Support for HTTP errors payload (fixes #264) (#265)

* Move errors to http package & update tests

* Introduce http Error with code & payload data

* Return encoded error response payload

Since the request has Accept/Content-Type headers we should use the
encoder on the response payload, just like handleSuccess does.

* Refactor http errors file

This commit introduces more functions for creating custom errors,
which use default payload content (provided by Golang's http package).
All existing functions have been renamed to New**ErrorWithPayload
indicating that the caller can provide a specific payload for the error.
Also a general purpose NewError method has been added, allowing the caller
to create Internal Server and other errors.

Signed-off-by: Fotis Papadopoulos <[email protected]>

sync/http/errors.go

@@ -0,0 +1,80 @@
+package http
+
+import (
+	"fmt"
+	"net/http"
+)
+
+// Error defines an abstract struct that can represent several types of HTTP errors.
+type Error struct {
+	code    int
+	payload interface{}
+}
+
+// Error returns the actual message of the error.
+func (e *Error) Error() string {
+	if e.payload == nil {
+		return fmt.Sprintf("HTTP error with code: %d", e.code)
+	}
+	return fmt.Sprintf("HTTP error with code: %d payload: %v", e.code, e.payload)
+}
+
+// NewValidationError creates a new validation error with default payload.
+func NewValidationError() *Error {
+	return &Error{http.StatusBadRequest, http.StatusText(http.StatusBadRequest)}
+}
+
+// NewValidationErrorWithPayload creates a new validation error with the specified payload.
+func NewValidationErrorWithPayload(payload interface{}) *Error {
+	return &Error{http.StatusBadRequest, payload}
+}
+
+// NewUnauthorizedError creates a new validation error with default payload.
+func NewUnauthorizedError() *Error {
+	return &Error{http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized)}
+}
+
+// NewUnauthorizedErrorWithPayload creates a new unauthorized error with the specified payload.
+func NewUnauthorizedErrorWithPayload(payload interface{}) *Error {
+	return &Error{http.StatusUnauthorized, payload}
+}
+
+// NewForbiddenError creates a new forbidden error with default payload.
+func NewForbiddenError() *Error {
+	return &Error{http.StatusForbidden, http.StatusText(http.StatusForbidden)}
+}
+
+// NewForbiddenErrorWithPayload creates a new forbidden error with the specified payload.
+func NewForbiddenErrorWithPayload(payload interface{}) *Error {
+	return &Error{http.StatusForbidden, payload}
+}
+
+// NewNotFoundError creates a new not found error with default payload.
+func NewNotFoundError() *Error {
+	return &Error{http.StatusNotFound, http.StatusText(http.StatusNotFound)}
+}
+
+// NewNotFoundErrorWithPayload creates a new not found error with the specified payload.
+func NewNotFoundErrorWithPayload(payload interface{}) *Error {
+	return &Error{http.StatusNotFound, payload}
+}
+
+// NewServiceUnavailableError creates a new service unavailable error with default payload.
+func NewServiceUnavailableError() *Error {
+	return &Error{http.StatusServiceUnavailable, http.StatusText(http.StatusServiceUnavailable)}
+}
+
+// NewServiceUnavailableErrorWithPayload creates a new service unavailable error with the specified payload.
+func NewServiceUnavailableErrorWithPayload(payload interface{}) *Error {
+	return &Error{http.StatusServiceUnavailable, payload}
+}
+
+// NewError creates a new error with default Internal Server Error payload.
+func NewError() *Error {
+	return &Error{http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)}
+}
+
+// NewErrorWithCodeAndPayload creates a fully customizable error with the specified status code and payload.
+func NewErrorWithCodeAndPayload(code int, payload interface{}) *Error {
+	return &Error{code, payload}
+}

sync/http/errors_test.go

@@ -0,0 +1,84 @@
+package http
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidationError(t *testing.T) {
+	err := NewValidationError()
+	assert.EqualError(t, err, "HTTP error with code: 400 payload: Bad Request")
+	assert.Equal(t, 400, err.code)
+}
+
+func TestValidationErrorWithPayload(t *testing.T) {
+	err := NewValidationErrorWithPayload("test")
+	assert.EqualError(t, err, "HTTP error with code: 400 payload: test")
+	assert.Equal(t, 400, err.code)
+}
+func TestUnauthorizedError(t *testing.T) {
+	err := NewUnauthorizedError()
+	assert.EqualError(t, err, "HTTP error with code: 401 payload: Unauthorized")
+	assert.Equal(t, 401, err.code)
+}
+
+func TestUnauthorizedErrorWithPayload(t *testing.T) {
+	err := NewUnauthorizedErrorWithPayload("test")
+	assert.EqualError(t, err, "HTTP error with code: 401 payload: test")
+	assert.Equal(t, 401, err.code)
+}
+
+func TestForbiddenError(t *testing.T) {
+	err := NewForbiddenError()
+	assert.EqualError(t, err, "HTTP error with code: 403 payload: Forbidden")
+	assert.Equal(t, 403, err.code)
+}
+
+func TestForbiddenErrorWithPayload(t *testing.T) {
+	err := NewForbiddenErrorWithPayload("test")
+	assert.EqualError(t, err, "HTTP error with code: 403 payload: test")
+	assert.Equal(t, 403, err.code)
+}
+
+func TestNotFoundError(t *testing.T) {
+	err := NewNotFoundError()
+	assert.EqualError(t, err, "HTTP error with code: 404 payload: Not Found")
+	assert.Equal(t, 404, err.code)
+}
+
+func TestNotFoundErrorWithPayload(t *testing.T) {
+	err := NewNotFoundErrorWithPayload("test")
+	assert.EqualError(t, err, "HTTP error with code: 404 payload: test")
+	assert.Equal(t, 404, err.code)
+}
+
+func TestServiceUnavailableError(t *testing.T) {
+	err := NewServiceUnavailableError()
+	assert.EqualError(t, err, "HTTP error with code: 503 payload: Service Unavailable")
+	assert.Equal(t, 503, err.code)
+}
+
+func TestServiceUnavailableErrorWithPayload(t *testing.T) {
+	err := NewServiceUnavailableErrorWithPayload("test")
+	assert.EqualError(t, err, "HTTP error with code: 503 payload: test")
+	assert.Equal(t, 503, err.code)
+}
+
+func TestNewError(t *testing.T) {
+	err := NewError()
+	assert.EqualError(t, err, "HTTP error with code: 500 payload: Internal Server Error")
+	assert.Equal(t, 500, err.code)
+}
+
+func TestNewErrorWithCodeAndPayload(t *testing.T) {
+	err := NewErrorWithCodeAndPayload(409, "Conflict")
+	assert.EqualError(t, err, "HTTP error with code: 409 payload: Conflict")
+	assert.Equal(t, 409, err.code)
+}
+
+func TestNewErrorWithCodeAndNoPayload(t *testing.T) {
+	err := NewErrorWithCodeAndPayload(409, nil)
+	assert.EqualError(t, err, "HTTP error with code: 409")
+	assert.Equal(t, 409, err.code)
+}

sync/http/handler.go

@@ -33,7 +33,7 @@ func handler(hnd sync.ProcessorFunc) http.HandlerFunc {
 		req := sync.NewRequest(f, r.Body, dec)
 		rsp, err := hnd(ctx, req)
 		if err != nil {
-			handleError(w, err)
+			handleError(w, enc, err)
 			return
 		}
 
@@ -122,21 +122,20 @@ func handleSuccess(w http.ResponseWriter, r *http.Request, rsp *sync.Response, e
 	return err
 }
 
-func handleError(w http.ResponseWriter, err error) {
-	switch err.(type) {
-	case *sync.ValidationError:
-		http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
-	case *sync.UnauthorizedError:
-		http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
-	case *sync.ForbiddenError:
-		http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
-	case *sync.NotFoundError:
-		http.Error(w, "", http.StatusNotFound)
-	case *sync.ServiceUnavailableError:
-		http.Error(w, http.StatusText(http.StatusServiceUnavailable), http.StatusServiceUnavailable)
-	default:
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+func handleError(w http.ResponseWriter, enc encoding.EncodeFunc, err error) {
+	// Assert error to type Error in order to leverage the code and payload values that such errors contain.
+	if err, ok := err.(*Error); ok {
+		p, encErr := enc(err.payload)
+		if encErr != nil {
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			return
+		}
+		w.WriteHeader(err.code)
+		w.Write(p)
+		return
 	}
+	// Using http.Error helper hijacks the content type header of the response returning plain text payload.
+	http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 }
 
 func prepareResponse(w http.ResponseWriter, ct string) {

sync/http/handler_test.go

@@ -129,23 +129,26 @@ func Test_handleSuccess(t *testing.T) {
 func Test_handleError(t *testing.T) {
 	type args struct {
 		err error
+		enc encoding.EncodeFunc
 	}
 	tests := []struct {
 		name         string
 		args         args
 		expectedCode int
 	}{
-		{"bad request", args{err: &sync.ValidationError{}}, http.StatusBadRequest},
-		{"unauthorized request", args{err: &sync.UnauthorizedError{}}, http.StatusUnauthorized},
-		{"forbidden request", args{err: &sync.ForbiddenError{}}, http.StatusForbidden},
-		{"not found error", args{err: &sync.NotFoundError{}}, http.StatusNotFound},
-		{"service unavailable error", args{err: &sync.ServiceUnavailableError{}}, http.StatusServiceUnavailable},
-		{"default error", args{err: errors.New("Test")}, http.StatusInternalServerError},
+		{"bad request", args{err: NewValidationError(), enc: json.Encode}, http.StatusBadRequest},
+		{"unauthorized request", args{err: NewUnauthorizedError(), enc: json.Encode}, http.StatusUnauthorized},
+		{"forbidden request", args{err: NewForbiddenError(), enc: json.Encode}, http.StatusForbidden},
+		{"not found error", args{err: NewNotFoundError(), enc: json.Encode}, http.StatusNotFound},
+		{"service unavailable error", args{err: NewServiceUnavailableError(), enc: json.Encode}, http.StatusServiceUnavailable},
+		{"internal server error", args{err: NewError(), enc: json.Encode}, http.StatusInternalServerError},
+		{"default error", args{err: errors.New("Test"), enc: json.Encode}, http.StatusInternalServerError},
+		{"payload encoding error", args{err: NewErrorWithCodeAndPayload(http.StatusBadRequest, make(chan int)), enc: json.Encode}, http.StatusInternalServerError},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			rsp := httptest.NewRecorder()
-			handleError(rsp, tt.args.err)
+			handleError(rsp, tt.args.enc, tt.args.err)
 			assert.Equal(t, tt.expectedCode, rsp.Code)
 		})
 	}