feat(service/rcp): unified error handling (celestiaorg#710)

* error handling is unified for all error types and endpoints * duplicated code is extracted into function * errors are returned as valid JSON, not plain text Co-authored-by: rene <[email protected]>
omercinar300 · May 15, 2022 · c436d6d · c436d6d
1 parent f3ccb56
commit c436d6d
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 76 deletions.
diff --git a/service/rpc/header.go b/service/rpc/header.go
@@ -27,8 +27,7 @@ func (h *Handler) handleHeaderRequest(w http.ResponseWriter, r *http.Request) {
 	// marshal and write response
 	resp, err := json.Marshal(header)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		log.Errorw("serving request", "endpoint", headerByHeightEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, headerByHeightEndpoint, err)
 		return
 	}
 	_, err = w.Write(resp)
@@ -48,23 +47,13 @@ func (h *Handler) performGetHeaderRequest(
 	heightStr := vars[heightKey]
 	height, err := strconv.Atoi(heightStr)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		_, werr := w.Write([]byte("must provide height as int"))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", endpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", endpoint, "err", err)
+		writeError(w, http.StatusBadRequest, endpoint, err)
 		return nil, err
 	}
 	// perform request
 	header, err := h.header.GetByHeight(r.Context(), uint64(height))
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		_, werr := w.Write([]byte(err.Error()))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", endpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", endpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, endpoint, err)
 		return nil, err
 	}
 	return header, nil

diff --git a/service/rpc/share.go b/service/rpc/share.go
@@ -35,16 +35,14 @@ func (h *Handler) handleSharesByNamespaceRequest(w http.ResponseWriter, r *http.
 	if strHeight, ok := vars[heightKey]; ok {
 		height, err = strconv.Atoi(strHeight)
 		if err != nil {
-			w.WriteHeader(http.StatusBadRequest)
-			log.Errorw("serving request", "endpoint", namespacedSharesEndpoint, "err", err)
+			writeError(w, http.StatusBadRequest, namespacedSharesEndpoint, err)
 			return
 		}
 	}
 	hexNID := vars[nIDKey]
 	nID, err := hex.DecodeString(hexNID)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		log.Errorw("serving request", "endpoint", namespacedSharesEndpoint, "err", err)
+		writeError(w, http.StatusBadRequest, namespacedSharesEndpoint, err)
 		return
 	}
 	// get header
@@ -56,28 +54,21 @@ func (h *Handler) handleSharesByNamespaceRequest(w http.ResponseWriter, r *http.
 		header, err = h.header.GetByHeight(r.Context(), uint64(height))
 	}
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		log.Errorw("serving request", "endpoint", namespacedSharesEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, namespacedSharesEndpoint, err)
 		return
 	}
 	// perform request
 	shares, err := h.share.GetSharesByNamespace(r.Context(), header.DAH, nID)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		_, werr := w.Write([]byte(err.Error()))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", namespacedSharesEndpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", namespacedSharesEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, namespacedSharesEndpoint, err)
 		return
 	}
 	resp, err := json.Marshal(&NamespacedSharesResponse{
 		Shares: shares,
 		Height: uint64(header.Height),
 	})
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		log.Errorw("serving request", "endpoint", namespacedSharesEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, namespacedSharesEndpoint, err)
 		return
 	}
 	_, err = w.Write(resp)

diff --git a/service/rpc/state.go b/service/rpc/state.go
@@ -33,18 +33,12 @@ type submitPFDRequest struct {
 func (h *Handler) handleBalanceRequest(w http.ResponseWriter, r *http.Request) {
 	bal, err := h.state.Balance(r.Context())
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		_, werr := w.Write([]byte(err.Error()))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", balanceEndpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", balanceEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, balanceEndpoint, err)
 		return
 	}
 	resp, err := json.Marshal(bal)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		log.Errorw("serving request", "endpoint", balanceEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, balanceEndpoint, err)
 		return
 	}
 	_, err = w.Write(resp)
@@ -60,28 +54,17 @@ func (h *Handler) handleBalanceForAddrRequest(w http.ResponseWriter, r *http.Req
 	// convert address to Address type
 	addr, err := types.AccAddressFromBech32(addrStr)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		_, werr := w.Write([]byte(err.Error()))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", balanceEndpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", balanceEndpoint, "err", err)
+		writeError(w, http.StatusBadRequest, balanceEndpoint, err)
 		return
 	}
 	bal, err := h.state.BalanceForAddress(r.Context(), addr)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		_, werr := w.Write([]byte(err.Error()))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", balanceEndpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", balanceEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, balanceEndpoint, err)
 		return
 	}
 	resp, err := json.Marshal(bal)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		log.Errorw("serving request", "endpoint", balanceEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, balanceEndpoint, err)
 		return
 	}
 	_, err = w.Write(resp)
@@ -95,31 +78,23 @@ func (h *Handler) handleSubmitTx(w http.ResponseWriter, r *http.Request) {
 	var req submitTxRequest
 	err := json.NewDecoder(r.Body).Decode(&req)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		log.Errorw("serving request", "endpoint", submitTxEndpoint, "err", err)
+		writeError(w, http.StatusBadRequest, submitTxEndpoint, err)
 		return
 	}
 	rawTx, err := hex.DecodeString(req.Tx)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		log.Errorw("serving request", "endpoint", submitTxEndpoint, "err", err)
+		writeError(w, http.StatusBadRequest, submitTxEndpoint, err)
 		return
 	}
 	// perform request
 	txResp, err := h.state.SubmitTx(r.Context(), rawTx)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		_, werr := w.Write([]byte(err.Error()))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", submitTxEndpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", submitTxEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, submitTxEndpoint, err)
 		return
 	}
 	resp, err := json.Marshal(txResp)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		log.Errorw("serving request", "endpoint", submitTxEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, submitTxEndpoint, err)
 		return
 	}
 	_, err = w.Write(resp)
@@ -133,37 +108,28 @@ func (h *Handler) handleSubmitPFD(w http.ResponseWriter, r *http.Request) {
 	var req submitPFDRequest
 	err := json.NewDecoder(r.Body).Decode(&req)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		log.Errorw("serving request", "endpoint", submitPFDEndpoint, "err", err)
+		writeError(w, http.StatusBadRequest, submitPFDEndpoint, err)
 		return
 	}
 	nID, err := hex.DecodeString(req.NamespaceID)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		log.Errorw("serving request", "endpoint", submitPFDEndpoint, "err", err)
+		writeError(w, http.StatusBadRequest, submitPFDEndpoint, err)
 		return
 	}
 	data, err := hex.DecodeString(req.Data)
 	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		log.Errorw("serving request", "endpoint", submitPFDEndpoint, "err", err)
+		writeError(w, http.StatusBadRequest, submitPFDEndpoint, err)
 		return
 	}
 	// perform request
 	txResp, err := h.state.SubmitPayForData(r.Context(), nID, data, req.GasLimit)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		_, werr := w.Write([]byte(err.Error()))
-		if werr != nil {
-			log.Errorw("writing response", "endpoint", submitPFDEndpoint, "err", werr)
-		}
-		log.Errorw("serving request", "endpoint", submitPFDEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, submitPFDEndpoint, err)
 		return
 	}
 	resp, err := json.Marshal(txResp)
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
-		log.Errorw("serving request", "endpoint", submitPFDEndpoint, "err", err)
+		writeError(w, http.StatusInternalServerError, submitPFDEndpoint, err)
 		return
 	}
 	_, err = w.Write(resp)

diff --git a/service/rpc/util.go b/service/rpc/util.go
@@ -0,0 +1,20 @@
+package rpc
+
+import (
+	"encoding/json"
+	"net/http"
+)
+
+func writeError(w http.ResponseWriter, statusCode int, endpoint string, err error) {
+	w.WriteHeader(statusCode)
+	errBody, jerr := json.Marshal(err.Error())
+	if jerr != nil {
+		log.Errorw("serializing error", "endpoint", endpoint, "err", jerr)
+		return
+	}
+	_, werr := w.Write(errBody)
+	if werr != nil {
+		log.Errorw("writing response", "endpoint", endpoint, "err", werr)
+	}
+	log.Errorw("serving request", "endpoint", endpoint, "err", err)
+}