From 88b24612217ba18a777343d298351706b05e230f Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 5 Oct 2018 13:25:17 +0200 Subject: [PATCH 01/17] Standarize REST error responses --- client/context/broadcast.go | 8 ++---- client/context/query.go | 2 +- client/lcd/version.go | 4 +-- client/tx/search.go | 12 ++++----- x/auth/client/rest/query.go | 9 +++---- x/gov/client/rest/rest.go | 43 +++++---------------------------- x/slashing/client/rest/query.go | 15 +++++------- x/slashing/client/rest/tx.go | 6 +---- x/stake/client/rest/query.go | 3 +-- x/stake/client/rest/tx.go | 38 +++++++++++++---------------- 10 files changed, 46 insertions(+), 94 deletions(-) diff --git a/client/context/broadcast.go b/client/context/broadcast.go index b15b2a8e32c4..9f88ce7b9892 100644 --- a/client/context/broadcast.go +++ b/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 diff --git a/client/context/query.go b/client/context/query.go index e4e48819047d..8fca9becff07 100644 --- a/client/context/query.go +++ b/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 diff --git a/client/lcd/version.go b/client/lcd/version.go index a124388e6dfc..a114e5a614fc 100644 --- a/client/lcd/version.go +++ b/client/lcd/version.go @@ -1,7 +1,6 @@ package lcd import ( - "fmt" "net/http" "github.com/cosmos/cosmos-sdk/client/context" @@ -20,10 +19,11 @@ func NodeVersionRequestHandler(cliCtx context.CLIContext) http.HandlerFunc { 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()))) + w.Write([]byte(err.Error())) return } + w.Header().Set("Content-Type", "application/json") w.Write(version) } } diff --git a/client/tx/search.go b/client/tx/search.go index 672df0e339f2..679d93987b3a 100644 --- a/client/tx/search.go +++ b/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,8 @@ 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())) + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(err.Error())) return } @@ -161,7 +161,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.WriteHeader(http.StatusBadRequest) w.Write([]byte(err.Error())) return } @@ -171,7 +171,7 @@ func SearchTxRequestHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http. txs, err := searchTxs(cliCtx, cdc, []string{tag}) if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } diff --git a/x/auth/client/rest/query.go b/x/auth/client/rest/query.go index ccd3a4bf79c6..740748bfbca2 100644 --- a/x/auth/client/rest/query.go +++ b/x/auth/client/rest/query.go @@ -1,7 +1,6 @@ package rest import ( - "fmt" "net/http" "github.com/cosmos/cosmos-sdk/client/context" @@ -48,7 +47,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 } @@ -61,7 +60,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 } @@ -87,7 +86,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 } @@ -100,7 +99,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 } diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index f66331ad5a18..a762be8b7abf 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -198,6 +198,7 @@ func queryProposalHandlerFn(cdc *codec.Codec) http.HandlerFunc { return } + w.Header().Set("Content-Type", "application/json") w.Write(res) } } @@ -227,7 +228,6 @@ func queryDepositHandlerFn(cdc *codec.Codec) http.HandlerFunc { 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 } @@ -251,20 +251,7 @@ func queryDepositHandlerFn(cdc *codec.Codec) http.HandlerFunc { return } - var deposit gov.Deposit - cdc.UnmarshalJSON(res, &deposit) - if deposit.Empty() { - res, err := cliCtx.QueryWithData("custom/gov/proposal", cdc.MustMarshalBinary(gov.QueryProposalParams{params.ProposalID})) - if err != nil || len(res) == 0 { - err := errors.Errorf("proposalID [%d] does not exist", proposalID) - utils.WriteErrorResponse(w, http.StatusNotFound, err.Error()) - return - } - err = errors.Errorf("depositer [%s] did not deposit on proposalID [%d]", bechDepositerAddr, proposalID) - utils.WriteErrorResponse(w, http.StatusNotFound, err.Error()) - return - } - + w.Header().Set("Content-Type", "application/json") w.Write(res) } } @@ -294,7 +281,6 @@ func queryVoteHandlerFn(cdc *codec.Codec) http.HandlerFunc { 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 } @@ -317,24 +303,7 @@ func queryVoteHandlerFn(cdc *codec.Codec) http.HandlerFunc { return } - var vote gov.Vote - cdc.UnmarshalJSON(res, &vote) - if vote.Empty() { - bz, err := cdc.MarshalJSON(gov.QueryProposalParams{params.ProposalID}) - if err != nil { - utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) - return - } - res, err := cliCtx.QueryWithData("custom/gov/proposal", bz) - if err != nil || len(res) == 0 { - err := errors.Errorf("proposalID [%d] does not exist", proposalID) - utils.WriteErrorResponse(w, http.StatusNotFound, err.Error()) - return - } - err = errors.Errorf("voter [%s] did not deposit on proposalID [%d]", bechVoterAddr, proposalID) - utils.WriteErrorResponse(w, http.StatusNotFound, err.Error()) - return - } + w.Header().Set("Content-Type", "application/json") w.Write(res) } } @@ -373,6 +342,7 @@ func queryVotesOnProposalHandlerFn(cdc *codec.Codec) http.HandlerFunc { return } + w.Header().Set("Content-Type", "application/json") w.Write(res) } } @@ -390,7 +360,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec) http.HandlerFunc { 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 } @@ -400,7 +369,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec) http.HandlerFunc { 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 } @@ -410,7 +378,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec) http.HandlerFunc { 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 } @@ -438,6 +405,7 @@ func queryProposalsWithParameterFn(cdc *codec.Codec) http.HandlerFunc { return } + w.Header().Set("Content-Type", "application/json") w.Write(res) } } @@ -480,6 +448,7 @@ func queryTallyOnProposalHandlerFn(cdc *codec.Codec) http.HandlerFunc { return } + w.Header().Set("Content-Type", "application/json") w.Write(res) } } diff --git a/x/slashing/client/rest/query.go b/x/slashing/client/rest/query.go index e83f1a235dcd..408b1f592643 100644 --- a/x/slashing/client/rest/query.go +++ b/x/slashing/client/rest/query.go @@ -1,10 +1,10 @@ package rest import ( - "fmt" "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/slashing" @@ -26,8 +26,7 @@ func signingInfoHandlerFn(cliCtx context.CLIContext, storeName string, cdc *code pk, err := sdk.GetConsPubKeyBech32(vars["validator"]) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -35,8 +34,7 @@ func signingInfoHandlerFn(cliCtx context.CLIContext, storeName string, cdc *code res, err := cliCtx.QueryStore(key, storeName) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("couldn't query signing info. Error: %s", err.Error()))) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -44,18 +42,17 @@ func signingInfoHandlerFn(cliCtx context.CLIContext, storeName string, cdc *code err = cdc.UnmarshalBinary(res, &signingInfo) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("couldn't decode signing info. Error: %s", err.Error()))) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } output, err := cdc.MarshalJSON(signingInfo) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } + w.Header().Set("Content-Type", "application/json") w.Write(output) } } diff --git a/x/slashing/client/rest/tx.go b/x/slashing/client/rest/tx.go index 972d4351fadb..eca88c65cdbd 100644 --- a/x/slashing/client/rest/tx.go +++ b/x/slashing/client/rest/tx.go @@ -2,7 +2,6 @@ package rest import ( "bytes" - "fmt" "net/http" "github.com/cosmos/cosmos-sdk/client/context" @@ -49,10 +48,7 @@ func unjailRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.CL valAddr, err := sdk.ValAddressFromBech32(req.ValidatorAddr) if err != nil { - utils.WriteErrorResponse( - w, http.StatusInternalServerError, - fmt.Sprintf("failed to decode validator; error: %s", err.Error()), - ) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } diff --git a/x/stake/client/rest/query.go b/x/stake/client/rest/query.go index a8fbfecf4d76..b5d277e065bc 100644 --- a/x/stake/client/rest/query.go +++ b/x/stake/client/rest/query.go @@ -1,7 +1,6 @@ package rest import ( - "fmt" "net/http" "strings" @@ -140,7 +139,7 @@ func delegatorTxsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Han node, err := cliCtx.GetNode() if err != nil { w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fmt.Sprintf("Couldn't get current Node information. Error: %s", err.Error()))) + w.Write([]byte(err.Error())) return } diff --git a/x/stake/client/rest/tx.go b/x/stake/client/rest/tx.go index ee2432228110..e07588f3159c 100644 --- a/x/stake/client/rest/tx.go +++ b/x/stake/client/rest/tx.go @@ -2,7 +2,6 @@ package rest import ( "bytes" - "fmt" "io/ioutil" "net/http" @@ -79,15 +78,13 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte body, err := ioutil.ReadAll(r.Body) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } err = cdc.UnmarshalJSON(body, &req) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -98,8 +95,7 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte info, err := kb.Get(baseReq.Name) if err != nil { - w.WriteHeader(http.StatusUnauthorized) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusUnauthorized, err.Error()) return } @@ -114,13 +110,13 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte for _, msg := range req.Delegations { delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -141,7 +137,7 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte for _, msg := range req.BeginRedelegates { delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -152,18 +148,18 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte valSrcAddr, err := sdk.ValAddressFromBech32(msg.ValidatorSrcAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } valDstAddr, err := sdk.ValAddressFromBech32(msg.ValidatorDstAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } shares, err := sdk.NewDecFromStr(msg.SharesAmount) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -180,19 +176,19 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte for _, msg := range req.CompleteRedelegates { delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } valSrcAddr, err := sdk.ValAddressFromBech32(msg.ValidatorSrcAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } valDstAddr, err := sdk.ValAddressFromBech32(msg.ValidatorDstAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -213,7 +209,7 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte for _, msg := range req.BeginUnbondings { delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -224,13 +220,13 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } shares, err := sdk.NewDecFromStr(msg.SharesAmount) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -246,13 +242,13 @@ func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx conte for _, msg := range req.CompleteUnbondings { delAddr, err := sdk.AccAddressFromBech32(msg.DelegatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode delegator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddr) if err != nil { - utils.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } From 493a3c7507eab57daf02929cfdea0449836f2916 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 5 Oct 2018 14:16:30 +0200 Subject: [PATCH 02/17] use utils error function everywhere --- client/keys/add.go | 18 ++++++-- client/keys/delete.go | 6 +-- client/keys/list.go | 13 ++++-- client/keys/show.go | 9 ++-- client/keys/update.go | 7 +-- client/lcd/version.go | 4 +- client/tx/query.go | 6 +-- client/tx/search.go | 9 ++-- x/auth/client/rest/query.go | 2 +- x/stake/client/rest/query.go | 88 +++++++++++------------------------- 10 files changed, 69 insertions(+), 93 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index be2825a55d2b..9fc3befef1d0 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -168,10 +168,11 @@ type NewKeyBody struct { func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) { var kb keys.Keybase var m NewKeyBody + var msg string kb, err := GetKeyBase() if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } @@ -185,13 +186,15 @@ func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) { return } if m.Name == "" { + msg = "You have to specify a name for the locally stored account." w.WriteHeader(http.StatusBadRequest) - w.Write([]byte("You have to specify a name for the locally stored account.")) + w.Write([]byte(msg)) return } if m.Password == "" { + msg = "You have to specify a password for the locally stored account." w.WriteHeader(http.StatusBadRequest) - w.Write([]byte("You have to specify a password for the locally stored account.")) + w.Write([]byte(msg)) return } @@ -199,8 +202,9 @@ func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) { infos, err := kb.List() for _, i := range infos { if i.GetName() == m.Name { + msg = fmt.Sprintf("Account with name %s already exists.", m.Name) w.WriteHeader(http.StatusConflict) - w.Write([]byte(fmt.Sprintf("Account with name %s already exists.", m.Name))) + w.Write([]byte(msg)) return } } @@ -233,6 +237,7 @@ func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) { return } + w.Header().Set("Content-Type", "application/json") w.Write(bz) } @@ -256,5 +261,8 @@ func SeedRequestHandler(w http.ResponseWriter, r *http.Request) { algo := keys.SigningAlgo(algoType) seed := getSeed(algo) - w.Write([]byte(seed)) + bz := []byte(seed) + + w.Header().Set("Content-Type", "application/json") + w.Write(bz) } diff --git a/client/keys/delete.go b/client/keys/delete.go index 944feb4b19b2..99a9fc503444 100644 --- a/client/keys/delete.go +++ b/client/keys/delete.go @@ -68,14 +68,14 @@ func DeleteKeyRequestHandler(w http.ResponseWriter, r *http.Request) { decoder := json.NewDecoder(r.Body) err := decoder.Decode(&m) if err != nil { - w.WriteHeader(400) + w.WriteHeader(http.StatusBadRequest) w.Write([]byte(err.Error())) return } kb, err = GetKeyBase() if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } @@ -83,7 +83,7 @@ func DeleteKeyRequestHandler(w http.ResponseWriter, r *http.Request) { // TODO handle error if key is not available or pass is wrong err = kb.Delete(name, m.Password) if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } diff --git a/client/keys/list.go b/client/keys/list.go index 22f163f1d8ff..ff650bd48167 100644 --- a/client/keys/list.go +++ b/client/keys/list.go @@ -38,32 +38,35 @@ func runListCmd(cmd *cobra.Command, args []string) error { func QueryKeysRequestHandler(w http.ResponseWriter, r *http.Request) { kb, err := GetKeyBase() if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } infos, err := kb.List() if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } // an empty list will be JSONized as null, but we want to keep the empty list if len(infos) == 0 { - w.Write([]byte("[]")) + w.WriteHeader(http.StatusNoContent) + w.Write([]byte("No content for keys")) return } keysOutput, err := Bech32KeysOutput(infos) if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } output, err := json.MarshalIndent(keysOutput, "", " ") if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } + + w.Header().Set("Content-Type", "application/json") w.Write(output) } diff --git a/client/keys/show.go b/client/keys/show.go index 82c6f9883d18..9e656a0e0b55 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -102,7 +102,7 @@ func GetKeyRequestHandler(w http.ResponseWriter, r *http.Request) { bechKeyOut, err := getBechKeyOut(bechPrefix) if err != nil { - w.WriteHeader(400) + w.WriteHeader(http.StatusBadRequest) w.Write([]byte(err.Error())) return } @@ -111,24 +111,25 @@ func GetKeyRequestHandler(w http.ResponseWriter, r *http.Request) { // TODO: check for the error if key actually does not exist, instead of // assuming this as the reason if err != nil { - w.WriteHeader(404) + w.WriteHeader(http.StatusNotFound) w.Write([]byte(err.Error())) return } keyOutput, err := bechKeyOut(info) if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } output, err := json.MarshalIndent(keyOutput, "", " ") if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } + w.Header().Set("Content-Type", "application/json") w.Write(output) } diff --git a/client/keys/update.go b/client/keys/update.go index 78a81bf0e605..37af5a7d079e 100644 --- a/client/keys/update.go +++ b/client/keys/update.go @@ -69,14 +69,14 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) { decoder := json.NewDecoder(r.Body) err := decoder.Decode(&m) if err != nil { - w.WriteHeader(400) + w.WriteHeader(http.StatusBadRequest) w.Write([]byte(err.Error())) return } kb, err = GetKeyBase() if err != nil { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return } @@ -86,10 +86,11 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) { // TODO check if account exists and if password is correct err = kb.Update(name, m.OldPassword, getNewpass) if err != nil { - w.WriteHeader(401) + w.WriteHeader(http.StatusUnauthorized) w.Write([]byte(err.Error())) return } + w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) } diff --git a/client/lcd/version.go b/client/lcd/version.go index a114e5a614fc..fe1d25edf9c8 100644 --- a/client/lcd/version.go +++ b/client/lcd/version.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/version" ) @@ -18,8 +19,7 @@ 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(err.Error())) + utils.WriteErrorResponse(w, http.StatusUnauthorized, err.Error()) return } diff --git a/client/tx/query.go b/client/tx/query.go index ec7daf41b4f8..da94404f2d24 100644 --- a/client/tx/query.go +++ b/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) diff --git a/client/tx/search.go b/client/tx/search.go index 679d93987b3a..e750f0e4c2ea 100644 --- a/client/tx/search.go +++ b/client/tx/search.go @@ -151,8 +151,7 @@ func SearchTxRequestHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http. value, err := url.QueryUnescape(keyValue[1]) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, 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(http.StatusBadRequest) - 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(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } diff --git a/x/auth/client/rest/query.go b/x/auth/client/rest/query.go index 740748bfbca2..031843860100 100644 --- a/x/auth/client/rest/query.go +++ b/x/auth/client/rest/query.go @@ -92,7 +92,7 @@ func QueryBalancesRequestHandlerFn( // the query will return empty if there is no data for this account if len(res) == 0 { - w.WriteHeader(http.StatusNoContent) + utils.WriteErrorResponse(w, http.StatusNoContent, "") return } diff --git a/x/stake/client/rest/query.go b/x/stake/client/rest/query.go index b5d277e065bc..1995efcf6973 100644 --- a/x/stake/client/rest/query.go +++ b/x/stake/client/rest/query.go @@ -6,6 +6,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/tx" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/stake" @@ -91,8 +92,7 @@ func delegatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Handle delegatorAddr, err := sdk.AccAddressFromBech32(bech32delegator) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -102,16 +102,13 @@ func delegatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Handle bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/stake/delegator", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -131,8 +128,7 @@ func delegatorTxsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Han _, err := sdk.AccAddressFromBech32(delegatorAddr) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -181,16 +177,14 @@ func delegatorTxsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Han for _, action := range actions { foundTxs, errQuery := queryTxs(node, cliCtx, cdc, action, delegatorAddr) if errQuery != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(errQuery.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) } txs = append(txs, foundTxs...) } output, err = cdc.MarshalJSON(txs) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } w.Write(output) @@ -208,15 +202,13 @@ func unbondingDelegationHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) h delegatorAddr, err := sdk.AccAddressFromBech32(bech32delegator) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } validatorAddr, err := sdk.ValAddressFromBech32(bech32validator) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -227,16 +219,13 @@ func unbondingDelegationHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) h bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/stake/unbondingDelegation", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -256,15 +245,13 @@ func delegationHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Handl delegatorAddr, err := sdk.AccAddressFromBech32(bech32delegator) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } validatorAddr, err := sdk.ValAddressFromBech32(bech32validator) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -275,16 +262,13 @@ func delegationHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Handl bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/stake/delegation", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -303,8 +287,7 @@ func delegatorValidatorsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) h delegatorAddr, err := sdk.AccAddressFromBech32(bech32delegator) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -314,16 +297,13 @@ func delegatorValidatorsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) h bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/stake/delegatorValidators", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -344,8 +324,7 @@ func delegatorValidatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) ht delegatorAddr, err := sdk.AccAddressFromBech32(bech32delegator) validatorAddr, err := sdk.ValAddressFromBech32(bech32validator) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -356,16 +335,13 @@ func delegatorValidatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) ht bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/stake/delegatorValidator", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -381,9 +357,7 @@ func validatorsHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { res, err := cliCtx.QueryWithData("custom/stake/validators", nil) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -403,8 +377,7 @@ func validatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Handle validatorAddr, err := sdk.ValAddressFromBech32(bech32validatorAddr) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } @@ -414,16 +387,13 @@ func validatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Handle bz, err := cdc.MarshalJSON(params) if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } res, err := cliCtx.QueryWithData("custom/stake/validator", bz) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -439,9 +409,7 @@ func poolHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { res, err := cliCtx.QueryWithData("custom/stake/pool", nil) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } @@ -457,9 +425,7 @@ func paramsHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { res, err := cliCtx.QueryWithData("custom/stake/parameters", nil) if err != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(err.Error())) - + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } From f1195e5742bd3fe87a44f10f4bc51a3221332e4b Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 5 Oct 2018 14:37:46 +0200 Subject: [PATCH 03/17] PENDING.md --- PENDING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index 84d38ecf3208..7f19a5f49564 100644 --- a/PENDING.md +++ b/PENDING.md @@ -41,7 +41,7 @@ BREAKING CHANGES * [x/gov] [#2195] Governance uses BFT Time * [x/gov] \#2256 Removed slashing for governance non-voting validators * [simulation] \#2162 Added back correct supply invariants - + * SDK * [core] \#2219 Update to Tendermint 0.24.0 * Validator set updates delayed by one block @@ -122,6 +122,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 CLI (`gaiacli`) * [cli] #2060 removed `--select` from `block` command From bd5c1a947e2675d84317033fa3100b010e2cba3c Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Tue, 9 Oct 2018 13:57:38 -0700 Subject: [PATCH 04/17] check error format test --- client/utils/rest.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 07b258ce9686..b35cf2247288 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -22,14 +22,28 @@ const ( queryArgGenerateOnly = "generate_only" ) +//---------------------------------------- +// REST error utilities + +// checks if the reported REST error has the expected format of 'Error{}' +func errMustHaveValidFormat(msg string) { + msg = strings.TrimSpace(msg) + if len(msg) == 0 { + panic("Error can't be blank") + } else if !strings.HasPrefix(msg, "Error{") { + panic(fmt.Sprintf("%s doesn't match the expected format", msg)) + } +} + //---------------------------------------- // Basic HTTP utilities // 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, msg ...string) { + errMustHaveValidFormat(err) w.WriteHeader(status) - w.Write([]byte(msg)) + w.Write([]byte(err)) } // WriteGasEstimateResponse prepares and writes an HTTP From 3bfc961b616ff52b5aaf53e48bd9ce0bc2e6cc39 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Thu, 11 Oct 2018 15:10:07 -0700 Subject: [PATCH 05/17] More standarization --- client/lcd/version.go | 2 +- client/utils/rest.go | 20 ++++++++++++++------ x/gov/errors.go | 4 ++-- x/gov/queryable.go | 35 +++++++++++++++++------------------ x/stake/querier/queryable.go | 33 ++++++++++++++++----------------- 5 files changed, 50 insertions(+), 44 deletions(-) diff --git a/client/lcd/version.go b/client/lcd/version.go index fe1d25edf9c8..4f4ce611cee7 100644 --- a/client/lcd/version.go +++ b/client/lcd/version.go @@ -19,7 +19,7 @@ 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 { - utils.WriteErrorResponse(w, http.StatusUnauthorized, err.Error()) + utils.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return } diff --git a/client/utils/rest.go b/client/utils/rest.go index b35cf2247288..58b8ae6c992b 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -26,15 +26,23 @@ const ( // REST error utilities // checks if the reported REST error has the expected format of 'Error{}' -func errMustHaveValidFormat(msg string) { - msg = strings.TrimSpace(msg) - if len(msg) == 0 { +func errMustHaveValidFormat(err string) { + err = strings.TrimSpace(err) + if len(err) == 0 { panic("Error can't be blank") - } else if !strings.HasPrefix(msg, "Error{") { - panic(fmt.Sprintf("%s doesn't match the expected format", msg)) + } else if !strings.HasPrefix(err, "Error{") { + panic(fmt.Sprintf("%s doesn't match the expected format", err)) } } +// appends a message to the head of the given error +func AppendMsgToErr(msg string, err string) string { + if strings.HasPrefix(err, "Error{") { + err = err[6 : len(err)-1] + } + return fmt.Sprintf("Error{%s; %s}", msg, err) +} + //---------------------------------------- // Basic HTTP utilities @@ -46,7 +54,7 @@ func WriteErrorResponse(w http.ResponseWriter, status int, err string, msg ...st 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) diff --git a/x/gov/errors.go b/x/gov/errors.go index 0825e00f8cc9..cd1cf8a17715 100644 --- a/x/gov/errors.go +++ b/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 { diff --git a/x/gov/queryable.go b/x/gov/queryable.go index 606f73a9ca49..7206949261d2 100644 --- a/x/gov/queryable.go +++ b/x/gov/queryable.go @@ -1,8 +1,7 @@ package gov import ( - "fmt" - + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" abci "github.com/tendermint/tendermint/abci/types" @@ -52,17 +51,17 @@ func queryProposal(ctx sdk.Context, path []string, req abci.RequestQuery, keeper var params QueryProposalParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } proposal := keeper.GetProposal(ctx, params.ProposalID) if proposal == nil { - return []byte{}, ErrUnknownProposal(DefaultCodespace, params.ProposalID) + return nil, ErrUnknownProposal(DefaultCodespace, params.ProposalID) } bz, err2 := codec.MarshalJSONIndent(keeper.cdc, proposal) if err2 != nil { - panic("could not marshal result to JSON") + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -78,13 +77,13 @@ func queryDeposit(ctx sdk.Context, path []string, req abci.RequestQuery, keeper var params QueryDepositParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } deposit, _ := keeper.GetDeposit(ctx, params.ProposalID, params.Depositer) bz, err2 := codec.MarshalJSONIndent(keeper.cdc, deposit) if err2 != nil { - panic("could not marshal result to JSON") + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -100,13 +99,13 @@ func queryVote(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Kee var params QueryVoteParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } vote, _ := keeper.GetVote(ctx, params.ProposalID, params.Voter) bz, err2 := codec.MarshalJSONIndent(keeper.cdc, vote) if err2 != nil { - panic("could not marshal result to JSON") + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -121,7 +120,7 @@ func queryDeposits(ctx sdk.Context, path []string, req abci.RequestQuery, keeper var params QueryDepositParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } var deposits []Deposit @@ -134,7 +133,7 @@ func queryDeposits(ctx sdk.Context, path []string, req abci.RequestQuery, keeper bz, err2 := codec.MarshalJSONIndent(keeper.cdc, deposits) if err2 != nil { - panic("could not marshal result to JSON") + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -150,7 +149,7 @@ func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } var votes []Vote @@ -163,7 +162,7 @@ func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke bz, err2 := codec.MarshalJSONIndent(keeper.cdc, votes) if err2 != nil { - panic("could not marshal result to JSON") + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -181,14 +180,14 @@ func queryProposals(ctx sdk.Context, path []string, req abci.RequestQuery, keepe var params QueryProposalsParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } proposals := keeper.GetProposalsFiltered(ctx, params.Voter, params.Depositer, params.ProposalStatus, params.NumLatestProposals) bz, err2 := codec.MarshalJSONIndent(keeper.cdc, proposals) if err2 != nil { - panic("could not marshal result to JSON") + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -205,12 +204,12 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke var proposalID int64 err2 := keeper.cdc.UnmarshalJSON(req.Data, proposalID) if err2 != nil { - return res, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } proposal := keeper.GetProposal(ctx, proposalID) if proposal == nil { - return res, ErrUnknownProposal(DefaultCodespace, proposalID) + return nil, ErrUnknownProposal(DefaultCodespace, proposalID) } var tallyResult TallyResult @@ -225,7 +224,7 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke bz, err2 := codec.MarshalJSONIndent(keeper.cdc, tallyResult) if err2 != nil { - panic("could not marshal result to JSON") + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } diff --git a/x/stake/querier/queryable.go b/x/stake/querier/queryable.go index 8537f2bd49e7..6955264cf919 100644 --- a/x/stake/querier/queryable.go +++ b/x/stake/querier/queryable.go @@ -1,8 +1,7 @@ package querier import ( - "fmt" - + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" keep "github.com/cosmos/cosmos-sdk/x/stake/keeper" @@ -79,7 +78,7 @@ func queryValidators(ctx sdk.Context, cdc *codec.Codec, k keep.Keeper) (res []by res, errRes := codec.MarshalJSONIndent(cdc, validators) if err != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(errRes.Error()) } return res, nil } @@ -89,7 +88,7 @@ func queryValidator(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k errRes := cdc.UnmarshalJSON(req.Data, ¶ms) if errRes != nil { - return []byte{}, sdk.ErrUnknownAddress(fmt.Sprintf("incorrectly formatted request address: %s", err.Error())) + return []byte{}, sdk.ErrUnknownAddress("") } validator, found := k.GetValidator(ctx, params.ValidatorAddr) @@ -99,7 +98,7 @@ func queryValidator(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k res, errRes = codec.MarshalJSONIndent(cdc, validator) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -108,7 +107,7 @@ func queryDelegator(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k var params QueryDelegatorParams errRes := cdc.UnmarshalJSON(req.Data, ¶ms) if errRes != nil { - return []byte{}, sdk.ErrUnknownAddress(fmt.Sprintf("incorrectly formatted request address: %s", errRes.Error())) + return []byte{}, sdk.ErrUnknownAddress("") } delegations := k.GetAllDelegatorDelegations(ctx, params.DelegatorAddr) unbondingDelegations := k.GetAllUnbondingDelegations(ctx, params.DelegatorAddr) @@ -122,7 +121,7 @@ func queryDelegator(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k res, errRes = codec.MarshalJSONIndent(cdc, summary) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -134,14 +133,14 @@ func queryDelegatorValidators(ctx sdk.Context, cdc *codec.Codec, req abci.Reques errRes := cdc.UnmarshalJSON(req.Data, ¶ms) if errRes != nil { - return []byte{}, sdk.ErrUnknownAddress(fmt.Sprintf("incorrectly formatted request address: %s", errRes.Error())) + return []byte{}, sdk.ErrUnknownAddress("") } validators := k.GetDelegatorValidators(ctx, params.DelegatorAddr, stakeParams.MaxValidators) res, errRes = codec.MarshalJSONIndent(cdc, validators) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -151,7 +150,7 @@ func queryDelegatorValidator(ctx sdk.Context, cdc *codec.Codec, req abci.Request errRes := cdc.UnmarshalJSON(req.Data, ¶ms) if errRes != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request address: %s", errRes.Error())) + return []byte{}, sdk.ErrUnknownAddress("") } validator, err := k.GetDelegatorValidator(ctx, params.DelegatorAddr, params.ValidatorAddr) @@ -161,7 +160,7 @@ func queryDelegatorValidator(ctx sdk.Context, cdc *codec.Codec, req abci.Request res, errRes = codec.MarshalJSONIndent(cdc, validator) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -171,7 +170,7 @@ func queryDelegation(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k errRes := cdc.UnmarshalJSON(req.Data, ¶ms) if errRes != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request address: %s", errRes.Error())) + return []byte{}, sdk.ErrUnknownAddress("") } delegation, found := k.GetDelegation(ctx, params.DelegatorAddr, params.ValidatorAddr) @@ -181,7 +180,7 @@ func queryDelegation(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k res, errRes = codec.MarshalJSONIndent(cdc, delegation) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -191,7 +190,7 @@ func queryUnbondingDelegation(ctx sdk.Context, cdc *codec.Codec, req abci.Reques errRes := cdc.UnmarshalJSON(req.Data, ¶ms) if errRes != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request address: %s", errRes.Error())) + return []byte{}, sdk.ErrUnknownAddress("") } unbond, found := k.GetUnbondingDelegation(ctx, params.DelegatorAddr, params.ValidatorAddr) @@ -201,7 +200,7 @@ func queryUnbondingDelegation(ctx sdk.Context, cdc *codec.Codec, req abci.Reques res, errRes = codec.MarshalJSONIndent(cdc, unbond) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -211,7 +210,7 @@ func queryPool(ctx sdk.Context, cdc *codec.Codec, k keep.Keeper) (res []byte, er res, errRes := codec.MarshalJSONIndent(cdc, pool) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -221,7 +220,7 @@ func queryParameters(ctx sdk.Context, cdc *codec.Codec, k keep.Keeper) (res []by res, errRes := codec.MarshalJSONIndent(cdc, params) if errRes != nil { - return nil, sdk.ErrInternal(fmt.Sprintf("could not marshal result to JSON: %s", errRes.Error())) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } From 07839c2ad4bc284051a4f0532699c649083aff12 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Thu, 11 Oct 2018 15:33:42 -0700 Subject: [PATCH 06/17] minor fixes --- types/errors.go | 6 +++--- x/auth/client/rest/query.go | 2 +- x/stake/querier/queryable.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/types/errors.go b/types/errors.go index 266e6d14c331..77a5a6083161 100644 --- a/types/errors.go +++ b/types/errors.go @@ -257,7 +257,7 @@ func (err *sdkError) Code() CodeType { func (err *sdkError) ABCILog() string { cdc := codec.New() errMsg := err.cmnError.Error() - jsonErr := humanReadableError{ + jsonErr := HumanReadableError{ Codespace: err.codespace, Code: err.code, ABCICode: err.ABCICode(), @@ -286,8 +286,8 @@ func (err *sdkError) QueryResult() abci.ResponseQuery { } } -// nolint -type humanReadableError struct { +// parses the error into an object-like struct for exporting +type HumanReadableError struct { Codespace CodespaceType `json:"codespace"` Code CodeType `json:"code"` ABCICode ABCICodeType `json:"abci_code"` diff --git a/x/auth/client/rest/query.go b/x/auth/client/rest/query.go index 96b65e56cfc3..0629cc939e17 100644 --- a/x/auth/client/rest/query.go +++ b/x/auth/client/rest/query.go @@ -91,7 +91,7 @@ func QueryBalancesRequestHandlerFn( // the query will return empty if there is no data for this account if len(res) == 0 { - utils.WriteErrorResponse(w, http.StatusNoContent, "") + w.WriteHeader(http.StatusNoContent) return } diff --git a/x/stake/querier/queryable.go b/x/stake/querier/queryable.go index 6955264cf919..b953509b6e93 100644 --- a/x/stake/querier/queryable.go +++ b/x/stake/querier/queryable.go @@ -78,7 +78,7 @@ func queryValidators(ctx sdk.Context, cdc *codec.Codec, k keep.Keeper) (res []by res, errRes := codec.MarshalJSONIndent(cdc, validators) if err != nil { - return nil, sdk.ErrInternal(errRes.Error()) + return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } From 3491300c88e8a80a5f86a5bdc697cf1b6a0123ed Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 12 Oct 2018 12:24:56 -0700 Subject: [PATCH 07/17] err util funcs --- client/utils/rest.go | 26 +++----------------------- types/errors.go | 34 ++++++++++++++++++++++++++++++++++ types/errors_test.go | 33 +++++++++++++++++++++++++++++++++ x/gov/queryable.go | 29 ++++++++++++++--------------- x/stake/querier/queryable.go | 19 +++++++++---------- 5 files changed, 93 insertions(+), 48 deletions(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 58b8ae6c992b..9528df2a4137 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -22,34 +22,14 @@ const ( queryArgGenerateOnly = "generate_only" ) -//---------------------------------------- -// REST error utilities - -// checks if the reported REST error has the expected format of 'Error{}' -func errMustHaveValidFormat(err string) { - err = strings.TrimSpace(err) - if len(err) == 0 { - panic("Error can't be blank") - } else if !strings.HasPrefix(err, "Error{") { - panic(fmt.Sprintf("%s doesn't match the expected format", err)) - } -} - -// appends a message to the head of the given error -func AppendMsgToErr(msg string, err string) string { - if strings.HasPrefix(err, "Error{") { - err = err[6 : len(err)-1] - } - return fmt.Sprintf("Error{%s; %s}", msg, err) -} - //---------------------------------------- // Basic HTTP utilities // WriteErrorResponse prepares and writes a HTTP error // given a status code and an error message. -func WriteErrorResponse(w http.ResponseWriter, status int, err string, msg ...string) { - errMustHaveValidFormat(err) +func WriteErrorResponse(w http.ResponseWriter, status int, err string) { + msg := sdk.GetABCILogMsg(err) + sdk.ErrMustHaveValidFormat(msg) w.WriteHeader(status) w.Write([]byte(err)) } diff --git a/types/errors.go b/types/errors.go index 77a5a6083161..49207cab5e22 100644 --- a/types/errors.go +++ b/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,6 +287,39 @@ func (err *sdkError) QueryResult() abci.ResponseQuery { } } +//---------------------------------------- +// 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 { + err = MustGetABCILogMsg(err) + } + if strings.HasPrefix(err, "Error{") && strings.HasSuffix(err, "}") { + err = err[6 : len(err)-1] + } + return fmt.Sprintf("Error{%s; %s}", msg, err) +} + +// returns the 'message' value from the ABCILog or panics if wrong format +func MustGetABCILogMsg(abciLog string) string { + msgIdx := strings.Index(abciLog, "message\":\"") + if msgIdx == -1 { + panic(fmt.Sprintf("invalid format: %s", abciLog)) + } + msg := abciLog[msgIdx+len("message\":\"") : len(abciLog)-2] + return msg +} + +// checks if the reported REST error has the expected format of 'Error{}' +func ErrMustHaveValidFormat(err string) { + err = strings.TrimSpace(err) + if !strings.HasPrefix(err, "Error{") || !strings.HasSuffix(err, "}") { + panic(fmt.Sprintf("%s doesn't match the expected format", err)) + } +} + // parses the error into an object-like struct for exporting type HumanReadableError struct { Codespace CodespaceType `json:"codespace"` diff --git a/types/errors_test.go b/types/errors_test.go index f00b2600ca85..e0ea1ddd2951 100644 --- a/types/errors_test.go +++ b/types/errors_test.go @@ -65,3 +65,36 @@ func TestErrFn(t *testing.T) { require.Equal(t, ABCICodeOK, ToABCICode(CodespaceRoot, CodeOK)) } + +func TestErrorFormat(t *testing.T) { + // default err msg + err := errFns[0]("") + msg := err.Stacktrace().Error() + require.NotPanicsf(t, func() { ErrMustHaveValidFormat(msg) }, "Should have a valid format") + + // custom err msg + err = errFns[1]("this is a custom error") + msg = err.Stacktrace().Error() + require.NotPanicsf(t, func() { ErrMustHaveValidFormat(msg) }, "Should have a valid format") + + // custom err msg with aditional value + err = errFns[2]("") + msg = AppendMsgToErr("Error", err.Stacktrace().Error()) + require.NotPanicsf(t, func() { ErrMustHaveValidFormat(msg) }, "Should have a valid format") + + // unexpected err msg + err = errFns[3]("") + require.Panicsf(t, func() { ErrMustHaveValidFormat(err.ABCILog()) }, "Shouldn't have a valid format") +} + +func TestGetABCILogMsg(t *testing.T) { + for i, errFn := range errFns { + err := errFn("") + msg := MustGetABCILogMsg(err.ABCILog()) + require.Equal(t, err.Stacktrace().Error(), msg, "Err function expected to return the 'message' value from the ABCI Log. tc #%d", i) + } + + // invalid format + err := errFns[0]("") + require.Panicsf(t, func() { MustGetABCILogMsg(err.Stacktrace().Error()) }, "Shouldn't have a valid format") +} diff --git a/x/gov/queryable.go b/x/gov/queryable.go index 7206949261d2..8490f5c51da7 100644 --- a/x/gov/queryable.go +++ b/x/gov/queryable.go @@ -1,7 +1,6 @@ package gov import ( - "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" abci "github.com/tendermint/tendermint/abci/types" @@ -51,7 +50,7 @@ func queryProposal(ctx sdk.Context, path []string, req abci.RequestQuery, keeper var params QueryProposalParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } proposal := keeper.GetProposal(ctx, params.ProposalID) @@ -61,7 +60,7 @@ func queryProposal(ctx sdk.Context, path []string, req abci.RequestQuery, keeper bz, err2 := codec.MarshalJSONIndent(keeper.cdc, proposal) if err2 != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -77,13 +76,13 @@ func queryDeposit(ctx sdk.Context, path []string, req abci.RequestQuery, keeper var params QueryDepositParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } deposit, _ := keeper.GetDeposit(ctx, params.ProposalID, params.Depositer) bz, err2 := codec.MarshalJSONIndent(keeper.cdc, deposit) if err2 != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -99,13 +98,13 @@ func queryVote(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Kee var params QueryVoteParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } vote, _ := keeper.GetVote(ctx, params.ProposalID, params.Voter) bz, err2 := codec.MarshalJSONIndent(keeper.cdc, vote) if err2 != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -120,7 +119,7 @@ func queryDeposits(ctx sdk.Context, path []string, req abci.RequestQuery, keeper var params QueryDepositParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } var deposits []Deposit @@ -133,7 +132,7 @@ func queryDeposits(ctx sdk.Context, path []string, req abci.RequestQuery, keeper bz, err2 := codec.MarshalJSONIndent(keeper.cdc, deposits) if err2 != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -149,7 +148,7 @@ func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } var votes []Vote @@ -162,7 +161,7 @@ func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke bz, err2 := codec.MarshalJSONIndent(keeper.cdc, votes) if err2 != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -180,14 +179,14 @@ func queryProposals(ctx sdk.Context, path []string, req abci.RequestQuery, keepe var params QueryProposalsParams err2 := keeper.cdc.UnmarshalJSON(req.Data, ¶ms) if err2 != nil { - return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } proposals := keeper.GetProposalsFiltered(ctx, params.Voter, params.Depositer, params.ProposalStatus, params.NumLatestProposals) bz, err2 := codec.MarshalJSONIndent(keeper.cdc, proposals) if err2 != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } @@ -204,7 +203,7 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke var proposalID int64 err2 := keeper.cdc.UnmarshalJSON(req.Data, proposalID) if err2 != nil { - return nil, sdk.ErrUnknownRequest(utils.AppendMsgToErr("incorrectly formatted request data", err2.Error())) + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err2.Error())) } proposal := keeper.GetProposal(ctx, proposalID) @@ -224,7 +223,7 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke bz, err2 := codec.MarshalJSONIndent(keeper.cdc, tallyResult) if err2 != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", err2.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err2.Error())) } return bz, nil } diff --git a/x/stake/querier/queryable.go b/x/stake/querier/queryable.go index b953509b6e93..7cf01d0d3f50 100644 --- a/x/stake/querier/queryable.go +++ b/x/stake/querier/queryable.go @@ -1,7 +1,6 @@ package querier import ( - "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" keep "github.com/cosmos/cosmos-sdk/x/stake/keeper" @@ -78,7 +77,7 @@ func queryValidators(ctx sdk.Context, cdc *codec.Codec, k keep.Keeper) (res []by res, errRes := codec.MarshalJSONIndent(cdc, validators) if err != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -98,7 +97,7 @@ func queryValidator(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k res, errRes = codec.MarshalJSONIndent(cdc, validator) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -121,7 +120,7 @@ func queryDelegator(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k res, errRes = codec.MarshalJSONIndent(cdc, summary) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -140,7 +139,7 @@ func queryDelegatorValidators(ctx sdk.Context, cdc *codec.Codec, req abci.Reques res, errRes = codec.MarshalJSONIndent(cdc, validators) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -160,7 +159,7 @@ func queryDelegatorValidator(ctx sdk.Context, cdc *codec.Codec, req abci.Request res, errRes = codec.MarshalJSONIndent(cdc, validator) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -180,7 +179,7 @@ func queryDelegation(ctx sdk.Context, cdc *codec.Codec, req abci.RequestQuery, k res, errRes = codec.MarshalJSONIndent(cdc, delegation) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -200,7 +199,7 @@ func queryUnbondingDelegation(ctx sdk.Context, cdc *codec.Codec, req abci.Reques res, errRes = codec.MarshalJSONIndent(cdc, unbond) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -210,7 +209,7 @@ func queryPool(ctx sdk.Context, cdc *codec.Codec, k keep.Keeper) (res []byte, er res, errRes := codec.MarshalJSONIndent(cdc, pool) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } @@ -220,7 +219,7 @@ func queryParameters(ctx sdk.Context, cdc *codec.Codec, k keep.Keeper) (res []by res, errRes := codec.MarshalJSONIndent(cdc, params) if errRes != nil { - return nil, sdk.ErrInternal(utils.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", errRes.Error())) } return res, nil } From 51082b2563032ff53110865769f8dd16f1e3e27d Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 12 Oct 2018 13:09:49 -0700 Subject: [PATCH 08/17] Minor updates --- client/utils/rest.go | 2 +- types/errors.go | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 9528df2a4137..465a17562d99 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -28,7 +28,7 @@ const ( // WriteErrorResponse prepares and writes a HTTP error // given a status code and an error message. func WriteErrorResponse(w http.ResponseWriter, status int, err string) { - msg := sdk.GetABCILogMsg(err) + msg := sdk.MustGetABCILogMsg(err) sdk.ErrMustHaveValidFormat(msg) w.WriteHeader(status) w.Write([]byte(err)) diff --git a/types/errors.go b/types/errors.go index 49207cab5e22..a580c0ddfb03 100644 --- a/types/errors.go +++ b/types/errors.go @@ -258,7 +258,7 @@ func (err *sdkError) Code() CodeType { func (err *sdkError) ABCILog() string { cdc := codec.New() errMsg := err.cmnError.Error() - jsonErr := HumanReadableError{ + jsonErr := humanReadableError{ Codespace: err.codespace, Code: err.code, ABCICode: err.ABCICode(), @@ -292,10 +292,6 @@ func (err *sdkError) QueryResult() abci.ResponseQuery { // 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 { - err = MustGetABCILogMsg(err) - } if strings.HasPrefix(err, "Error{") && strings.HasSuffix(err, "}") { err = err[6 : len(err)-1] } @@ -321,7 +317,7 @@ func ErrMustHaveValidFormat(err string) { } // parses the error into an object-like struct for exporting -type HumanReadableError struct { +type humanReadableError struct { Codespace CodespaceType `json:"codespace"` Code CodeType `json:"code"` ABCICode ABCICodeType `json:"abci_code"` From 6679de34336982c504303d79db544a1a49533773 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 12 Oct 2018 16:24:22 -0700 Subject: [PATCH 09/17] unit tests --- client/utils/rest.go | 4 +-- types/errors.go | 43 +++++++++++++++++++++++--------- types/errors_test.go | 59 +++++++++++++++++++++++++------------------- 3 files changed, 66 insertions(+), 40 deletions(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 465a17562d99..55ca3643c6ca 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -28,8 +28,8 @@ const ( // WriteErrorResponse prepares and writes a HTTP error // given a status code and an error message. func WriteErrorResponse(w http.ResponseWriter, status int, err string) { - msg := sdk.MustGetABCILogMsg(err) - sdk.ErrMustHaveValidFormat(msg) + msgIdx := sdk.mustGetMsgIndex(err) + err = sdk.ErrEnsureFormat(err, msgIdx) w.WriteHeader(status) w.Write([]byte(err)) } diff --git a/types/errors.go b/types/errors.go index a580c0ddfb03..4d11e944dca8 100644 --- a/types/errors.go +++ b/types/errors.go @@ -290,30 +290,49 @@ func (err *sdkError) QueryResult() abci.ResponseQuery { //---------------------------------------- // REST error utilities +// ensures that the value of the ABCI Log error message is correctly formatted +func ErrEnsureFormat(abciLog string, msgIdx int) string { + msg := abciLog[msgIdx : len(abciLog)-2] + msg = parseErrorMsg(msg) + return fmt.Sprintf("%s%s%s", + abciLog[:msgIdx], + msg, + abciLog[len(abciLog)-3:], + ) +} + // 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, parseErrorMsg(errMsg)) + return fmt.Sprintf("%s%s%s", + err[:msgIdx+len("message\":\"")], + errMsg, + err[len(err)-3:], + ) + } if strings.HasPrefix(err, "Error{") && strings.HasSuffix(err, "}") { - err = err[6 : len(err)-1] + err = parseErrorMsg(err) } - return fmt.Sprintf("Error{%s; %s}", msg, err) + return fmt.Sprintf("%s; %s", msg, err) } -// returns the 'message' value from the ABCILog or panics if wrong format -func MustGetABCILogMsg(abciLog string) string { +func mustGetMsgIndex(abciLog string) int { msgIdx := strings.Index(abciLog, "message\":\"") if msgIdx == -1 { - panic(fmt.Sprintf("invalid format: %s", abciLog)) + panic(fmt.Sprintf("invalid error format: %s", abciLog)) } - msg := abciLog[msgIdx+len("message\":\"") : len(abciLog)-2] - return msg + return msgIdx + len("message\":\"") } -// checks if the reported REST error has the expected format of 'Error{}' -func ErrMustHaveValidFormat(err string) { - err = strings.TrimSpace(err) - if !strings.HasPrefix(err, "Error{") || !strings.HasSuffix(err, "}") { - panic(fmt.Sprintf("%s doesn't match the expected format", err)) +// parses the error 'message' value from err.Stacktrace().Error() +func parseErrorMsg(err string) string { + if strings.HasPrefix(err, "Error{") && strings.HasSuffix(err, "}") { + err = err[len("Error{") : len(err)-1] } + return strings.TrimSpace(err) } // parses the error into an object-like struct for exporting diff --git a/types/errors_test.go b/types/errors_test.go index e0ea1ddd2951..8aacc793a42c 100644 --- a/types/errors_test.go +++ b/types/errors_test.go @@ -1,6 +1,7 @@ package types import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -66,35 +67,41 @@ func TestErrFn(t *testing.T) { require.Equal(t, ABCICodeOK, ToABCICode(CodespaceRoot, CodeOK)) } -func TestErrorFormat(t *testing.T) { - // default err msg - err := errFns[0]("") - msg := err.Stacktrace().Error() - require.NotPanicsf(t, func() { ErrMustHaveValidFormat(msg) }, "Should have a valid format") - - // custom err msg - err = errFns[1]("this is a custom error") - msg = err.Stacktrace().Error() - require.NotPanicsf(t, func() { ErrMustHaveValidFormat(msg) }, "Should have a valid format") - - // custom err msg with aditional value - err = errFns[2]("") - msg = AppendMsgToErr("Error", err.Stacktrace().Error()) - require.NotPanicsf(t, func() { ErrMustHaveValidFormat(msg) }, "Should have a valid format") - - // unexpected err msg - err = errFns[3]("") - require.Panicsf(t, func() { ErrMustHaveValidFormat(err.ABCILog()) }, "Shouldn't have a valid format") +func TestErrVerifyFormat(t *testing.T) { + for i, errFn := range errFns { + err := errFn("") + abciLog := err.ABCILog() + msgIdx := mustGetMsgIndex(abciLog) + msg := ErrEnsureFormat(abciLog, msgIdx) + require.Equal(t, fmt.Sprintf("%s%s}", + abciLog[:msgIdx], + abciLog[msgIdx+len("Error{"):len(abciLog)-1]), + msg, + fmt.Sprintf("Should have formatted the error message. tc #%d", i)) + } } -func TestGetABCILogMsg(t *testing.T) { +func TestAppendMsgToErr(t *testing.T) { for i, errFn := range errFns { err := errFn("") - msg := MustGetABCILogMsg(err.ABCILog()) - require.Equal(t, err.Stacktrace().Error(), msg, "Err function expected to return the 'message' value from the ABCI Log. tc #%d", i) + 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[len("Error{"):len(errMsg)-1]), + 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("Error{"):len(abciLog)-1]), + msg, + fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i)) } - - // invalid format - err := errFns[0]("") - require.Panicsf(t, func() { MustGetABCILogMsg(err.Stacktrace().Error()) }, "Shouldn't have a valid format") } From ab129a689262c88a93a2a0ddd75d87a4af5cc127 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 12 Oct 2018 16:27:59 -0700 Subject: [PATCH 10/17] minor comment --- types/errors.go | 1 + 1 file changed, 1 insertion(+) diff --git a/types/errors.go b/types/errors.go index 4d11e944dca8..9e9ce8dc7ac3 100644 --- a/types/errors.go +++ b/types/errors.go @@ -319,6 +319,7 @@ func AppendMsgToErr(msg string, err string) string { 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 { From 28526cda1934246252951a5f11ecb6502c4d045a Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 12 Oct 2018 16:32:32 -0700 Subject: [PATCH 11/17] minor fix --- client/utils/rest.go | 3 +-- types/errors.go | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 55ca3643c6ca..a9d8928b069b 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -28,8 +28,7 @@ const ( // WriteErrorResponse prepares and writes a HTTP error // given a status code and an error message. func WriteErrorResponse(w http.ResponseWriter, status int, err string) { - msgIdx := sdk.mustGetMsgIndex(err) - err = sdk.ErrEnsureFormat(err, msgIdx) + err = sdk.ErrEnsureFormat(err) w.WriteHeader(status) w.Write([]byte(err)) } diff --git a/types/errors.go b/types/errors.go index 9e9ce8dc7ac3..289ca7ad08da 100644 --- a/types/errors.go +++ b/types/errors.go @@ -291,7 +291,8 @@ func (err *sdkError) QueryResult() abci.ResponseQuery { // REST error utilities // ensures that the value of the ABCI Log error message is correctly formatted -func ErrEnsureFormat(abciLog string, msgIdx int) string { +func ErrEnsureFormat(abciLog string) string { + msgIdx := mustGetMsgIndex(abciLog) msg := abciLog[msgIdx : len(abciLog)-2] msg = parseErrorMsg(msg) return fmt.Sprintf("%s%s%s", From b513162d9c6e3370b616a535b99bf2275672d40e Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 12 Oct 2018 17:53:02 -0700 Subject: [PATCH 12/17] fix lint --- types/errors_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/errors_test.go b/types/errors_test.go index 8aacc793a42c..28b64843c713 100644 --- a/types/errors_test.go +++ b/types/errors_test.go @@ -72,7 +72,7 @@ func TestErrVerifyFormat(t *testing.T) { err := errFn("") abciLog := err.ABCILog() msgIdx := mustGetMsgIndex(abciLog) - msg := ErrEnsureFormat(abciLog, msgIdx) + msg := ErrEnsureFormat(abciLog) require.Equal(t, fmt.Sprintf("%s%s}", abciLog[:msgIdx], abciLog[msgIdx+len("Error{"):len(abciLog)-1]), From 45ef9bee1d922f7a6013afec473459d4e7382b8b Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 17 Oct 2018 14:53:37 +0200 Subject: [PATCH 13/17] remove var --- client/keys/add.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index 72bc21ab8d09..f13fac547b05 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -262,10 +262,9 @@ func SeedRequestHandler(w http.ResponseWriter, r *http.Request) { algo := keys.SigningAlgo(algoType) seed := getSeed(algo) - bz := []byte(seed) w.Header().Set("Content-Type", "application/json") - w.Write(bz) + w.Write([]byte(seed)) } // RecoverKeyBody is recover key request REST body From 50f10d8c8bf4e9ca75d8f996ff2b0604d8a14c55 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 19 Oct 2018 17:18:32 +0200 Subject: [PATCH 14/17] Updated Append function --- types/errors.go | 28 +++------------------------- types/errors_test.go | 18 ++---------------- 2 files changed, 5 insertions(+), 41 deletions(-) diff --git a/types/errors.go b/types/errors.go index 289ca7ad08da..9715e0c81def 100644 --- a/types/errors.go +++ b/types/errors.go @@ -290,33 +290,19 @@ func (err *sdkError) QueryResult() abci.ResponseQuery { //---------------------------------------- // REST error utilities -// ensures that the value of the ABCI Log error message is correctly formatted -func ErrEnsureFormat(abciLog string) string { - msgIdx := mustGetMsgIndex(abciLog) - msg := abciLog[msgIdx : len(abciLog)-2] - msg = parseErrorMsg(msg) - return fmt.Sprintf("%s%s%s", - abciLog[:msgIdx], - msg, - abciLog[len(abciLog)-3:], - ) -} - // 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 { + fmt.Println(err) errMsg := err[msgIdx+len("message\":\"") : len(err)-2] - errMsg = fmt.Sprintf("%s; %s", msg, parseErrorMsg(errMsg)) + errMsg = fmt.Sprintf("%s; %s", msg, errMsg) return fmt.Sprintf("%s%s%s", err[:msgIdx+len("message\":\"")], errMsg, - err[len(err)-3:], + err[len(err)-2:], ) } - if strings.HasPrefix(err, "Error{") && strings.HasSuffix(err, "}") { - err = parseErrorMsg(err) - } return fmt.Sprintf("%s; %s", msg, err) } @@ -329,14 +315,6 @@ func mustGetMsgIndex(abciLog string) int { return msgIdx + len("message\":\"") } -// parses the error 'message' value from err.Stacktrace().Error() -func parseErrorMsg(err string) string { - if strings.HasPrefix(err, "Error{") && strings.HasSuffix(err, "}") { - err = err[len("Error{") : len(err)-1] - } - return strings.TrimSpace(err) -} - // parses the error into an object-like struct for exporting type humanReadableError struct { Codespace CodespaceType `json:"codespace"` diff --git a/types/errors_test.go b/types/errors_test.go index 28b64843c713..1d63e09908e2 100644 --- a/types/errors_test.go +++ b/types/errors_test.go @@ -67,20 +67,6 @@ func TestErrFn(t *testing.T) { require.Equal(t, ABCICodeOK, ToABCICode(CodespaceRoot, CodeOK)) } -func TestErrVerifyFormat(t *testing.T) { - for i, errFn := range errFns { - err := errFn("") - abciLog := err.ABCILog() - msgIdx := mustGetMsgIndex(abciLog) - msg := ErrEnsureFormat(abciLog) - require.Equal(t, fmt.Sprintf("%s%s}", - abciLog[:msgIdx], - abciLog[msgIdx+len("Error{"):len(abciLog)-1]), - msg, - fmt.Sprintf("Should have formatted the error message. tc #%d", i)) - } -} - func TestAppendMsgToErr(t *testing.T) { for i, errFn := range errFns { err := errFn("") @@ -90,7 +76,7 @@ func TestAppendMsgToErr(t *testing.T) { // plain msg error msg := AppendMsgToErr("something unexpected happened", errMsg) require.Equal(t, fmt.Sprintf("something unexpected happened; %s", - errMsg[len("Error{"):len(errMsg)-1]), + errMsg), msg, fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i)) @@ -100,7 +86,7 @@ func TestAppendMsgToErr(t *testing.T) { require.Equal(t, fmt.Sprintf("%s%s; %s}", abciLog[:msgIdx], "something unexpected happened", - abciLog[msgIdx+len("Error{"):len(abciLog)-1]), + abciLog[msgIdx:len(abciLog)-1]), msg, fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i)) } From 1121f437370318715861be7cb905ea85da375465 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 19 Oct 2018 17:19:53 +0200 Subject: [PATCH 15/17] Delete println --- types/errors.go | 1 - 1 file changed, 1 deletion(-) diff --git a/types/errors.go b/types/errors.go index 9715e0c81def..e05800b539dd 100644 --- a/types/errors.go +++ b/types/errors.go @@ -294,7 +294,6 @@ func (err *sdkError) QueryResult() abci.ResponseQuery { func AppendMsgToErr(msg string, err string) string { msgIdx := strings.Index(err, "message\":\"") if msgIdx != -1 { - fmt.Println(err) errMsg := err[msgIdx+len("message\":\"") : len(err)-2] errMsg = fmt.Sprintf("%s; %s", msg, errMsg) return fmt.Sprintf("%s%s%s", From edcb2c8cf47c22637865f730f06b484ebf84655e Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 19 Oct 2018 17:30:13 +0200 Subject: [PATCH 16/17] append msg to response --- client/tx/search.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/tx/search.go b/client/tx/search.go index e750f0e4c2ea..198bc4b3364b 100644 --- a/client/tx/search.go +++ b/client/tx/search.go @@ -151,7 +151,7 @@ func SearchTxRequestHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http. value, err := url.QueryUnescape(keyValue[1]) if err != nil { - utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) + utils.WriteErrorResponse(w, http.StatusBadRequest, sdk.AppendMsgToErr("could not decode address", err.Error())) return } From 2e29fc56a007f51040f6718b80a2329c9a50135b Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 19 Oct 2018 17:31:11 +0200 Subject: [PATCH 17/17] fix error --- client/utils/rest.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 1fe64f30c601..b74b91d035cf 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -27,7 +27,6 @@ const ( // WriteErrorResponse prepares and writes a HTTP error // given a status code and an error message. func WriteErrorResponse(w http.ResponseWriter, status int, err string) { - err = sdk.ErrEnsureFormat(err) w.WriteHeader(status) w.Write([]byte(err)) }