Merge PR #6551: Ethanfrey/fix trace flag

baseapp/abci.go

@@ -163,7 +163,7 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc
 func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 	tx, err := app.txDecoder(req.Tx)
 	if err != nil {
-		return sdkerrors.ResponseCheckTx(err, 0, 0)
+		return sdkerrors.ResponseCheckTx(err, 0, 0, app.trace)
 	}
 
 	var mode runTxMode
@@ -181,7 +181,7 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 
 	gInfo, result, err := app.runTx(mode, req.Tx, tx)
 	if err != nil {
-		return sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed)
+		return sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed, app.trace)
 	}
 
 	return abci.ResponseCheckTx{
@@ -201,12 +201,12 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
 	tx, err := app.txDecoder(req.Tx)
 	if err != nil {
-		return sdkerrors.ResponseDeliverTx(err, 0, 0)
+		return sdkerrors.ResponseDeliverTx(err, 0, 0, app.trace)
 	}
 
 	gInfo, result, err := app.runTx(runTxModeDeliver, req.Tx, tx)
 	if err != nil {
-		return sdkerrors.ResponseDeliverTx(err, gInfo.GasWanted, gInfo.GasUsed)
+		return sdkerrors.ResponseDeliverTx(err, gInfo.GasWanted, gInfo.GasUsed, app.trace)
 	}
 
 	return abci.ResponseDeliverTx{

baseapp/baseapp.go

@@ -106,6 +106,9 @@ type BaseApp struct { // nolint: maligned
 
 	// application's version string
 	appVersion string
+
+	// trace set will return full stack traces for errors in ABCI Log field
+	trace bool
 }
 
 // NewBaseApp returns a reference to an initialized BaseApp. It accepts a
@@ -127,6 +130,7 @@ func NewBaseApp(
 		queryRouter:    NewQueryRouter(),
 		txDecoder:      txDecoder,
 		fauxMerkleMode: false,
+		trace:          false,
 	}
 	for _, option := range options {
 		option(app)
@@ -354,6 +358,10 @@ func (app *BaseApp) setInterBlockCache(cache sdk.MultiStorePersistentCache) {
 	app.interBlockCache = cache
 }
 
+func (app *BaseApp) setTrace(trace bool) {
+	app.trace = trace
+}
+
 // Router returns the router of the BaseApp.
 func (app *BaseApp) Router() sdk.Router {
 	if app.sealed {

baseapp/options.go

@@ -44,6 +44,11 @@ func SetInterBlockCache(cache sdk.MultiStorePersistentCache) func(*BaseApp) {
 	return func(app *BaseApp) { app.setInterBlockCache(cache) }
 }
 
+// SetTrace will turn on or off trace flag
+func SetTrace(trace bool) func(*BaseApp) {
+	return func(app *BaseApp) { app.setTrace(trace) }
+}
+
 func (app *BaseApp) SetName(name string) {
 	if app.sealed {
 		panic("SetName() on sealed BaseApp")

server/start.go

@@ -31,6 +31,7 @@ const (
 	FlagHaltTime           = "halt-time"
 	FlagInterBlockCache    = "inter-block-cache"
 	FlagUnsafeSkipUpgrades = "unsafe-skip-upgrades"
+	FlagTrace              = "trace"
 
 	FlagPruning           = "pruning"
 	FlagPruningKeepRecent = "pruning-keep-recent"
@@ -87,6 +88,7 @@ which accepts a path for the resulting pprof file.
 	cmd.Flags().Bool(flagWithTendermint, true, "Run abci app embedded in-process with tendermint")
 	cmd.Flags().String(flagAddress, "tcp://0.0.0.0:26658", "Listen address")
 	cmd.Flags().String(flagTraceStore, "", "Enable KVStore tracing to an output file")
+	cmd.Flags().Bool(FlagTrace, false, "Provide full stack traces for errors in ABCI Log")
 	cmd.Flags().String(
 		FlagMinGasPrices, "",
 		"Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)",
@@ -101,6 +103,7 @@ which accepts a path for the resulting pprof file.
 	cmd.Flags().Uint64(FlagPruningKeepRecent, 0, "Number of recent heights to keep on disk (ignored if pruning is not 'custom')")
 	cmd.Flags().Uint64(FlagPruningKeepEvery, 0, "Offset heights to keep on disk after 'keep-every' (ignored if pruning is not 'custom')")
 	cmd.Flags().Uint64(FlagPruningInterval, 0, "Height interval at which pruned heights are removed from disk (ignored if pruning is not 'custom')")
+	viper.BindPFlag(FlagTrace, cmd.Flags().Lookup(FlagTrace))
 	viper.BindPFlag(FlagPruning, cmd.Flags().Lookup(FlagPruning))
 	viper.BindPFlag(FlagPruningKeepRecent, cmd.Flags().Lookup(FlagPruningKeepRecent))
 	viper.BindPFlag(FlagPruningKeepEvery, cmd.Flags().Lookup(FlagPruningKeepEvery))

types/errors/abci.go

@@ -1,7 +1,6 @@
 package errors
 
 import (
-	"errors"
 	"fmt"
 	"reflect"
 
@@ -18,8 +17,6 @@ const (
 	// detailed error string.
 	internalABCICodespace        = UndefinedCodespace
 	internalABCICode      uint32 = 1
-	internalABCILog       string = "internal error"
-	// multiErrorABCICode uint32 = 1000
 )
 
 // ABCIInfo returns the ABCI error information as consumed by the tendermint
@@ -44,8 +41,8 @@ func ABCIInfo(err error, debug bool) (codespace string, code uint32, log string)
 
 // ResponseCheckTx returns an ABCI ResponseCheckTx object with fields filled in
 // from the given error and gas values.
-func ResponseCheckTx(err error, gw, gu uint64) abci.ResponseCheckTx {
-	space, code, log := ABCIInfo(err, false)
+func ResponseCheckTx(err error, gw, gu uint64, debug bool) abci.ResponseCheckTx {
+	space, code, log := ABCIInfo(err, debug)
 	return abci.ResponseCheckTx{
 		Codespace: space,
 		Code:      code,
@@ -57,8 +54,8 @@ func ResponseCheckTx(err error, gw, gu uint64) abci.ResponseCheckTx {
 
 // ResponseDeliverTx returns an ABCI ResponseDeliverTx object with fields filled in
 // from the given error and gas values.
-func ResponseDeliverTx(err error, gw, gu uint64) abci.ResponseDeliverTx {
-	space, code, log := ABCIInfo(err, false)
+func ResponseDeliverTx(err error, gw, gu uint64, debug bool) abci.ResponseDeliverTx {
+	space, code, log := ABCIInfo(err, debug)
 	return abci.ResponseDeliverTx{
 		Codespace: space,
 		Code:      code,
@@ -160,10 +157,10 @@ func errIsNil(err error) bool {
 // originates.
 func Redact(err error) error {
 	if ErrPanic.Is(err) {
-		return errors.New(internalABCILog)
+		return ErrPanic
 	}
 	if abciCode(err) == internalABCICode {
-		return errors.New(internalABCILog)
+		return errInternal
 	}
 	return err
 }

types/errors/abci_test.go

@@ -5,6 +5,8 @@ import (
 	"io"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestABCInfo(t *testing.T) {
@@ -46,7 +48,7 @@ func TestABCInfo(t *testing.T) {
 		"stdlib is generic message": {
 			err:       io.EOF,
 			debug:     false,
-			wantLog:   "internal error",
+			wantLog:   "internal",
 			wantCode:  1,
 			wantSpace: UndefinedCodespace,
 		},
@@ -60,7 +62,7 @@ func TestABCInfo(t *testing.T) {
 		"wrapped stdlib is only a generic message": {
 			err:       Wrap(io.EOF, "cannot read file"),
 			debug:     false,
-			wantLog:   "internal error",
+			wantLog:   "internal",
 			wantCode:  1,
 			wantSpace: UndefinedCodespace,
 		},
@@ -134,7 +136,7 @@ func TestABCIInfoStacktrace(t *testing.T) {
 			err:            Wrap(fmt.Errorf("stdlib"), "wrapped"),
 			debug:          false,
 			wantStacktrace: false,
-			wantErrMsg:     "internal error",
+			wantErrMsg:     "internal",
 		},
 	}
 
@@ -169,21 +171,42 @@ func TestABCIInfoHidesStacktrace(t *testing.T) {
 }
 
 func TestRedact(t *testing.T) {
-	if err := Redact(ErrPanic); ErrPanic.Is(err) {
-		t.Error("reduct must not pass through panic error")
-	}
-	if err := Redact(ErrUnauthorized); !ErrUnauthorized.Is(err) {
-		t.Error("reduct should pass through SDK error")
-	}
-
-	var cerr customErr
-	if err := Redact(cerr); err != cerr {
-		t.Error("reduct should pass through ABCI code error")
+	cases := map[string]struct {
+		err       error
+		untouched bool  // if true we expect the same error after redact
+		changed   error // if untouched == false, expect this error
+	}{
+		"panic looses message": {
+			err:     Wrap(ErrPanic, "some secret stack trace"),
+			changed: ErrPanic,
+		},
+		"sdk errors untouched": {
+			err:       Wrap(ErrUnauthorized, "cannot drop db"),
+			untouched: true,
+		},
+		"pass though custom errors with ABCI code": {
+			err:       customErr{},
+			untouched: true,
+		},
+		"redact stdlib error": {
+			err:     fmt.Errorf("stdlib error"),
+			changed: errInternal,
+		},
 	}
 
-	serr := fmt.Errorf("stdlib error")
-	if err := Redact(serr); err == serr {
-		t.Error("reduct must not pass through a stdlib error")
+	for name, tc := range cases {
+		spec := tc
+		t.Run(name, func(t *testing.T) {
+			redacted := Redact(spec.err)
+			if spec.untouched {
+				require.Equal(t, spec.err, redacted)
+			} else {
+				// see if we got the expected redact
+				require.Equal(t, spec.changed, redacted)
+				// make sure the ABCI code did not change
+				require.Equal(t, abciCode(spec.err), abciCode(redacted))
+			}
+		})
 	}
 }
 
@@ -215,41 +238,15 @@ func TestABCIInfoSerializeErr(t *testing.T) {
 			debug: true,
 			exp:   fmt.Sprintf("%+v", myErrDecode),
 		},
-		// "multi error default encoder": {
-		// 	src: Append(myErrMsg, myErrAddr),
-		// 	exp: Append(myErrMsg, myErrAddr).Error(),
-		// },
-		// "multi error default with internal": {
-		// 	src: Append(myErrMsg, myPanic),
-		// 	exp: "internal error",
-		// },
 		"redact in default encoder": {
 			src: myPanic,
-			exp: "internal error",
+			exp: "panic",
 		},
 		"do not redact in debug encoder": {
 			src:   myPanic,
 			debug: true,
 			exp:   fmt.Sprintf("%+v", myPanic),
 		},
-		// 		"redact in multi error": {
-		// 			src:   Append(myPanic, myErrMsg),
-		// 			debug: false,
-		// 			exp:   "internal error",
-		// 		},
-		// 		"no redact in multi error": {
-		// 			src:   Append(myPanic, myErrMsg),
-		// 			debug: true,
-		// 			exp: `2 errors occurred:
-		// 	* panic
-		// 	* test: invalid message
-		// `,
-		// 		},
-		// 		"wrapped multi error with redact": {
-		// 			src:   Wrap(Append(myPanic, myErrMsg), "wrap"),
-		// 			debug: false,
-		// 			exp:   "internal error",
-		// 		},
 	}
 	for msg, spec := range specs {
 		spec := spec