Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

types/rest: convenience functions for error checking #5900

Merged
merged 1 commit into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ functionality that requires an online connection.
* (types/module) [\#5724](https://github.com/cosmos/cosmos-sdk/issues/5724) The `types/module` package does no longer depend on `x/simulation`.
* (client) [\#5856](https://github.com/cosmos/cosmos-sdk/pull/5856) Added the possibility to set `--offline` flag with config command.
* (client) [\#5895](https://github.com/cosmos/cosmos-sdk/issues/5895) show config options in the config command's help screen.
* (types/rest) [\#5900](https://github.com/cosmos/cosmos-sdk/pull/5900) Add Check*Error function family to spare developers from replicating tons of boilerplate code.

## [v0.38.2] - 2020-03-25

Expand Down
6 changes: 2 additions & 4 deletions client/rpc/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ func BlockRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
}

output, err := getBlock(cliCtx, &height)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand All @@ -149,8 +148,7 @@ func BlockRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
func LatestBlockRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
output, err := getBlock(cliCtx, nil)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand Down
6 changes: 2 additions & 4 deletions client/rpc/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ type NodeInfoResponse struct {
func NodeInfoRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
status, err := getNodeStatus(cliCtx)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand All @@ -98,8 +97,7 @@ type SyncingResponse struct {
func NodeSyncingRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
status, err := getNodeStatus(cliCtx)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand Down
6 changes: 2 additions & 4 deletions client/rpc/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,7 @@ func ValidatorSetRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
}

output, err := GetValidators(cliCtx, &height, page, limit)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}
rest.PostProcessResponse(w, cliCtx, output)
Expand All @@ -202,8 +201,7 @@ func LatestValidatorSetRequestHandlerFn(cliCtx context.CLIContext) http.HandlerF
}

output, err := GetValidators(cliCtx, nil, page, limit)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand Down
12 changes: 4 additions & 8 deletions client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ func WriteGeneratedTxResponse(
}

simAndExec, gas, err := flags.ParseGas(br.Gas)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

Expand All @@ -182,8 +181,7 @@ func WriteGeneratedTxResponse(
}

_, adjusted, err := CalculateGas(ctx.QueryWithData, txf, msgs...)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand All @@ -196,14 +194,12 @@ func WriteGeneratedTxResponse(
}

tx, err := BuildUnsignedTx(txf, msgs...)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

output, err := ctx.Marshaler.MarshalJSON(tx)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand Down
49 changes: 35 additions & 14 deletions types/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ func (br BaseReq) ValidateBasic(w http.ResponseWriter) bool {
// Writes an error response to ResponseWriter and returns true if errors occurred.
func ReadRESTReq(w http.ResponseWriter, r *http.Request, m codec.JSONMarshaler, req interface{}) bool {
body, err := ioutil.ReadAll(r.Body)
if err != nil {
WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if CheckBadRequestError(w, err) {
return false
}

Expand All @@ -148,6 +147,34 @@ func NewErrorResponse(code int, err string) ErrorResponse {
return ErrorResponse{Code: code, Error: err}
}

// CheckError takes care of writing an error response if err is not nil.
// Returns false when err is nil; it returns true otherwise.
func CheckError(w http.ResponseWriter, status int, err error) bool {
if err != nil {
WriteErrorResponse(w, status, err.Error())
return true
}
return false
}

// CheckBadRequestError attaches an error message to an HTTP 400 BAD REQUEST response.
// Returns false when err is nil; it returns true otherwise.
func CheckBadRequestError(w http.ResponseWriter, err error) bool {
return CheckError(w, http.StatusBadRequest, err)
}

// CheckInternalServerError attaches an error message to an HTTP 500 INTERNAL SERVER ERROR response.
// Returns false when err is nil; it returns true otherwise.
func CheckInternalServerError(w http.ResponseWriter, err error) bool {
return CheckError(w, http.StatusInternalServerError, err)
}

// CheckNotFoundError attaches an error message to an HTTP 404 NOT FOUND response.
// Returns false when err is nil; it returns true otherwise.
func CheckNotFoundError(w http.ResponseWriter, err error) bool {
return CheckError(w, http.StatusNotFound, err)
}

// WriteErrorResponse prepares and writes a HTTP error
// given a status code and an error message.
func WriteErrorResponse(w http.ResponseWriter, status int, err string) {
Expand All @@ -162,8 +189,7 @@ func WriteSimulationResponse(w http.ResponseWriter, m codec.JSONMarshaler, gas u
gasEst := GasEstimateResponse{GasEstimate: gas}

resp, err := m.MarshalJSON(gasEst)
if err != nil {
WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if CheckInternalServerError(w, err) {
return
}

Expand Down Expand Up @@ -194,8 +220,7 @@ func ParseFloat64OrReturnBadRequest(w http.ResponseWriter, s string, defaultIfEm
}

n, err := strconv.ParseFloat(s, 64)
if err != nil {
WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if CheckBadRequestError(w, err) {
return n, false
}

Expand All @@ -208,8 +233,7 @@ func ParseQueryHeightOrReturnBadRequest(w http.ResponseWriter, cliCtx context.CL
heightStr := r.FormValue("height")
if heightStr != "" {
height, err := strconv.ParseInt(heightStr, 10, 64)
if err != nil {
WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if CheckBadRequestError(w, err) {
return cliCtx, false
}

Expand Down Expand Up @@ -257,8 +281,7 @@ func PostProcessResponseBare(w http.ResponseWriter, ctx context.CLIContext, body
resp, err = codec.MarshalIndentFromJSON(resp)
}

if err != nil {
WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if CheckInternalServerError(w, err) {
return
}
}
Expand Down Expand Up @@ -302,8 +325,7 @@ func PostProcessResponse(w http.ResponseWriter, ctx context.CLIContext, resp int
result, err = codec.MarshalIndentFromJSON(result)
}

if err != nil {
WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if CheckInternalServerError(w, err) {
return
}
}
Expand All @@ -315,8 +337,7 @@ func PostProcessResponse(w http.ResponseWriter, ctx context.CLIContext, resp int
output, err = codec.MarshalIndentFromJSON(output)
}

if err != nil {
WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if CheckInternalServerError(w, err) {
return
}

Expand Down
32 changes: 32 additions & 0 deletions types/rest/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,3 +440,35 @@ func mustNewRequest(t *testing.T, method, url string, body io.Reader) *http.Requ
require.NoError(t, err)
return req
}

func TestCheckErrors(t *testing.T) {
t.Parallel()
err := errors.New("ERROR")
tests := []struct {
name string
checkerFn func(w http.ResponseWriter, err error) bool
error error
wantErr bool
wantString string
wantStatus int
}{
{"500", CheckInternalServerError, err, true, `{"error":"ERROR"}`, http.StatusInternalServerError},
{"500 (no error)", CheckInternalServerError, nil, false, ``, http.StatusInternalServerError},
{"400", CheckBadRequestError, err, true, `{"error":"ERROR"}`, http.StatusBadRequest},
{"400 (no error)", CheckBadRequestError, nil, false, ``, http.StatusBadRequest},
{"404", CheckNotFoundError, err, true, `{"error":"ERROR"}`, http.StatusNotFound},
{"404 (no error)", CheckNotFoundError, nil, false, ``, http.StatusNotFound},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
w := httptest.NewRecorder()
require.Equal(t, tt.wantErr, tt.checkerFn(w, tt.error))
if tt.wantErr {
require.Equal(t, w.Body.String(), tt.wantString)
require.Equal(t, w.Code, tt.wantStatus)
}
})
}
}
12 changes: 4 additions & 8 deletions x/auth/client/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ func WriteGenerateStdTxResponse(w http.ResponseWriter, cliCtx context.CLIContext
}

simAndExec, gas, err := flags.ParseGas(br.Gas)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

Expand All @@ -36,8 +35,7 @@ func WriteGenerateStdTxResponse(w http.ResponseWriter, cliCtx context.CLIContext
}

txBldr, err = EnrichWithGas(txBldr, cliCtx, msgs)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand All @@ -48,14 +46,12 @@ func WriteGenerateStdTxResponse(w http.ResponseWriter, cliCtx context.CLIContext
}

stdMsg, err := txBldr.BuildSignMsg(msgs)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

output, err := cliCtx.Codec.MarshalJSON(types.NewStdTx(stdMsg.Msgs, stdMsg.Fee, nil, stdMsg.Memo))
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand Down
13 changes: 4 additions & 9 deletions x/auth/client/rest/broadcast.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,28 +23,23 @@ func BroadcastTxRequest(cliCtx context.CLIContext) http.HandlerFunc {
var req BroadcastReq

body, err := ioutil.ReadAll(r.Body)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

err = cliCtx.Codec.UnmarshalJSON(body, &req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if err := cliCtx.Codec.UnmarshalJSON(body, &req); rest.CheckBadRequestError(w, err) {
return
}

txBytes, err := cliCtx.Codec.MarshalBinaryBare(req.Tx)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

cliCtx = cliCtx.WithBroadcastMode(req.Mode)

res, err := cliCtx.BroadcastTx(txBytes)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand Down
12 changes: 4 additions & 8 deletions x/auth/client/rest/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,23 @@ func DecodeTxRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
var req DecodeReq

body, err := ioutil.ReadAll(r.Body)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

err = cliCtx.Codec.UnmarshalJSON(body, &req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

txBytes, err := base64.StdEncoding.DecodeString(req.Tx)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

var stdTx authtypes.StdTx
err = cliCtx.Codec.UnmarshalBinaryBare(txBytes, &stdTx)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

Expand Down
9 changes: 3 additions & 6 deletions x/auth/client/rest/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,18 @@ func EncodeTxRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
var req types.StdTx

body, err := ioutil.ReadAll(r.Body)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

err = cliCtx.Codec.UnmarshalJSON(body, &req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if rest.CheckBadRequestError(w, err) {
return
}

// re-encode it via the Amino wire protocol
txBytes, err := cliCtx.Codec.MarshalBinaryBare(req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
if rest.CheckInternalServerError(w, err) {
return
}

Expand Down
Loading