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 {