refactor: remove redacted message (#11960)
## Description Closes: #11539 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
This commit is contained in:
parent
54d764b9a8
commit
bcf2088925
|
@ -220,6 +220,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
|||
* (gov) [\#11287](https://github.com/cosmos/cosmos-sdk/pull/11287) Fix error message when no flags are provided while executing `submit-legacy-proposal` transaction.
|
||||
* (x/auth) [\#11482](https://github.com/cosmos/cosmos-sdk/pull/11482) Improve panic message when attempting to register a method handler for a message that does not implement sdk.Msg
|
||||
* (x/staking) [\#11596](https://github.com/cosmos/cosmos-sdk/pull/11596) Add (re)delegation getters
|
||||
* (errors) [\#11960](https://github.com/cosmos/cosmos-sdk/pull/11960) Removed 'redacted' error message from defaultErrEncoder
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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,
|
||||
|
|
Loading…
Reference in New Issue