From 8a282a5fee50fea850f722e6c8b6abf8bccf4d18 Mon Sep 17 00:00:00 2001 From: Marko Date: Sun, 11 Aug 2019 19:03:40 +0200 Subject: [PATCH] replace errors.go with github.com/pkg/errors (2/2) (#3890) * init of (2/2) common errors * Remove instances of cmn.Error (2/2) - Replace usage of cmnError and errorWrap - ref #3862 Signed-off-by: Marko Baricevic * comment wording * simplify IsErrXXX functions * log panic along with stopping the MConnection --- cmd/tendermint/commands/lite.go | 5 +++-- crypto/merkle/simple_proof.go | 7 +++--- lite/base_verifier.go | 6 ++++-- lite/errors/errors.go | 38 +++++++++++---------------------- lite/proxy/errors.go | 11 ++++------ lite/proxy/query.go | 7 +++--- lite/proxy/verifier.go | 7 +++--- p2p/conn/connection.go | 9 +++++--- p2p/peer_test.go | 7 +++--- privval/socket_dialers_test.go | 4 ++-- privval/utils.go | 15 +++++++------ privval/utils_test.go | 8 +++---- types/params.go | 16 ++++++++------ types/validator_set.go | 23 +++++++------------- 14 files changed, 75 insertions(+), 88 deletions(-) diff --git a/cmd/tendermint/commands/lite.go b/cmd/tendermint/commands/lite.go index d3a4ac53..906af930 100644 --- a/cmd/tendermint/commands/lite.go +++ b/cmd/tendermint/commands/lite.go @@ -4,6 +4,7 @@ import ( "fmt" "net/url" + "github.com/pkg/errors" "github.com/spf13/cobra" cmn "github.com/tendermint/tendermint/libs/common" @@ -80,7 +81,7 @@ func runProxy(cmd *cobra.Command, args []string) error { logger.Info("Constructing Verifier...") cert, err := proxy.NewVerifier(chainID, home, node, logger, cacheSize) if err != nil { - return cmn.ErrorWrap(err, "constructing Verifier") + return errors.Wrap(err, "constructing Verifier") } cert.SetLogger(logger) sc := proxy.SecureClient(node, cert) @@ -88,7 +89,7 @@ func runProxy(cmd *cobra.Command, args []string) error { logger.Info("Starting proxy...") err = proxy.StartProxy(sc, listenAddr, logger, maxOpenConnections) if err != nil { - return cmn.ErrorWrap(err, "starting proxy") + return errors.Wrap(err, "starting proxy") } // Run forever diff --git a/crypto/merkle/simple_proof.go b/crypto/merkle/simple_proof.go index d3be5d7e..da32157d 100644 --- a/crypto/merkle/simple_proof.go +++ b/crypto/merkle/simple_proof.go @@ -2,10 +2,9 @@ package merkle import ( "bytes" - "errors" "fmt" - cmn "github.com/tendermint/tendermint/libs/common" + "github.com/pkg/errors" ) // SimpleProof represents a simple Merkle proof. @@ -75,11 +74,11 @@ func (sp *SimpleProof) Verify(rootHash []byte, leaf []byte) error { return errors.New("Proof index cannot be negative") } if !bytes.Equal(sp.LeafHash, leafHash) { - return cmn.NewError("invalid leaf hash: wanted %X got %X", leafHash, sp.LeafHash) + return errors.Errorf("invalid leaf hash: wanted %X got %X", leafHash, sp.LeafHash) } computedHash := sp.ComputeRootHash() if !bytes.Equal(computedHash, rootHash) { - return cmn.NewError("invalid root hash: wanted %X got %X", rootHash, computedHash) + return errors.Errorf("invalid root hash: wanted %X got %X", rootHash, computedHash) } return nil } diff --git a/lite/base_verifier.go b/lite/base_verifier.go index 9eb880bb..9fe583b0 100644 --- a/lite/base_verifier.go +++ b/lite/base_verifier.go @@ -3,6 +3,8 @@ package lite import ( "bytes" + "github.com/pkg/errors" + cmn "github.com/tendermint/tendermint/libs/common" lerr "github.com/tendermint/tendermint/lite/errors" "github.com/tendermint/tendermint/types" @@ -63,7 +65,7 @@ func (bv *BaseVerifier) Verify(signedHeader types.SignedHeader) error { // Do basic sanity checks. err := signedHeader.ValidateBasic(bv.chainID) if err != nil { - return cmn.ErrorWrap(err, "in verify") + return errors.Wrap(err, "in verify") } // Check commit signatures. @@ -71,7 +73,7 @@ func (bv *BaseVerifier) Verify(signedHeader types.SignedHeader) error { bv.chainID, signedHeader.Commit.BlockID, signedHeader.Height, signedHeader.Commit) if err != nil { - return cmn.ErrorWrap(err, "in verify") + return errors.Wrap(err, "in verify") } return nil diff --git a/lite/errors/errors.go b/lite/errors/errors.go index 75442c72..5bb829b0 100644 --- a/lite/errors/errors.go +++ b/lite/errors/errors.go @@ -3,7 +3,7 @@ package errors import ( "fmt" - cmn "github.com/tendermint/tendermint/libs/common" + "github.com/pkg/errors" ) //---------------------------------------- @@ -49,15 +49,12 @@ func (e errEmptyTree) Error() string { // ErrCommitNotFound indicates that a the requested commit was not found. func ErrCommitNotFound() error { - return cmn.ErrorWrap(errCommitNotFound{}, "") + return errors.Wrap(errCommitNotFound{}, "") } func IsErrCommitNotFound(err error) bool { - if err_, ok := err.(cmn.Error); ok { - _, ok := err_.Data().(errCommitNotFound) - return ok - } - return false + _, ok := errors.Cause(err).(errCommitNotFound) + return ok } //----------------- @@ -65,18 +62,15 @@ func IsErrCommitNotFound(err error) bool { // ErrUnexpectedValidators indicates a validator set mismatch. func ErrUnexpectedValidators(got, want []byte) error { - return cmn.ErrorWrap(errUnexpectedValidators{ + return errors.Wrap(errUnexpectedValidators{ got: got, want: want, }, "") } func IsErrUnexpectedValidators(err error) bool { - if err_, ok := err.(cmn.Error); ok { - _, ok := err_.Data().(errUnexpectedValidators) - return ok - } - return false + _, ok := errors.Cause(err).(errUnexpectedValidators) + return ok } //----------------- @@ -84,28 +78,22 @@ func IsErrUnexpectedValidators(err error) bool { // ErrUnknownValidators indicates that some validator set was missing or unknown. func ErrUnknownValidators(chainID string, height int64) error { - return cmn.ErrorWrap(errUnknownValidators{chainID, height}, "") + return errors.Wrap(errUnknownValidators{chainID, height}, "") } func IsErrUnknownValidators(err error) bool { - if err_, ok := err.(cmn.Error); ok { - _, ok := err_.Data().(errUnknownValidators) - return ok - } - return false + _, ok := errors.Cause(err).(errUnknownValidators) + return ok } //----------------- // ErrEmptyTree func ErrEmptyTree() error { - return cmn.ErrorWrap(errEmptyTree{}, "") + return errors.Wrap(errEmptyTree{}, "") } func IsErrEmptyTree(err error) bool { - if err_, ok := err.(cmn.Error); ok { - _, ok := err_.Data().(errEmptyTree) - return ok - } - return false + _, ok := errors.Cause(err).(errEmptyTree) + return ok } diff --git a/lite/proxy/errors.go b/lite/proxy/errors.go index 6a7c2354..41923659 100644 --- a/lite/proxy/errors.go +++ b/lite/proxy/errors.go @@ -1,7 +1,7 @@ package proxy import ( - cmn "github.com/tendermint/tendermint/libs/common" + "github.com/pkg/errors" ) type errNoData struct{} @@ -12,13 +12,10 @@ func (e errNoData) Error() string { // IsErrNoData checks whether an error is due to a query returning empty data func IsErrNoData(err error) bool { - if err_, ok := err.(cmn.Error); ok { - _, ok := err_.Data().(errNoData) - return ok - } - return false + _, ok := errors.Cause(err).(errNoData) + return ok } func ErrNoData() error { - return cmn.ErrorWrap(errNoData{}, "") + return errors.Wrap(errNoData{}, "") } diff --git a/lite/proxy/query.go b/lite/proxy/query.go index fd10e0bb..2e0b9322 100644 --- a/lite/proxy/query.go +++ b/lite/proxy/query.go @@ -4,9 +4,10 @@ import ( "fmt" "strings" - cmn "github.com/tendermint/tendermint/libs/common" + "github.com/pkg/errors" "github.com/tendermint/tendermint/crypto/merkle" + cmn "github.com/tendermint/tendermint/libs/common" "github.com/tendermint/tendermint/lite" lerr "github.com/tendermint/tendermint/lite/errors" rpcclient "github.com/tendermint/tendermint/rpc/client" @@ -83,7 +84,7 @@ func GetWithProofOptions(prt *merkle.ProofRuntime, path string, key []byte, opts kp = kp.AppendKey(resp.Key, merkle.KeyEncodingURL) err = prt.VerifyValue(resp.Proof, signedHeader.AppHash, kp.String(), resp.Value) if err != nil { - return nil, cmn.ErrorWrap(err, "Couldn't verify value proof") + return nil, errors.Wrap(err, "Couldn't verify value proof") } return &ctypes.ResultABCIQuery{Response: resp}, nil } else { @@ -92,7 +93,7 @@ func GetWithProofOptions(prt *merkle.ProofRuntime, path string, key []byte, opts // XXX How do we encode the key into a string... err = prt.VerifyAbsence(resp.Proof, signedHeader.AppHash, string(resp.Key)) if err != nil { - return nil, cmn.ErrorWrap(err, "Couldn't verify absence proof") + return nil, errors.Wrap(err, "Couldn't verify absence proof") } return &ctypes.ResultABCIQuery{Response: resp}, nil } diff --git a/lite/proxy/verifier.go b/lite/proxy/verifier.go index 2119a7ae..3673b66c 100644 --- a/lite/proxy/verifier.go +++ b/lite/proxy/verifier.go @@ -1,7 +1,8 @@ package proxy import ( - cmn "github.com/tendermint/tendermint/libs/common" + "github.com/pkg/errors" + log "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/lite" lclient "github.com/tendermint/tendermint/lite/client" @@ -29,11 +30,11 @@ func NewVerifier(chainID, rootDir string, client lclient.SignStatusClient, logge logger.Info("lite/proxy/NewVerifier found no trusted full commit, initializing from source from height 1...") fc, err := source.LatestFullCommit(chainID, 1, 1) if err != nil { - return nil, cmn.ErrorWrap(err, "fetching source full commit @ height 1") + return nil, errors.Wrap(err, "fetching source full commit @ height 1") } err = trust.SaveFullCommit(fc) if err != nil { - return nil, cmn.ErrorWrap(err, "saving full commit to trusted") + return nil, errors.Wrap(err, "saving full commit to trusted") } } diff --git a/p2p/conn/connection.go b/p2p/conn/connection.go index a206af54..68066a7c 100644 --- a/p2p/conn/connection.go +++ b/p2p/conn/connection.go @@ -2,7 +2,8 @@ package conn import ( "bufio" - "errors" + "runtime/debug" + "fmt" "io" "math" @@ -12,6 +13,8 @@ import ( "sync/atomic" "time" + "github.com/pkg/errors" + amino "github.com/tendermint/go-amino" cmn "github.com/tendermint/tendermint/libs/common" flow "github.com/tendermint/tendermint/libs/flowrate" @@ -313,8 +316,8 @@ func (c *MConnection) flush() { // Catch panics, usually caused by remote disconnects. func (c *MConnection) _recover() { if r := recover(); r != nil { - err := cmn.ErrorWrap(r, "recovered panic in MConnection") - c.stopForError(err) + c.Logger.Error("MConnection panicked", "err", r, "stack", string(debug.Stack())) + c.stopForError(errors.Errorf("recovered from panic: %v", r)) } } diff --git a/p2p/peer_test.go b/p2p/peer_test.go index bf61beb4..37a3009c 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -113,13 +114,13 @@ func testOutboundPeerConn( var pc peerConn conn, err := testDial(addr, config) if err != nil { - return pc, cmn.ErrorWrap(err, "Error creating peer") + return pc, errors.Wrap(err, "Error creating peer") } pc, err = testPeerConn(conn, config, true, persistent, ourNodePrivKey, addr) if err != nil { if cerr := conn.Close(); cerr != nil { - return pc, cmn.ErrorWrap(err, cerr.Error()) + return pc, errors.Wrap(err, cerr.Error()) } return pc, err } @@ -127,7 +128,7 @@ func testOutboundPeerConn( // ensure dialed ID matches connection ID if addr.ID != pc.ID() { if cerr := conn.Close(); cerr != nil { - return pc, cmn.ErrorWrap(err, cerr.Error()) + return pc, errors.Wrap(err, cerr.Error()) } return pc, ErrSwitchAuthenticationFailure{addr, pc.ID()} } diff --git a/privval/socket_dialers_test.go b/privval/socket_dialers_test.go index c77261bc..d7b372b8 100644 --- a/privval/socket_dialers_test.go +++ b/privval/socket_dialers_test.go @@ -5,11 +5,11 @@ import ( "testing" "time" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/ed25519" - cmn "github.com/tendermint/tendermint/libs/common" ) func getDialerTestCases(t *testing.T) []dialerTestCase { @@ -44,6 +44,6 @@ func TestIsConnTimeoutForWrappedConnTimeouts(t *testing.T) { dialer := DialTCPFn(tcpAddr, time.Millisecond, ed25519.GenPrivKey()) _, err := dialer() assert.Error(t, err) - err = cmn.ErrorWrap(ErrConnectionTimeout, err.Error()) + err = errors.Wrap(ErrConnectionTimeout, err.Error()) assert.True(t, IsConnTimeout(err)) } diff --git a/privval/utils.go b/privval/utils.go index a707e2ee..65368eb2 100644 --- a/privval/utils.go +++ b/privval/utils.go @@ -4,6 +4,8 @@ import ( "fmt" "net" + "github.com/pkg/errors" + "github.com/tendermint/tendermint/crypto/ed25519" cmn "github.com/tendermint/tendermint/libs/common" "github.com/tendermint/tendermint/libs/log" @@ -13,15 +15,14 @@ import ( // report that a connection timeout occurred. This detects both fundamental // network timeouts, as well as ErrConnTimeout errors. func IsConnTimeout(err error) bool { - if cmnErr, ok := err.(cmn.Error); ok { - if cmnErr.Data() == ErrConnectionTimeout { - return true - } - } - if _, ok := err.(timeoutError); ok { + switch errors.Cause(err).(type) { + case EndpointTimeoutError: return true + case timeoutError: + return true + default: + return false } - return false } // NewSignerListener creates a new SignerListenerEndpoint using the corresponding listen address diff --git a/privval/utils_test.go b/privval/utils_test.go index b07186f6..5648efec 100644 --- a/privval/utils_test.go +++ b/privval/utils_test.go @@ -1,15 +1,13 @@ package privval import ( - "fmt" "testing" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" - - cmn "github.com/tendermint/tendermint/libs/common" ) func TestIsConnTimeoutForNonTimeoutErrors(t *testing.T) { - assert.False(t, IsConnTimeout(cmn.ErrorWrap(ErrDialRetryMax, "max retries exceeded"))) - assert.False(t, IsConnTimeout(fmt.Errorf("completely irrelevant error"))) + assert.False(t, IsConnTimeout(errors.Wrap(ErrDialRetryMax, "max retries exceeded"))) + assert.False(t, IsConnTimeout(errors.New("completely irrelevant error"))) } diff --git a/types/params.go b/types/params.go index 162aaead..c9ab4aaf 100644 --- a/types/params.go +++ b/types/params.go @@ -1,6 +1,8 @@ package types import ( + "github.com/pkg/errors" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/tmhash" cmn "github.com/tendermint/tendermint/libs/common" @@ -95,38 +97,38 @@ func (params *ValidatorParams) IsValidPubkeyType(pubkeyType string) bool { // allowed limits, and returns an error if they are not. func (params *ConsensusParams) Validate() error { if params.Block.MaxBytes <= 0 { - return cmn.NewError("Block.MaxBytes must be greater than 0. Got %d", + return errors.Errorf("Block.MaxBytes must be greater than 0. Got %d", params.Block.MaxBytes) } if params.Block.MaxBytes > MaxBlockSizeBytes { - return cmn.NewError("Block.MaxBytes is too big. %d > %d", + return errors.Errorf("Block.MaxBytes is too big. %d > %d", params.Block.MaxBytes, MaxBlockSizeBytes) } if params.Block.MaxGas < -1 { - return cmn.NewError("Block.MaxGas must be greater or equal to -1. Got %d", + return errors.Errorf("Block.MaxGas must be greater or equal to -1. Got %d", params.Block.MaxGas) } if params.Block.TimeIotaMs <= 0 { - return cmn.NewError("Block.TimeIotaMs must be greater than 0. Got %v", + return errors.Errorf("Block.TimeIotaMs must be greater than 0. Got %v", params.Block.TimeIotaMs) } if params.Evidence.MaxAge <= 0 { - return cmn.NewError("EvidenceParams.MaxAge must be greater than 0. Got %d", + return errors.Errorf("EvidenceParams.MaxAge must be greater than 0. Got %d", params.Evidence.MaxAge) } if len(params.Validator.PubKeyTypes) == 0 { - return cmn.NewError("len(Validator.PubKeyTypes) must be greater than 0") + return errors.New("len(Validator.PubKeyTypes) must be greater than 0") } // Check if keyType is a known ABCIPubKeyType for i := 0; i < len(params.Validator.PubKeyTypes); i++ { keyType := params.Validator.PubKeyTypes[i] if _, ok := ABCIPubKeyTypesToAminoNames[keyType]; !ok { - return cmn.NewError("params.Validator.PubKeyTypes[%d], %s, is an unknown pubkey type", + return errors.Errorf("params.Validator.PubKeyTypes[%d], %s, is an unknown pubkey type", i, keyType) } } diff --git a/types/validator_set.go b/types/validator_set.go index 33636d09..8cb8a2ff 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -2,15 +2,15 @@ package types import ( "bytes" - "errors" "fmt" "math" "math/big" "sort" "strings" + "github.com/pkg/errors" + "github.com/tendermint/tendermint/crypto/merkle" - cmn "github.com/tendermint/tendermint/libs/common" ) // MaxTotalVotingPower - the maximum allowed total voting power. @@ -681,13 +681,13 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin continue } if precommit.Height != height { - return cmn.NewError("Blocks don't match - %d vs %d", round, precommit.Round) + return errors.Errorf("Blocks don't match - %d vs %d", round, precommit.Round) } if precommit.Round != round { - return cmn.NewError("Invalid commit -- wrong round: %v vs %v", round, precommit.Round) + return errors.Errorf("Invalid commit -- wrong round: %v vs %v", round, precommit.Round) } if precommit.Type != PrecommitType { - return cmn.NewError("Invalid commit -- not precommit @ index %v", idx) + return errors.Errorf("Invalid commit -- not precommit @ index %v", idx) } // See if this validator is in oldVals. oldIdx, val := oldVals.GetByAddress(precommit.ValidatorAddress) @@ -699,7 +699,7 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin // Validate signature. precommitSignBytes := commit.VoteSignBytes(chainID, idx) if !val.PubKey.VerifyBytes(precommitSignBytes, precommit.Signature) { - return cmn.NewError("Invalid commit -- invalid signature: %v", precommit) + return errors.Errorf("Invalid commit -- invalid signature: %v", precommit) } // Good precommit! if blockID.Equals(precommit.BlockID) { @@ -721,15 +721,8 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin // ErrTooMuchChange func IsErrTooMuchChange(err error) bool { - switch err_ := err.(type) { - case cmn.Error: - _, ok := err_.Data().(errTooMuchChange) - return ok - case errTooMuchChange: - return true - default: - return false - } + _, ok := errors.Cause(err).(errTooMuchChange) + return ok } type errTooMuchChange struct {