From 865acc725d3fa7a96de78cca2fd0d61aff0047db Mon Sep 17 00:00:00 2001 From: tyler Date: Fri, 13 May 2022 15:13:38 -0700 Subject: [PATCH] refactor: remove redacted message --- errors/abci.go | 19 +--------- errors/abci_test.go | 91 ++++++++------------------------------------- 2 files changed, 16 insertions(+), 94 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index f40d00c2cbcb..603f3b36ff1c 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -42,9 +42,8 @@ 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() + return err.Error() } type coder interface { @@ -111,19 +110,3 @@ func errIsNil(err error) bool { } return false } - -var errPanicWithMsg = Wrapf(ErrPanic, "error message redacted to hide potential sensitive info. Use the '--trace' flag if you are running a node to see the full stack trace error") - -// 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 index 0cfb4a16b65c..0982709bf7e6 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -57,13 +57,6 @@ func (s *abciTestSuite) TestABCInfo() { 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, @@ -71,13 +64,6 @@ func (s *abciTestSuite) TestABCInfo() { 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": { @@ -103,10 +89,12 @@ func (s *abciTestSuite) TestABCInfo() { } 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) + s.T().Run(testName, func(t *testing.T) { + space, code, log := ABCIInfo(tc.err, tc.debug) + s.Require().Equal(tc.wantSpace, space, testName) + s.Require().Equal(tc.wantCode, code, testName) + s.Require().Equal(tc.wantLog, log, testName) + }) } } @@ -135,25 +123,20 @@ func (s *abciTestSuite) TestABCIInfoStacktrace() { 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 = "cosmossdk.io/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) + s.T().Run(testName, func(t *testing.T) { + _, _, log := ABCIInfo(tc.err, tc.debug) + if !tc.wantStacktrace { + s.Require().Equal(tc.wantErrMsg, log, testName) + } else { + s.Require().True(strings.Contains(log, thisTestSrc), testName) + s.Require().True(strings.Contains(log, tc.wantErrMsg), testName) + } + }) } } @@ -163,46 +146,6 @@ func (s *abciTestSuite) TestABCIInfoHidesStacktrace() { 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. @@ -231,10 +174,6 @@ func (s *abciTestSuite) TestABCIInfoSerializeErr() { debug: true, exp: fmt.Sprintf("%+v", myErrDecode), }, - "redact in default encoder": { - src: myPanic, - exp: "error message redacted to hide potential sensitive info. Use the '--trace' flag if you are running a node to see the full stack trace error: panic", - }, "do not redact in debug encoder": { src: myPanic, debug: true,