Merge PR #5429: Refactor Error Handling - II (Modules)

baseapp/baseapp.go

@@ -656,12 +656,12 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s
 		msgRoute := msg.Route()
 		handler := app.router.Route(msgRoute)
 		if handler == nil {
-			return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized message route: %s, message index: %d", msgRoute, i)
+			return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized message route: %s; message index: %d", msgRoute, i)
 		}
 
 		msgResult, err := handler(ctx, msg)
 		if err != nil {
-			return nil, sdkerrors.Wrapf(err, "failed to execute message, message index: %d", i)
+			return nil, sdkerrors.Wrapf(err, "failed to execute message; message index: %d", i)
 		}
 
 		msgEvents := sdk.Events{

client/context/broadcast.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // BroadcastTx broadcasts a transactions either synchronously or asynchronously
@@ -52,19 +53,19 @@ func CheckTendermintError(err error, txBytes []byte) *sdk.TxResponse {
 	switch {
 	case strings.Contains(errStr, strings.ToLower(mempool.ErrTxInCache.Error())):
 		return &sdk.TxResponse{
-			Code:   uint32(sdk.CodeTxInMempoolCache),
+			Code:   sdkerrors.ErrTxInMempoolCache.ABCICode(),
 			TxHash: txHash,
 		}
 
 	case strings.Contains(errStr, "mempool is full"):
 		return &sdk.TxResponse{
-			Code:   uint32(sdk.CodeMempoolIsFull),
+			Code:   sdkerrors.ErrMempoolIsFull.ABCICode(),
 			TxHash: txHash,
 		}
 
 	case strings.Contains(errStr, "tx too large"):
 		return &sdk.TxResponse{
-			Code:   uint32(sdk.CodeTxTooLarge),
+			Code:   sdkerrors.ErrTxTooLarge.ABCICode(),
 			TxHash: txHash,
 		}
 

client/context/broadcast_test.go

@@ -4,16 +4,15 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/tendermint/tendermint/crypto/tmhash"
-
 	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto/tmhash"
 	"github.com/tendermint/tendermint/mempool"
 	"github.com/tendermint/tendermint/rpc/client/mock"
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
 	tmtypes "github.com/tendermint/tendermint/types"
 
 	"github.com/cosmos/cosmos-sdk/client/flags"
-	"github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 type MockClient struct {
@@ -43,9 +42,9 @@ func CreateContextWithErrorAndMode(err error, mode string) CLIContext {
 // Test the correct code is returned when
 func TestBroadcastError(t *testing.T) {
 	errors := map[error]uint32{
-		mempool.ErrTxInCache:       uint32(types.CodeTxInMempoolCache),
-		mempool.ErrTxTooLarge{}:    uint32(types.CodeTxTooLarge),
-		mempool.ErrMempoolIsFull{}: uint32(types.CodeMempoolIsFull),
+		mempool.ErrTxInCache:       sdkerrors.ErrTxInMempoolCache.ABCICode(),
+		mempool.ErrTxTooLarge{}:    sdkerrors.ErrTxTooLarge.ABCICode(),
+		mempool.ErrMempoolIsFull{}: sdkerrors.ErrMempoolIsFull.ABCICode(),
 	}
 
 	modes := []string{

docs/architecture/adr-015-ibc-packet-receiver.md

@@ -204,7 +204,7 @@ type PacketDataI interface {
 	GetCommitment() []byte // Commitment form that will be stored in the state.
 	GetTimeoutHeight() uint64
 
-	ValidateBasic() sdk.Error
+	ValidateBasic() error
 	Type() string
 }
 ```

server/mock/app.go

@@ -2,6 +2,7 @@ package mock
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -49,10 +50,10 @@ func NewApp(rootDir string, logger log.Logger) (abci.Application, error) {
 // KVStoreHandler is a simple handler that takes kvstoreTx and writes
 // them to the db
 func KVStoreHandler(storeKey sdk.StoreKey) sdk.Handler {
-	return func(ctx sdk.Context, msg sdk.Msg) sdk.Result {
+	return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
 		dTx, ok := msg.(kvstoreTx)
 		if !ok {
-			panic("KVStoreHandler should only receive kvstoreTx")
+			return nil, errors.New("KVStoreHandler should only receive kvstoreTx")
 		}
 
 		// tx is already unmarshalled
@@ -62,10 +63,9 @@ func KVStoreHandler(storeKey sdk.StoreKey) sdk.Handler {
 		store := ctx.KVStore(storeKey)
 		store.Set(key, value)
 
-		return sdk.Result{
-			Code: 0,
-			Log:  fmt.Sprintf("set %s=%s", key, value),
-		}
+		return &sdk.Result{
+			Log: fmt.Sprintf("set %s=%s", key, value),
+		}, nil
 	}
 }
 

server/mock/tx.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // An sdk.Tx which is its own sdk.Msg.
@@ -47,7 +48,7 @@ func (tx kvstoreTx) GetSignBytes() []byte {
 }
 
 // Should the app be calling this? Or only handlers?
-func (tx kvstoreTx) ValidateBasic() sdk.Error {
+func (tx kvstoreTx) ValidateBasic() error {
 	return nil
 }
 
@@ -57,7 +58,7 @@ func (tx kvstoreTx) GetSigners() []sdk.AccAddress {
 
 // takes raw transaction bytes and decodes them into an sdk.Tx. An sdk.Tx has
 // all the signatures and can be used to authenticate.
-func decodeTx(txBytes []byte) (sdk.Tx, sdk.Error) {
+func decodeTx(txBytes []byte) (sdk.Tx, error) {
 	var tx sdk.Tx
 
 	split := bytes.Split(txBytes, []byte("="))
@@ -68,7 +69,7 @@ func decodeTx(txBytes []byte) (sdk.Tx, sdk.Error) {
 		k, v := split[0], split[1]
 		tx = kvstoreTx{k, v, txBytes}
 	} else {
-		return nil, sdk.ErrTxDecode("too many =")
+		return nil, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "too many '='")
 	}
 
 	return tx, nil

simapp/app.go

@@ -158,7 +158,7 @@ func NewSimApp(
 	}
 
 	// init params keeper and subspaces
-	app.ParamsKeeper = params.NewKeeper(app.cdc, keys[params.StoreKey], tkeys[params.TStoreKey], params.DefaultCodespace)
+	app.ParamsKeeper = params.NewKeeper(app.cdc, keys[params.StoreKey], tkeys[params.TStoreKey])
 	app.subspaces[auth.ModuleName] = app.ParamsKeeper.Subspace(auth.DefaultParamspace)
 	app.subspaces[bank.ModuleName] = app.ParamsKeeper.Subspace(bank.DefaultParamspace)
 	app.subspaces[staking.ModuleName] = app.ParamsKeeper.Subspace(staking.DefaultParamspace)
@@ -174,25 +174,24 @@ func NewSimApp(
 		app.cdc, keys[auth.StoreKey], app.subspaces[auth.ModuleName], auth.ProtoBaseAccount,
 	)
 	app.BankKeeper = bank.NewBaseKeeper(
-		app.AccountKeeper, app.subspaces[bank.ModuleName], bank.DefaultCodespace,
-		app.BlacklistedAccAddrs(),
+		app.AccountKeeper, app.subspaces[bank.ModuleName], app.BlacklistedAccAddrs(),
 	)
 	app.SupplyKeeper = supply.NewKeeper(
 		app.cdc, keys[supply.StoreKey], app.AccountKeeper, app.BankKeeper, maccPerms,
 	)
 	stakingKeeper := staking.NewKeeper(
 		app.cdc, keys[staking.StoreKey], app.SupplyKeeper, app.subspaces[staking.ModuleName],
-		staking.DefaultCodespace)
+	)
 	app.MintKeeper = mint.NewKeeper(
 		app.cdc, keys[mint.StoreKey], app.subspaces[mint.ModuleName], &stakingKeeper,
 		app.SupplyKeeper, auth.FeeCollectorName,
 	)
 	app.DistrKeeper = distr.NewKeeper(
 		app.cdc, keys[distr.StoreKey], app.subspaces[distr.ModuleName], &stakingKeeper,
-		app.SupplyKeeper, distr.DefaultCodespace, auth.FeeCollectorName, app.ModuleAccountAddrs(),
+		app.SupplyKeeper, auth.FeeCollectorName, app.ModuleAccountAddrs(),
 	)
 	app.SlashingKeeper = slashing.NewKeeper(
-		app.cdc, keys[slashing.StoreKey], &stakingKeeper, app.subspaces[slashing.ModuleName], slashing.DefaultCodespace,
+		app.cdc, keys[slashing.StoreKey], &stakingKeeper, app.subspaces[slashing.ModuleName],
 	)
 	app.CrisisKeeper = crisis.NewKeeper(
 		app.subspaces[crisis.ModuleName], invCheckPeriod, app.SupplyKeeper, auth.FeeCollectorName,
@@ -201,8 +200,7 @@ func NewSimApp(
 
 	// create evidence keeper with router
 	evidenceKeeper := evidence.NewKeeper(
-		app.cdc, keys[evidence.StoreKey], app.subspaces[evidence.ModuleName], evidence.DefaultCodespace,
-		&app.StakingKeeper, app.SlashingKeeper,
+		app.cdc, keys[evidence.StoreKey], app.subspaces[evidence.ModuleName], &app.StakingKeeper, app.SlashingKeeper,
 	)
 	evidenceRouter := evidence.NewRouter()
 	// TODO: Register evidence routes.
@@ -217,7 +215,7 @@ func NewSimApp(
 		AddRoute(upgrade.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper))
 	app.GovKeeper = gov.NewKeeper(
 		app.cdc, keys[gov.StoreKey], app.subspaces[gov.ModuleName], app.SupplyKeeper,
-		&stakingKeeper, gov.DefaultCodespace, govRouter,
+		&stakingKeeper, govRouter,
 	)
 
 	// register the staking hooks

simapp/test_helpers.go

@@ -115,7 +115,7 @@ func CheckBalance(t *testing.T, app *SimApp, addr sdk.AccAddress, exp sdk.Coins)
 func SignCheckDeliver(
 	t *testing.T, cdc *codec.Codec, app *bam.BaseApp, header abci.Header, msgs []sdk.Msg,
 	accNums, seq []uint64, expSimPass, expPass bool, priv ...crypto.PrivKey,
-) sdk.Result {
+) (sdk.GasInfo, *sdk.Result, error) {
 
 	tx := helpers.GenTx(
 		msgs,
@@ -131,28 +131,32 @@ func SignCheckDeliver(
 	require.Nil(t, err)
 
 	// Must simulate now as CheckTx doesn't run Msgs anymore
-	res := app.Simulate(txBytes, tx)
+	_, res, err := app.Simulate(txBytes, tx)
 
 	if expSimPass {
-		require.Equal(t, sdk.CodeOK, res.Code, res.Log)
+		require.NoError(t, err)
+		require.NotNil(t, res)
 	} else {
-		require.NotEqual(t, sdk.CodeOK, res.Code, res.Log)
+		require.Error(t, err)
+		require.Nil(t, res)
 	}
 
 	// Simulate a sending a transaction and committing a block
 	app.BeginBlock(abci.RequestBeginBlock{Header: header})
-	res = app.Deliver(tx)
+	gInfo, res, err := app.Deliver(tx)
 
 	if expPass {
-		require.Equal(t, sdk.CodeOK, res.Code, res.Log)
+		require.NoError(t, err)
+		require.NotNil(t, res)
 	} else {
-		require.NotEqual(t, sdk.CodeOK, res.Code, res.Log)
+		require.Error(t, err)
+		require.Nil(t, res)
 	}
 
 	app.EndBlock(abci.RequestEndBlock{})
 	app.Commit()
 
-	return res
+	return gInfo, res, err
 }
 
 // GenSequenceOfTxs generates a set of signed transactions of messages, such

types/errors/abci_test.go

@@ -15,17 +15,17 @@ func TestABCInfo(t *testing.T) {
 		wantSpace string
 		wantLog   string
 	}{
-		"plain weave error": {
+		"plain SDK error": {
 			err:       ErrUnauthorized,
 			debug:     false,
 			wantLog:   "unauthorized",
 			wantCode:  ErrUnauthorized.code,
 			wantSpace: RootCodespace,
 		},
-		"wrapped weave error": {
+		"wrapped SDK error": {
 			err:       Wrap(Wrap(ErrUnauthorized, "foo"), "bar"),
 			debug:     false,
-			wantLog:   "bar: foo: unauthorized",
+			wantLog:   "unauthorized: foo: bar",
 			wantCode:  ErrUnauthorized.code,
 			wantSpace: RootCodespace,
 		},
@@ -36,7 +36,7 @@ func TestABCInfo(t *testing.T) {
 			wantCode:  0,
 			wantSpace: "",
 		},
-		"nil weave error is not an error": {
+		"nil SDK error is not an error": {
 			err:       (*Error)(nil),
 			debug:     false,
 			wantLog:   "",
@@ -112,23 +112,23 @@ func TestABCIInfoStacktrace(t *testing.T) {
 		wantStacktrace bool
 		wantErrMsg     string
 	}{
-		"wrapped weave error in debug mode provides stacktrace": {
+		"wrapped SDK error in debug mode provides stacktrace": {
 			err:            Wrap(ErrUnauthorized, "wrapped"),
 			debug:          true,
 			wantStacktrace: true,
-			wantErrMsg:     "wrapped: unauthorized",
+			wantErrMsg:     "unauthorized: wrapped",
 		},
-		"wrapped weave error in non-debug mode does not have stacktrace": {
+		"wrapped SDK error in non-debug mode does not have stacktrace": {
 			err:            Wrap(ErrUnauthorized, "wrapped"),
 			debug:          false,
 			wantStacktrace: false,
-			wantErrMsg:     "wrapped: unauthorized",
+			wantErrMsg:     "unauthorized: wrapped",
 		},
 		"wrapped stdlib error in debug mode provides stacktrace": {
 			err:            Wrap(fmt.Errorf("stdlib"), "wrapped"),
 			debug:          true,
 			wantStacktrace: true,
-			wantErrMsg:     "wrapped: stdlib",
+			wantErrMsg:     "stdlib: wrapped",
 		},
 		"wrapped stdlib error in non-debug mode does not have stacktrace": {
 			err:            Wrap(fmt.Errorf("stdlib"), "wrapped"),
@@ -163,7 +163,7 @@ func TestABCIInfoHidesStacktrace(t *testing.T) {
 	err := Wrap(ErrUnauthorized, "wrapped")
 	_, _, log := ABCIInfo(err, false)
 
-	if log != "wrapped: unauthorized" {
+	if log != "unauthorized: wrapped" {
 		t.Fatalf("unexpected message in non debug mode: %s", log)
 	}
 }
@@ -173,7 +173,7 @@ func TestRedact(t *testing.T) {
 		t.Error("reduct must not pass through panic error")
 	}
 	if err := Redact(ErrUnauthorized); !ErrUnauthorized.Is(err) {
-		t.Error("reduct should pass through weave error")
+		t.Error("reduct should pass through SDK error")
 	}
 
 	var cerr customErr
@@ -203,12 +203,12 @@ func TestABCIInfoSerializeErr(t *testing.T) {
 		"single error": {
 			src:   myErrDecode,
 			debug: false,
-			exp:   "test: tx parse error",
+			exp:   "tx parse error: test",
 		},
 		"second error": {
 			src:   myErrAddr,
 			debug: false,
-			exp:   "tester: invalid address",
+			exp:   "invalid address: tester",
 		},
 		"single error with debug": {
 			src:   myErrDecode,