From c78cab32ef1cf2cf865576c555a0b37ce9f071fe Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Fri, 19 Nov 2021 11:55:33 -0500 Subject: [PATCH 01/11] feat: add errors module --- errors/abci.go | 129 ++++++++++++++++++ errors/abci_test.go | 259 +++++++++++++++++++++++++++++++++++++ errors/doc.go | 34 +++++ errors/errors.go | 266 ++++++++++++++++++++++++++++++++++++++ errors/errors_test.go | 222 +++++++++++++++++++++++++++++++ errors/go.mod | 11 ++ errors/go.sum | 12 ++ errors/handle.go | 12 ++ errors/stacktrace.go | 121 +++++++++++++++++ errors/stacktrace_test.go | 62 +++++++++ 10 files changed, 1128 insertions(+) create mode 100644 errors/abci.go create mode 100644 errors/abci_test.go create mode 100644 errors/doc.go create mode 100644 errors/errors.go create mode 100644 errors/errors_test.go create mode 100644 errors/go.mod create mode 100644 errors/go.sum create mode 100644 errors/handle.go create mode 100644 errors/stacktrace.go create mode 100644 errors/stacktrace_test.go diff --git a/errors/abci.go b/errors/abci.go new file mode 100644 index 000000000000..8e33347fc5ec --- /dev/null +++ b/errors/abci.go @@ -0,0 +1,129 @@ +package errors + +import ( + "fmt" + "reflect" +) + +const ( + // SuccessABCICode declares an ABCI response use 0 to signal that the + // processing was successful and no error is returned. + SuccessABCICode uint32 = 0 + + // All unclassified errors that do not provide an ABCI code are clubbed + // under an internal error code and a generic message instead of + // detailed error string. + internalABCICodespace = UndefinedCodespace + internalABCICode uint32 = 1 +) + +// ABCIInfo returns the ABCI error information as consumed by the tendermint +// client. Returned codespace, code, and log message should be used as a ABCI response. +// Any error that does not provide ABCICode information is categorized as error +// with code 1, codespace UndefinedCodespace +// When not running in a debug mode all messages of errors that do not provide +// ABCICode information are replaced with generic "internal error". Errors +// without an ABCICode information as considered internal. +func ABCIInfo(err error, debug bool) (codespace string, code uint32, log string) { + if errIsNil(err) { + return "", SuccessABCICode, "" + } + + encode := defaultErrEncoder + if debug { + encode = debugErrEncoder + } + + return abciCodespace(err), abciCode(err), encode(err) +} + +// The debugErrEncoder encodes the error with a stacktrace. +func debugErrEncoder(err error) string { + return fmt.Sprintf("%+v", err) +} + +// The defaultErrEncoder applies Redact on the error before encoding it with its internal error message. +func defaultErrEncoder(err error) string { + return Redact(err).Error() +} + +type coder interface { + ABCICode() uint32 +} + +// abciCode test 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 { + if errIsNil(err) { + return SuccessABCICode + } + + for { + if c, ok := err.(coder); ok { + return c.ABCICode() + } + + if c, ok := err.(causer); ok { + err = c.Cause() + } 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. +func abciCodespace(err error) string { + if errIsNil(err) { + return "" + } + + for { + if c, ok := err.(codespacer); ok { + return c.Codespace() + } + + if c, ok := err.(causer); ok { + err = c.Cause() + } else { + return internalABCICodespace + } + } +} + +// errIsNil returns true if value represented by the given error is nil. +// +// Most of the time a simple == check is enough. There is a very narrowed +// spectrum of cases (mostly in tests) where a more sophisticated check is +// required. +func errIsNil(err error) bool { + if err == nil { + return true + } + if val := reflect.ValueOf(err); val.Kind() == reflect.Ptr { + return val.IsNil() + } + return false +} + +var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentially sensitive system info") + +// Redact replaces an error that is not initialized as an ABCI Error with a +// generic internal error instance. If the error is an ABCI Error, that error is +// simply returned. +func Redact(err error) error { + if ErrPanic.Is(err) { + return errPanicWithMsg + } + if abciCode(err) == internalABCICode { + return errInternal + } + + return err +} diff --git a/errors/abci_test.go b/errors/abci_test.go new file mode 100644 index 000000000000..02c12e7bbdd6 --- /dev/null +++ b/errors/abci_test.go @@ -0,0 +1,259 @@ +package errors + +import ( + "fmt" + "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() { + cases := map[string]struct { + err error + debug bool + wantCode uint32 + wantSpace string + wantLog string + }{ + "plain SDK error": { + err: ErrUnauthorized, + debug: false, + wantLog: "unauthorized", + wantCode: ErrUnauthorized.code, + wantSpace: RootCodespace, + }, + "wrapped SDK error": { + err: Wrap(Wrap(ErrUnauthorized, "foo"), "bar"), + debug: false, + wantLog: "bar: foo: unauthorized", + wantCode: ErrUnauthorized.code, + wantSpace: RootCodespace, + }, + "nil is empty message": { + err: nil, + debug: false, + wantLog: "", + wantCode: 0, + wantSpace: "", + }, + "nil SDK error is not an error": { + err: (*Error)(nil), + debug: false, + wantLog: "", + wantCode: 0, + wantSpace: "", + }, + "stdlib is generic message": { + err: io.EOF, + debug: false, + wantLog: "internal", + wantCode: 1, + wantSpace: UndefinedCodespace, + }, + "stdlib returns error message in debug mode": { + err: io.EOF, + debug: true, + wantLog: "EOF", + wantCode: 1, + wantSpace: UndefinedCodespace, + }, + "wrapped stdlib is only a generic message": { + err: Wrap(io.EOF, "cannot read file"), + debug: false, + wantLog: "internal", + 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, + //}, + "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 { + 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) + } +} + +func (s *abciTestSuite) TestABCIInfoStacktrace() { + 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", + }, + "wrapped stdlib error in non-debug mode does not have stacktrace": { + err: Wrap(fmt.Errorf("stdlib"), "wrapped"), + debug: false, + wantStacktrace: false, + wantErrMsg: "internal", + }, + } + + const thisTestSrc = "github.com/cosmos/cosmos-sdk/types/errors.(*abciTestSuite).TestABCIInfoStacktrace" + + for testName, tc := range cases { + _, _, log := ABCIInfo(tc.err, tc.debug) + if !tc.wantStacktrace { + s.Require().Equal(tc.wantErrMsg, log, testName) + continue + } + + s.Require().True(strings.Contains(log, thisTestSrc), testName) + s.Require().True(strings.Contains(log, tc.wantErrMsg), testName) + } +} + +func (s *abciTestSuite) TestABCIInfoHidesStacktrace() { + err := Wrap(ErrUnauthorized, "wrapped") + _, _, log := ABCIInfo(err, false) + s.Require().Equal("wrapped: unauthorized", log) +} + +func (s *abciTestSuite) TestRedact() { + cases := map[string]struct { + err error + untouched bool // if true we expect the same error after redact + changed error // if untouched == false, expect this error + }{ + "panic looses message": { + err: Wrap(ErrPanic, "some secret stack trace"), + changed: errPanicWithMsg, + }, + "sdk errors untouched": { + err: Wrap(ErrUnauthorized, "cannot drop db"), + untouched: true, + }, + "pass though custom errors with ABCI code": { + err: customErr{}, + untouched: true, + }, + "redact stdlib error": { + err: fmt.Errorf("stdlib error"), + changed: errInternal, + }, + } + + for name, tc := range cases { + spec := tc + redacted := Redact(spec.err) + if spec.untouched { + s.Require().Equal(spec.err, redacted, name) + continue + } + + // see if we got the expected redact + s.Require().Equal(spec.changed, redacted, name) + // make sure the ABCI code did not change + s.Require().Equal(abciCode(spec.err), abciCode(redacted), name) + + } +} + +func (s *abciTestSuite) TestABCIInfoSerializeErr() { + var ( + // Create errors with stacktrace for equal comparison. + myErrDecode = Wrap(ErrTxDecode, "test") + myErrAddr = Wrap(ErrInvalidAddress, "tester") + myPanic = ErrPanic + ) + + specs := map[string]struct { + src error + debug bool + exp string + }{ + "single error": { + src: myErrDecode, + debug: false, + exp: "test: tx parse error", + }, + "second error": { + src: myErrAddr, + debug: false, + exp: "tester: invalid address", + }, + "single error with debug": { + src: myErrDecode, + debug: true, + exp: fmt.Sprintf("%+v", myErrDecode), + }, + "redact in default encoder": { + src: myPanic, + exp: "panic message redacted to hide potentially sensitive system info: panic", + }, + "do not redact in debug encoder": { + src: myPanic, + debug: true, + exp: fmt.Sprintf("%+v", myPanic), + }, + } + for msg, spec := range specs { + spec := spec + _, _, log := ABCIInfo(spec.src, spec.debug) + s.Require().Equal(spec.exp, log, msg) + } +} + +// 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" } diff --git a/errors/doc.go b/errors/doc.go new file mode 100644 index 000000000000..6cca61580d06 --- /dev/null +++ b/errors/doc.go @@ -0,0 +1,34 @@ +/* +Package errors implements custom error interfaces for cosmos-sdk. + +Error declarations should be generic and cover broad range of cases. Each +returned error instance can wrap a generic error declaration to provide more +details. + +This package provides a broad range of errors declared that fits all common +cases. If an error is very specific for an extension it can be registered outside +of the errors package. If it will be needed my many extensions, please consider +registering it in the errors package. To create a new error instance use Register +function. You must provide a unique, non zero error code and a short description, for example: + + var ErrZeroDivision = errors.Register(9241, "zero division") + +When returning an error, you can attach to it an additional context +information by using Wrap function, for example: + + func safeDiv(val, div int) (int, err) { + if div == 0 { + return 0, errors.Wrapf(ErrZeroDivision, "cannot divide %d", val) + } + return val / div, nil + } + +The first time an error instance is wrapped a stacktrace is attached as well. +Stacktrace information can be printed using %+v and %v formats. + + %s is just the error message + %+v is the full stack trace + %v appends a compressed [filename:line] where the error was created + +*/ +package errors diff --git a/errors/errors.go b/errors/errors.go new file mode 100644 index 000000000000..b9a3c03f6a80 --- /dev/null +++ b/errors/errors.go @@ -0,0 +1,266 @@ +package errors + +import ( + "fmt" + "reflect" + + "github.com/pkg/errors" +) + +// UndefinedCodespace when we explicitly declare no codespace +const UndefinedCodespace = "undefined" + +var ( + // errInternal should never be exposed, but we reserve this code for non-specified errors + errInternal = Register(UndefinedCodespace, 1, "internal") + + // ErrPanic is only set when we recover from a panic, so we know to + // redact potentially sensitive system info + ErrPanic = Register(UndefinedCodespace, 111222, "panic") +) + +// Register returns an error instance that should be used as the base for +// creating error instances during runtime. +// +// Popular root errors are declared in this package, but extensions may want to +// declare custom codes. This function ensures that no error code is used +// twice. Attempt to reuse an error code results in panic. +// +// Use this function only during a program startup phase. +func Register(codespace string, code uint32, description string) *Error { + // TODO - uniqueness is (codespace, code) combo + if e := getUsed(codespace, code); e != nil { + panic(fmt.Sprintf("error with code %d is already registered: %q", code, e.desc)) + } + + err := New(codespace, code, description) + setUsed(err) + + return err +} + +// usedCodes is keeping track of used codes to ensure their uniqueness. No two +// error instances should share the same (codespace, code) tuple. +var usedCodes = map[string]*Error{} + +func errorID(codespace string, code uint32) string { + return fmt.Sprintf("%s:%d", codespace, code) +} + +func getUsed(codespace string, code uint32) *Error { + return usedCodes[errorID(codespace, code)] +} + +func setUsed(err *Error) { + usedCodes[errorID(err.codespace, err.code)] = err +} + +// ABCIError will resolve an error code/log from an abci result into +// an error message. If the code is registered, it will map it back to +// the canonical error, so we can do eg. ErrNotFound.Is(err) on something +// we get back from an external API. +// +// This should *only* be used in clients, not in the server side. +// 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) + } + // This is a unique error, will never match on .Is() + // Use Wrap here to get a stack trace + return Wrap(New(codespace, code, "unknown"), log) +} + +// Error represents a root error. +// +// Weave framework is using root error to categorize issues. Each instance +// created during the runtime should wrap one of the declared root errors. This +// allows error tests and returning all errors to the client in a safe manner. +// +// All popular root errors are declared in this package. If an extension has to +// declare a custom root error, always use Register function to ensure +// error code uniqueness. +type Error struct { + codespace string + code uint32 + desc string +} + +func New(codespace string, code uint32, desc string) *Error { + return &Error{codespace: codespace, code: code, desc: desc} +} + +func (e Error) Error() string { + return e.desc +} + +func (e Error) ABCICode() uint32 { + return e.code +} + +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) } + +// 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...) } + +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), +// 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 + } + + // 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, + } +} + +// 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 +} + +// 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 +} + +// 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 { + 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 +} + +type unpacker interface { + Unpack() []error +} diff --git a/errors/errors_test.go b/errors/errors_test.go new file mode 100644 index 000000000000..e0c3f11f6625 --- /dev/null +++ b/errors/errors_test.go @@ -0,0 +1,222 @@ +package errors + +import ( + stdlib "errors" + "fmt" + "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: errors.Wrap(ErrUnauthorized, "gone"), + wantIs: true, + }, + "unsuccessful comparison to a wrapped error": { + a: ErrUnauthorized, + b: errors.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: errors.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() + + var errNil *Error + var err = ErrInvalidAddress + var errW = Wrap(ErrLogic, "more info") + + require.False(IsOf(errNil), "nil error should always have no causer") + require.False(IsOf(errNil, err), "nil error should always have no causer") + + require.False(IsOf(err)) + require.False(IsOf(err, nil)) + require.False(IsOf(err, ErrLogic)) + + require.True(IsOf(errW, ErrLogic)) + require.True(IsOf(errW, err, ErrLogic)) + require.True(IsOf(errW, nil, errW), "error should much itself") + var 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" +} + +func (s *errorsTestSuite) TestWrapEmpty() { + s.Require().Nil(Wrap(nil, "wrapping ")) +} + +func (s *errorsTestSuite) TestWrappedIs() { + require := s.Require() + err := Wrap(ErrTxTooLarge, "context") + require.True(stdlib.Is(err, ErrTxTooLarge)) + + err = Wrap(err, "more context") + require.True(stdlib.Is(err, ErrTxTooLarge)) + + err = Wrap(err, "even more context") + require.True(stdlib.Is(err, ErrTxTooLarge)) + + err = Wrap(ErrInsufficientFee, "...") + require.False(stdlib.Is(err, ErrTxTooLarge)) + + errs := stdlib.New("other") + require.True(stdlib.Is(errs, errs)) + + errw := &wrappedError{"msg", errs} + require.True(errw.Is(errw), "should match itself") +} + +func (s *errorsTestSuite) TestWrappedIsMultiple() { + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") + err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) + s.Require().True(stdlib.Is(err, errTest2)) +} + +func (s *errorsTestSuite) TestWrappedIsFail() { + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") + err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) + s.Require().False(stdlib.Is(err, errTest)) +} + +func (s *errorsTestSuite) TestWrappedUnwrap() { + var errTest = errors.New("test error") + err := Wrap(errTest, "some random description") + s.Require().Equal(errTest, stdlib.Unwrap(err)) +} + +func (s *errorsTestSuite) TestWrappedUnwrapMultiple() { + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") + err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) + s.Require().Equal(errTest2, stdlib.Unwrap(err)) +} + +func (s *errorsTestSuite) TestWrappedUnwrapFail() { + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") + err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) + s.Require().NotEqual(errTest, stdlib.Unwrap(err)) +} + +func (s *errorsTestSuite) TestABCIError() { + s.Require().Equal("custom: tx parse error", ABCIError(RootCodespace, 2, "custom").Error()) + s.Require().Equal("custom: unknown", ABCIError("unknown", 1, "custom").Error()) +} + +func ExampleWrap() { + err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") + err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") + fmt.Println(err1.Error()) + fmt.Println(err2.Error()) + // Output: + // 90 is smaller than 100: insufficient funds + // 90 is smaller than 100: insufficient funds +} + +func ExampleWrapf() { + err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") + err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") + fmt.Println(err1.Error()) + fmt.Println(err2.Error()) + // Output: + // 90 is smaller than 100: insufficient funds + // 90 is smaller than 100: insufficient funds +} diff --git a/errors/go.mod b/errors/go.mod new file mode 100644 index 000000000000..be0e25d7a1df --- /dev/null +++ b/errors/go.mod @@ -0,0 +1,11 @@ +module errors + +go 1.17 + +require ( + github.com/davecgh/go-spew v1.1.0 // indirect + github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/testify v1.7.0 // indirect + gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect +) diff --git a/errors/go.sum b/errors/go.sum new file mode 100644 index 000000000000..c2a26e73bf7d --- /dev/null +++ b/errors/go.sum @@ -0,0 +1,12 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +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/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/errors/handle.go b/errors/handle.go new file mode 100644 index 000000000000..33c3fbfdeac2 --- /dev/null +++ b/errors/handle.go @@ -0,0 +1,12 @@ +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)) + } +} diff --git a/errors/stacktrace.go b/errors/stacktrace.go new file mode 100644 index 000000000000..f7079c56d831 --- /dev/null +++ b/errors/stacktrace.go @@ -0,0 +1,121 @@ +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 + "github.com/cosmos/cosmos-sdk/types/errors.Wrap", + "github.com/cosmos/cosmos-sdk/types/errors.Wrapf", + "github.com/cosmos/cosmos-sdk/types/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 new file mode 100644 index 000000000000..06eb829f0f44 --- /dev/null +++ b/errors/stacktrace_test.go @@ -0,0 +1,62 @@ +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{ + "github.com/cosmos/cosmos-sdk/types/errors.Wrap\n", + "github.com/cosmos/cosmos-sdk/types/errors.Wrapf\n", + "runtime.goexit\n", + } + const thisTestSrc = "types/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 2ff010404a33c95b6435de17042f2e67eb6cc86d Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 23 Nov 2021 21:10:12 -0500 Subject: [PATCH 02/11] add test errors --- errors/abci.go | 2 +- errors/abci_test.go | 30 +++++++++++++++--------------- errors/errors.go | 2 +- errors/errors_test.go | 28 ++++++++++++++-------------- errors/stacktrace.go | 12 ++++++------ errors/testerrors.go | 35 +++++++++++++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 37 deletions(-) create mode 100644 errors/testerrors.go diff --git a/errors/abci.go b/errors/abci.go index 8e33347fc5ec..111e68a6db2d 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -51,7 +51,7 @@ type coder interface { ABCICode() uint32 } -// abciCode test if given error contains an ABCI code and returns the value of +// abciCode testCodespace 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 { diff --git a/errors/abci_test.go b/errors/abci_test.go index 02c12e7bbdd6..b8aaed736ca9 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -30,18 +30,18 @@ func (s *abciTestSuite) TestABCInfo() { wantLog string }{ "plain SDK error": { - err: ErrUnauthorized, + err: errUnauthorized, debug: false, wantLog: "unauthorized", - wantCode: ErrUnauthorized.code, - wantSpace: RootCodespace, + wantCode: errUnauthorized.code, + wantSpace: testCodespace, }, "wrapped SDK error": { - err: Wrap(Wrap(ErrUnauthorized, "foo"), "bar"), + err: Wrap(Wrap(errUnauthorized, "foo"), "bar"), debug: false, wantLog: "bar: foo: unauthorized", - wantCode: ErrUnauthorized.code, - wantSpace: RootCodespace, + wantCode: errUnauthorized.code, + wantSpace: testCodespace, }, "nil is empty message": { err: nil, @@ -78,8 +78,8 @@ func (s *abciTestSuite) TestABCInfo() { wantCode: 1, wantSpace: UndefinedCodespace, }, - // This is hard to test because of attached stacktrace. This - // case is tested in an another test. + // This is hard to testCodespace because of attached stacktrace. This + // case is tested in an another testCodespace. //"wrapped stdlib is a full message in debug mode": { // err: Wrap(io.EOF, "cannot read file"), // debug: true, @@ -118,13 +118,13 @@ func (s *abciTestSuite) TestABCIInfoStacktrace() { wantErrMsg string }{ "wrapped SDK error in debug mode provides stacktrace": { - err: Wrap(ErrUnauthorized, "wrapped"), + 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"), + err: Wrap(errUnauthorized, "wrapped"), debug: false, wantStacktrace: false, wantErrMsg: "wrapped: unauthorized", @@ -158,7 +158,7 @@ func (s *abciTestSuite) TestABCIInfoStacktrace() { } func (s *abciTestSuite) TestABCIInfoHidesStacktrace() { - err := Wrap(ErrUnauthorized, "wrapped") + err := Wrap(errUnauthorized, "wrapped") _, _, log := ABCIInfo(err, false) s.Require().Equal("wrapped: unauthorized", log) } @@ -174,7 +174,7 @@ func (s *abciTestSuite) TestRedact() { changed: errPanicWithMsg, }, "sdk errors untouched": { - err: Wrap(ErrUnauthorized, "cannot drop db"), + err: Wrap(errUnauthorized, "cannot drop db"), untouched: true, }, "pass though custom errors with ABCI code": { @@ -206,8 +206,8 @@ func (s *abciTestSuite) TestRedact() { func (s *abciTestSuite) TestABCIInfoSerializeErr() { var ( // Create errors with stacktrace for equal comparison. - myErrDecode = Wrap(ErrTxDecode, "test") - myErrAddr = Wrap(ErrInvalidAddress, "tester") + myErrDecode = Wrap(errTxDecode, "testCodespace") + myErrAddr = Wrap(errInvalidAddress, "tester") myPanic = ErrPanic ) @@ -219,7 +219,7 @@ func (s *abciTestSuite) TestABCIInfoSerializeErr() { "single error": { src: myErrDecode, debug: false, - exp: "test: tx parse error", + exp: "testCodespace: tx parse error", }, "second error": { src: myErrAddr, diff --git a/errors/errors.go b/errors/errors.go index b9a3c03f6a80..aa84ad7fe5a4 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -256,7 +256,7 @@ func IsOf(received error, targets ...error) bool { } // causer is an interface implemented by an error that supports wrapping. Use -// it to test if an error wraps another error instance. +// it to testCodespace 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 e0c3f11f6625..d366819ff167 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -60,7 +60,7 @@ func (s *errorsTestSuite) TestErrorIs() { }, "two different coded errors": { a: ErrUnauthorized, - b: ErrOutOfGas, + b: errOutOfGas, wantIs: false, }, "successful comparison to a wrapped error": { @@ -70,7 +70,7 @@ func (s *errorsTestSuite) TestErrorIs() { }, "unsuccessful comparison to a wrapped error": { a: ErrUnauthorized, - b: errors.Wrap(ErrInsufficientFee, "too big"), + b: errors.Wrap(errInsufficientFee, "too big"), wantIs: false, }, "not equal to stdlib error": { @@ -113,7 +113,7 @@ func (s *errorsTestSuite) TestIsOf() { require := s.Require() var errNil *Error - var err = ErrInvalidAddress + var err = errInvalidAddress var errW = Wrap(ErrLogic, "more info") require.False(IsOf(errNil), "nil error should always have no causer") @@ -152,7 +152,7 @@ func (s *errorsTestSuite) TestWrappedIs() { err = Wrap(err, "even more context") require.True(stdlib.Is(err, ErrTxTooLarge)) - err = Wrap(ErrInsufficientFee, "...") + err = Wrap(errInsufficientFee, "...") require.False(stdlib.Is(err, ErrTxTooLarge)) errs := stdlib.New("other") @@ -163,41 +163,41 @@ func (s *errorsTestSuite) TestWrappedIs() { } func (s *errorsTestSuite) TestWrappedIsMultiple() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") + var errTest = errors.New("testCodespace error") + var errTest2 = errors.New("testCodespace error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().True(stdlib.Is(err, errTest2)) } func (s *errorsTestSuite) TestWrappedIsFail() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") + var errTest = errors.New("testCodespace error") + var errTest2 = errors.New("testCodespace error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().False(stdlib.Is(err, errTest)) } func (s *errorsTestSuite) TestWrappedUnwrap() { - var errTest = errors.New("test error") + var errTest = errors.New("testCodespace error") err := Wrap(errTest, "some random description") s.Require().Equal(errTest, stdlib.Unwrap(err)) } func (s *errorsTestSuite) TestWrappedUnwrapMultiple() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") + var errTest = errors.New("testCodespace error") + var errTest2 = errors.New("testCodespace error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().Equal(errTest2, stdlib.Unwrap(err)) } func (s *errorsTestSuite) TestWrappedUnwrapFail() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") + var errTest = errors.New("testCodespace error") + var errTest2 = errors.New("testCodespace error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().NotEqual(errTest, stdlib.Unwrap(err)) } func (s *errorsTestSuite) TestABCIError() { - s.Require().Equal("custom: tx parse error", ABCIError(RootCodespace, 2, "custom").Error()) + s.Require().Equal("custom: tx parse error", ABCIError(testCodespace, 2, "custom").Error()) s.Require().Equal("custom: unknown", ABCIError("unknown", 1, "custom").Error()) } diff --git a/errors/stacktrace.go b/errors/stacktrace.go index f7079c56d831..180f62c8761d 100644 --- a/errors/stacktrace.go +++ b/errors/stacktrace.go @@ -58,7 +58,7 @@ func trimInternal(st errors.StackTrace) errors.StackTrace { ) { st = st[1:] } - // trim out outer wrappers (runtime.goexit and test library if present) + // trim out outer wrappers (runtime.goexit and testCodespace library if present) for l := len(st) - 1; l > 0 && matchesFunc(st[l], "runtime.", "testing."); l-- { st = st[:l] } @@ -73,7 +73,7 @@ func writeSimpleFrame(s io.Writer, f errors.Frame) { if len(chunks) == 2 { file = chunks[1] } - fmt.Fprintf(s, " [%s:%d]", file, line) + _, _ = fmt.Fprintf(s, " [%s:%d]", file, line) } // Format works like pkg/errors, with additions. @@ -86,16 +86,16 @@ func writeSimpleFrame(s io.Writer, f errors.Frame) { func (e *wrappedError) Format(s fmt.State, verb rune) { // normal output here.... if verb != 'v' { - fmt.Fprint(s, e.Error()) + _, _ = 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()) + _, _ = fmt.Fprintf(s, "%+v\n", stack) + _, _ = fmt.Fprint(s, e.Error()) } else { - fmt.Fprint(s, e.Error()) + _, _ = fmt.Fprint(s, e.Error()) writeSimpleFrame(s, stack[0]) } } diff --git a/errors/testerrors.go b/errors/testerrors.go new file mode 100644 index 000000000000..acd60ad8ee10 --- /dev/null +++ b/errors/testerrors.go @@ -0,0 +1,35 @@ +package errors + +const testCodespace = "sdk" + +var ( + errTxDecode = Register(testCodespace, 2, "tx parse error") + errInvalidSequence = Register(testCodespace, 3, "invalid sequence") + errUnauthorized = Register(testCodespace, 4, "unauthorized") + errInsufficientFunds = Register(testCodespace, 5, "insufficient funds") + errUnknownRequest = Register(testCodespace, 6, "unknown request") + errInvalidAddress = Register(testCodespace, 7, "invalid address") + errInvalidPubKey = Register(testCodespace, 8, "invalid pubkey") + errUnknownAddress = Register(testCodespace, 9, "unknown address") + errInvalidCoins = Register(testCodespace, 10, "invalid coins") + errOutOfGas = Register(testCodespace, 11, "out of gas") + errInsufficientFee = Register(testCodespace, 13, "insufficient fee") + errTooManySignatures = Register(testCodespace, 14, "maximum number of signatures exceeded") + ErrNoSignatures = Register(testCodespace, 15, "no signatures supplied") + ErrJSONMarshal = Register(testCodespace, 16, "failed to marshal JSON bytes") + ErrJSONUnmarshal = Register(testCodespace, 17, "failed to unmarshal JSON bytes") + ErrInvalidRequest = Register(testCodespace, 18, "invalid request") + ErrMempoolIsFull = Register(testCodespace, 20, "mempool is full") + ErrTxTooLarge = Register(testCodespace, 21, "tx too large") + ErrKeyNotFound = Register(testCodespace, 22, "key not found") + ErrorInvalidSigner = Register(testCodespace, 24, "tx intended signer does not match the given signer") + ErrInvalidChainID = Register(testCodespace, 28, "invalid chain-id") + ErrInvalidType = Register(testCodespace, 29, "invalid type") + ErrUnknownExtensionOptions = Register(testCodespace, 31, "unknown extension options") + ErrPackAny = Register(testCodespace, 33, "failed packing protobuf message to Any") + ErrLogic = Register(testCodespace, 35, "internal logic error") + ErrConflict = Register(testCodespace, 36, "conflict") + ErrNotSupported = Register(testCodespace, 37, "feature not supported") + ErrNotFound = Register(testCodespace, 38, "not found") + ErrIO = Register(testCodespace, 39, "Internal IO error") +) From c9dfe48a4b80e45f2f1acb7894b980cb160c691c Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 23 Nov 2021 21:11:05 -0500 Subject: [PATCH 03/11] add test errors --- errors/errors_test.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/errors/errors_test.go b/errors/errors_test.go index d366819ff167..2c5b03384bbe 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -29,12 +29,12 @@ func (s *errorsTestSuite) TestCause() { root error }{ "Errors are self-causing": { - err: ErrUnauthorized, - root: ErrUnauthorized, + err: errUnauthorized, + root: errUnauthorized, }, "Wrap reveals root cause": { - err: Wrap(ErrUnauthorized, "foo"), - root: ErrUnauthorized, + err: Wrap(errUnauthorized, "foo"), + root: errUnauthorized, }, "Cause works for stderr as root": { err: Wrap(std, "Some helpful text"), @@ -54,32 +54,32 @@ func (s *errorsTestSuite) TestErrorIs() { wantIs bool }{ "instance of the same error": { - a: ErrUnauthorized, - b: ErrUnauthorized, + a: errUnauthorized, + b: errUnauthorized, wantIs: true, }, "two different coded errors": { - a: ErrUnauthorized, + a: errUnauthorized, b: errOutOfGas, wantIs: false, }, "successful comparison to a wrapped error": { - a: ErrUnauthorized, - b: errors.Wrap(ErrUnauthorized, "gone"), + a: errUnauthorized, + b: errors.Wrap(errUnauthorized, "gone"), wantIs: true, }, "unsuccessful comparison to a wrapped error": { - a: ErrUnauthorized, + a: errUnauthorized, b: errors.Wrap(errInsufficientFee, "too big"), wantIs: false, }, "not equal to stdlib error": { - a: ErrUnauthorized, + a: errUnauthorized, b: fmt.Errorf("stdlib error"), wantIs: false, }, "not equal to a wrapped stdlib error": { - a: ErrUnauthorized, + a: errUnauthorized, b: errors.Wrap(fmt.Errorf("stdlib error"), "wrapped"), wantIs: false, }, @@ -95,11 +95,11 @@ func (s *errorsTestSuite) TestErrorIs() { }, "nil is not not-nil": { a: nil, - b: ErrUnauthorized, + b: errUnauthorized, wantIs: false, }, "not-nil is not nil": { - a: ErrUnauthorized, + a: errUnauthorized, b: nil, wantIs: false, }, @@ -202,8 +202,8 @@ func (s *errorsTestSuite) TestABCIError() { } func ExampleWrap() { - err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") - err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") + err1 := Wrap(errInsufficientFunds, "90 is smaller than 100") + err2 := errors.Wrap(errInsufficientFunds, "90 is smaller than 100") fmt.Println(err1.Error()) fmt.Println(err2.Error()) // Output: @@ -212,8 +212,8 @@ func ExampleWrap() { } func ExampleWrapf() { - err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") - err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") + err1 := Wrap(errInsufficientFunds, "90 is smaller than 100") + err2 := errors.Wrap(errInsufficientFunds, "90 is smaller than 100") fmt.Println(err1.Error()) fmt.Println(err2.Error()) // Output: From 459f2b9166cb70b20e0c487130d100ce7e1c9f4b Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 14 Dec 2021 15:14:44 -0500 Subject: [PATCH 04/11] alias types/errors -> errors --- .gitignore | 2 + errors/errors.go | 4 +- errors/errors_test.go | 5 + go.mod | 3 + types/errors/abci.go | 126 --------------- types/errors/abci_test.go | 259 ----------------------------- types/errors/doc.go | 34 ---- types/errors/errors.go | 277 +++----------------------------- types/errors/errors_test.go | 267 ------------------------------ types/errors/handle.go | 12 -- types/errors/stacktrace.go | 121 -------------- types/errors/stacktrace_test.go | 62 ------- 12 files changed, 33 insertions(+), 1139 deletions(-) delete mode 100644 types/errors/abci_test.go delete mode 100644 types/errors/doc.go delete mode 100644 types/errors/errors_test.go delete mode 100644 types/errors/handle.go delete mode 100644 types/errors/stacktrace.go delete mode 100644 types/errors/stacktrace_test.go diff --git a/.gitignore b/.gitignore index 14e117b3bad6..a0ac427c1d65 100644 --- a/.gitignore +++ b/.gitignore @@ -53,3 +53,5 @@ dependency-graph.png *.aux *.out *.synctex.gz +/x/genutil/config/priv_validator_key.json +/x/genutil/data/priv_validator_state.json diff --git a/errors/errors.go b/errors/errors.go index aa84ad7fe5a4..e285f1349123 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -136,11 +136,11 @@ func (e *Error) Is(err error) bool { // 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) } +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...) } +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 diff --git a/errors/errors_test.go b/errors/errors_test.go index 2c5b03384bbe..f0f962186f0a 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -160,6 +160,11 @@ func (s *errorsTestSuite) TestWrappedIs() { errw := &wrappedError{"msg", errs} require.True(errw.Is(errw), "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)) } func (s *errorsTestSuite) TestWrappedIsMultiple() { diff --git a/go.mod b/go.mod index 2ba3115190d4..ec5ffae4fff6 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/cosmos/cosmos-proto v0.0.0-20211123144845-528f5002c9f8 github.com/cosmos/cosmos-sdk/db v0.0.0 github.com/cosmos/cosmos-sdk/x/group v0.0.0-00010101000000-000000000000 + github.com/cosmos/cosmos-sdk/errors v0.0.0 github.com/cosmos/go-bip39 v1.0.0 github.com/cosmos/iavl v0.17.3 github.com/cosmos/ledger-cosmos-go v0.11.1 @@ -156,3 +157,5 @@ replace github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.7.0 replace github.com/cosmos/cosmos-sdk/db => ./db replace github.com/cosmos/cosmos-sdk/x/group => ./x/group + +replace github.com/cosmos/cosmos-sdk/errors => ./errors diff --git a/types/errors/abci.go b/types/errors/abci.go index d09b31b15b36..fef8dfa8ab69 100644 --- a/types/errors/abci.go +++ b/types/errors/abci.go @@ -1,44 +1,9 @@ package errors import ( - "fmt" - "reflect" - abci "github.com/tendermint/tendermint/abci/types" ) -const ( - // SuccessABCICode declares an ABCI response use 0 to signal that the - // processing was successful and no error is returned. - SuccessABCICode uint32 = 0 - - // All unclassified errors that do not provide an ABCI code are clubbed - // under an internal error code and a generic message instead of - // detailed error string. - internalABCICodespace = UndefinedCodespace - internalABCICode uint32 = 1 -) - -// ABCIInfo returns the ABCI error information as consumed by the tendermint -// client. Returned codespace, code, and log message should be used as a ABCI response. -// Any error that does not provide ABCICode information is categorized as error -// with code 1, codespace UndefinedCodespace -// When not running in a debug mode all messages of errors that do not provide -// ABCICode information are replaced with generic "internal error". Errors -// without an ABCICode information as considered internal. -func ABCIInfo(err error, debug bool) (codespace string, code uint32, log string) { - if errIsNil(err) { - return "", SuccessABCICode, "" - } - - encode := defaultErrEncoder - if debug { - encode = debugErrEncoder - } - - return abciCodespace(err), abciCode(err), encode(err) -} - // ResponseCheckTx returns an ABCI ResponseCheckTx object with fields filled in // from the given error and gas values. func ResponseCheckTx(err error, gw, gu uint64, debug bool) abci.ResponseCheckTx { @@ -75,94 +40,3 @@ func QueryResult(err error, debug bool) abci.ResponseQuery { Log: log, } } - -// The debugErrEncoder encodes the error with a stacktrace. -func debugErrEncoder(err error) string { - return fmt.Sprintf("%+v", err) -} - -// The defaultErrEncoder applies Redact on the error before encoding it with its internal error message. -func defaultErrEncoder(err error) string { - return Redact(err).Error() -} - -type coder interface { - ABCICode() uint32 -} - -// abciCode test 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 { - if errIsNil(err) { - return SuccessABCICode - } - - for { - if c, ok := err.(coder); ok { - return c.ABCICode() - } - - if c, ok := err.(causer); ok { - err = c.Cause() - } 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. -func abciCodespace(err error) string { - if errIsNil(err) { - return "" - } - - for { - if c, ok := err.(codespacer); ok { - return c.Codespace() - } - - if c, ok := err.(causer); ok { - err = c.Cause() - } else { - return internalABCICodespace - } - } -} - -// errIsNil returns true if value represented by the given error is nil. -// -// Most of the time a simple == check is enough. There is a very narrowed -// spectrum of cases (mostly in tests) where a more sophisticated check is -// required. -func errIsNil(err error) bool { - if err == nil { - return true - } - if val := reflect.ValueOf(err); val.Kind() == reflect.Ptr { - return val.IsNil() - } - return false -} - -var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentially sensitive system info") - -// Redact replaces an error that is not initialized as an ABCI Error with a -// generic internal error instance. If the error is an ABCI Error, that error is -// simply returned. -func Redact(err error) error { - if ErrPanic.Is(err) { - return errPanicWithMsg - } - if abciCode(err) == internalABCICode { - return errInternal - } - - return err -} diff --git a/types/errors/abci_test.go b/types/errors/abci_test.go deleted file mode 100644 index 02c12e7bbdd6..000000000000 --- a/types/errors/abci_test.go +++ /dev/null @@ -1,259 +0,0 @@ -package errors - -import ( - "fmt" - "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() { - cases := map[string]struct { - err error - debug bool - wantCode uint32 - wantSpace string - wantLog string - }{ - "plain SDK error": { - err: ErrUnauthorized, - debug: false, - wantLog: "unauthorized", - wantCode: ErrUnauthorized.code, - wantSpace: RootCodespace, - }, - "wrapped SDK error": { - err: Wrap(Wrap(ErrUnauthorized, "foo"), "bar"), - debug: false, - wantLog: "bar: foo: unauthorized", - wantCode: ErrUnauthorized.code, - wantSpace: RootCodespace, - }, - "nil is empty message": { - err: nil, - debug: false, - wantLog: "", - wantCode: 0, - wantSpace: "", - }, - "nil SDK error is not an error": { - err: (*Error)(nil), - debug: false, - wantLog: "", - wantCode: 0, - wantSpace: "", - }, - "stdlib is generic message": { - err: io.EOF, - debug: false, - wantLog: "internal", - wantCode: 1, - wantSpace: UndefinedCodespace, - }, - "stdlib returns error message in debug mode": { - err: io.EOF, - debug: true, - wantLog: "EOF", - wantCode: 1, - wantSpace: UndefinedCodespace, - }, - "wrapped stdlib is only a generic message": { - err: Wrap(io.EOF, "cannot read file"), - debug: false, - wantLog: "internal", - 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, - //}, - "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 { - 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) - } -} - -func (s *abciTestSuite) TestABCIInfoStacktrace() { - 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", - }, - "wrapped stdlib error in non-debug mode does not have stacktrace": { - err: Wrap(fmt.Errorf("stdlib"), "wrapped"), - debug: false, - wantStacktrace: false, - wantErrMsg: "internal", - }, - } - - const thisTestSrc = "github.com/cosmos/cosmos-sdk/types/errors.(*abciTestSuite).TestABCIInfoStacktrace" - - for testName, tc := range cases { - _, _, log := ABCIInfo(tc.err, tc.debug) - if !tc.wantStacktrace { - s.Require().Equal(tc.wantErrMsg, log, testName) - continue - } - - s.Require().True(strings.Contains(log, thisTestSrc), testName) - s.Require().True(strings.Contains(log, tc.wantErrMsg), testName) - } -} - -func (s *abciTestSuite) TestABCIInfoHidesStacktrace() { - err := Wrap(ErrUnauthorized, "wrapped") - _, _, log := ABCIInfo(err, false) - s.Require().Equal("wrapped: unauthorized", log) -} - -func (s *abciTestSuite) TestRedact() { - cases := map[string]struct { - err error - untouched bool // if true we expect the same error after redact - changed error // if untouched == false, expect this error - }{ - "panic looses message": { - err: Wrap(ErrPanic, "some secret stack trace"), - changed: errPanicWithMsg, - }, - "sdk errors untouched": { - err: Wrap(ErrUnauthorized, "cannot drop db"), - untouched: true, - }, - "pass though custom errors with ABCI code": { - err: customErr{}, - untouched: true, - }, - "redact stdlib error": { - err: fmt.Errorf("stdlib error"), - changed: errInternal, - }, - } - - for name, tc := range cases { - spec := tc - redacted := Redact(spec.err) - if spec.untouched { - s.Require().Equal(spec.err, redacted, name) - continue - } - - // see if we got the expected redact - s.Require().Equal(spec.changed, redacted, name) - // make sure the ABCI code did not change - s.Require().Equal(abciCode(spec.err), abciCode(redacted), name) - - } -} - -func (s *abciTestSuite) TestABCIInfoSerializeErr() { - var ( - // Create errors with stacktrace for equal comparison. - myErrDecode = Wrap(ErrTxDecode, "test") - myErrAddr = Wrap(ErrInvalidAddress, "tester") - myPanic = ErrPanic - ) - - specs := map[string]struct { - src error - debug bool - exp string - }{ - "single error": { - src: myErrDecode, - debug: false, - exp: "test: tx parse error", - }, - "second error": { - src: myErrAddr, - debug: false, - exp: "tester: invalid address", - }, - "single error with debug": { - src: myErrDecode, - debug: true, - exp: fmt.Sprintf("%+v", myErrDecode), - }, - "redact in default encoder": { - src: myPanic, - exp: "panic message redacted to hide potentially sensitive system info: panic", - }, - "do not redact in debug encoder": { - src: myPanic, - debug: true, - exp: fmt.Sprintf("%+v", myPanic), - }, - } - for msg, spec := range specs { - spec := spec - _, _, log := ABCIInfo(spec.src, spec.debug) - s.Require().Equal(spec.exp, log, msg) - } -} - -// 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" } diff --git a/types/errors/doc.go b/types/errors/doc.go deleted file mode 100644 index 6cca61580d06..000000000000 --- a/types/errors/doc.go +++ /dev/null @@ -1,34 +0,0 @@ -/* -Package errors implements custom error interfaces for cosmos-sdk. - -Error declarations should be generic and cover broad range of cases. Each -returned error instance can wrap a generic error declaration to provide more -details. - -This package provides a broad range of errors declared that fits all common -cases. If an error is very specific for an extension it can be registered outside -of the errors package. If it will be needed my many extensions, please consider -registering it in the errors package. To create a new error instance use Register -function. You must provide a unique, non zero error code and a short description, for example: - - var ErrZeroDivision = errors.Register(9241, "zero division") - -When returning an error, you can attach to it an additional context -information by using Wrap function, for example: - - func safeDiv(val, div int) (int, err) { - if div == 0 { - return 0, errors.Wrapf(ErrZeroDivision, "cannot divide %d", val) - } - return val / div, nil - } - -The first time an error instance is wrapped a stacktrace is attached as well. -Stacktrace information can be printed using %+v and %v formats. - - %s is just the error message - %+v is the full stack trace - %v appends a compressed [filename:line] where the error was created - -*/ -package errors diff --git a/types/errors/errors.go b/types/errors/errors.go index 12a912cc49ce..d01e64b23b8c 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -1,22 +1,33 @@ package errors import ( - "fmt" - "reflect" + errorsmod "github.com/cosmos/cosmos-sdk/errors" +) + +var ( + SuccessABCICode = errorsmod.SuccessABCICode + ABCIInfo = errorsmod.ABCIInfo + Redact = errorsmod.Redact + UndefinedCodespace = errorsmod.UndefinedCodespace + Register = errorsmod.Register + ABCIError = errorsmod.ABCIError + New = errorsmod.New + Wrap = errorsmod.Wrap + Wrapf = errorsmod.Wrapf + Recover = errorsmod.Recover + WithType = errorsmod.WithType + IsOf = errorsmod.IsOf + AssertNil = errorsmod.AssertNil +) - "github.com/pkg/errors" +type ( + Error = errorsmod.Error ) // RootCodespace is the codespace for all errors defined in this package const RootCodespace = "sdk" -// UndefinedCodespace when we explicitly declare no codespace -const UndefinedCodespace = "undefined" - var ( - // errInternal should never be exposed, but we reserve this code for non-specified errors - errInternal = Register(UndefinedCodespace, 1, "internal") - // ErrTxDecode is returned if we cannot parse a transaction ErrTxDecode = Register(RootCodespace, 2, "tx parse error") @@ -143,254 +154,8 @@ var ( // ErrPanic is only set when we recover from a panic, so we know to // redact potentially sensitive system info - ErrPanic = Register(UndefinedCodespace, 111222, "panic") + ErrPanic = errorsmod.ErrPanic // ErrAppConfig defines an error occurred if min-gas-prices field in BaseConfig is empty. ErrAppConfig = Register(RootCodespace, 40, "error in app.toml") ) - -// Register returns an error instance that should be used as the base for -// creating error instances during runtime. -// -// Popular root errors are declared in this package, but extensions may want to -// declare custom codes. This function ensures that no error code is used -// twice. Attempt to reuse an error code results in panic. -// -// Use this function only during a program startup phase. -func Register(codespace string, code uint32, description string) *Error { - // TODO - uniqueness is (codespace, code) combo - if e := getUsed(codespace, code); e != nil { - panic(fmt.Sprintf("error with code %d is already registered: %q", code, e.desc)) - } - - err := New(codespace, code, description) - setUsed(err) - - return err -} - -// usedCodes is keeping track of used codes to ensure their uniqueness. No two -// error instances should share the same (codespace, code) tuple. -var usedCodes = map[string]*Error{} - -func errorID(codespace string, code uint32) string { - return fmt.Sprintf("%s:%d", codespace, code) -} - -func getUsed(codespace string, code uint32) *Error { - return usedCodes[errorID(codespace, code)] -} - -func setUsed(err *Error) { - usedCodes[errorID(err.codespace, err.code)] = err -} - -// ABCIError will resolve an error code/log from an abci result into -// an error message. If the code is registered, it will map it back to -// the canonical error, so we can do eg. ErrNotFound.Is(err) on something -// we get back from an external API. -// -// This should *only* be used in clients, not in the server side. -// 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) - } - // This is a unique error, will never match on .Is() - // Use Wrap here to get a stack trace - return Wrap(New(codespace, code, "unknown"), log) -} - -// Error represents a root error. -// -// Weave framework is using root error to categorize issues. Each instance -// created during the runtime should wrap one of the declared root errors. This -// allows error tests and returning all errors to the client in a safe manner. -// -// All popular root errors are declared in this package. If an extension has to -// declare a custom root error, always use Register function to ensure -// error code uniqueness. -type Error struct { - codespace string - code uint32 - desc string -} - -func New(codespace string, code uint32, desc string) *Error { - return &Error{codespace: codespace, code: code, desc: desc} -} - -func (e Error) Error() string { - return e.desc -} - -func (e Error) ABCICode() uint32 { - return e.code -} - -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) } - -// 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...) } - -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), -// 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 - } - - // 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, - } -} - -// 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 -} - -// 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 -} - -// 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 { - 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 -} - -type unpacker interface { - Unpack() []error -} diff --git a/types/errors/errors_test.go b/types/errors/errors_test.go deleted file mode 100644 index b7e9c478fe26..000000000000 --- a/types/errors/errors_test.go +++ /dev/null @@ -1,267 +0,0 @@ -package errors - -import ( - stdlib "errors" - "fmt" - "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: errors.Wrap(ErrUnauthorized, "gone"), - wantIs: true, - }, - "unsuccessful comparison to a wrapped error": { - a: ErrUnauthorized, - b: errors.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: errors.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, - }, - // "multierr with the same error": { - // a: ErrUnauthorized, - // b: Append(ErrUnauthorized, ErrState), - // wantIs: true, - // }, - // "multierr with random order": { - // a: ErrUnauthorized, - // b: Append(ErrState, ErrUnauthorized), - // wantIs: true, - // }, - // "multierr with wrapped err": { - // a: ErrUnauthorized, - // b: Append(ErrState, Wrap(ErrUnauthorized, "test")), - // wantIs: true, - // }, - // "multierr with nil error": { - // a: ErrUnauthorized, - // b: Append(nil, nil), - // wantIs: false, - // }, - // "multierr with different error": { - // a: ErrUnauthorized, - // b: Append(ErrState, nil), - // wantIs: false, - // }, - // "multierr from nil": { - // a: nil, - // b: Append(ErrState, ErrUnauthorized), - // wantIs: false, - // }, - // "field error wrapper": { - // a: ErrEmpty, - // b: Field("name", ErrEmpty, "name is required"), - // wantIs: true, - // }, - // "nil field error wrapper": { - // a: nil, - // b: Field("name", nil, "name is required"), - // wantIs: true, - // }, - } - for testName, tc := range cases { - s.Require().Equal(tc.wantIs, tc.a.Is(tc.b), testName) - } -} - -func (s *errorsTestSuite) TestIsOf() { - require := s.Require() - - var errNil *Error - var err = ErrInvalidAddress - var errW = Wrap(ErrLogic, "more info") - - require.False(IsOf(errNil), "nil error should always have no causer") - require.False(IsOf(errNil, err), "nil error should always have no causer") - - require.False(IsOf(err)) - require.False(IsOf(err, nil)) - require.False(IsOf(err, ErrLogic)) - - require.True(IsOf(errW, ErrLogic)) - require.True(IsOf(errW, err, ErrLogic)) - require.True(IsOf(errW, nil, errW), "error should much itself") - var 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" -} - -func (s *errorsTestSuite) TestWrapEmpty() { - s.Require().Nil(Wrap(nil, "wrapping ")) -} - -func (s *errorsTestSuite) TestWrappedIs() { - require := s.Require() - err := Wrap(ErrTxTooLarge, "context") - require.True(stdlib.Is(err, ErrTxTooLarge)) - - err = Wrap(err, "more context") - require.True(stdlib.Is(err, ErrTxTooLarge)) - - err = Wrap(err, "even more context") - require.True(stdlib.Is(err, ErrTxTooLarge)) - - err = Wrap(ErrInsufficientFee, "...") - require.False(stdlib.Is(err, ErrTxTooLarge)) - - errs := stdlib.New("other") - require.True(stdlib.Is(errs, errs)) - - errw := &wrappedError{"msg", errs} - require.True(errw.Is(errw), "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)) -} - -func (s *errorsTestSuite) TestWrappedIsMultiple() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().True(stdlib.Is(err, errTest2)) -} - -func (s *errorsTestSuite) TestWrappedIsFail() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().False(stdlib.Is(err, errTest)) -} - -func (s *errorsTestSuite) TestWrappedUnwrap() { - var errTest = errors.New("test error") - err := Wrap(errTest, "some random description") - s.Require().Equal(errTest, stdlib.Unwrap(err)) -} - -func (s *errorsTestSuite) TestWrappedUnwrapMultiple() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().Equal(errTest2, stdlib.Unwrap(err)) -} - -func (s *errorsTestSuite) TestWrappedUnwrapFail() { - var errTest = errors.New("test error") - var errTest2 = errors.New("test error 2") - err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) - s.Require().NotEqual(errTest, stdlib.Unwrap(err)) -} - -func (s *errorsTestSuite) TestABCIError() { - s.Require().Equal("custom: tx parse error", ABCIError(RootCodespace, 2, "custom").Error()) - s.Require().Equal("custom: unknown", ABCIError("unknown", 1, "custom").Error()) -} - -func ExampleWrap() { - err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") - err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") - fmt.Println(err1.Error()) - fmt.Println(err2.Error()) - // Output: - // 90 is smaller than 100: insufficient funds - // 90 is smaller than 100: insufficient funds -} - -func ExampleWrapf() { - err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") - err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") - fmt.Println(err1.Error()) - fmt.Println(err2.Error()) - // Output: - // 90 is smaller than 100: insufficient funds - // 90 is smaller than 100: insufficient funds -} diff --git a/types/errors/handle.go b/types/errors/handle.go deleted file mode 100644 index 33c3fbfdeac2..000000000000 --- a/types/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)) - } -} diff --git a/types/errors/stacktrace.go b/types/errors/stacktrace.go deleted file mode 100644 index f7079c56d831..000000000000 --- a/types/errors/stacktrace.go +++ /dev/null @@ -1,121 +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 - "github.com/cosmos/cosmos-sdk/types/errors.Wrap", - "github.com/cosmos/cosmos-sdk/types/errors.Wrapf", - "github.com/cosmos/cosmos-sdk/types/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/types/errors/stacktrace_test.go b/types/errors/stacktrace_test.go deleted file mode 100644 index 06eb829f0f44..000000000000 --- a/types/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{ - "github.com/cosmos/cosmos-sdk/types/errors.Wrap\n", - "github.com/cosmos/cosmos-sdk/types/errors.Wrapf\n", - "runtime.goexit\n", - } - const thisTestSrc = "types/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 dc4f7fa9a0c96fe18cc8b68b7cd3424e058b13c2 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 14 Dec 2021 15:36:31 -0500 Subject: [PATCH 05/11] cleanup --- errors/abci_test.go | 6 +++--- errors/errors_test.go | 18 +++++++++--------- go.mod | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/errors/abci_test.go b/errors/abci_test.go index b8aaed736ca9..fd19c472fbbc 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -78,8 +78,8 @@ func (s *abciTestSuite) TestABCInfo() { wantCode: 1, wantSpace: UndefinedCodespace, }, - // This is hard to testCodespace because of attached stacktrace. This - // case is tested in an another testCodespace. + // 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, @@ -219,7 +219,7 @@ func (s *abciTestSuite) TestABCIInfoSerializeErr() { "single error": { src: myErrDecode, debug: false, - exp: "testCodespace: tx parse error", + exp: "test: tx parse error", }, "second error": { src: myErrAddr, diff --git a/errors/errors_test.go b/errors/errors_test.go index f0f962186f0a..fd99d046cefd 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -168,35 +168,35 @@ func (s *errorsTestSuite) TestWrappedIs() { } func (s *errorsTestSuite) TestWrappedIsMultiple() { - var errTest = errors.New("testCodespace error") - var errTest2 = errors.New("testCodespace error 2") + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().True(stdlib.Is(err, errTest2)) } func (s *errorsTestSuite) TestWrappedIsFail() { - var errTest = errors.New("testCodespace error") - var errTest2 = errors.New("testCodespace error 2") + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().False(stdlib.Is(err, errTest)) } func (s *errorsTestSuite) TestWrappedUnwrap() { - var errTest = errors.New("testCodespace error") + var errTest = errors.New("test error") err := Wrap(errTest, "some random description") s.Require().Equal(errTest, stdlib.Unwrap(err)) } func (s *errorsTestSuite) TestWrappedUnwrapMultiple() { - var errTest = errors.New("testCodespace error") - var errTest2 = errors.New("testCodespace error 2") + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().Equal(errTest2, stdlib.Unwrap(err)) } func (s *errorsTestSuite) TestWrappedUnwrapFail() { - var errTest = errors.New("testCodespace error") - var errTest2 = errors.New("testCodespace error 2") + var errTest = errors.New("test error") + var errTest2 = errors.New("test error 2") err := Wrap(errTest2, Wrap(errTest, "some random description").Error()) s.Require().NotEqual(errTest, stdlib.Unwrap(err)) } diff --git a/go.mod b/go.mod index ec5ffae4fff6..6c5ec6af5125 100644 --- a/go.mod +++ b/go.mod @@ -12,8 +12,8 @@ require ( github.com/cosmos/btcutil v1.0.4 github.com/cosmos/cosmos-proto v0.0.0-20211123144845-528f5002c9f8 github.com/cosmos/cosmos-sdk/db v0.0.0 - github.com/cosmos/cosmos-sdk/x/group v0.0.0-00010101000000-000000000000 github.com/cosmos/cosmos-sdk/errors v0.0.0 + github.com/cosmos/cosmos-sdk/x/group v0.0.0-00010101000000-000000000000 github.com/cosmos/go-bip39 v1.0.0 github.com/cosmos/iavl v0.17.3 github.com/cosmos/ledger-cosmos-go v0.11.1 From cf6b885dfc43972b5fb5652058dcb843628e7653 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 14 Dec 2021 15:39:03 -0500 Subject: [PATCH 06/11] cleanup --- errors/abci_test.go | 2 +- errors/stacktrace.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/errors/abci_test.go b/errors/abci_test.go index fd19c472fbbc..113a9897067b 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -206,7 +206,7 @@ func (s *abciTestSuite) TestRedact() { func (s *abciTestSuite) TestABCIInfoSerializeErr() { var ( // Create errors with stacktrace for equal comparison. - myErrDecode = Wrap(errTxDecode, "testCodespace") + myErrDecode = Wrap(errTxDecode, "test") myErrAddr = Wrap(errInvalidAddress, "tester") myPanic = ErrPanic ) diff --git a/errors/stacktrace.go b/errors/stacktrace.go index 180f62c8761d..e7d809a1a113 100644 --- a/errors/stacktrace.go +++ b/errors/stacktrace.go @@ -58,7 +58,7 @@ func trimInternal(st errors.StackTrace) errors.StackTrace { ) { st = st[1:] } - // trim out outer wrappers (runtime.goexit and testCodespace library if present) + // 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] } From 34c094df3c660feeabb2c3eae7a629c695d9d8f2 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 14 Dec 2021 15:41:16 -0500 Subject: [PATCH 07/11] cleanup --- errors/abci.go | 2 +- errors/errors.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index 111e68a6db2d..afd535e198c6 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -51,7 +51,7 @@ type coder interface { ABCICode() uint32 } -// abciCode testCodespace if given error contains an ABCI code and returns the value of +// 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. func abciCode(err error) uint32 { diff --git a/errors/errors.go b/errors/errors.go index e285f1349123..b01ebaf59ef6 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -256,7 +256,7 @@ func IsOf(received error, targets ...error) bool { } // causer is an interface implemented by an error that supports wrapping. Use -// it to testCodespace if an error wraps another error instance. +// it to test if an error wraps another error instance. type causer interface { Cause() error } From 47fe8bcff79a768f9ebec0aca1e9e0c2208d4788 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 14 Dec 2021 15:50:56 -0500 Subject: [PATCH 08/11] add docs --- types/errors/errors.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/types/errors/errors.go b/types/errors/errors.go index d01e64b23b8c..2aa53c1deb53 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -1,3 +1,11 @@ +//Package errors provides a shared set of errors for use in the SDK, +//aliases functionality in the github.com/cosmos/cosmos-sdk/errors module +//that used to be in this package, and provides some helpers for converting +//errors to ABCI response code. +// +//New code should generally import github.com/cosmos/cosmos-sdk/errors directly +//and define a custom set of errors in custom codespace, rather than importing +//this package. package errors import ( From 782d66deb964b234211b0415c2b788d9b8b4f343 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 14 Dec 2021 15:59:37 -0500 Subject: [PATCH 09/11] add CHANGELOG entries --- CHANGELOG.md | 1 + errors/CHANGELOG.md | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 errors/CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e433b520ead..3b04564ff8dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -149,6 +149,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (types) [\#10630](https://github.com/cosmos/cosmos-sdk/pull/10630) Add an `Events` field to the `TxResponse` type that captures _all_ events emitted by a transaction, unlike `Logs` which only contains events emitted during message execution. * (deps) [\#10706](https://github.com/cosmos/cosmos-sdk/issues/10706) Bump rosetta-sdk-go to v0.7.2 and rosetta-cli to v0.7.3 * (module) [\#10711](https://github.com/cosmos/cosmos-sdk/pull/10711) Panic at startup if the app developer forgot to add modules in the `SetOrder{BeginBlocker, EndBlocker, InitGenesis, ExportGenesis}` functions. This means that all modules, even those who have empty implementations for those methods, need to be added to `SetOrder*`. +* (types/errors) [\#10779](https://github.com/cosmos/cosmos-sdk/pull/10779) Move most functionality in `types/errors` to a standalone `errors` go module, except the `RootCodespace` errors and ABCI response helpers. All functions and types that used to live in `types/errors` are now aliased so this is not a breaking change. ### Bug Fixes diff --git a/errors/CHANGELOG.md b/errors/CHANGELOG.md new file mode 100644 index 000000000000..339ebe921c05 --- /dev/null +++ b/errors/CHANGELOG.md @@ -0,0 +1,40 @@ + + +# Changelog + +## [Unreleased] + +### Features +* [\#10779](https://github.com/cosmos/cosmos-sdk/pull/10779) Import code from the `github.com/cosmos/cosmos-sdk/types/errors` package. From d2187b3e73e6bc5a4ad336e2c87f1e4d769ee13c Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Wed, 15 Dec 2021 16:07:32 -0500 Subject: [PATCH 10/11] update CHANGELOG --- errors/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/CHANGELOG.md b/errors/CHANGELOG.md index 339ebe921c05..6b6faf4c5cd5 100644 --- a/errors/CHANGELOG.md +++ b/errors/CHANGELOG.md @@ -34,7 +34,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog -## [Unreleased] +## v1.0.0 ### Features * [\#10779](https://github.com/cosmos/cosmos-sdk/pull/10779) Import code from the `github.com/cosmos/cosmos-sdk/types/errors` package. From 73e5a5e0957f36901d16f41f63207f1f0e68160a Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Fri, 17 Dec 2021 16:42:01 -0500 Subject: [PATCH 11/11] fix tests --- errors/abci_test.go | 20 +++++----- errors/errors_test.go | 86 ++++++++++++++++++++++++++++++------------- errors/testerrors.go | 35 ------------------ x/group/go.mod | 3 ++ 4 files changed, 73 insertions(+), 71 deletions(-) delete mode 100644 errors/testerrors.go diff --git a/errors/abci_test.go b/errors/abci_test.go index 113a9897067b..8c653abeafc2 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -30,17 +30,17 @@ func (s *abciTestSuite) TestABCInfo() { wantLog string }{ "plain SDK error": { - err: errUnauthorized, + err: ErrUnauthorized, debug: false, wantLog: "unauthorized", - wantCode: errUnauthorized.code, + wantCode: ErrUnauthorized.code, wantSpace: testCodespace, }, "wrapped SDK error": { - err: Wrap(Wrap(errUnauthorized, "foo"), "bar"), + err: Wrap(Wrap(ErrUnauthorized, "foo"), "bar"), debug: false, wantLog: "bar: foo: unauthorized", - wantCode: errUnauthorized.code, + wantCode: ErrUnauthorized.code, wantSpace: testCodespace, }, "nil is empty message": { @@ -118,13 +118,13 @@ func (s *abciTestSuite) TestABCIInfoStacktrace() { wantErrMsg string }{ "wrapped SDK error in debug mode provides stacktrace": { - err: Wrap(errUnauthorized, "wrapped"), + 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"), + err: Wrap(ErrUnauthorized, "wrapped"), debug: false, wantStacktrace: false, wantErrMsg: "wrapped: unauthorized", @@ -158,7 +158,7 @@ func (s *abciTestSuite) TestABCIInfoStacktrace() { } func (s *abciTestSuite) TestABCIInfoHidesStacktrace() { - err := Wrap(errUnauthorized, "wrapped") + err := Wrap(ErrUnauthorized, "wrapped") _, _, log := ABCIInfo(err, false) s.Require().Equal("wrapped: unauthorized", log) } @@ -174,7 +174,7 @@ func (s *abciTestSuite) TestRedact() { changed: errPanicWithMsg, }, "sdk errors untouched": { - err: Wrap(errUnauthorized, "cannot drop db"), + err: Wrap(ErrUnauthorized, "cannot drop db"), untouched: true, }, "pass though custom errors with ABCI code": { @@ -206,8 +206,8 @@ func (s *abciTestSuite) TestRedact() { func (s *abciTestSuite) TestABCIInfoSerializeErr() { var ( // Create errors with stacktrace for equal comparison. - myErrDecode = Wrap(errTxDecode, "test") - myErrAddr = Wrap(errInvalidAddress, "tester") + myErrDecode = Wrap(ErrTxDecode, "test") + myErrAddr = Wrap(ErrInvalidAddress, "tester") myPanic = ErrPanic ) diff --git a/errors/errors_test.go b/errors/errors_test.go index fd99d046cefd..dbe05615daa3 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -29,12 +29,12 @@ func (s *errorsTestSuite) TestCause() { root error }{ "Errors are self-causing": { - err: errUnauthorized, - root: errUnauthorized, + err: ErrUnauthorized, + root: ErrUnauthorized, }, "Wrap reveals root cause": { - err: Wrap(errUnauthorized, "foo"), - root: errUnauthorized, + err: Wrap(ErrUnauthorized, "foo"), + root: ErrUnauthorized, }, "Cause works for stderr as root": { err: Wrap(std, "Some helpful text"), @@ -54,32 +54,32 @@ func (s *errorsTestSuite) TestErrorIs() { wantIs bool }{ "instance of the same error": { - a: errUnauthorized, - b: errUnauthorized, + a: ErrUnauthorized, + b: ErrUnauthorized, wantIs: true, }, "two different coded errors": { - a: errUnauthorized, - b: errOutOfGas, + a: ErrUnauthorized, + b: ErrOutOfGas, wantIs: false, }, "successful comparison to a wrapped error": { - a: errUnauthorized, - b: errors.Wrap(errUnauthorized, "gone"), + a: ErrUnauthorized, + b: errors.Wrap(ErrUnauthorized, "gone"), wantIs: true, }, "unsuccessful comparison to a wrapped error": { - a: errUnauthorized, - b: errors.Wrap(errInsufficientFee, "too big"), + a: ErrUnauthorized, + b: errors.Wrap(ErrInsufficientFee, "too big"), wantIs: false, }, "not equal to stdlib error": { - a: errUnauthorized, + a: ErrUnauthorized, b: fmt.Errorf("stdlib error"), wantIs: false, }, "not equal to a wrapped stdlib error": { - a: errUnauthorized, + a: ErrUnauthorized, b: errors.Wrap(fmt.Errorf("stdlib error"), "wrapped"), wantIs: false, }, @@ -95,11 +95,11 @@ func (s *errorsTestSuite) TestErrorIs() { }, "nil is not not-nil": { a: nil, - b: errUnauthorized, + b: ErrUnauthorized, wantIs: false, }, "not-nil is not nil": { - a: errUnauthorized, + a: ErrUnauthorized, b: nil, wantIs: false, }, @@ -113,7 +113,7 @@ func (s *errorsTestSuite) TestIsOf() { require := s.Require() var errNil *Error - var err = errInvalidAddress + var err = ErrInvalidAddress var errW = Wrap(ErrLogic, "more info") require.False(IsOf(errNil), "nil error should always have no causer") @@ -152,7 +152,7 @@ func (s *errorsTestSuite) TestWrappedIs() { err = Wrap(err, "even more context") require.True(stdlib.Is(err, ErrTxTooLarge)) - err = Wrap(errInsufficientFee, "...") + err = Wrap(ErrInsufficientFee, "...") require.False(stdlib.Is(err, ErrTxTooLarge)) errs := stdlib.New("other") @@ -161,10 +161,10 @@ func (s *errorsTestSuite) TestWrappedIs() { errw := &wrappedError{"msg", errs} require.True(errw.Is(errw), "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)) + 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)) } func (s *errorsTestSuite) TestWrappedIsMultiple() { @@ -207,8 +207,8 @@ func (s *errorsTestSuite) TestABCIError() { } func ExampleWrap() { - err1 := Wrap(errInsufficientFunds, "90 is smaller than 100") - err2 := errors.Wrap(errInsufficientFunds, "90 is smaller than 100") + err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") + err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") fmt.Println(err1.Error()) fmt.Println(err2.Error()) // Output: @@ -217,11 +217,45 @@ func ExampleWrap() { } func ExampleWrapf() { - err1 := Wrap(errInsufficientFunds, "90 is smaller than 100") - err2 := errors.Wrap(errInsufficientFunds, "90 is smaller than 100") + err1 := Wrap(ErrInsufficientFunds, "90 is smaller than 100") + err2 := errors.Wrap(ErrInsufficientFunds, "90 is smaller than 100") fmt.Println(err1.Error()) fmt.Println(err2.Error()) // Output: // 90 is smaller than 100: insufficient funds // 90 is smaller than 100: insufficient funds } + +const testCodespace = "testtesttest" + +var ( + ErrTxDecode = Register(testCodespace, 2, "tx parse error") + ErrInvalidSequence = Register(testCodespace, 3, "invalid sequence") + ErrUnauthorized = Register(testCodespace, 4, "unauthorized") + ErrInsufficientFunds = Register(testCodespace, 5, "insufficient funds") + ErrUnknownRequest = Register(testCodespace, 6, "unknown request") + ErrInvalidAddress = Register(testCodespace, 7, "invalid address") + ErrInvalidPubKey = Register(testCodespace, 8, "invalid pubkey") + ErrUnknownAddress = Register(testCodespace, 9, "unknown address") + ErrInvalidCoins = Register(testCodespace, 10, "invalid coins") + ErrOutOfGas = Register(testCodespace, 11, "out of gas") + ErrInsufficientFee = Register(testCodespace, 13, "insufficient fee") + ErrTooManySignatures = Register(testCodespace, 14, "maximum number of signatures exceeded") + ErrNoSignatures = Register(testCodespace, 15, "no signatures supplied") + ErrJSONMarshal = Register(testCodespace, 16, "failed to marshal JSON bytes") + ErrJSONUnmarshal = Register(testCodespace, 17, "failed to unmarshal JSON bytes") + ErrInvalidRequest = Register(testCodespace, 18, "invalid request") + ErrMempoolIsFull = Register(testCodespace, 20, "mempool is full") + ErrTxTooLarge = Register(testCodespace, 21, "tx too large") + ErrKeyNotFound = Register(testCodespace, 22, "key not found") + ErrorInvalidSigner = Register(testCodespace, 24, "tx intended signer does not match the given signer") + ErrInvalidChainID = Register(testCodespace, 28, "invalid chain-id") + ErrInvalidType = Register(testCodespace, 29, "invalid type") + ErrUnknownExtensionOptions = Register(testCodespace, 31, "unknown extension options") + ErrPackAny = Register(testCodespace, 33, "failed packing protobuf message to Any") + ErrLogic = Register(testCodespace, 35, "internal logic error") + ErrConflict = Register(testCodespace, 36, "conflict") + ErrNotSupported = Register(testCodespace, 37, "feature not supported") + ErrNotFound = Register(testCodespace, 38, "not found") + ErrIO = Register(testCodespace, 39, "Internal IO error") +) diff --git a/errors/testerrors.go b/errors/testerrors.go deleted file mode 100644 index acd60ad8ee10..000000000000 --- a/errors/testerrors.go +++ /dev/null @@ -1,35 +0,0 @@ -package errors - -const testCodespace = "sdk" - -var ( - errTxDecode = Register(testCodespace, 2, "tx parse error") - errInvalidSequence = Register(testCodespace, 3, "invalid sequence") - errUnauthorized = Register(testCodespace, 4, "unauthorized") - errInsufficientFunds = Register(testCodespace, 5, "insufficient funds") - errUnknownRequest = Register(testCodespace, 6, "unknown request") - errInvalidAddress = Register(testCodespace, 7, "invalid address") - errInvalidPubKey = Register(testCodespace, 8, "invalid pubkey") - errUnknownAddress = Register(testCodespace, 9, "unknown address") - errInvalidCoins = Register(testCodespace, 10, "invalid coins") - errOutOfGas = Register(testCodespace, 11, "out of gas") - errInsufficientFee = Register(testCodespace, 13, "insufficient fee") - errTooManySignatures = Register(testCodespace, 14, "maximum number of signatures exceeded") - ErrNoSignatures = Register(testCodespace, 15, "no signatures supplied") - ErrJSONMarshal = Register(testCodespace, 16, "failed to marshal JSON bytes") - ErrJSONUnmarshal = Register(testCodespace, 17, "failed to unmarshal JSON bytes") - ErrInvalidRequest = Register(testCodespace, 18, "invalid request") - ErrMempoolIsFull = Register(testCodespace, 20, "mempool is full") - ErrTxTooLarge = Register(testCodespace, 21, "tx too large") - ErrKeyNotFound = Register(testCodespace, 22, "key not found") - ErrorInvalidSigner = Register(testCodespace, 24, "tx intended signer does not match the given signer") - ErrInvalidChainID = Register(testCodespace, 28, "invalid chain-id") - ErrInvalidType = Register(testCodespace, 29, "invalid type") - ErrUnknownExtensionOptions = Register(testCodespace, 31, "unknown extension options") - ErrPackAny = Register(testCodespace, 33, "failed packing protobuf message to Any") - ErrLogic = Register(testCodespace, 35, "internal logic error") - ErrConflict = Register(testCodespace, 36, "conflict") - ErrNotSupported = Register(testCodespace, 37, "feature not supported") - ErrNotFound = Register(testCodespace, 38, "not found") - ErrIO = Register(testCodespace, 39, "Internal IO error") -) diff --git a/x/group/go.mod b/x/group/go.mod index c252cfc07818..c745bb4e32d4 100644 --- a/x/group/go.mod +++ b/x/group/go.mod @@ -42,6 +42,7 @@ require ( github.com/confio/ics23/go v0.6.6 // indirect github.com/cosmos/btcutil v1.0.4 // indirect github.com/cosmos/cosmos-sdk/db v0.0.0 // indirect + github.com/cosmos/cosmos-sdk/errors v0.0.0 // indirect github.com/cosmos/go-bip39 v1.0.0 // indirect github.com/cosmos/iavl v0.17.3 // indirect github.com/cosmos/ledger-cosmos-go v0.11.1 // indirect @@ -150,3 +151,5 @@ replace github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alp replace github.com/cosmos/cosmos-sdk => ../../ replace github.com/cosmos/cosmos-sdk/db => ../../db + +replace github.com/cosmos/cosmos-sdk/errors => ../../errors