types/rest: add convenience functions for error checking

cosmos · Mar 31, 2020 · 7e2476c · 7e2476c
1 parent 35e1552
commit 7e2476c
Show file tree

Hide file tree

Showing 30 changed files with 227 additions and 333 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
 

diff --git a/client/rpc/block.go b/client/rpc/block.go
@@ -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
 		}
 
@@ -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
 		}
 

diff --git a/client/rpc/status.go b/client/rpc/status.go
@@ -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
 		}
 
@@ -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
 		}
 

diff --git a/client/rpc/validators.go b/client/rpc/validators.go
@@ -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)
@@ -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
 		}
 

diff --git a/client/tx/tx.go b/client/tx/tx.go
@@ -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
 	}
 
@@ -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
 		}
 
@@ -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
 	}
 

diff --git a/types/rest/rest.go b/types/rest/rest.go
@@ -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
 	}
 
@@ -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) {
@@ -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
 	}
 
@@ -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
 	}
 
@@ -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
 		}
 
@@ -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
 		}
 	}
@@ -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
 		}
 	}
@@ -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
 	}
 

diff --git a/types/rest/rest_test.go b/types/rest/rest_test.go
@@ -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)
+			}
+		})
+	}
+}
diff --git a/x/auth/client/rest.go b/x/auth/client/rest.go
@@ -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
 	}
 
@@ -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
 		}
 
@@ -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
 	}
 

diff --git a/x/auth/client/rest/broadcast.go b/x/auth/client/rest/broadcast.go
@@ -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
 		}
 

diff --git a/x/auth/client/rest/decode.go b/x/auth/client/rest/decode.go
@@ -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
 		}
 

diff --git a/x/auth/client/rest/encode.go b/x/auth/client/rest/encode.go
@@ -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
 		}