Skip to content

Commit

Permalink
Merge PR #5421: Refactor Error Handling
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderbez authored Dec 27, 2019
1 parent 3c82b66 commit 9a183ff
Show file tree
Hide file tree
Showing 170 changed files with 2,309 additions and 2,926 deletions.
15 changes: 14 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
176 changes: 99 additions & 77 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
}
}
Expand Down Expand Up @@ -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] {
Expand All @@ -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:], "/")
Expand All @@ -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)
Expand All @@ -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]
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
}
Expand Down
Loading

0 comments on commit 9a183ff

Please sign in to comment.