From edff842c3b5aeec6f5e6b37292938d4e9d50fd85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Hurtado?= Date: Mon, 18 Nov 2019 14:32:41 +0100 Subject: [PATCH] New srverrors package added. New error handling added to control package --- internal/server/control/control.go | 17 ++--- internal/server/control/control_test.go | 87 +++++++++++++++---------- internal/server/srverrors/error.go | 19 ++++++ internal/server/srverrors/error_test.go | 47 +++++++++++++ 4 files changed, 128 insertions(+), 42 deletions(-) create mode 100644 internal/server/srverrors/error.go create mode 100644 internal/server/srverrors/error_test.go diff --git a/internal/server/control/control.go b/internal/server/control/control.go index fe0a21b..d29d790 100644 --- a/internal/server/control/control.go +++ b/internal/server/control/control.go @@ -25,6 +25,7 @@ import ( "github.com/gorilla/mux" "github.com/BBVA/kapow/internal/server/model" + "github.com/BBVA/kapow/internal/server/srverrors" "github.com/BBVA/kapow/internal/server/user" ) @@ -52,7 +53,7 @@ func removeRoute(res http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) id := vars["id"] if err := funcRemove(id); err != nil { - res.WriteHeader(http.StatusNotFound) + srverrors.WriteErrorResponse(http.StatusNotFound, "Route Not Found", res) return } @@ -93,29 +94,29 @@ func addRoute(res http.ResponseWriter, req *http.Request) { payload, _ := ioutil.ReadAll(req.Body) err := json.Unmarshal(payload, &route) if err != nil { - res.WriteHeader(http.StatusBadRequest) + srverrors.WriteErrorResponse(http.StatusBadRequest, "Malformed JSON", res) return } if route.Method == "" { - res.WriteHeader(http.StatusUnprocessableEntity) + srverrors.WriteErrorResponse(http.StatusUnprocessableEntity, "Invalid Route", res) return } if route.Pattern == "" { - res.WriteHeader(http.StatusUnprocessableEntity) + srverrors.WriteErrorResponse(http.StatusUnprocessableEntity, "Invalid Route", res) return } err = pathValidator(route.Pattern) if err != nil { - res.WriteHeader(http.StatusUnprocessableEntity) + srverrors.WriteErrorResponse(http.StatusUnprocessableEntity, "Invalid Route", res) return } id, err := idGenerator() if err != nil { - res.WriteHeader(http.StatusInternalServerError) + srverrors.WriteErrorResponse(http.StatusInternalServerError, "Internal Server Error", res) return } @@ -124,8 +125,8 @@ func addRoute(res http.ResponseWriter, req *http.Request) { created := funcAdd(route) createdBytes, _ := json.Marshal(created) - res.WriteHeader(http.StatusCreated) res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusCreated) _, _ = res.Write(createdBytes) } @@ -137,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 { - res.WriteHeader(http.StatusNotFound) + srverrors.WriteErrorResponse(http.StatusNotFound, "Route Not Found", res) } else { res.Header().Set("Content-Type", "application/json") rBytes, _ := json.Marshal(r) diff --git a/internal/server/control/control_test.go b/internal/server/control/control_test.go index 975039e..c40b70b 100644 --- a/internal/server/control/control_test.go +++ b/internal/server/control/control_test.go @@ -19,6 +19,7 @@ package control import ( "encoding/json" "errors" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -30,9 +31,33 @@ import ( "github.com/gorilla/mux" "github.com/BBVA/kapow/internal/server/model" + "github.com/BBVA/kapow/internal/server/srverrors" "github.com/BBVA/kapow/internal/server/user" ) +func checkErrorResponse(r *http.Response, expectedErrcode int, expectedReason string) []error { + errList := make([]error, 0) + + if r.StatusCode != expectedErrcode { + errList = append(errList, fmt.Errorf("HTTP status mismatch. Expected: %d, got: %d", expectedErrcode, r.StatusCode)) + } + + if v := r.Header.Get("Content-Type"); v != "application/json; charset=utf-8" { + errList = append(errList, fmt.Errorf("Content-Type header mismatch. Expected: %q, got: %q", "application/json; charset=utf-8", v)) + } + + errMsg := srverrors.ServerErrMessage{} + if bodyBytes, err := ioutil.ReadAll(r.Body); err != nil { + errList = append(errList, fmt.Errorf("Unexpected error reading response body: %v", err)) + } else if err := json.Unmarshal(bodyBytes, &errMsg); err != nil { + errList = append(errList, fmt.Errorf("Response body contains invalid JSON entity: %v", err)) + } else if errMsg.Reason != expectedReason { + errList = append(errList, fmt.Errorf("Unexpected reason in response. Expected: %q, got: %q", expectedReason, errMsg.Reason)) + } + + return errList +} + func TestConfigRouterHasRoutesWellConfigured(t *testing.T) { testCases := []struct { pattern, method string @@ -101,17 +126,15 @@ func TestAddRouteReturnsBadRequestWhenMalformedJSONBody(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/routes", strings.NewReader(reqPayload)) resp := httptest.NewRecorder() - handler := http.HandlerFunc(addRoute) - handler.ServeHTTP(resp, req) - if resp.Code != http.StatusBadRequest { - t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusBadRequest, resp.Code) + addRoute(resp, req) + + for _, e := range checkErrorResponse(resp.Result(), http.StatusBadRequest, "Malformed JSON") { + t.Error(e.Error()) } - } func TestAddRouteReturns422ErrorWhenMandatoryFieldsMissing(t *testing.T) { - handler := http.HandlerFunc(addRoute) tc := []struct { payload, testCase string testMustFail bool @@ -167,18 +190,19 @@ func TestAddRouteReturns422ErrorWhenMandatoryFieldsMissing(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/routes", strings.NewReader(test.payload)) resp := httptest.NewRecorder() - handler.ServeHTTP(resp, req) + addRoute(resp, req) + r := resp.Result() if test.testMustFail { - if resp.Code != http.StatusUnprocessableEntity { - t.Errorf("HTTP status mismatch in case %s. Expected: %d, got: %d", test.testCase, http.StatusUnprocessableEntity, resp.Code) + for _, e := range checkErrorResponse(r, http.StatusUnprocessableEntity, "Invalid Route") { + t.Error(e.Error()) } } else if !test.testMustFail { - if resp.Code != http.StatusCreated { - t.Errorf("HTTP status mismatch in case %s. Expected: %d, got: %d", test.testCase, http.StatusUnprocessableEntity, resp.Code) + if r.StatusCode != http.StatusCreated { + t.Errorf("HTTP status mismatch in case %s. Expected: %d, got: %d", test.testCase, http.StatusUnprocessableEntity, r.StatusCode) } - if ct := resp.Header().Get("Content-Type"); ct != "application/json" { - t.Errorf("Incorrect content type in response. Expected: application/json, got: %s", ct) + if ct := r.Header.Get("Content-Type"); ct != "application/json" { + t.Errorf("Incorrect content type in response. Expected: application/json, got: %q", ct) } } } @@ -193,7 +217,6 @@ func TestAddRouteGeneratesRouteID(t *testing.T) { }` req := httptest.NewRequest(http.MethodPost, "/routes", strings.NewReader(reqPayload)) resp := httptest.NewRecorder() - handler := http.HandlerFunc(addRoute) var genID string funcAdd = func(input model.Route) model.Route { genID = input.ID @@ -204,7 +227,7 @@ func TestAddRouteGeneratesRouteID(t *testing.T) { defer func() { pathValidator = origPathValidator }() pathValidator = func(path string) error { return nil } - handler.ServeHTTP(resp, req) + addRoute(resp, req) if _, err := uuid.Parse(genID); err != nil { t.Error("ID not generated properly") @@ -220,7 +243,6 @@ func TestAddRoute500sWhenIDGeneratorFails(t *testing.T) { }` req := httptest.NewRequest(http.MethodPost, "/routes", strings.NewReader(reqPayload)) resp := httptest.NewRecorder() - handler := http.HandlerFunc(addRoute) origPathValidator := pathValidator defer func() { pathValidator = origPathValidator }() @@ -230,14 +252,13 @@ func TestAddRoute500sWhenIDGeneratorFails(t *testing.T) { defer func() { idGenerator = idGenOrig }() idGenerator = func() (uuid.UUID, error) { var uuid uuid.UUID - return uuid, errors.New( - "End of Time reached; Try again before, or in the next Big Bang cycle") + return uuid, errors.New("End of Time reached; Try again before, or in the next Big Bang cycle") } - handler.ServeHTTP(resp, req) + addRoute(resp, req) - if resp.Result().StatusCode != http.StatusInternalServerError { - t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusInternalServerError, resp.Result().StatusCode) + for _, e := range checkErrorResponse(resp.Result(), http.StatusInternalServerError, "Internal Server Error") { + t.Error(e.Error()) } } @@ -251,7 +272,6 @@ func TestAddRouteReturnsCreated(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/routes", strings.NewReader(reqPayload)) resp := httptest.NewRecorder() - handler := http.HandlerFunc(addRoute) var genID string funcAdd = func(input model.Route) model.Route { expected := model.Route{ID: input.ID, Method: "GET", Pattern: "/hello", Entrypoint: "/bin/sh -c", Command: "echo Hello World | kapow set /response/body"} @@ -267,7 +287,7 @@ func TestAddRouteReturnsCreated(t *testing.T) { defer func() { pathValidator = origPathValidator }() pathValidator = func(path string) error { return nil } - handler.ServeHTTP(resp, req) + addRoute(resp, req) if resp.Code != http.StatusCreated { t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusCreated, resp.Code) @@ -297,15 +317,14 @@ func TestAddRoute422sWhenInvalidRoute(t *testing.T) { }` req := httptest.NewRequest(http.MethodPost, "/routes", strings.NewReader(reqPayload)) resp := httptest.NewRecorder() - handler := http.HandlerFunc(addRoute) origPathValidator := pathValidator defer func() { pathValidator = origPathValidator }() pathValidator = func(path string) error { return errors.New("Invalid route") } - handler.ServeHTTP(resp, req) + addRoute(resp, req) - if resp.Code != http.StatusUnprocessableEntity { - t.Error("Invalid route registered") + for _, e := range checkErrorResponse(resp.Result(), http.StatusUnprocessableEntity, "Invalid Route") { + t.Error(e.Error()) } } @@ -315,7 +334,6 @@ func TestRemoveRouteReturnsNotFound(t *testing.T) { handler := mux.NewRouter() handler.HandleFunc("/routes/{id}", removeRoute). Methods("DELETE") - funcRemove = func(id string) error { if id == "ROUTE_XXXXXXXXXXXXXXXXXX" { return errors.New(id) @@ -325,8 +343,9 @@ func TestRemoveRouteReturnsNotFound(t *testing.T) { } handler.ServeHTTP(resp, req) - if resp.Code != http.StatusNotFound { - t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusNotFound, resp.Code) + + for _, e := range checkErrorResponse(resp.Result(), http.StatusNotFound, "Route Not Found") { + t.Error(e.Error()) } } @@ -345,6 +364,7 @@ func TestRemoveRouteReturnsNoContent(t *testing.T) { } handler.ServeHTTP(resp, req) + if resp.Code != http.StatusNoContent { t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusNoContent, resp.Code) } @@ -415,13 +435,12 @@ func TestGetRouteReturns404sWhenRouteDoesntExist(t *testing.T) { handler.ServeHTTP(w, r) - resp := w.Result() - if resp.StatusCode != http.StatusNotFound { - t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusNotFound, resp.StatusCode) + for _, e := range checkErrorResponse(w.Result(), http.StatusNotFound, "Route Not Found") { + t.Error(e.Error()) } } -func TestGetRouteSetsCorrctContentType(t *testing.T) { +func TestGetRouteSetsCorrectContentType(t *testing.T) { handler := mux.NewRouter() handler.HandleFunc("/routes/{id}", getRoute). Methods("GET") diff --git a/internal/server/srverrors/error.go b/internal/server/srverrors/error.go new file mode 100644 index 0000000..cc40f54 --- /dev/null +++ b/internal/server/srverrors/error.go @@ -0,0 +1,19 @@ +package srverrors + +import ( + "encoding/json" + "net/http" +) + +type ServerErrMessage struct { + Reason string +} + +func WriteErrorResponse(statusCode int, reasonMsg string, res http.ResponseWriter) { + respBody := ServerErrMessage{} + respBody.Reason = reasonMsg + bb, _ := json.Marshal(respBody) + res.Header().Add("Content-Type", "application/json; charset=utf-8") + res.WriteHeader(statusCode) + _, _ = res.Write(bb) +} diff --git a/internal/server/srverrors/error_test.go b/internal/server/srverrors/error_test.go new file mode 100644 index 0000000..adda7b5 --- /dev/null +++ b/internal/server/srverrors/error_test.go @@ -0,0 +1,47 @@ +package srverrors_test + +import ( + "encoding/json" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "github.com/BBVA/kapow/internal/server/srverrors" +) + +func TestWriteErrorResponseSetsAppJsonContentType(t *testing.T) { + w := httptest.NewRecorder() + + srverrors.WriteErrorResponse(0, "Not Important Here", w) + + 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) { + w := httptest.NewRecorder() + + srverrors.WriteErrorResponse(http.StatusGone, "Not Important Here", w) + + 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) { + expectedReason := "Something Not Found" + w := httptest.NewRecorder() + + srverrors.WriteErrorResponse(http.StatusNotFound, expectedReason, w) + + errMsg := srverrors.ServerErrMessage{} + if bodyBytes, err := ioutil.ReadAll(w.Result().Body); err != nil { + t.Errorf("Unexpected error reading response body: %v", err) + } else if err := json.Unmarshal(bodyBytes, &errMsg); err != nil { + t.Errorf("Response body contains invalid JSON entity: %v", err) + } else if errMsg.Reason != expectedReason { + t.Errorf("Unexpected reason in response. Expected: %q, got: %q", expectedReason, errMsg.Reason) + } +}