From 679583718c433b45a974fa160697deca0a47a611 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Tue, 4 Jun 2024 18:38:08 +0200 Subject: [PATCH 01/12] remove all deps from errors --- errors/abci_test.go | 58 ++++----- errors/errors.go | 81 +----------- errors/errors_test.go | 250 +++++++++++++++----------------------- errors/go.mod | 14 --- errors/go.sum | 28 ----- errors/stacktrace.go | 120 ------------------ errors/stacktrace_test.go | 62 ---------- 7 files changed, 126 insertions(+), 487 deletions(-) delete mode 100644 errors/stacktrace.go delete mode 100644 errors/stacktrace_test.go diff --git a/errors/abci_test.go b/errors/abci_test.go index 333731d01518..4c693eeae358 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -5,23 +5,9 @@ import ( "io" "strings" "testing" - - "github.com/stretchr/testify/suite" ) -type abciTestSuite struct { - suite.Suite -} - -func TestABCITestSuite(t *testing.T) { - suite.Run(t, new(abciTestSuite)) -} - -func (s *abciTestSuite) SetupSuite() { - s.T().Parallel() -} - -func (s *abciTestSuite) TestABCInfo() { +func TestABCInfo(t *testing.T) { cases := map[string]struct { err error debug bool @@ -89,16 +75,22 @@ func (s *abciTestSuite) TestABCInfo() { } for testName, tc := range cases { - s.T().Run(testName, func(t *testing.T) { + t.Run(testName, func(t *testing.T) { space, code, log := ABCIInfo(tc.err, tc.debug) - s.Require().Equal(tc.wantSpace, space, testName) - s.Require().Equal(tc.wantCode, code, testName) - s.Require().Equal(tc.wantLog, log, testName) + if space != tc.wantSpace { + t.Errorf("%s: expected space %s, got %s", testName, tc.wantSpace, space) + } + if code != tc.wantCode { + t.Errorf("%s: expected code %d, got %d", testName, tc.wantCode, code) + } + if log != tc.wantLog { + t.Errorf("%s: expected log %s, got %s", testName, tc.wantLog, log) + } }) } } -func (s *abciTestSuite) TestABCIInfoStacktrace() { +func TestABCIInfoStacktrace(t *testing.T) { cases := map[string]struct { err error debug bool @@ -128,25 +120,33 @@ func (s *abciTestSuite) TestABCIInfoStacktrace() { const thisTestSrc = "cosmossdk.io/errors.(*abciTestSuite).TestABCIInfoStacktrace" for testName, tc := range cases { - s.T().Run(testName, func(t *testing.T) { + t.Run(testName, func(t *testing.T) { _, _, log := ABCIInfo(tc.err, tc.debug) if !tc.wantStacktrace { - s.Require().Equal(tc.wantErrMsg, log, testName) + if log != tc.wantErrMsg { + t.Errorf("%s: expected log %s, got %s", testName, tc.wantErrMsg, log) + } } else { - s.Require().True(strings.Contains(log, thisTestSrc), testName) - s.Require().True(strings.Contains(log, tc.wantErrMsg), testName) + if !strings.Contains(log, thisTestSrc) { + t.Errorf("%s: expected log to contain %s", testName, thisTestSrc) + } + if !strings.Contains(log, tc.wantErrMsg) { + t.Errorf("%s: expected log to contain %s", testName, tc.wantErrMsg) + } } }) } } -func (s *abciTestSuite) TestABCIInfoHidesStacktrace() { +func TestABCIInfoHidesStacktrace(t *testing.T) { err := Wrap(ErrUnauthorized, "wrapped") _, _, log := ABCIInfo(err, false) - s.Require().Equal("wrapped: unauthorized", log) + if log != "wrapped: unauthorized" { + t.Errorf("expected log %s, got %s", "wrapped: unauthorized", log) + } } -func (s *abciTestSuite) TestABCIInfoSerializeErr() { +func TestABCIInfoSerializeErr(t *testing.T) { var ( // Create errors with stacktrace for equal comparison. myErrDecode = Wrap(ErrTxDecode, "test") @@ -183,7 +183,9 @@ func (s *abciTestSuite) TestABCIInfoSerializeErr() { for msg, spec := range specs { spec := spec _, _, log := ABCIInfo(spec.src, spec.debug) - s.Require().Equal(spec.exp, log, msg) + if log != spec.exp { + t.Errorf("%s: expected log %s, got %s", msg, spec.exp, log) + } } } diff --git a/errors/errors.go b/errors/errors.go index 99c6cf01540a..9b1b2f382553 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -1,10 +1,8 @@ package errors import ( + "errors" "fmt" - "reflect" - - "github.com/pkg/errors" ) // UndefinedCodespace when we explicitly declare no codespace @@ -102,38 +100,6 @@ func (e Error) Codespace() string { return e.codespace } -// Is check if given error instance is of a given kind/type. This involves -// unwrapping given error using the Cause method if available. -func (e *Error) Is(err error) bool { - // Reflect usage is necessary to correctly compare with - // a nil implementation of an error. - if e == nil { - return isNilErr(err) - } - - for { - if err == e { - return true - } - - // If this is a collection of errors, this function must return - // true if at least one from the group match. - if u, ok := err.(unpacker); ok { - for _, er := range u.Unpack() { - if e.Is(er) { - return true - } - } - } - - if c, ok := err.(causer); ok { - err = c.Cause() - } else { - return false - } - } -} - // Wrap extends this error with an additional information. // It's a handy function to call Wrap with sdk errors. func (e *Error) Wrap(desc string) error { return Wrap(e, desc) } @@ -142,18 +108,6 @@ func (e *Error) Wrap(desc string) error { return Wrap(e, desc) } // It's a handy function to call Wrapf with sdk errors. func (e *Error) Wrapf(desc string, args ...interface{}) error { return Wrapf(e, desc, args...) } -func isNilErr(err error) bool { - // Reflect usage is necessary to correctly compare with - // a nil implementation of an error. - if err == nil { - return true - } - if reflect.ValueOf(err).Kind() == reflect.Struct { - return false - } - return reflect.ValueOf(err).IsNil() -} - // Wrap extends given error with an additional information. // // If the wrapped error does not provide ABCICode method (ie. stdlib errors), @@ -166,13 +120,6 @@ func Wrap(err error, description string) error { return nil } - // If this error does not carry the stacktrace information yet, attach - // one. This should be done only once per error at the lowest frame - // possible (most inner wrap). - if stackTrace(err) == nil { - err = errors.WithStack(err) - } - return &wrappedError{ parent: err, msg: description, @@ -203,28 +150,6 @@ func (e *wrappedError) Cause() error { return e.parent } -// Is reports whether any error in e's chain matches a target. -func (e *wrappedError) Is(target error) bool { - if e == target { - return true - } - - w := e.Cause() - for { - if w == target { - return true - } - - x, ok := w.(causer) - if ok { - w = x.Cause() - } - if x == nil { - return false - } - } -} - // Unwrap implements the built-in errors.Unwrap func (e *wrappedError) Unwrap() error { return e.parent @@ -263,7 +188,3 @@ func IsOf(received error, targets ...error) bool { type causer interface { Cause() error } - -type unpacker interface { - Unpack() []error -} diff --git a/errors/errors_test.go b/errors/errors_test.go index f3e99e2bab9b..b71910eaa09a 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -1,210 +1,150 @@ package errors import ( - stdlib "errors" - "fmt" + "errors" "testing" - - "github.com/pkg/errors" - "github.com/stretchr/testify/suite" ) -type errorsTestSuite struct { - suite.Suite -} - -func TestErrorsTestSuite(t *testing.T) { - suite.Run(t, new(errorsTestSuite)) -} - -func (s *errorsTestSuite) SetupSuite() { - s.T().Parallel() -} - -func (s *errorsTestSuite) TestCause() { - std := stdlib.New("this is a stdlib error") - - cases := map[string]struct { - err error - root error - }{ - "Errors are self-causing": { - err: ErrUnauthorized, - root: ErrUnauthorized, - }, - "Wrap reveals root cause": { - err: Wrap(ErrUnauthorized, "foo"), - root: ErrUnauthorized, - }, - "Cause works for stderr as root": { - err: Wrap(std, "Some helpful text"), - root: std, - }, - } - - for testName, tc := range cases { - s.Require().Equal(tc.root, errors.Cause(tc.err), testName) - } -} - -func (s *errorsTestSuite) TestErrorIs() { - cases := map[string]struct { - a *Error - b error - wantIs bool - }{ - "instance of the same error": { - a: ErrUnauthorized, - b: ErrUnauthorized, - wantIs: true, - }, - "two different coded errors": { - a: ErrUnauthorized, - b: ErrOutOfGas, - wantIs: false, - }, - "successful comparison to a wrapped error": { - a: ErrUnauthorized, - b: Wrap(ErrUnauthorized, "gone"), - wantIs: true, - }, - "unsuccessful comparison to a wrapped error": { - a: ErrUnauthorized, - b: Wrap(ErrInsufficientFee, "too big"), - wantIs: false, - }, - "not equal to stdlib error": { - a: ErrUnauthorized, - b: fmt.Errorf("stdlib error"), - wantIs: false, - }, - "not equal to a wrapped stdlib error": { - a: ErrUnauthorized, - b: Wrap(fmt.Errorf("stdlib error"), "wrapped"), - wantIs: false, - }, - "nil is nil": { - a: nil, - b: nil, - wantIs: true, - }, - "nil is any error nil": { - a: nil, - b: (*customError)(nil), - wantIs: true, - }, - "nil is not not-nil": { - a: nil, - b: ErrUnauthorized, - wantIs: false, - }, - "not-nil is not nil": { - a: ErrUnauthorized, - b: nil, - wantIs: false, - }, - } - for testName, tc := range cases { - s.Require().Equal(tc.wantIs, tc.a.Is(tc.b), testName) - } -} - -func (s *errorsTestSuite) TestIsOf() { - require := s.Require() - +func TestIsOf(t *testing.T) { var errNil *Error err := ErrInvalidAddress errW := Wrap(ErrLogic, "more info") - require.False(IsOf(nil), "nil should always have no causer") - require.False(IsOf(nil, err), "nil should always have no causer") - require.False(IsOf(errNil), "nil error should always have no causer") - require.False(IsOf(errNil, err), "nil error should always have no causer") + if IsOf(nil) { + t.Errorf("nil should always have no causer") + } + if IsOf(nil, err) { + t.Errorf("nil should always have no causer") + } + if IsOf(errNil) { + t.Errorf("nil error should always have no causer") + } + if IsOf(errNil, err) { + t.Errorf("nil error should always have no causer") + } - require.False(IsOf(err)) - require.False(IsOf(err, nil)) - require.False(IsOf(err, ErrLogic)) + if IsOf(err) { + t.Errorf("error should always have no causer") + } + if IsOf(err, nil) { + t.Errorf("error should always have no causer") + } + if IsOf(err, ErrLogic) { + t.Errorf("error should always have no causer") + } - require.True(IsOf(errW, ErrLogic)) - require.True(IsOf(errW, err, ErrLogic)) - require.True(IsOf(errW, nil, errW), "error should much itself") + if !IsOf(errW, ErrLogic) { + t.Errorf("error should have causer") + } + if !IsOf(errW, err, ErrLogic) { + t.Errorf("error should have causer") + } + if !IsOf(errW, nil, errW) { + t.Errorf("error should much itself") + } err2 := errors.New("other error") - require.True(IsOf(err2, nil, err2), "error should much itself") -} - -type customError struct{} - -func (customError) Error() string { - return "custom error" + if !IsOf(err2, nil, err2) { + t.Errorf("error should much itself") + } } -func (s *errorsTestSuite) TestWrapEmpty() { - s.Require().Nil(Wrap(nil, "wrapping ")) +func TestWrapEmpty(t *testing.T) { + if err := Wrap(nil, "wrapping "); err != nil { + t.Errorf("expected nil error, got: %v", err) + } } -func (s *errorsTestSuite) TestWrappedIs() { - require := s.Require() +func TestWrappedIs(t *testing.T) { err := Wrap(ErrTxTooLarge, "context") - require.True(stdlib.Is(err, ErrTxTooLarge)) + if !errors.Is(err, ErrTxTooLarge) { + t.Errorf("expected error to be of type ErrTxTooLarge") + } err = Wrap(err, "more context") - require.True(stdlib.Is(err, ErrTxTooLarge)) + if !errors.Is(err, ErrTxTooLarge) { + t.Errorf("expected error to be of type ErrTxTooLarge") + } err = Wrap(err, "even more context") - require.True(stdlib.Is(err, ErrTxTooLarge)) + if !errors.Is(err, ErrTxTooLarge) { + t.Errorf("expected error to be of type ErrTxTooLarge") + } err = Wrap(ErrInsufficientFee, "...") - require.False(stdlib.Is(err, ErrTxTooLarge)) - - errs := stdlib.New("other") - require.True(stdlib.Is(errs, errs)) + if !errors.Is(err, ErrTxTooLarge) { + t.Errorf("expected error to be of type ErrTxTooLarge") + } - errw := &wrappedError{"msg", errs} - require.True(errw.Is(errw), "should match itself") + errs := errors.New("other") + if !errors.Is(errs, errs) { + t.Errorf("error should match itself") + } - require.True(stdlib.Is(ErrInsufficientFee.Wrap("wrapped"), ErrInsufficientFee)) - require.True(IsOf(ErrInsufficientFee.Wrap("wrapped"), ErrInsufficientFee)) - require.True(stdlib.Is(ErrInsufficientFee.Wrapf("wrapped"), ErrInsufficientFee)) - require.True(IsOf(ErrInsufficientFee.Wrapf("wrapped"), ErrInsufficientFee)) + if !errors.Is(ErrInsufficientFee.Wrap("wrapped"), ErrInsufficientFee) { + t.Errorf("expected error to be of type ErrInsufficientFee") + } + if !IsOf(ErrInsufficientFee.Wrap("wrapped"), ErrInsufficientFee) { + t.Errorf("expected error to be of type ErrInsufficientFee") + } + if !errors.Is(ErrInsufficientFee.Wrapf("wrapped"), ErrInsufficientFee) { + t.Errorf("expected error to be of type ErrInsufficientFee") + } + if !IsOf(ErrInsufficientFee.Wrapf("wrapped"), ErrInsufficientFee) { + t.Errorf("expected error to be of type ErrInsufficientFee") + } } -func (s *errorsTestSuite) TestWrappedIsMultiple() { +func TestWrappedIsMultiple(t *testing.T) { errTest := errors.New("test error") errTest2 := errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().True(stdlib.Is(err, errTest2)) + if !errors.Is(err, errTest2) { + t.Errorf("expected error to be of type errTest2") + } } -func (s *errorsTestSuite) TestWrappedIsFail() { +func TestWrappedIsFail(t *testing.T) { errTest := errors.New("test error") errTest2 := errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().False(stdlib.Is(err, errTest)) + if errors.Is(err, errTest) { + t.Errorf("expected error to not be of type errTest") + } } -func (s *errorsTestSuite) TestWrappedUnwrap() { +func TestWrappedUnwrap(t *testing.T) { errTest := errors.New("test error") err := Wrap(errTest, "some random description") - s.Require().Equal(errTest, stdlib.Unwrap(err)) + if unwrappedErr := errors.Unwrap(err); errors.Is(unwrappedErr, errTest) { + t.Errorf("expected unwrapped error to be %v, got %v", errTest, unwrappedErr) + } } -func (s *errorsTestSuite) TestWrappedUnwrapMultiple() { +func TestWrappedUnwrapMultiple(t *testing.T) { errTest := errors.New("test error") errTest2 := errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().Equal(errTest2, stdlib.Unwrap(err)) + if unwrappedErr := errors.Unwrap(err); errors.Is(unwrappedErr, errTest2) { + t.Errorf("expected unwrapped error to be %v, got %v", errTest2, unwrappedErr) + } } -func (s *errorsTestSuite) TestWrappedUnwrapFail() { +func TestWrappedUnwrapFail(t *testing.T) { errTest := errors.New("test error") errTest2 := errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().NotEqual(errTest, stdlib.Unwrap(err)) + if errors.Is(errTest, errors.Unwrap(err)) { + t.Errorf("expected unwrapped error to not be %v", errTest) + } } -func (s *errorsTestSuite) TestABCIError() { - s.Require().Equal("custom: tx parse error", ABCIError(testCodespace, 2, "custom").Error()) - s.Require().Equal("custom: unknown", ABCIError("unknown", 1, "custom").Error()) +func TestABCIError(t *testing.T) { + if err := ABCIError(testCodespace, 2, "custom"); err.Error() != "custom: tx parse error" { + t.Errorf("expected error message: custom: tx parse error, got: %v", err.Error()) + } + if err := ABCIError("unknown", 1, "custom"); err.Error() != "custom: unknown" { + t.Errorf("expected error message: custom: unknown, got: %v", err.Error()) + } } const testCodespace = "testtesttest" diff --git a/errors/go.mod b/errors/go.mod index 91cf5a175e20..afcb133dd2b3 100644 --- a/errors/go.mod +++ b/errors/go.mod @@ -1,17 +1,3 @@ module cosmossdk.io/errors go 1.20 - -require ( - github.com/pkg/errors v0.9.1 - github.com/stretchr/testify v1.9.0 -) - -require ( - github.com/davecgh/go-spew v1.1.1 // indirect - github.com/kr/pretty v0.3.0 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/rogpeppe/go-internal v1.8.1 // indirect - gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect -) diff --git a/errors/go.sum b/errors/go.sum index 4c118e9927a8..e69de29bb2d1 100644 --- a/errors/go.sum +++ b/errors/go.sum @@ -1,28 +0,0 @@ -github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= -github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= -github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= -github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= -github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= -github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= -github.com/rogpeppe/go-internal v1.8.1 h1:geMPLpDpQOgVyCg5z5GoRwLHepNdb71NXb67XFkP+Eg= -github.com/rogpeppe/go-internal v1.8.1/go.mod h1:JeRgkft04UBgHMgCIwADu4Pn6Mtm5d4nPKWu0nJ5d+o= -github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= -github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= -gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/errors/stacktrace.go b/errors/stacktrace.go deleted file mode 100644 index d7021085db34..000000000000 --- a/errors/stacktrace.go +++ /dev/null @@ -1,120 +0,0 @@ -package errors - -import ( - "fmt" - "io" - "runtime" - "strings" - - "github.com/pkg/errors" -) - -func matchesFunc(f errors.Frame, prefixes ...string) bool { - fn := funcName(f) - for _, prefix := range prefixes { - if strings.HasPrefix(fn, prefix) { - return true - } - } - return false -} - -// funcName returns the name of this function, if known. -func funcName(f errors.Frame) string { - // this looks a bit like magic, but follows example here: - // https://github.com/pkg/errors/blob/v0.8.1/stack.go#L43-L50 - pc := uintptr(f) - 1 - fn := runtime.FuncForPC(pc) - if fn == nil { - return "unknown" - } - return fn.Name() -} - -func fileLine(f errors.Frame) (string, int) { - // this looks a bit like magic, but follows example here: - // https://github.com/pkg/errors/blob/v0.8.1/stack.go#L14-L27 - // as this is where we get the Frames - pc := uintptr(f) - 1 - fn := runtime.FuncForPC(pc) - if fn == nil { - return "unknown", 0 - } - return fn.FileLine(pc) -} - -func trimInternal(st errors.StackTrace) errors.StackTrace { - // trim our internal parts here - // manual error creation, or runtime for caught panics - for matchesFunc(st[0], - // where we create errors - "cosmossdk.io/errors.Wrap", - "cosmossdk.io/errors.Wrapf", - "cosmossdk.io/errors.WithType", - // runtime are added on panics - "runtime.", - // _test is defined in coverage tests, causing failure - // "/_test/" - ) { - st = st[1:] - } - // trim out outer wrappers (runtime.goexit and test library if present) - for l := len(st) - 1; l > 0 && matchesFunc(st[l], "runtime.", "testing."); l-- { - st = st[:l] - } - return st -} - -func writeSimpleFrame(s io.Writer, f errors.Frame) { - file, line := fileLine(f) - // cut file at "github.com/" - // TODO: generalize better for other hosts? - chunks := strings.SplitN(file, "github.com/", 2) - if len(chunks) == 2 { - file = chunks[1] - } - _, _ = fmt.Fprintf(s, " [%s:%d]", file, line) -} - -// Format works like pkg/errors, with additions. -// %s is just the error message -// %+v is the full stack trace -// %v appends a compressed [filename:line] where the error was created -// -// Inspired by https://github.com/pkg/errors/blob/v0.8.1/errors.go#L162-L176 -func (e *wrappedError) Format(s fmt.State, verb rune) { - // normal output here.... - if verb != 'v' { - _, _ = fmt.Fprint(s, e.Error()) - return - } - // work with the stack trace... whole or part - stack := trimInternal(stackTrace(e)) - if s.Flag('+') { - _, _ = fmt.Fprintf(s, "%+v\n", stack) - _, _ = fmt.Fprint(s, e.Error()) - } else { - _, _ = fmt.Fprint(s, e.Error()) - writeSimpleFrame(s, stack[0]) - } -} - -// stackTrace returns the first found stack trace frame carried by given error -// or any wrapped error. It returns nil if no stack trace is found. -func stackTrace(err error) errors.StackTrace { - type stackTracer interface { - StackTrace() errors.StackTrace - } - - for { - if st, ok := err.(stackTracer); ok { - return st.StackTrace() - } - - if c, ok := err.(causer); ok { - err = c.Cause() - } else { - return nil - } - } -} diff --git a/errors/stacktrace_test.go b/errors/stacktrace_test.go deleted file mode 100644 index c0a8d6141c25..000000000000 --- a/errors/stacktrace_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package errors - -import ( - "errors" - "fmt" - "reflect" - "strings" -) - -func (s *errorsTestSuite) TestStackTrace() { - cases := map[string]struct { - err error - wantError string - }{ - "New gives us a stacktrace": { - err: Wrap(ErrNoSignatures, "name"), - wantError: "name: no signatures supplied", - }, - "Wrapping stderr gives us a stacktrace": { - err: Wrap(fmt.Errorf("foo"), "standard"), - wantError: "standard: foo", - }, - "Wrapping pkg/errors gives us clean stacktrace": { - err: Wrap(errors.New("bar"), "pkg"), - wantError: "pkg: bar", - }, - "Wrapping inside another function is still clean": { - err: Wrap(fmt.Errorf("indirect"), "do the do"), - wantError: "do the do: indirect", - }, - } - - // Wrapping code is unwanted in the errors stack trace. - unwantedSrc := []string{ - "cosmossdk.io/errors.Wrap\n", - "cosmossdk.io/errors.Wrapf\n", - "runtime.goexit\n", - } - const thisTestSrc = "errors/stacktrace_test.go" - - for _, tc := range cases { - s.Require().True(reflect.DeepEqual(tc.err.Error(), tc.wantError)) - s.Require().NotNil(stackTrace(tc.err)) - fullStack := fmt.Sprintf("%+v", tc.err) - s.Require().True(strings.Contains(fullStack, thisTestSrc)) - s.Require().True(strings.Contains(fullStack, tc.wantError)) - - for _, src := range unwantedSrc { - if strings.Contains(fullStack, src) { - s.T().Logf("Stack trace below\n----%s\n----", fullStack) - s.T().Logf("full stack contains unwanted source file path: %q", src) - } - } - - tinyStack := fmt.Sprintf("%v", tc.err) - s.Require().True(strings.HasPrefix(tinyStack, tc.wantError)) - s.Require().False(strings.Contains(tinyStack, "\n")) - // contains a link to where it was created, which must - // be here, not the Wrap() function - s.Require().True(strings.Contains(tinyStack, thisTestSrc)) - } -} From 4cbf2544e8c6d581a1af6f5ad691cd01a7aa0232 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 5 Jun 2024 09:40:21 +0200 Subject: [PATCH 02/12] more changes --- errors/abci_test.go | 49 ------------------------------------------- errors/errors_test.go | 5 ----- errors/go.mod | 2 +- 3 files changed, 1 insertion(+), 55 deletions(-) diff --git a/errors/abci_test.go b/errors/abci_test.go index 4c693eeae358..e72626d3a845 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -3,7 +3,6 @@ package errors import ( "fmt" "io" - "strings" "testing" ) @@ -90,54 +89,6 @@ func TestABCInfo(t *testing.T) { } } -func TestABCIInfoStacktrace(t *testing.T) { - cases := map[string]struct { - err error - debug bool - wantStacktrace bool - wantErrMsg string - }{ - "wrapped SDK error in debug mode provides stacktrace": { - err: Wrap(ErrUnauthorized, "wrapped"), - debug: true, - wantStacktrace: true, - wantErrMsg: "wrapped: unauthorized", - }, - "wrapped SDK error in non-debug mode does not have stacktrace": { - err: Wrap(ErrUnauthorized, "wrapped"), - debug: false, - wantStacktrace: false, - wantErrMsg: "wrapped: unauthorized", - }, - "wrapped stdlib error in debug mode provides stacktrace": { - err: Wrap(fmt.Errorf("stdlib"), "wrapped"), - debug: true, - wantStacktrace: true, - wantErrMsg: "wrapped: stdlib", - }, - } - - const thisTestSrc = "cosmossdk.io/errors.(*abciTestSuite).TestABCIInfoStacktrace" - - for testName, tc := range cases { - t.Run(testName, func(t *testing.T) { - _, _, log := ABCIInfo(tc.err, tc.debug) - if !tc.wantStacktrace { - if log != tc.wantErrMsg { - t.Errorf("%s: expected log %s, got %s", testName, tc.wantErrMsg, log) - } - } else { - if !strings.Contains(log, thisTestSrc) { - t.Errorf("%s: expected log to contain %s", testName, thisTestSrc) - } - if !strings.Contains(log, tc.wantErrMsg) { - t.Errorf("%s: expected log to contain %s", testName, tc.wantErrMsg) - } - } - }) - } -} - func TestABCIInfoHidesStacktrace(t *testing.T) { err := Wrap(ErrUnauthorized, "wrapped") _, _, log := ABCIInfo(err, false) diff --git a/errors/errors_test.go b/errors/errors_test.go index b71910eaa09a..a3f87f581c85 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -70,11 +70,6 @@ func TestWrappedIs(t *testing.T) { t.Errorf("expected error to be of type ErrTxTooLarge") } - err = Wrap(ErrInsufficientFee, "...") - if !errors.Is(err, ErrTxTooLarge) { - t.Errorf("expected error to be of type ErrTxTooLarge") - } - errs := errors.New("other") if !errors.Is(errs, errs) { t.Errorf("error should match itself") diff --git a/errors/go.mod b/errors/go.mod index afcb133dd2b3..20ae1c3f4a92 100644 --- a/errors/go.mod +++ b/errors/go.mod @@ -1,3 +1,3 @@ -module cosmossdk.io/errors +module cosmossdk.io/errors/v2 go 1.20 From 72e4bd8639a593ed86fedb6df4f61830cfc54e5f Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 5 Jun 2024 09:55:33 +0200 Subject: [PATCH 03/12] remove wrap errors in favour of fmt.Errorf --- errors/abci.go | 6 ++ errors/abci_test.go | 8 +-- errors/errors.go | 94 +----------------------------- errors/errors_test.go | 129 ------------------------------------------ 4 files changed, 12 insertions(+), 225 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index 603f3b36ff1c..19d043715624 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -110,3 +110,9 @@ func errIsNil(err error) bool { } return false } + +// causer is an interface implemented by an error that supports wrapping. Use +// it to test if an error wraps another error instance. +type causer interface { + Cause() error +} diff --git a/errors/abci_test.go b/errors/abci_test.go index e72626d3a845..ffa5d63edf31 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -22,7 +22,7 @@ func TestABCInfo(t *testing.T) { wantSpace: testCodespace, }, "wrapped SDK error": { - err: Wrap(Wrap(ErrUnauthorized, "foo"), "bar"), + err: fmt.Errorf("bar: %w", fmt.Errorf("foo: %w", ErrUnauthorized)), debug: false, wantLog: "bar: foo: unauthorized", wantCode: ErrUnauthorized.code, @@ -90,7 +90,7 @@ func TestABCInfo(t *testing.T) { } func TestABCIInfoHidesStacktrace(t *testing.T) { - err := Wrap(ErrUnauthorized, "wrapped") + err := fmt.Errorf("wrapped: %w", ErrUnauthorized) _, _, log := ABCIInfo(err, false) if log != "wrapped: unauthorized" { t.Errorf("expected log %s, got %s", "wrapped: unauthorized", log) @@ -100,8 +100,8 @@ func TestABCIInfoHidesStacktrace(t *testing.T) { func TestABCIInfoSerializeErr(t *testing.T) { var ( // Create errors with stacktrace for equal comparison. - myErrDecode = Wrap(ErrTxDecode, "test") - myErrAddr = Wrap(ErrInvalidAddress, "tester") + myErrDecode = fmt.Errorf("test, %w", ErrTxDecode) + myErrAddr = fmt.Errorf("tester, %w", ErrInvalidAddress) myPanic = ErrPanic ) diff --git a/errors/errors.go b/errors/errors.go index 9b1b2f382553..8528b8f1e8ac 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -1,7 +1,6 @@ package errors import ( - "errors" "fmt" ) @@ -61,11 +60,11 @@ func setUsed(err *Error) { // The server (abci app / blockchain) should only refer to registered errors func ABCIError(codespace string, code uint32, log string) error { if e := getUsed(codespace, code); e != nil { - return Wrap(e, log) + return fmt.Errorf("%s: %w", log, e) } // This is a unique error, will never match on .Is() // Use Wrap here to get a stack trace - return Wrap(&Error{codespace: codespace, code: code, desc: "unknown"}, log) + return fmt.Errorf("%s: %w", log, &Error{codespace: codespace, code: code, desc: "unknown"}) } // Error represents a root error. @@ -99,92 +98,3 @@ func (e Error) ABCICode() uint32 { func (e Error) Codespace() string { return e.codespace } - -// Wrap extends this error with an additional information. -// It's a handy function to call Wrap with sdk errors. -func (e *Error) Wrap(desc string) error { return Wrap(e, desc) } - -// Wrapf extends this error with an additional information. -// It's a handy function to call Wrapf with sdk errors. -func (e *Error) Wrapf(desc string, args ...interface{}) error { return Wrapf(e, desc, args...) } - -// Wrap extends given error with an additional information. -// -// If the wrapped error does not provide ABCICode method (ie. stdlib errors), -// it will be labeled as internal error. -// -// If err is nil, this returns nil, avoiding the need for an if statement when -// wrapping a error returned at the end of a function -func Wrap(err error, description string) error { - if err == nil { - return nil - } - - return &wrappedError{ - parent: err, - msg: description, - } -} - -// Wrapf extends given error with an additional information. -// -// This function works like Wrap function with additional functionality of -// formatting the input as specified. -func Wrapf(err error, format string, args ...interface{}) error { - desc := fmt.Sprintf(format, args...) - return Wrap(err, desc) -} - -type wrappedError struct { - // This error layer description. - msg string - // The underlying error that triggered this one. - parent error -} - -func (e *wrappedError) Error() string { - return fmt.Sprintf("%s: %s", e.msg, e.parent.Error()) -} - -func (e *wrappedError) Cause() error { - return e.parent -} - -// Unwrap implements the built-in errors.Unwrap -func (e *wrappedError) Unwrap() error { - return e.parent -} - -// Recover captures a panic and stop its propagation. If panic happens it is -// transformed into a ErrPanic instance and assigned to given error. Call this -// function using defer in order to work as expected. -func Recover(err *error) { - if r := recover(); r != nil { - *err = Wrapf(ErrPanic, "%v", r) - } -} - -// WithType is a helper to augment an error with a corresponding type message -func WithType(err error, obj interface{}) error { - return Wrap(err, fmt.Sprintf("%T", obj)) -} - -// IsOf checks if a received error is caused by one of the target errors. -// It extends the errors.Is functionality to a list of errors. -func IsOf(received error, targets ...error) bool { - if received == nil { - return false - } - for _, t := range targets { - if errors.Is(received, t) { - return true - } - } - return false -} - -// causer is an interface implemented by an error that supports wrapping. Use -// it to test if an error wraps another error instance. -type causer interface { - Cause() error -} diff --git a/errors/errors_test.go b/errors/errors_test.go index a3f87f581c85..ce146ea412f0 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -1,138 +1,9 @@ package errors import ( - "errors" "testing" ) -func TestIsOf(t *testing.T) { - var errNil *Error - err := ErrInvalidAddress - errW := Wrap(ErrLogic, "more info") - - if IsOf(nil) { - t.Errorf("nil should always have no causer") - } - if IsOf(nil, err) { - t.Errorf("nil should always have no causer") - } - if IsOf(errNil) { - t.Errorf("nil error should always have no causer") - } - if IsOf(errNil, err) { - t.Errorf("nil error should always have no causer") - } - - if IsOf(err) { - t.Errorf("error should always have no causer") - } - if IsOf(err, nil) { - t.Errorf("error should always have no causer") - } - if IsOf(err, ErrLogic) { - t.Errorf("error should always have no causer") - } - - if !IsOf(errW, ErrLogic) { - t.Errorf("error should have causer") - } - if !IsOf(errW, err, ErrLogic) { - t.Errorf("error should have causer") - } - if !IsOf(errW, nil, errW) { - t.Errorf("error should much itself") - } - err2 := errors.New("other error") - if !IsOf(err2, nil, err2) { - t.Errorf("error should much itself") - } -} - -func TestWrapEmpty(t *testing.T) { - if err := Wrap(nil, "wrapping "); err != nil { - t.Errorf("expected nil error, got: %v", err) - } -} - -func TestWrappedIs(t *testing.T) { - err := Wrap(ErrTxTooLarge, "context") - if !errors.Is(err, ErrTxTooLarge) { - t.Errorf("expected error to be of type ErrTxTooLarge") - } - - err = Wrap(err, "more context") - if !errors.Is(err, ErrTxTooLarge) { - t.Errorf("expected error to be of type ErrTxTooLarge") - } - - err = Wrap(err, "even more context") - if !errors.Is(err, ErrTxTooLarge) { - t.Errorf("expected error to be of type ErrTxTooLarge") - } - - errs := errors.New("other") - if !errors.Is(errs, errs) { - t.Errorf("error should match itself") - } - - if !errors.Is(ErrInsufficientFee.Wrap("wrapped"), ErrInsufficientFee) { - t.Errorf("expected error to be of type ErrInsufficientFee") - } - if !IsOf(ErrInsufficientFee.Wrap("wrapped"), ErrInsufficientFee) { - t.Errorf("expected error to be of type ErrInsufficientFee") - } - if !errors.Is(ErrInsufficientFee.Wrapf("wrapped"), ErrInsufficientFee) { - t.Errorf("expected error to be of type ErrInsufficientFee") - } - if !IsOf(ErrInsufficientFee.Wrapf("wrapped"), ErrInsufficientFee) { - t.Errorf("expected error to be of type ErrInsufficientFee") - } -} - -func TestWrappedIsMultiple(t *testing.T) { - errTest := errors.New("test error") - errTest2 := errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - if !errors.Is(err, errTest2) { - t.Errorf("expected error to be of type errTest2") - } -} - -func TestWrappedIsFail(t *testing.T) { - errTest := errors.New("test error") - errTest2 := errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - if errors.Is(err, errTest) { - t.Errorf("expected error to not be of type errTest") - } -} - -func TestWrappedUnwrap(t *testing.T) { - errTest := errors.New("test error") - err := Wrap(errTest, "some random description") - if unwrappedErr := errors.Unwrap(err); errors.Is(unwrappedErr, errTest) { - t.Errorf("expected unwrapped error to be %v, got %v", errTest, unwrappedErr) - } -} - -func TestWrappedUnwrapMultiple(t *testing.T) { - errTest := errors.New("test error") - errTest2 := errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - if unwrappedErr := errors.Unwrap(err); errors.Is(unwrappedErr, errTest2) { - t.Errorf("expected unwrapped error to be %v, got %v", errTest2, unwrappedErr) - } -} - -func TestWrappedUnwrapFail(t *testing.T) { - errTest := errors.New("test error") - errTest2 := errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - if errors.Is(errTest, errors.Unwrap(err)) { - t.Errorf("expected unwrapped error to not be %v", errTest) - } -} - func TestABCIError(t *testing.T) { if err := ABCIError(testCodespace, 2, "custom"); err.Error() != "custom: tx parse error" { t.Errorf("expected error message: custom: tx parse error, got: %v", err.Error()) From 6e065b3f63afdd03417e0d9dd718fc6493cf7e50 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 5 Jun 2024 12:58:10 +0200 Subject: [PATCH 04/12] minor cleanup --- errors/abci.go | 27 +++++++-------------------- errors/abci_test.go | 10 ++++------ 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index 19d043715624..cf3b0aad7b41 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -1,6 +1,7 @@ package errors import ( + "errors" "fmt" "reflect" ) @@ -46,10 +47,6 @@ func defaultErrEncoder(err error) string { return err.Error() } -type coder interface { - ABCICode() uint32 -} - // abciCode tests if given error contains an ABCI code and returns the value of // it if available. This function is testing for the causer interface as well // and unwraps the error. @@ -59,22 +56,15 @@ func abciCode(err error) uint32 { } for { - if c, ok := err.(coder); ok { - return c.ABCICode() - } - - if c, ok := err.(causer); ok { - err = c.Cause() + var customErr *Error + if errors.As(err, &customErr) { + return customErr.ABCICode() } else { return internalABCICode } } } -type codespacer interface { - Codespace() string -} - // abciCodespace tests if given error contains a codespace and returns the value of // it if available. This function is testing for the causer interface as well // and unwraps the error. @@ -84,12 +74,9 @@ func abciCodespace(err error) string { } for { - if c, ok := err.(codespacer); ok { - return c.Codespace() - } - - if c, ok := err.(causer); ok { - err = c.Cause() + var customErr *Error + if errors.As(err, &customErr) { + return customErr.Codespace() } else { return internalABCICodespace } diff --git a/errors/abci_test.go b/errors/abci_test.go index ffa5d63edf31..63edaae8d807 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -49,13 +49,11 @@ func TestABCInfo(t *testing.T) { wantCode: 1, wantSpace: UndefinedCodespace, }, - // This is hard to test because of attached stacktrace. This - // case is tested in an another test. // "wrapped stdlib is a full message in debug mode": { - // err: Wrap(io.EOF, "cannot read file"), - // debug: true, - // wantLog: "cannot read file: EOF", - // wantCode: 1, + // err: fmt.Errorf("cannot read file: %w", io.EOF), + // debug: true, + // wantLog: "cannot read file: EOF", + // wantCode: 1, // }, "custom error": { err: customErr{}, From afbc2b2dac80bcb3ed288a42e4994221b988740a Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 5 Jun 2024 13:03:24 +0200 Subject: [PATCH 05/12] fix test --- errors/abci_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/errors/abci_test.go b/errors/abci_test.go index 63edaae8d807..ddf5b4e5f434 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -98,8 +98,8 @@ func TestABCIInfoHidesStacktrace(t *testing.T) { func TestABCIInfoSerializeErr(t *testing.T) { var ( // Create errors with stacktrace for equal comparison. - myErrDecode = fmt.Errorf("test, %w", ErrTxDecode) - myErrAddr = fmt.Errorf("tester, %w", ErrInvalidAddress) + myErrDecode = fmt.Errorf("test: %w", ErrTxDecode) + myErrAddr = fmt.Errorf("tester: %w", ErrInvalidAddress) myPanic = ErrPanic ) From e64794a1169c3a1777d5ebf56824f2a080d1071f Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Wed, 5 Jun 2024 15:50:25 +0200 Subject: [PATCH 06/12] remove invalid test cases --- errors/abci_test.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/errors/abci_test.go b/errors/abci_test.go index ddf5b4e5f434..cd276cf3e977 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -49,26 +49,6 @@ func TestABCInfo(t *testing.T) { wantCode: 1, wantSpace: UndefinedCodespace, }, - // "wrapped stdlib is a full message in debug mode": { - // err: fmt.Errorf("cannot read file: %w", io.EOF), - // debug: true, - // wantLog: "cannot read file: EOF", - // wantCode: 1, - // }, - "custom error": { - err: customErr{}, - debug: false, - wantLog: "custom", - wantCode: 999, - wantSpace: "extern", - }, - "custom error in debug mode": { - err: customErr{}, - debug: true, - wantLog: "custom", - wantCode: 999, - wantSpace: "extern", - }, } for testName, tc := range cases { From 4e699311e74b9f428d8528b8221531738906ba0c Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Thu, 6 Jun 2024 10:55:10 +0200 Subject: [PATCH 07/12] remove unused interface --- errors/abci.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index cf3b0aad7b41..df534dbfe4eb 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -97,9 +97,3 @@ func errIsNil(err error) bool { } return false } - -// causer is an interface implemented by an error that supports wrapping. Use -// it to test if an error wraps another error instance. -type causer interface { - Cause() error -} From d3bf0d274f10701867c5f749505628eba42ee2c4 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Thu, 6 Jun 2024 10:55:26 +0200 Subject: [PATCH 08/12] remove unused error --- errors/abci_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/errors/abci_test.go b/errors/abci_test.go index cd276cf3e977..3bdc2b34e2c2 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -117,13 +117,3 @@ func TestABCIInfoSerializeErr(t *testing.T) { } } } - -// customErr is a custom implementation of an error that provides an ABCICode -// method. -type customErr struct{} - -func (customErr) Codespace() string { return "extern" } - -func (customErr) ABCICode() uint32 { return 999 } - -func (customErr) Error() string { return "custom" } From a065237ccc9bba548ef23d1c51cdd1d97ff1e918 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Thu, 6 Jun 2024 19:22:48 +0200 Subject: [PATCH 09/12] address comments --- errors/abci_test.go | 10 +--------- errors/errors.go | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/errors/abci_test.go b/errors/abci_test.go index 3bdc2b34e2c2..ca283997ae91 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -67,17 +67,9 @@ func TestABCInfo(t *testing.T) { } } -func TestABCIInfoHidesStacktrace(t *testing.T) { - err := fmt.Errorf("wrapped: %w", ErrUnauthorized) - _, _, log := ABCIInfo(err, false) - if log != "wrapped: unauthorized" { - t.Errorf("expected log %s, got %s", "wrapped: unauthorized", log) - } -} - func TestABCIInfoSerializeErr(t *testing.T) { var ( - // Create errors with stacktrace for equal comparison. + // Create errors for equal comparison. myErrDecode = fmt.Errorf("test: %w", ErrTxDecode) myErrAddr = fmt.Errorf("tester: %w", ErrInvalidAddress) myPanic = ErrPanic diff --git a/errors/errors.go b/errors/errors.go index 8528b8f1e8ac..1255e88003de 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -98,3 +98,27 @@ func (e Error) ABCICode() uint32 { func (e Error) Codespace() string { return e.codespace } + +// Wrap extends given error with an additional information. +// +// If the wrapped error does not provide ABCICode method (ie. stdlib errors), +// it will be labeled as internal error. +// +// If err is nil, this returns nil, avoiding the need for an if statement when +// wrapping a error returned at the end of a function +func Wrap(err error, description string) error { + if err == nil { + return nil + } + + return fmt.Errorf("%s: %w", description, err) +} + +// Wrapf extends given error with an additional information. +// +// This function works like Wrap function with additional functionality of +// formatting the input as specified. +func Wrapf(err error, format string, args ...interface{}) error { + desc := fmt.Sprintf(format, args...) + return Wrap(err, desc) +} From 98eef6d22f852140a09536d56fe2993a4f1c2451 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Thu, 6 Jun 2024 19:27:00 +0200 Subject: [PATCH 10/12] address comments --- errors/abci.go | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index df534dbfe4eb..4fa5b959dbee 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -35,7 +35,8 @@ func ABCIInfo(err error, debug bool) (codespace string, code uint32, log string) encode = debugErrEncoder } - return abciCodespace(err), abciCode(err), encode(err) + code, space := abciInfo(err) + return space, code, encode(err) } // The debugErrEncoder encodes the error with a stacktrace. @@ -47,40 +48,25 @@ func defaultErrEncoder(err error) string { return err.Error() } -// abciCode tests if given error contains an ABCI code and returns the value of +// abciInfo tests if given error contains an ABCI code and returns the value of // it if available. This function is testing for the causer interface as well // and unwraps the error. -func abciCode(err error) uint32 { +func abciInfo(err error) (code uint32, codespace string) { if errIsNil(err) { - return SuccessABCICode + return SuccessABCICode, "" } - for { - var customErr *Error - if errors.As(err, &customErr) { - return customErr.ABCICode() - } else { - return internalABCICode - } - } -} + var customErr *Error -// abciCodespace tests if given error contains a codespace and returns the value of -// it if available. This function is testing for the causer interface as well -// and unwraps the error. -func abciCodespace(err error) string { - if errIsNil(err) { - return "" + if errors.As(err, &customErr) { + code = customErr.ABCICode() + codespace = customErr.Codespace() + } else { + code = internalABCICode + codespace = internalABCICodespace } - for { - var customErr *Error - if errors.As(err, &customErr) { - return customErr.Codespace() - } else { - return internalABCICodespace - } - } + return } // errIsNil returns true if value represented by the given error is nil. From 5afda00eab60385f5b887129d241bd02bbffe583 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Tue, 11 Jun 2024 11:12:37 +0200 Subject: [PATCH 11/12] changelog entry --- errors/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/errors/CHANGELOG.md b/errors/CHANGELOG.md index 0135befe9a73..9678589ad8c2 100644 --- a/errors/CHANGELOG.md +++ b/errors/CHANGELOG.md @@ -34,6 +34,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking * [#20402](https://github.com/cosmos/cosmos-sdk/pull/20402) Remove Grpc error codes from the error package. This is done in order to keep the dependency graph of errors minimal +* [#20539](https://github.com/cosmos/cosmos-sdk/pull/20539) v2 errors removes `IsOf`, `Recover`, `WithType` and wrapped error. The errors package uses the go std library errors. It provides a `Wrap` and `Wrapf` to help in the migration from v1 to v2. ## [v1.0.1](https://github.com/cosmos/cosmos-sdk/releases/tag/errors%2Fv1.0.1) From 9fdbb30ced4bc447f4fe716fee577f11c928806c Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Tue, 11 Jun 2024 11:54:50 +0200 Subject: [PATCH 12/12] remove handle.go --- errors/handle.go | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 errors/handle.go diff --git a/errors/handle.go b/errors/handle.go deleted file mode 100644 index 33c3fbfdeac2..000000000000 --- a/errors/handle.go +++ /dev/null @@ -1,12 +0,0 @@ -package errors - -import "fmt" - -// AssertNil panics on error -// Should be only used with interface methods, which require return error, but the -// error is always nil -func AssertNil(err error) { - if err != nil { - panic(fmt.Errorf("logic error - this should never happen. %w", err)) - } -}