From 2a455be46c2ee47a7e4fb2957eaa5544e4ac0099 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Mon, 15 Nov 2021 14:25:29 -0500 Subject: [PATCH] libs/os: remove arbitrary os.Exit (#7284) I think calling os.Exit at arbitrary points is _bad_ and is good to delete. I think panics in the case of data courruption have a chance of providing useful information. --- internal/consensus/replay_file.go | 23 +++++++++++++---------- internal/state/store.go | 13 ++++--------- libs/os/os.go | 5 ----- rpc/jsonrpc/test/main.go | 7 +++++-- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/internal/consensus/replay_file.go b/internal/consensus/replay_file.go index 8243e4f22..1f5e8e375 100644 --- a/internal/consensus/replay_file.go +++ b/internal/consensus/replay_file.go @@ -18,7 +18,6 @@ import ( sm "github.com/tendermint/tendermint/internal/state" "github.com/tendermint/tendermint/internal/store" "github.com/tendermint/tendermint/libs/log" - tmos "github.com/tendermint/tendermint/libs/os" tmpubsub "github.com/tendermint/tendermint/libs/pubsub" "github.com/tendermint/tendermint/types" ) @@ -92,7 +91,10 @@ func (cs *State) ReplayFile(file string, console bool) error { var msg *TimedWALMessage for { if nextN == 0 && console { - nextN = pb.replayConsoleLoop() + nextN, err = pb.replayConsoleLoop() + if err != nil { + return err + } } msg, err = pb.dec.Decode() @@ -194,16 +196,17 @@ func (cs *State) startForReplay() { }()*/ } -// console function for parsing input and running commands -func (pb *playback) replayConsoleLoop() int { +// console function for parsing input and running commands. The integer +// return value is invalid unless the error is nil. +func (pb *playback) replayConsoleLoop() (int, error) { for { fmt.Printf("> ") bufReader := bufio.NewReader(os.Stdin) line, more, err := bufReader.ReadLine() if more { - tmos.Exit("input is too long") + return 0, fmt.Errorf("input is too long") } else if err != nil { - tmos.Exit(err.Error()) + return 0, err } tokens := strings.Split(string(line), " ") @@ -217,13 +220,13 @@ func (pb *playback) replayConsoleLoop() int { // "next N" -> replay next N messages if len(tokens) == 1 { - return 0 + return 0, nil } i, err := strconv.Atoi(tokens[1]) if err != nil { fmt.Println("next takes an integer argument") } else { - return i + return i, nil } case "back": @@ -233,7 +236,7 @@ func (pb *playback) replayConsoleLoop() int { // NOTE: "back" is not supported in the state machine design, // so we restart and replay up to - ctx := context.Background() + ctx := context.TODO() // ensure all new step events are regenerated as expected newStepSub, err := pb.cs.eventBus.SubscribeWithArgs(ctx, tmpubsub.SubscribeArgs{ @@ -241,7 +244,7 @@ func (pb *playback) replayConsoleLoop() int { Query: types.EventQueryNewRoundStep, }) if err != nil { - tmos.Exit(fmt.Sprintf("failed to subscribe %s to %v", subscriber, types.EventQueryNewRoundStep)) + return 0, fmt.Errorf("failed to subscribe %s to %v", subscriber, types.EventQueryNewRoundStep) } defer func() { args := tmpubsub.UnsubscribeArgs{Subscriber: subscriber, Query: types.EventQueryNewRoundStep} diff --git a/internal/state/store.go b/internal/state/store.go index aff165aa1..0f1d2b444 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -11,7 +11,6 @@ import ( abci "github.com/tendermint/tendermint/abci/types" tmmath "github.com/tendermint/tendermint/libs/math" - tmos "github.com/tendermint/tendermint/libs/os" tmstate "github.com/tendermint/tendermint/proto/tendermint/state" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "github.com/tendermint/tendermint/types" @@ -126,8 +125,7 @@ func (store dbStore) loadState(key []byte) (state State, err error) { err = proto.Unmarshal(buf, sp) if err != nil { // DATA HAS BEEN CORRUPTED OR THE SPEC HAS CHANGED - tmos.Exit(fmt.Sprintf(`LoadState: Data has been corrupted or its spec has changed: - %v\n`, err)) + panic(fmt.Sprintf("data has been corrupted or its spec has changed: %+v", err)) } sm, err := FromProto(sp) @@ -424,8 +422,7 @@ func (store dbStore) LoadABCIResponses(height int64) (*tmstate.ABCIResponses, er err = abciResponses.Unmarshal(buf) if err != nil { // DATA HAS BEEN CORRUPTED OR THE SPEC HAS CHANGED - tmos.Exit(fmt.Sprintf(`LoadABCIResponses: Data has been corrupted or its spec has - changed: %v\n`, err)) + panic(fmt.Sprintf("data has been corrupted or its spec has changed: %+v", err)) } // TODO: ensure that buf is completely read. @@ -544,8 +541,7 @@ func loadValidatorsInfo(db dbm.DB, height int64) (*tmstate.ValidatorsInfo, error err = v.Unmarshal(buf) if err != nil { // DATA HAS BEEN CORRUPTED OR THE SPEC HAS CHANGED - tmos.Exit(fmt.Sprintf(`LoadValidators: Data has been corrupted or its spec has changed: - %v\n`, err)) + panic(fmt.Sprintf("data has been corrupted or its spec has changed: %+v", err)) } // TODO: ensure that buf is completely read. @@ -632,8 +628,7 @@ func (store dbStore) loadConsensusParamsInfo(height int64) (*tmstate.ConsensusPa paramsInfo := new(tmstate.ConsensusParamsInfo) if err = paramsInfo.Unmarshal(buf); err != nil { // DATA HAS BEEN CORRUPTED OR THE SPEC HAS CHANGED - tmos.Exit(fmt.Sprintf(`LoadConsensusParams: Data has been corrupted or its spec has changed: - %v\n`, err)) + panic(fmt.Sprintf(`data has been corrupted or its spec has changed: %+v`, err)) } // TODO: ensure that buf is completely read. diff --git a/libs/os/os.go b/libs/os/os.go index f4b0f1810..02b98c52a 100644 --- a/libs/os/os.go +++ b/libs/os/os.go @@ -29,11 +29,6 @@ func TrapSignal(logger logger, cb func()) { }() } -func Exit(s string) { - fmt.Printf(s + "\n") - os.Exit(1) -} - // EnsureDir ensures the given directory exists, creating it if necessary. // Errors if the path already exists as a non-directory. func EnsureDir(dir string, mode os.FileMode) error { diff --git a/rpc/jsonrpc/test/main.go b/rpc/jsonrpc/test/main.go index d348e1639..64f9de87a 100644 --- a/rpc/jsonrpc/test/main.go +++ b/rpc/jsonrpc/test/main.go @@ -3,6 +3,7 @@ package main import ( "fmt" "net/http" + "os" "github.com/tendermint/tendermint/libs/log" tmos "github.com/tendermint/tendermint/libs/os" @@ -35,10 +36,12 @@ func main() { config := rpcserver.DefaultConfig() listener, err := rpcserver.Listen("tcp://127.0.0.1:8008", config.MaxOpenConnections) if err != nil { - tmos.Exit(err.Error()) + logger.Error("rpc listening", "err", err) + os.Exit(1) } if err = rpcserver.Serve(listener, mux, logger, config); err != nil { - tmos.Exit(err.Error()) + logger.Error("rpc serve", "err", err) + os.Exit(1) } }