diff --git a/CHANGELOG.md b/CHANGELOG.md index 08cd61e2c234..bbf05de9a23b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -272,6 +272,7 @@ functionality that requires an online connection. * (x/ibc) [\#5948](https://github.com/cosmos/cosmos-sdk/issues/5948) Add `InitGenesis` and `ExportGenesis` functions for `ibc` module. * (types) [\#6128](https://github.com/cosmos/cosmos-sdk/pull/6137) Add `String()` method to `GasMeter`. * (types) [\#6195](https://github.com/cosmos/cosmos-sdk/pull/6195) Add codespace to broadcast(sync/async) response. +* (baseapp) [\#6053](https://github.com/cosmos/cosmos-sdk/pull/6053) Customizable panic recovery handling added for `app.runTx()` method (as proposed in the [ADR 22](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-022-custom-panic-handling.md)). Adds ability for developers to register custom panic handlers extending standard ones. ## [v0.38.4] - 2020-05-21 diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 77c95cef8b93..83d32a0b5214 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -3,7 +3,6 @@ package baseapp import ( "fmt" "reflect" - "runtime/debug" "strings" "github.com/gogo/protobuf/grpc" @@ -93,6 +92,9 @@ type BaseApp struct { // nolint: maligned // application's version string appVersion string + + // recovery handler for app.runTx method + runTxRecoveryMiddleware recoveryMiddleware } // NewBaseApp returns a reference to an initialized BaseApp. It accepts a @@ -124,6 +126,8 @@ func NewBaseApp( app.cms.SetInterBlockCache(app.interBlockCache) } + app.runTxRecoveryMiddleware = newDefaultRecoveryMiddleware() + return app } @@ -352,6 +356,13 @@ func (app *BaseApp) GetConsensusParams(ctx sdk.Context) *abci.ConsensusParams { return cp } +// AddRunTxRecoveryHandler adds custom app.runTx method panic handlers. +func (app *BaseApp) AddRunTxRecoveryHandler(handlers ...RecoveryHandler) { + for _, h := range handlers { + app.runTxRecoveryMiddleware = newRecoveryMiddleware(h, app.runTxRecoveryMiddleware) + } +} + // StoreConsensusParams sets the consensus parameters to the baseapp's param store. func (app *BaseApp) StoreConsensusParams(ctx sdk.Context, cp *abci.ConsensusParams) { if app.paramStore == nil { @@ -496,26 +507,8 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (gInfo sdk. defer func() { if r := recover(); r != nil { - switch rType := r.(type) { - // TODO: Use ErrOutOfGas instead of ErrorOutOfGas which would allow us - // to keep the stracktrace. - case sdk.ErrorOutOfGas: - err = sdkerrors.Wrap( - sdkerrors.ErrOutOfGas, fmt.Sprintf( - "out of gas in location: %v; gasWanted: %d, gasUsed: %d", - rType.Descriptor, gasWanted, ctx.GasMeter().GasConsumed(), - ), - ) - - default: - err = sdkerrors.Wrap( - sdkerrors.ErrPanic, fmt.Sprintf( - "recovered: %v\nstack:\n%v", r, string(debug.Stack()), - ), - ) - } - - result = nil + recoveryMW := newOutOfGasRecoveryMiddleware(gasWanted, ctx, app.runTxRecoveryMiddleware) + err, result = processRecovery(r, recoveryMW), nil } gInfo = sdk.GasInfo{GasWanted: gasWanted, GasUsed: ctx.GasMeter().GasConsumed()} diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index da3b70ff905a..6d42972c106d 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -1305,6 +1305,49 @@ func TestMaxBlockGasLimits(t *testing.T) { } } +// Test custom panic handling within app.DeliverTx method +func TestCustomRunTxPanicHandler(t *testing.T) { + const customPanicMsg = "test panic" + anteErr := sdkerrors.Register("fakeModule", 100500, "fakeError") + + anteOpt := func(bapp *BaseApp) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { + panic(sdkerrors.Wrap(anteErr, "anteHandler")) + return + }) + } + routerOpt := func(bapp *BaseApp) { + bapp.Router().AddRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) { + return &sdk.Result{}, nil + }) + } + + app := setupBaseApp(t, anteOpt, routerOpt) + + header := abci.Header{Height: 1} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + + app.AddRunTxRecoveryHandler(func(recoveryObj interface{}) error { + err, ok := recoveryObj.(error) + if !ok { + return nil + } + + if anteErr.Is(err) { + panic(customPanicMsg) + } else { + return nil + } + }) + + // Transaction should panic with custom handler above + { + tx := newTxCounter(0, 0) + + require.PanicsWithValue(t, customPanicMsg, func() { app.Deliver(tx) }) + } +} + func TestBaseAppAnteHandler(t *testing.T) { anteKey := []byte("ante-key") anteOpt := func(bapp *BaseApp) { diff --git a/baseapp/recovery.go b/baseapp/recovery.go new file mode 100644 index 000000000000..7f0687800c65 --- /dev/null +++ b/baseapp/recovery.go @@ -0,0 +1,77 @@ +package baseapp + +import ( + "fmt" + "runtime/debug" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// RecoveryHandler handles recovery() object. +// Return a non-nil error if recoveryObj was processed. +// Return nil if recoveryObj was not processed. +type RecoveryHandler func(recoveryObj interface{}) error + +// recoveryMiddleware is wrapper for RecoveryHandler to create chained recovery handling. +// returns (recoveryMiddleware, nil) if recoveryObj was not processed and should be passed to the next middleware in chain. +// returns (nil, error) if recoveryObj was processed and middleware chain processing should be stopped. +type recoveryMiddleware func(recoveryObj interface{}) (recoveryMiddleware, error) + +// processRecovery processes recoveryMiddleware chain for recovery() object. +// Chain processing stops on non-nil error or when chain is processed. +func processRecovery(recoveryObj interface{}, middleware recoveryMiddleware) error { + if middleware == nil { + return nil + } + + next, err := middleware(recoveryObj) + if err != nil { + return err + } + + return processRecovery(recoveryObj, next) +} + +// newRecoveryMiddleware creates a RecoveryHandler middleware. +func newRecoveryMiddleware(handler RecoveryHandler, next recoveryMiddleware) recoveryMiddleware { + return func(recoveryObj interface{}) (recoveryMiddleware, error) { + if err := handler(recoveryObj); err != nil { + return nil, err + } + + return next, nil + } +} + +// newOutOfGasRecoveryMiddleware creates a standard OutOfGas recovery middleware for app.runTx method. +func newOutOfGasRecoveryMiddleware(gasWanted uint64, ctx sdk.Context, next recoveryMiddleware) recoveryMiddleware { + handler := func(recoveryObj interface{}) error { + err, ok := recoveryObj.(sdk.ErrorOutOfGas) + if !ok { + return nil + } + + return sdkerrors.Wrap( + sdkerrors.ErrOutOfGas, fmt.Sprintf( + "out of gas in location: %v; gasWanted: %d, gasUsed: %d", + err.Descriptor, gasWanted, ctx.GasMeter().GasConsumed(), + ), + ) + } + + return newRecoveryMiddleware(handler, next) +} + +// newDefaultRecoveryMiddleware creates a default (last in chain) recovery middleware for app.runTx method. +func newDefaultRecoveryMiddleware() recoveryMiddleware { + handler := func(recoveryObj interface{}) error { + return sdkerrors.Wrap( + sdkerrors.ErrPanic, fmt.Sprintf( + "recovered: %v\nstack:\n%v", recoveryObj, string(debug.Stack()), + ), + ) + } + + return newRecoveryMiddleware(handler, nil) +} diff --git a/baseapp/recovery_test.go b/baseapp/recovery_test.go new file mode 100644 index 000000000000..b75892c63818 --- /dev/null +++ b/baseapp/recovery_test.go @@ -0,0 +1,64 @@ +package baseapp + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +// Test that recovery chain produces expected error at specific middleware layer +func TestRecoveryChain(t *testing.T) { + createError := func(id int) error { + return fmt.Errorf("error from id: %d", id) + } + + createHandler := func(id int, handle bool) RecoveryHandler { + return func(_ interface{}) error { + if handle { + return createError(id) + } + return nil + } + } + + // check recovery chain [1] -> 2 -> 3 + { + mw := newRecoveryMiddleware(createHandler(3, false), nil) + mw = newRecoveryMiddleware(createHandler(2, false), mw) + mw = newRecoveryMiddleware(createHandler(1, true), mw) + receivedErr := processRecovery(nil, mw) + + require.Equal(t, createError(1), receivedErr) + } + + // check recovery chain 1 -> [2] -> 3 + { + mw := newRecoveryMiddleware(createHandler(3, false), nil) + mw = newRecoveryMiddleware(createHandler(2, true), mw) + mw = newRecoveryMiddleware(createHandler(1, false), mw) + receivedErr := processRecovery(nil, mw) + + require.Equal(t, createError(2), receivedErr) + } + + // check recovery chain 1 -> 2 -> [3] + { + mw := newRecoveryMiddleware(createHandler(3, true), nil) + mw = newRecoveryMiddleware(createHandler(2, false), mw) + mw = newRecoveryMiddleware(createHandler(1, false), mw) + receivedErr := processRecovery(nil, mw) + + require.Equal(t, createError(3), receivedErr) + } + + // check recovery chain 1 -> 2 -> 3 + { + mw := newRecoveryMiddleware(createHandler(3, false), nil) + mw = newRecoveryMiddleware(createHandler(2, false), mw) + mw = newRecoveryMiddleware(createHandler(1, false), mw) + receivedErr := processRecovery(nil, mw) + + require.Nil(t, receivedErr) + } +} diff --git a/docs/core/README.md b/docs/core/README.md index fab8a5361ba3..f8a024d59edc 100644 --- a/docs/core/README.md +++ b/docs/core/README.md @@ -16,5 +16,6 @@ This repository contains reference documentation on the core concepts of the Cos 6. [Encoding](./encoding.md) 7. [Events](./events.md) 8. [Object-Capabilities](./ocap.md) +9. [RunTx recovery middleware](./runtx_panics.md) After reading about the core concepts, head on to the [Building Modules documentation](../building-modules/README.md) to learn more about the process of building modules. \ No newline at end of file diff --git a/docs/core/runtx_panics.md b/docs/core/runtx_panics.md new file mode 100644 index 000000000000..66a7576ca626 --- /dev/null +++ b/docs/core/runtx_panics.md @@ -0,0 +1,66 @@ + + +# RunTx recovery middleware + +`BaseApp.runTx()` function handles Golang panics that might occur during transactions execution, for example, keeper has faced an invalid state and paniced. +Depending on the panic type different handler is used, for instance the default one prints an error log message. +Recovery middleware is used to add custom panic recovery for SDK application developers. + +More context could be found in the corresponding [ADR-022](../architecture/adr-022-custom-panic-handling.md). + +Implementation could be found in the [recovery.go](../../baseapp/recovery.go) file. + +## Interface + +```go +type RecoveryHandler func(recoveryObj interface{}) error +``` + +`recoveryObj` is a return value for `recover()` function from the `buildin` Golang package. + +**Contract:** +* RecoveryHandler returns `nil` if `recoveryObj` wasn't handled and should be passed to the next recovery middleware; +* RecoveryHandler returns a non-nil `error` if `recoveryObj` was handled; + +## Custom RecoveryHandler register + +``BaseApp.AddRunTxRecoveryHandler(handlers ...RecoveryHandler)`` + +BaseApp method adds recovery middleware to the default recovery chain. + +## Example + +Lets assume we want to emit the "Consensus failure" chain state if some particular error occurred. + +We have a module keeper that panics: +```go +func (k FooKeeper) Do(obj interface{}) { + if obj == nil { + // that shouldn't happen, we need to crash the app + err := sdkErrors.Wrap(fooTypes.InternalError, "obj is nil") + panic(err) + } +} +``` + +By default that panic would be recovered and an error message will be printed to log. To override that behaviour we should register a custom RecoveryHandler: +```go +// SDK application constructor +customHandler := func(recoveryObj interface{}) error { + err, ok := recoveryObj.(error) + if !ok { + return nil + } + + if fooTypes.InternalError.Is(err) { + panic(fmt.Errorf("FooKeeper did panic with error: %w", err)) + } + + return nil +} + +baseApp := baseapp.NewBaseApp(...) +baseApp.AddRunTxRecoveryHandler(customHandler) +```