From 89f4afaf24c9fdfb98db239212133357f43b51fd Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 31 Jul 2019 13:58:53 +0200 Subject: [PATCH] Return codespace as well in ABCIInfo --- errors/abci.go | 36 +++++++++++++++++++++++++++++++----- errors/abci_test.go | 23 +++++++++++++++++++---- errors/errors.go | 3 +-- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index 5942527c3440..4de360e2f887 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -20,15 +20,15 @@ const ( ) // ABCIInfo returns the ABCI error information as consumed by the tendermint -// client. Returned code and log message should be used as a ABCI response. +// 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. +// 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) (uint32, string) { +func ABCIInfo(err error, debug bool) (codespace string, code uint32, log string) { if errIsNil(err) { - return SuccessABCICode, "" + return "", SuccessABCICode, "" } encode := defaultErrEncoder @@ -36,7 +36,7 @@ func ABCIInfo(err error, debug bool) (uint32, string) { encode = debugErrEncoder } - return abciCode(err), encode(err) + return abciCodespace(err), abciCode(err), encode(err) } // The debugErrEncoder encodes the error with a stacktrace. @@ -74,6 +74,32 @@ func abciCode(err error) uint32 { } } +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 UndefinedCodespace + } + } +} + + // 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 diff --git a/errors/abci_test.go b/errors/abci_test.go index 80a8ad669bef..ae58cec2474c 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -12,6 +12,7 @@ func TestABCInfo(t *testing.T) { err error debug bool wantCode uint32 + wantSpace string wantLog string }{ "plain weave error": { @@ -19,42 +20,49 @@ func TestABCInfo(t *testing.T) { debug: false, wantLog: "unauthorized", wantCode: ErrUnauthorized.code, + wantSpace: RootCodespace, }, "wrapped weave 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 weave 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 error", 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 error", wantCode: 1, + wantSpace: UndefinedCodespace, }, // This is hard to test because of attached stacktrace. This // case is tested in an another test. @@ -69,18 +77,23 @@ func TestABCInfo(t *testing.T) { 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 { t.Run(testName, func(t *testing.T) { - code, log := ABCIInfo(tc.err, tc.debug) + space, code, log := ABCIInfo(tc.err, tc.debug) + if space != tc.wantSpace { + t.Errorf("want %s space, got %s", tc.wantSpace, space) + } if code != tc.wantCode { t.Errorf("want %d code, got %d", tc.wantCode, code) } @@ -128,7 +141,7 @@ func TestABCIInfoStacktrace(t *testing.T) { for testName, tc := range cases { t.Run(testName, func(t *testing.T) { - _, log := ABCIInfo(tc.err, tc.debug) + _, _, log := ABCIInfo(tc.err, tc.debug) if tc.wantStacktrace { if !strings.Contains(log, thisTestSrc) { t.Errorf("log does not contain this file stack trace: %s", log) @@ -148,7 +161,7 @@ func TestABCIInfoStacktrace(t *testing.T) { func TestABCIInfoHidesStacktrace(t *testing.T) { err := Wrap(ErrUnauthorized, "wrapped") - _, log := ABCIInfo(err, false) + _, _, log := ABCIInfo(err, false) if log != "wrapped: unauthorized" { t.Fatalf("unexpected message in non debug mode: %s", log) @@ -240,7 +253,7 @@ func TestABCIInfoSerializeErr(t *testing.T) { } for msg, spec := range specs { t.Run(msg, func(t *testing.T) { - _, log := ABCIInfo(spec.src, spec.debug) + _, _, log := ABCIInfo(spec.src, spec.debug) if exp, got := spec.exp, log; exp != got { t.Errorf("expected %v but got %v", exp, got) } @@ -252,6 +265,8 @@ func TestABCIInfoSerializeErr(t *testing.T) { // 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/errors.go b/errors/errors.go index 2658289fd2c7..ef806177397d 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -11,8 +11,7 @@ import ( const RootCodespace = "sdk" // UndefinedCodespace when we explicitly declare no codespace -const UndefinedCodespace = "" - +const UndefinedCodespace = "undefined" var ( // errInternal should never be exposed, but we reserve this code for non-specified errors