diff --git a/baseapp/abci.go b/baseapp/abci.go index 57bcec661eff..52f1273e3f61 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -163,7 +163,7 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { tx, err := app.txDecoder(req.Tx) if err != nil { - return sdkerrors.ResponseCheckTx(err, 0, 0) + return sdkerrors.ResponseCheckTx(err, 0, 0, app.trace) } var mode runTxMode @@ -181,7 +181,7 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { gInfo, result, err := app.runTx(mode, req.Tx, tx) if err != nil { - return sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed) + return sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed, app.trace) } return abci.ResponseCheckTx{ @@ -201,12 +201,12 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx { tx, err := app.txDecoder(req.Tx) if err != nil { - return sdkerrors.ResponseDeliverTx(err, 0, 0) + return sdkerrors.ResponseDeliverTx(err, 0, 0, app.trace) } gInfo, result, err := app.runTx(runTxModeDeliver, req.Tx, tx) if err != nil { - return sdkerrors.ResponseDeliverTx(err, gInfo.GasWanted, gInfo.GasUsed) + return sdkerrors.ResponseDeliverTx(err, gInfo.GasWanted, gInfo.GasUsed, app.trace) } return abci.ResponseDeliverTx{ diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 6fa6a943dca1..cc1f37cb4fc8 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -106,6 +106,9 @@ type BaseApp struct { // nolint: maligned // application's version string appVersion string + + // trace set will return full stack traces for errors in ABCI Log field + trace bool } // NewBaseApp returns a reference to an initialized BaseApp. It accepts a @@ -127,6 +130,7 @@ func NewBaseApp( queryRouter: NewQueryRouter(), txDecoder: txDecoder, fauxMerkleMode: false, + trace: false, } for _, option := range options { option(app) @@ -354,6 +358,10 @@ func (app *BaseApp) setInterBlockCache(cache sdk.MultiStorePersistentCache) { app.interBlockCache = cache } +func (app *BaseApp) setTrace(trace bool) { + app.trace = trace +} + // Router returns the router of the BaseApp. func (app *BaseApp) Router() sdk.Router { if app.sealed { diff --git a/baseapp/options.go b/baseapp/options.go index ea3b42ac2451..b8fdef58b7f5 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -44,6 +44,11 @@ func SetInterBlockCache(cache sdk.MultiStorePersistentCache) func(*BaseApp) { return func(app *BaseApp) { app.setInterBlockCache(cache) } } +// SetTrace will turn on or off trace flag +func SetTrace(trace bool) func(*BaseApp) { + return func(app *BaseApp) { app.setTrace(trace) } +} + func (app *BaseApp) SetName(name string) { if app.sealed { panic("SetName() on sealed BaseApp") diff --git a/server/start.go b/server/start.go index c96c1e71c84f..ff1725b671fb 100644 --- a/server/start.go +++ b/server/start.go @@ -31,6 +31,7 @@ const ( FlagHaltTime = "halt-time" FlagInterBlockCache = "inter-block-cache" FlagUnsafeSkipUpgrades = "unsafe-skip-upgrades" + FlagTrace = "trace" FlagPruning = "pruning" FlagPruningKeepRecent = "pruning-keep-recent" @@ -87,6 +88,7 @@ which accepts a path for the resulting pprof file. cmd.Flags().Bool(flagWithTendermint, true, "Run abci app embedded in-process with tendermint") cmd.Flags().String(flagAddress, "tcp://0.0.0.0:26658", "Listen address") cmd.Flags().String(flagTraceStore, "", "Enable KVStore tracing to an output file") + cmd.Flags().Bool(FlagTrace, false, "Provide full stack traces for errors in ABCI Log") cmd.Flags().String( FlagMinGasPrices, "", "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)", @@ -101,6 +103,7 @@ which accepts a path for the resulting pprof file. cmd.Flags().Uint64(FlagPruningKeepRecent, 0, "Number of recent heights to keep on disk (ignored if pruning is not 'custom')") cmd.Flags().Uint64(FlagPruningKeepEvery, 0, "Offset heights to keep on disk after 'keep-every' (ignored if pruning is not 'custom')") cmd.Flags().Uint64(FlagPruningInterval, 0, "Height interval at which pruned heights are removed from disk (ignored if pruning is not 'custom')") + viper.BindPFlag(FlagTrace, cmd.Flags().Lookup(FlagTrace)) viper.BindPFlag(FlagPruning, cmd.Flags().Lookup(FlagPruning)) viper.BindPFlag(FlagPruningKeepRecent, cmd.Flags().Lookup(FlagPruningKeepRecent)) viper.BindPFlag(FlagPruningKeepEvery, cmd.Flags().Lookup(FlagPruningKeepEvery)) diff --git a/types/errors/abci.go b/types/errors/abci.go index cc5633543192..a14719a9ae60 100644 --- a/types/errors/abci.go +++ b/types/errors/abci.go @@ -1,7 +1,6 @@ package errors import ( - "errors" "fmt" "reflect" @@ -18,8 +17,6 @@ const ( // detailed error string. internalABCICodespace = UndefinedCodespace internalABCICode uint32 = 1 - internalABCILog string = "internal error" - // multiErrorABCICode uint32 = 1000 ) // ABCIInfo returns the ABCI error information as consumed by the tendermint @@ -44,8 +41,8 @@ func ABCIInfo(err error, debug bool) (codespace string, code uint32, log string) // ResponseCheckTx returns an ABCI ResponseCheckTx object with fields filled in // from the given error and gas values. -func ResponseCheckTx(err error, gw, gu uint64) abci.ResponseCheckTx { - space, code, log := ABCIInfo(err, false) +func ResponseCheckTx(err error, gw, gu uint64, debug bool) abci.ResponseCheckTx { + space, code, log := ABCIInfo(err, debug) return abci.ResponseCheckTx{ Codespace: space, Code: code, @@ -57,8 +54,8 @@ func ResponseCheckTx(err error, gw, gu uint64) abci.ResponseCheckTx { // ResponseDeliverTx returns an ABCI ResponseDeliverTx object with fields filled in // from the given error and gas values. -func ResponseDeliverTx(err error, gw, gu uint64) abci.ResponseDeliverTx { - space, code, log := ABCIInfo(err, false) +func ResponseDeliverTx(err error, gw, gu uint64, debug bool) abci.ResponseDeliverTx { + space, code, log := ABCIInfo(err, debug) return abci.ResponseDeliverTx{ Codespace: space, Code: code, @@ -160,10 +157,10 @@ func errIsNil(err error) bool { // originates. func Redact(err error) error { if ErrPanic.Is(err) { - return errors.New(internalABCILog) + return ErrPanic } if abciCode(err) == internalABCICode { - return errors.New(internalABCILog) + return errInternal } return err } diff --git a/types/errors/abci_test.go b/types/errors/abci_test.go index c098811ea474..1908e4f068e2 100644 --- a/types/errors/abci_test.go +++ b/types/errors/abci_test.go @@ -5,6 +5,8 @@ import ( "io" "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestABCInfo(t *testing.T) { @@ -46,7 +48,7 @@ func TestABCInfo(t *testing.T) { "stdlib is generic message": { err: io.EOF, debug: false, - wantLog: "internal error", + wantLog: "internal", wantCode: 1, wantSpace: UndefinedCodespace, }, @@ -60,7 +62,7 @@ func TestABCInfo(t *testing.T) { "wrapped stdlib is only a generic message": { err: Wrap(io.EOF, "cannot read file"), debug: false, - wantLog: "internal error", + wantLog: "internal", wantCode: 1, wantSpace: UndefinedCodespace, }, @@ -134,7 +136,7 @@ func TestABCIInfoStacktrace(t *testing.T) { err: Wrap(fmt.Errorf("stdlib"), "wrapped"), debug: false, wantStacktrace: false, - wantErrMsg: "internal error", + wantErrMsg: "internal", }, } @@ -169,21 +171,42 @@ func TestABCIInfoHidesStacktrace(t *testing.T) { } func TestRedact(t *testing.T) { - if err := Redact(ErrPanic); ErrPanic.Is(err) { - t.Error("reduct must not pass through panic error") - } - if err := Redact(ErrUnauthorized); !ErrUnauthorized.Is(err) { - t.Error("reduct should pass through SDK error") - } - - var cerr customErr - if err := Redact(cerr); err != cerr { - t.Error("reduct should pass through ABCI code error") + 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: ErrPanic, + }, + "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, + }, } - serr := fmt.Errorf("stdlib error") - if err := Redact(serr); err == serr { - t.Error("reduct must not pass through a stdlib error") + for name, tc := range cases { + spec := tc + t.Run(name, func(t *testing.T) { + redacted := Redact(spec.err) + if spec.untouched { + require.Equal(t, spec.err, redacted) + } else { + // see if we got the expected redact + require.Equal(t, spec.changed, redacted) + // make sure the ABCI code did not change + require.Equal(t, abciCode(spec.err), abciCode(redacted)) + } + }) } } @@ -215,41 +238,15 @@ func TestABCIInfoSerializeErr(t *testing.T) { debug: true, exp: fmt.Sprintf("%+v", myErrDecode), }, - // "multi error default encoder": { - // src: Append(myErrMsg, myErrAddr), - // exp: Append(myErrMsg, myErrAddr).Error(), - // }, - // "multi error default with internal": { - // src: Append(myErrMsg, myPanic), - // exp: "internal error", - // }, "redact in default encoder": { src: myPanic, - exp: "internal error", + exp: "panic", }, "do not redact in debug encoder": { src: myPanic, debug: true, exp: fmt.Sprintf("%+v", myPanic), }, - // "redact in multi error": { - // src: Append(myPanic, myErrMsg), - // debug: false, - // exp: "internal error", - // }, - // "no redact in multi error": { - // src: Append(myPanic, myErrMsg), - // debug: true, - // exp: `2 errors occurred: - // * panic - // * test: invalid message - // `, - // }, - // "wrapped multi error with redact": { - // src: Wrap(Append(myPanic, myErrMsg), "wrap"), - // debug: false, - // exp: "internal error", - // }, } for msg, spec := range specs { spec := spec