From 0d66fe996370d67ddd1a01441fa6dd0692ea79a3 Mon Sep 17 00:00:00 2001 From: pancho horrillo Date: Wed, 20 Nov 2019 07:37:35 +0100 Subject: [PATCH] Rename WriteErrorResponse() to simply ErrorJSON() and reorder arguments This way we better mimic net/http.Error() https://pkg.go.dev/net/http?tab=doc#Error --- internal/server/control/control.go | 14 ++++++------ internal/server/data/decorator.go | 2 +- internal/server/data/resource.go | 30 ++++++++++++------------- internal/server/data/server.go | 2 +- internal/server/srverrors/error.go | 20 +++++++++-------- internal/server/srverrors/error_test.go | 12 +++++----- 6 files changed, 41 insertions(+), 39 deletions(-) diff --git a/internal/server/control/control.go b/internal/server/control/control.go index d29d790..d357cec 100644 --- a/internal/server/control/control.go +++ b/internal/server/control/control.go @@ -53,7 +53,7 @@ func removeRoute(res http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) id := vars["id"] if err := funcRemove(id); err != nil { - srverrors.WriteErrorResponse(http.StatusNotFound, "Route Not Found", res) + srverrors.ErrorJSON(res, "Route Not Found", http.StatusNotFound) return } @@ -94,29 +94,29 @@ func addRoute(res http.ResponseWriter, req *http.Request) { payload, _ := ioutil.ReadAll(req.Body) err := json.Unmarshal(payload, &route) if err != nil { - srverrors.WriteErrorResponse(http.StatusBadRequest, "Malformed JSON", res) + srverrors.ErrorJSON(res, "Malformed JSON", http.StatusBadRequest) return } if route.Method == "" { - srverrors.WriteErrorResponse(http.StatusUnprocessableEntity, "Invalid Route", res) + srverrors.ErrorJSON(res, "Invalid Route", http.StatusUnprocessableEntity) return } if route.Pattern == "" { - srverrors.WriteErrorResponse(http.StatusUnprocessableEntity, "Invalid Route", res) + srverrors.ErrorJSON(res, "Invalid Route", http.StatusUnprocessableEntity) return } err = pathValidator(route.Pattern) if err != nil { - srverrors.WriteErrorResponse(http.StatusUnprocessableEntity, "Invalid Route", res) + srverrors.ErrorJSON(res, "Invalid Route", http.StatusUnprocessableEntity) return } id, err := idGenerator() if err != nil { - srverrors.WriteErrorResponse(http.StatusInternalServerError, "Internal Server Error", res) + srverrors.ErrorJSON(res, "Internal Server Error", http.StatusInternalServerError) return } @@ -138,7 +138,7 @@ var funcGet func(string) (model.Route, error) = user.Routes.Get func getRoute(res http.ResponseWriter, req *http.Request) { id := mux.Vars(req)["id"] if r, err := funcGet(id); err != nil { - srverrors.WriteErrorResponse(http.StatusNotFound, "Route Not Found", res) + srverrors.ErrorJSON(res, "Route Not Found", http.StatusNotFound) } else { res.Header().Set("Content-Type", "application/json") rBytes, _ := json.Marshal(r) diff --git a/internal/server/data/decorator.go b/internal/server/data/decorator.go index bd708be..4a78ffe 100644 --- a/internal/server/data/decorator.go +++ b/internal/server/data/decorator.go @@ -40,7 +40,7 @@ func checkHandler(fn resourceHandler) func(http.ResponseWriter, *http.Request) { if h, ok := Handlers.Get(handlerID); ok { fn(w, r, h) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, "Handler ID Not Found", w) + srverrors.ErrorJSON(w, "Handler ID Not Found", http.StatusNotFound) } } } diff --git a/internal/server/data/resource.go b/internal/server/data/resource.go index cd99c3e..9633a67 100644 --- a/internal/server/data/resource.go +++ b/internal/server/data/resource.go @@ -40,7 +40,7 @@ func getRequestBody(w http.ResponseWriter, r *http.Request, h *model.Handler) { n, err := io.Copy(w, h.Request.Body) if err != nil { if n == 0 { - srverrors.WriteErrorResponse(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), w) + srverrors.ErrorJSON(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) } else { // Only way to abort current connection as of go 1.13 // https://github.com/golang/go/issues/16542 @@ -72,7 +72,7 @@ func getRequestMatches(w http.ResponseWriter, r *http.Request, h *model.Handler) if value, ok := vars[name]; ok { _, _ = w.Write([]byte(value)) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } } @@ -82,7 +82,7 @@ func getRequestParams(w http.ResponseWriter, r *http.Request, h *model.Handler) if values, ok := h.Request.URL.Query()[name]; ok { _, _ = w.Write([]byte(values[0])) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } } @@ -92,7 +92,7 @@ func getRequestHeaders(w http.ResponseWriter, r *http.Request, h *model.Handler) if values, ok := h.Request.Header[textproto.CanonicalMIMEHeaderKey(name)]; ok { _, _ = w.Write([]byte(values[0])) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } } @@ -102,7 +102,7 @@ func getRequestCookies(w http.ResponseWriter, r *http.Request, h *model.Handler) if cookie, err := h.Request.Cookie(name); err == nil { _, _ = w.Write([]byte(cookie.Value)) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } } @@ -118,11 +118,11 @@ func getRequestForm(w http.ResponseWriter, r *http.Request, h *model.Handler) { // We tried to exercise this execution path but didn't know how. err := h.Request.ParseForm() if err != nil { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } else if values, ok := h.Request.Form[name]; ok { _, _ = w.Write([]byte(values[0])) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } } @@ -133,7 +133,7 @@ func getRequestFileName(w http.ResponseWriter, r *http.Request, h *model.Handler if err == nil { _, _ = w.Write([]byte(header.Filename)) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } } @@ -144,7 +144,7 @@ func getRequestFileContent(w http.ResponseWriter, r *http.Request, h *model.Hand if err == nil { _, _ = io.Copy(w, file) } else { - srverrors.WriteErrorResponse(http.StatusNotFound, ResourceItemNotFound, w) + srverrors.ErrorJSON(w, ResourceItemNotFound, http.StatusNotFound) } } @@ -153,14 +153,14 @@ func getRequestFileContent(w http.ResponseWriter, r *http.Request, h *model.Hand func setResponseStatus(w http.ResponseWriter, r *http.Request, h *model.Handler) { sb, err := ioutil.ReadAll(r.Body) if err != nil { - srverrors.WriteErrorResponse(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), w) + srverrors.ErrorJSON(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } if si, err := strconv.Atoi(string(sb)); err != nil { - srverrors.WriteErrorResponse(http.StatusUnprocessableEntity, NonIntegerValue, w) + srverrors.ErrorJSON(w, NonIntegerValue, http.StatusUnprocessableEntity) } else if http.StatusText(si) == "" { - srverrors.WriteErrorResponse(http.StatusBadRequest, InvalidStatusCode, w) + srverrors.ErrorJSON(w, InvalidStatusCode, http.StatusBadRequest) } else { h.Writer.WriteHeader(int(si)) } @@ -170,7 +170,7 @@ func setResponseHeaders(w http.ResponseWriter, r *http.Request, h *model.Handler name := mux.Vars(r)["name"] vb, err := ioutil.ReadAll(r.Body) if err != nil { - srverrors.WriteErrorResponse(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), w) + srverrors.ErrorJSON(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } @@ -186,7 +186,7 @@ func setResponseCookies(w http.ResponseWriter, r *http.Request, h *model.Handler name := mux.Vars(r)["name"] vb, err := ioutil.ReadAll(r.Body) if err != nil { - srverrors.WriteErrorResponse(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), w) + srverrors.ErrorJSON(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } @@ -199,6 +199,6 @@ func setResponseBody(w http.ResponseWriter, r *http.Request, h *model.Handler) { if n > 0 { panic("Truncated body") } - srverrors.WriteErrorResponse(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), w) + srverrors.ErrorJSON(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) } } diff --git a/internal/server/data/server.go b/internal/server/data/server.go index 140b984..323b6e7 100644 --- a/internal/server/data/server.go +++ b/internal/server/data/server.go @@ -38,7 +38,7 @@ func configRouter(rs []routeSpec) (r *mux.Router) { r.HandleFunc( "/handlers/{handlerID}/{resource:.*}", func(w http.ResponseWriter, r *http.Request) { - srverrors.WriteErrorResponse(http.StatusBadRequest, "Invalid Resource Path", w) + srverrors.ErrorJSON(w, "Invalid Resource Path", http.StatusBadRequest) }) return r } diff --git a/internal/server/srverrors/error.go b/internal/server/srverrors/error.go index 4b303c5..59a0bdf 100644 --- a/internal/server/srverrors/error.go +++ b/internal/server/srverrors/error.go @@ -10,13 +10,15 @@ type ServerErrMessage struct { Reason string `json:"reason"` } -// WriteErrorResponse writes the error JSON body to the provided http.ResponseWriter, -// after setting the appropiate Content-Type header -func WriteErrorResponse(statusCode int, reasonMsg string, res http.ResponseWriter) { - respBody := ServerErrMessage{} - respBody.Reason = reasonMsg - bb, _ := json.Marshal(respBody) - res.Header().Set("Content-Type", "application/json; charset=utf-8") - res.WriteHeader(statusCode) - _, _ = res.Write(bb) +// ErrorJSON writes the provided error as a JSON body to the provided +// http.ResponseWriter, after setting the appropriate Content-Type header +func ErrorJSON(w http.ResponseWriter, error string, code int) { + body, _ := json.Marshal( + ServerErrMessage{ + Reason: error, + }, + ) + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.WriteHeader(code) + _, _ = w.Write(body) } diff --git a/internal/server/srverrors/error_test.go b/internal/server/srverrors/error_test.go index adda7b5..647f0e2 100644 --- a/internal/server/srverrors/error_test.go +++ b/internal/server/srverrors/error_test.go @@ -10,31 +10,31 @@ import ( "github.com/BBVA/kapow/internal/server/srverrors" ) -func TestWriteErrorResponseSetsAppJsonContentType(t *testing.T) { +func TestErrorJSONSetsAppJsonContentType(t *testing.T) { w := httptest.NewRecorder() - srverrors.WriteErrorResponse(0, "Not Important Here", w) + srverrors.ErrorJSON(w, "Not Important Here", 0) if v := w.Result().Header.Get("Content-Type"); v != "application/json; charset=utf-8" { t.Errorf("Content-Type header mismatch. Expected: %q, got: %q", "application/json; charset=utf-8", v) } } -func TestWriteErrorResponseSetsRequestedStatusCode(t *testing.T) { +func TestErrorJSONSetsRequestedStatusCode(t *testing.T) { w := httptest.NewRecorder() - srverrors.WriteErrorResponse(http.StatusGone, "Not Important Here", w) + srverrors.ErrorJSON(w, "Not Important Here", http.StatusGone) if v := w.Result().StatusCode; v != http.StatusGone { t.Errorf("Status code mismatch. Expected: %d, got: %d", http.StatusGone, v) } } -func TestWriteErrorResponseSetsBodyCorrectly(t *testing.T) { +func TestErrorJSONSetsBodyCorrectly(t *testing.T) { expectedReason := "Something Not Found" w := httptest.NewRecorder() - srverrors.WriteErrorResponse(http.StatusNotFound, expectedReason, w) + srverrors.ErrorJSON(w, expectedReason, http.StatusNotFound) errMsg := srverrors.ServerErrMessage{} if bodyBytes, err := ioutil.ReadAll(w.Result().Body); err != nil {