Refactor Error Handling

CHANGELOG.md

@@ -50,7 +50,20 @@ logic has been implemented for v0.38 target version. Applications can migrate vi
 
 ### API Breaking Changes
 
-* (client) [\#5442](https://github.com/cosmos/cosmos-sdk/pull/5442) Remove client/alias.go as it's not necessary and 
+* (baseapp/types) [\#5421](https://github.com/cosmos/cosmos-sdk/pull/5421) The `Error` interface (`types/errors.go`)
+  has been removed in favor of the concrete type defined in `types/errors/` which implements the standard `error`
+  interface. As a result, the `Handler` and `Querier` implementations now return a standard `error`.
+  Within `BaseApp`, `runTx` now returns a `(GasInfo, *Result, error)` tuple and `runMsgs` returns a
+  `(*Result, error)` tuple. A reference to a `Result` is now used to indicate success whereas an error
+  signals an invalid message or failed message execution. As a result, the fields `Code`, `Codespace`,
+  `GasWanted`, and `GasUsed` have been removed the `Result` type. The latter two fields are now found
+  in the `GasInfo` type which is always returned regardless of execution outcome.
+
+  Note to developers: Since all handlers and queriers must now return a standard `error`, the `types/errors/`
+  package contains all the relevant and pre-registered errors that you typically work with. A typical
+  error returned will look like `sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "...")`. You can retrieve
+  relevant ABCI information from the error via `ABCIInfo`.
+* (client) [\#5442](https://github.com/cosmos/cosmos-sdk/pull/5442) Remove client/alias.go as it's not necessary and
 components can be imported directly from the packages.
 * (store) [\#4748](https://github.com/cosmos/cosmos-sdk/pull/4748) The `CommitMultiStore` interface
 now requires a `SetInterBlockCache` method. Applications that do not wish to support this can simply

baseapp/abci.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // InitChain implements the ABCI interface. It runs the initialization logic
@@ -153,54 +154,66 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc
 	return
 }
 
-// CheckTx implements the ABCI interface. It runs the "basic checks" to see
-// whether or not a transaction can possibly be executed, first decoding and then
-// the ante handler (which checks signatures/fees/ValidateBasic).
-//
-// NOTE:CheckTx does not run the actual Msg handler function(s).
-func (app *BaseApp) CheckTx(req abci.RequestCheckTx) (res abci.ResponseCheckTx) {
-	var result sdk.Result
-
+// CheckTx implements the ABCI interface and executes a tx in CheckTx mode. In
+// CheckTx mode, messages are not executed. This means messages are only validated
+// and only the AnteHandler is executed. State is persisted to the BaseApp's
+// internal CheckTx state if the AnteHandler passes. Otherwise, the ResponseCheckTx
+// will contain releveant error information. Regardless of tx execution outcome,
+// the ResponseCheckTx will contain relevant gas execution context.
+func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 	tx, err := app.txDecoder(req.Tx)
+	if err != nil {
+		return sdkerrors.ResponseCheckTx(err, 0, 0)
+	}
+
+	var mode runTxMode
+
 	switch {
-	case err != nil:
-		result = err.Result()
 	case req.Type == abci.CheckTxType_New:
-		result = app.runTx(runTxModeCheck, req.Tx, tx)
+		mode = runTxModeCheck
+
 	case req.Type == abci.CheckTxType_Recheck:
-		result = app.runTx(runTxModeReCheck, req.Tx, tx)
+		mode = runTxModeReCheck
+
 	default:
-		panic(fmt.Sprintf("Unknown RequestCheckTx Type: %v", req.Type))
+		panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type))
+	}
+
+	gInfo, result, err := app.runTx(mode, req.Tx, tx)
+	if err != nil {
+		return sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed)
 	}
 
 	return abci.ResponseCheckTx{
-		Code:      uint32(result.Code),
-		Data:      result.Data,
+		GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
+		GasUsed:   int64(gInfo.GasUsed),   // TODO: Should type accept unsigned ints?
 		Log:       result.Log,
-		GasWanted: int64(result.GasWanted), // TODO: Should type accept unsigned ints?
-		GasUsed:   int64(result.GasUsed),   // TODO: Should type accept unsigned ints?
+		Data:      result.Data,
 		Events:    result.Events.ToABCIEvents(),
 	}
 }
 
-// DeliverTx implements the ABCI interface.
-func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliverTx) {
-	var result sdk.Result
-
+// DeliverTx implements the ABCI interface and executes a tx in DeliverTx mode.
+// State only gets persisted if all messages are valid and get executed successfully.
+// Otherwise, the ResponseDeliverTx will contain releveant error information.
+// Regardless of tx execution outcome, the ResponseDeliverTx will contain relevant
+// gas execution context.
+func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
 	tx, err := app.txDecoder(req.Tx)
 	if err != nil {
-		result = err.Result()
-	} else {
-		result = app.runTx(runTxModeDeliver, req.Tx, tx)
+		return sdkerrors.ResponseDeliverTx(err, 0, 0)
+	}
+
+	gInfo, result, err := app.runTx(runTxModeDeliver, req.Tx, tx)
+	if err != nil {
+		return sdkerrors.ResponseDeliverTx(err, gInfo.GasWanted, gInfo.GasUsed)
 	}
 
 	return abci.ResponseDeliverTx{
-		Code:      uint32(result.Code),
-		Codespace: string(result.Codespace),
-		Data:      result.Data,
+		GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
+		GasUsed:   int64(gInfo.GasUsed),   // TODO: Should type accept unsigned ints?
 		Log:       result.Log,
-		GasWanted: int64(result.GasWanted), // TODO: Should type accept unsigned ints?
-		GasUsed:   int64(result.GasUsed),   // TODO: Should type accept unsigned ints?
+		Data:      result.Data,
 		Events:    result.Events.ToABCIEvents(),
 	}
 }
@@ -278,11 +291,10 @@ func (app *BaseApp) halt() {
 
 // Query implements the ABCI interface. It delegates to CommitMultiStore if it
 // implements Queryable.
-func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
+func (app *BaseApp) Query(req abci.RequestQuery) abci.ResponseQuery {
 	path := splitPath(req.Path)
 	if len(path) == 0 {
-		msg := "no query path provided"
-		return sdk.ErrUnknownRequest(msg).QueryResult()
+		sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "no query path provided"))
 	}
 
 	switch path[0] {
@@ -294,61 +306,59 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 		return handleQueryStore(app, path, req)
 
 	case "p2p":
-		return handleQueryP2P(app, path, req)
+		return handleQueryP2P(app, path)
 
 	case "custom":
 		return handleQueryCustom(app, path, req)
 	}
 
-	msg := "unknown query path"
-	return sdk.ErrUnknownRequest(msg).QueryResult()
+	return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "unknown query path"))
 }
 
-func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
+func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
 	if len(path) >= 2 {
-		var result sdk.Result
-
 		switch path[1] {
 		case "simulate":
 			txBytes := req.Data
+
 			tx, err := app.txDecoder(txBytes)
 			if err != nil {
-				result = err.Result()
-			} else {
-				result = app.Simulate(txBytes, tx)
+				return sdkerrors.QueryResult(sdkerrors.Wrap(err, "failed to decode tx"))
+			}
+
+			gInfo, _, _ := app.Simulate(txBytes, tx)
+
+			return abci.ResponseQuery{
+				Codespace: sdkerrors.RootCodespace,
+				Height:    req.Height,
+				Value:     codec.Cdc.MustMarshalBinaryLengthPrefixed(gInfo.GasUsed),
 			}
 
 		case "version":
 			return abci.ResponseQuery{
-				Code:      uint32(sdk.CodeOK),
-				Codespace: string(sdk.CodespaceRoot),
+				Codespace: sdkerrors.RootCodespace,
 				Height:    req.Height,
 				Value:     []byte(app.appVersion),
 			}
 
 		default:
-			result = sdk.ErrUnknownRequest(fmt.Sprintf("unknown query: %s", path)).Result()
-		}
-
-		value := codec.Cdc.MustMarshalBinaryLengthPrefixed(result)
-		return abci.ResponseQuery{
-			Code:      uint32(sdk.CodeOK),
-			Codespace: string(sdk.CodespaceRoot),
-			Height:    req.Height,
-			Value:     value,
+			return sdkerrors.QueryResult(sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unknown query: %s", path))
 		}
 	}
 
-	msg := "expected second parameter to be either 'simulate' or 'version', neither was present"
-	return sdk.ErrUnknownRequest(msg).QueryResult()
+	return sdkerrors.QueryResult(
+		sdkerrors.Wrap(
+			sdkerrors.ErrUnknownRequest,
+			"expected second parameter to be either 'simulate' or 'version', neither was present",
+		),
+	)
 }
 
 func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
 	// "/store" prefix for store queries
 	queryable, ok := app.cms.(sdk.Queryable)
 	if !ok {
-		msg := "multistore doesn't support queries"
-		return sdk.ErrUnknownRequest(msg).QueryResult()
+		return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "multistore doesn't support queries"))
 	}
 
 	req.Path = "/" + strings.Join(path[1:], "/")
@@ -359,7 +369,12 @@ func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.R
 	}
 
 	if req.Height <= 1 && req.Prove {
-		return sdk.ErrInternal("cannot query with proof when height <= 1; please provide a valid height").QueryResult()
+		return sdkerrors.QueryResult(
+			sdkerrors.Wrap(
+				sdkerrors.ErrInvalidRequest,
+				"cannot query with proof when height <= 1; please provide a valid height",
+			),
+		)
 	}
 
 	resp := queryable.Query(req)
@@ -368,7 +383,7 @@ func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.R
 	return resp
 }
 
-func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci.ResponseQuery) {
+func handleQueryP2P(app *BaseApp, path []string) abci.ResponseQuery {
 	// "/p2p" prefix for p2p queries
 	if len(path) >= 4 {
 		cmd, typ, arg := path[1], path[2], path[3]
@@ -383,28 +398,30 @@ func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci.
 			}
 
 		default:
-			msg := "expected second parameter to be 'filter'"
-			return sdk.ErrUnknownRequest(msg).QueryResult()
+			return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "expected second parameter to be 'filter'"))
 		}
 	}
 
-	msg := "Expected path is p2p filter <addr|id> <parameter>"
-	return sdk.ErrUnknownRequest(msg).QueryResult()
+	return sdkerrors.QueryResult(
+		sdkerrors.Wrap(
+			sdkerrors.ErrUnknownRequest, "expected path is p2p filter <addr|id> <parameter>",
+		),
+	)
 }
 
-func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
+func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
 	// path[0] should be "custom" because "/custom" prefix is required for keeper
 	// queries.
 	//
 	// The QueryRouter routes using path[1]. For example, in the path
 	// "custom/gov/proposal", QueryRouter routes using "gov".
 	if len(path) < 2 || path[1] == "" {
-		return sdk.ErrUnknownRequest("No route for custom query specified").QueryResult()
+		return sdkerrors.QueryResult(sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, "no route for custom query specified"))
 	}
 
 	querier := app.queryRouter.Route(path[1])
 	if querier == nil {
-		return sdk.ErrUnknownRequest(fmt.Sprintf("no custom querier found for route %s", path[1])).QueryResult()
+		return sdkerrors.QueryResult(sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "no custom querier found for route %s", path[1]))
 	}
 
 	// when a client did not provide a query height, manually inject the latest
@@ -413,17 +430,22 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
 	}
 
 	if req.Height <= 1 && req.Prove {
-		return sdk.ErrInternal("cannot query with proof when height <= 1; please provide a valid height").QueryResult()
+		return sdkerrors.QueryResult(
+			sdkerrors.Wrap(
+				sdkerrors.ErrInvalidRequest,
+				"cannot query with proof when height <= 1; please provide a valid height",
+			),
+		)
 	}
 
 	cacheMS, err := app.cms.CacheMultiStoreWithVersion(req.Height)
 	if err != nil {
-		return sdk.ErrInternal(
-			fmt.Sprintf(
-				"failed to load state at height %d; %s (latest height: %d)",
-				req.Height, err, app.LastBlockHeight(),
+		return sdkerrors.QueryResult(
+			sdkerrors.Wrapf(
+				sdkerrors.ErrInvalidRequest,
+				"failed to load state at height %d; %s (latest height: %d)", req.Height, err, app.LastBlockHeight(),
 			),
-		).QueryResult()
+		)
 	}
 
 	// cache wrap the commit-multistore for safety
@@ -435,18 +457,18 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
 	//
 	// For example, in the path "custom/gov/proposal/test", the gov querier gets
 	// []string{"proposal", "test"} as the path.
-	resBytes, queryErr := querier(ctx, path[2:], req)
-	if queryErr != nil {
+	resBytes, err := querier(ctx, path[2:], req)
+	if err != nil {
+		space, code, log := sdkerrors.ABCIInfo(err, false)
 		return abci.ResponseQuery{
-			Code:      uint32(queryErr.Code()),
-			Codespace: string(queryErr.Codespace()),
+			Code:      code,
+			Codespace: space,
+			Log:       log,
 			Height:    req.Height,
-			Log:       queryErr.ABCILog(),
 		}
 	}
 
 	return abci.ResponseQuery{
-		Code:   uint32(sdk.CodeOK),
 		Height: req.Height,
 		Value:  resBytes,
 	}