Baseapp recovery middleware

CHANGELOG.md

@@ -253,6 +253,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
 

baseapp/baseapp.go

@@ -3,7 +3,6 @@ package baseapp
 import (
 	"fmt"
 	"reflect"
-	"runtime/debug"
 	"strings"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -90,6 +89,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
@@ -120,6 +122,8 @@ func NewBaseApp(
 		app.cms.SetInterBlockCache(app.interBlockCache)
 	}
 
+	app.runTxRecoveryMiddleware = newDefaultRecoveryMiddleware()
+
 	return app
 }
 
@@ -345,6 +349,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 {
@@ -489,26 +500,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()}

baseapp/baseapp_test.go

@@ -1294,6 +1294,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) {

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)
+}

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)
+	}
+}

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. 

docs/core/runtx_panics.md

@@ -0,0 +1,66 @@
+<!--
+order: 9
+-->
+
+# 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)
+```