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)",

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