R4R: Standardize REST error responses

PENDING.md

@@ -142,6 +142,7 @@ IMPROVEMENTS
 
 * Gaia REST API (`gaiacli advanced rest-server`)
     * [x/stake] [\#2000](https://github.com/cosmos/cosmos-sdk/issues/2000) Added tests for new staking endpoints
+    * [gaia-lite] [\#2445](https://github.com/cosmos/cosmos-sdk/issues/2445) Standarized REST error responses
     * [gaia-lite] Added example to Swagger specification for /keys/seed.
 
 * Gaia CLI  (`gaiacli`)

client/context/broadcast.go

@@ -55,15 +55,11 @@ func (ctx CLIContext) BroadcastTxAndAwaitCommit(tx []byte) (*ctypes.ResultBroadc
 	}
 
 	if !res.CheckTx.IsOK() {
-		return res, errors.Errorf("checkTx failed: (%d) %s",
-			res.CheckTx.Code,
-			res.CheckTx.Log)
+		return res, errors.Errorf(res.CheckTx.Log)
 	}
 
 	if !res.DeliverTx.IsOK() {
-		return res, errors.Errorf("deliverTx failed: (%d) %s",
-			res.DeliverTx.Code,
-			res.DeliverTx.Log)
+		return res, errors.Errorf(res.DeliverTx.Log)
 	}
 
 	return res, err

client/context/query.go

@@ -168,7 +168,7 @@ func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, err erro
 
 	resp := result.Response
 	if !resp.IsOK() {
-		return res, errors.Errorf("query failed: (%d) %s", resp.Code, resp.Log)
+		return res, errors.Errorf(resp.Log)
 	}
 
 	// data from trusted node or subspace query doesn't need verification

client/keys/add.go

@@ -262,6 +262,8 @@ func SeedRequestHandler(w http.ResponseWriter, r *http.Request) {
 	algo := keys.SigningAlgo(algoType)
 
 	seed := getSeed(algo)
+
+	w.Header().Set("Content-Type", "application/json")
 	w.Write([]byte(seed))
 }
 

client/keys/update.go

@@ -91,5 +91,6 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
 }

client/lcd/version.go

@@ -1,10 +1,10 @@
 package lcd
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/cosmos/cosmos-sdk/client/context"
+	"github.com/cosmos/cosmos-sdk/client/utils"
 	"github.com/cosmos/cosmos-sdk/version"
 )
 
@@ -19,11 +19,11 @@ func NodeVersionRequestHandler(cliCtx context.CLIContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		version, err := cliCtx.Query("/app/version", nil)
 		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte(fmt.Sprintf("Could't query version. Error: %s", err.Error())))
+			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
+		w.Header().Set("Content-Type", "application/json")
 		w.Write(version)
 	}
 }

client/tx/query.go

@@ -3,9 +3,10 @@ package tx
 import (
 	"encoding/hex"
 	"fmt"
-	"github.com/tendermint/tendermint/libs/common"
 	"net/http"
 
+	"github.com/tendermint/tendermint/libs/common"
+
 	"github.com/gorilla/mux"
 	"github.com/spf13/cobra"
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -142,8 +143,7 @@ func QueryTxRequestHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.H
 
 		output, err := queryTx(cdc, cliCtx, hashHexStr)
 		if err != nil {
-			w.WriteHeader(500)
-			w.Write([]byte(err.Error()))
+			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 		utils.PostProcessResponse(w, cdc, output, cliCtx.Indent)

client/tx/search.go

@@ -30,7 +30,7 @@ func SearchTxCmd(cdc *codec.Codec) *cobra.Command {
 		Use:   "txs",
 		Short: "Search for all transactions that match the given tags.",
 		Long: strings.TrimSpace(`
-Search for transactions that match the given tags. By default, transactions must match ALL tags 
+Search for transactions that match the given tags. By default, transactions must match ALL tags
 passed to the --tags option. To match any transaction, use the --any option.
 
 For example:
@@ -141,7 +141,7 @@ func SearchTxRequestHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.
 	return func(w http.ResponseWriter, r *http.Request) {
 		tag := r.FormValue("tag")
 		if tag == "" {
-			w.WriteHeader(400)
+			w.WriteHeader(http.StatusBadRequest)
 			w.Write([]byte("You need to provide at least a tag as a key=value pair to search for. Postfix the key with _bech32 to search bech32-encoded addresses or public keys"))
 			return
 		}
@@ -151,8 +151,7 @@ func SearchTxRequestHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.
 
 		value, err := url.QueryUnescape(keyValue[1])
 		if err != nil {
-			w.WriteHeader(400)
-			w.Write([]byte("Could not decode address: " + err.Error()))
+			utils.WriteErrorResponse(w, http.StatusBadRequest, sdk.AppendMsgToErr("could not decode address", err.Error()))
 			return
 		}
 
@@ -161,8 +160,7 @@ func SearchTxRequestHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.
 			prefix := strings.Split(bech32address, "1")[0]
 			bz, err := sdk.GetFromBech32(bech32address, prefix)
 			if err != nil {
-				w.WriteHeader(400)
-				w.Write([]byte(err.Error()))
+				utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 				return
 			}
 
@@ -171,8 +169,7 @@ func SearchTxRequestHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.
 
 		txs, err := searchTxs(cliCtx, cdc, []string{tag})
 		if err != nil {
-			w.WriteHeader(500)
-			w.Write([]byte(err.Error()))
+			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 

client/utils/rest.go

@@ -26,12 +26,12 @@ const (
 
 // WriteErrorResponse prepares and writes a HTTP error
 // given a status code and an error message.
-func WriteErrorResponse(w http.ResponseWriter, status int, msg string) {
+func WriteErrorResponse(w http.ResponseWriter, status int, err string) {
 	w.WriteHeader(status)
-	w.Write([]byte(msg))
+	w.Write([]byte(err))
 }
 
-// WriteGasEstimateResponse prepares and writes an HTTP
+// WriteSimulationResponse prepares and writes an HTTP
 // response for transactions simulations.
 func WriteSimulationResponse(w http.ResponseWriter, gas int64) {
 	w.WriteHeader(http.StatusOK)

types/errors.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	cmn "github.com/tendermint/tendermint/libs/common"
@@ -286,7 +287,34 @@ func (err *sdkError) QueryResult() abci.ResponseQuery {
 	}
 }
 
-// nolint
+//----------------------------------------
+// REST error utilities
+
+// appends a message to the head of the given error
+func AppendMsgToErr(msg string, err string) string {
+	msgIdx := strings.Index(err, "message\":\"")
+	if msgIdx != -1 {
+		errMsg := err[msgIdx+len("message\":\"") : len(err)-2]
+		errMsg = fmt.Sprintf("%s; %s", msg, errMsg)
+		return fmt.Sprintf("%s%s%s",
+			err[:msgIdx+len("message\":\"")],
+			errMsg,
+			err[len(err)-2:],
+		)
+	}
+	return fmt.Sprintf("%s; %s", msg, err)
+}
+
+// returns the index of the message in the ABCI Log
+func mustGetMsgIndex(abciLog string) int {
+	msgIdx := strings.Index(abciLog, "message\":\"")
+	if msgIdx == -1 {
+		panic(fmt.Sprintf("invalid error format: %s", abciLog))
+	}
+	return msgIdx + len("message\":\"")
+}
+
+// parses the error into an object-like struct for exporting
 type humanReadableError struct {
 	Codespace CodespaceType `json:"codespace"`
 	Code      CodeType      `json:"code"`

types/errors_test.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -65,3 +66,28 @@ func TestErrFn(t *testing.T) {
 
 	require.Equal(t, ABCICodeOK, ToABCICode(CodespaceRoot, CodeOK))
 }
+
+func TestAppendMsgToErr(t *testing.T) {
+	for i, errFn := range errFns {
+		err := errFn("")
+		errMsg := err.Stacktrace().Error()
+		abciLog := err.ABCILog()
+
+		// plain msg error
+		msg := AppendMsgToErr("something unexpected happened", errMsg)
+		require.Equal(t, fmt.Sprintf("something unexpected happened; %s",
+			errMsg),
+			msg,
+			fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i))
+
+		// ABCI Log msg error
+		msg = AppendMsgToErr("something unexpected happened", abciLog)
+		msgIdx := mustGetMsgIndex(abciLog)
+		require.Equal(t, fmt.Sprintf("%s%s; %s}",
+			abciLog[:msgIdx],
+			"something unexpected happened",
+			abciLog[msgIdx:len(abciLog)-1]),
+			msg,
+			fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i))
+	}
+}

x/auth/client/rest/query.go

@@ -1,7 +1,6 @@
 package rest
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/cosmos/cosmos-sdk/client/context"
@@ -47,7 +46,7 @@ func QueryAccountRequestHandlerFn(
 
 		res, err := cliCtx.QueryStore(auth.AddressStoreKey(addr), storeName)
 		if err != nil {
-			utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("couldn't query account. Error: %s", err.Error()))
+			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
@@ -60,7 +59,7 @@ func QueryAccountRequestHandlerFn(
 		// decode the value
 		account, err := decoder(res)
 		if err != nil {
-			utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("couldn't parse query result. Result: %s. Error: %s", res, err.Error()))
+			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
@@ -86,7 +85,7 @@ func QueryBalancesRequestHandlerFn(
 
 		res, err := cliCtx.QueryStore(auth.AddressStoreKey(addr), storeName)
 		if err != nil {
-			utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("couldn't query account. Error: %s", err.Error()))
+			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
@@ -99,7 +98,7 @@ func QueryBalancesRequestHandlerFn(
 		// decode the value
 		account, err := decoder(res)
 		if err != nil {
-			utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("couldn't parse query result. Result: %s. Error: %s", res, err.Error()))
+			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 

x/gov/client/rest/rest.go

@@ -254,7 +254,6 @@ func queryDepositHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Han
 
 		depositerAddr, err := sdk.AccAddressFromBech32(bechDepositerAddr)
 		if err != nil {
-			err := errors.Errorf("'%s' needs to be bech32 encoded", RestDepositer)
 			utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 			return
 		}
@@ -319,7 +318,6 @@ func queryVoteHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Handle
 
 		voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr)
 		if err != nil {
-			err := errors.Errorf("'%s' needs to be bech32 encoded", RestVoter)
 			utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 			return
 		}
@@ -393,7 +391,6 @@ func queryVotesOnProposalHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext)
 			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
-
 		utils.PostProcessResponse(w, cdc, res, cliCtx.Indent)
 	}
 }
@@ -411,7 +408,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec, cliCtx context.CLIContext)
 		if len(bechVoterAddr) != 0 {
 			voterAddr, err := sdk.AccAddressFromBech32(bechVoterAddr)
 			if err != nil {
-				err := errors.Errorf("'%s' needs to be bech32 encoded", RestVoter)
 				utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 				return
 			}
@@ -421,7 +417,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec, cliCtx context.CLIContext)
 		if len(bechDepositerAddr) != 0 {
 			depositerAddr, err := sdk.AccAddressFromBech32(bechDepositerAddr)
 			if err != nil {
-				err := errors.Errorf("'%s' needs to be bech32 encoded", RestDepositer)
 				utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 				return
 			}
@@ -431,7 +426,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec, cliCtx context.CLIContext)
 		if len(strProposalStatus) != 0 {
 			proposalStatus, err := gov.ProposalStatusFromString(strProposalStatus)
 			if err != nil {
-				err := errors.Errorf("'%s' is not a valid Proposal Status", strProposalStatus)
 				utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 				return
 			}
@@ -456,7 +450,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec, cliCtx context.CLIContext)
 			utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
-
 		utils.PostProcessResponse(w, cdc, res, cliCtx.Indent)
 	}
 }
@@ -496,7 +489,6 @@ func queryTallyOnProposalHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext)
 			w.Write([]byte(err.Error()))
 			return
 		}
-
 		utils.PostProcessResponse(w, cdc, res, cliCtx.Indent)
 	}
 }

x/gov/errors.go

@@ -27,11 +27,11 @@ const (
 // Error constructors
 
 func ErrUnknownProposal(codespace sdk.CodespaceType, proposalID int64) sdk.Error {
-	return sdk.NewError(codespace, CodeUnknownProposal, fmt.Sprintf("Unknown proposal - %d", proposalID))
+	return sdk.NewError(codespace, CodeUnknownProposal, fmt.Sprintf("Unknown proposal with id %d", proposalID))
 }
 
 func ErrInactiveProposal(codespace sdk.CodespaceType, proposalID int64) sdk.Error {
-	return sdk.NewError(codespace, CodeInactiveProposal, fmt.Sprintf("Inactive proposal - %d", proposalID))
+	return sdk.NewError(codespace, CodeInactiveProposal, fmt.Sprintf("Inactive proposal with id %d", proposalID))
 }
 
 func ErrAlreadyActiveProposal(codespace sdk.CodespaceType, proposalID int64) sdk.Error {