New srverrors package added. New error handling added to control package

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)

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)
+	addRoute(resp, req)
-	if resp.Code != http.StatusBadRequest {
+
-		t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusBadRequest, resp.Code)
+	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 {
+			for _, e := range checkErrorResponse(r, http.StatusUnprocessableEntity, "Invalid Route") {
-				t.Errorf("HTTP status mismatch in case %s. Expected: %d, got: %d", test.testCase, http.StatusUnprocessableEntity, resp.Code)
+				t.Error(e.Error())
 			}
 		} else if !test.testMustFail {
-			if resp.Code != http.StatusCreated {
+			if r.StatusCode != http.StatusCreated {
-				t.Errorf("HTTP status mismatch in case %s. Expected: %d, got: %d", test.testCase, http.StatusUnprocessableEntity, resp.Code)
+				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" {
+			if ct := r.Header.Get("Content-Type"); ct != "application/json" {
-				t.Errorf("Incorrect content type in response. Expected: application/json, got: %s", ct)
+				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(
+		return uuid, errors.New("End of Time reached; Try again before, or in the next Big Bang cycle")
-			"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 {
+	for _, e := range checkErrorResponse(resp.Result(), http.StatusInternalServerError, "Internal Server Error") {
-		t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusInternalServerError, resp.Result().StatusCode)
+		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 {
+	for _, e := range checkErrorResponse(resp.Result(), http.StatusUnprocessableEntity, "Invalid Route") {
-		t.Error("Invalid route registered")
+		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()
+	for _, e := range checkErrorResponse(w.Result(), http.StatusNotFound, "Route Not Found") {
-	if resp.StatusCode != http.StatusNotFound {
+		t.Error(e.Error())
-		t.Errorf("HTTP status mismatch. Expected: %d, got: %d", http.StatusNotFound, resp.StatusCode)
 	}
 }
 
-func TestGetRouteSetsCorrctContentType(t *testing.T) {
+func TestGetRouteSetsCorrectContentType(t *testing.T) {
 	handler := mux.NewRouter()
 	handler.HandleFunc("/routes/{id}", getRoute).
 		Methods("GET")

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)
+}

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)
+	}
+}