feat: allow customizing http handler error parsing (#270)

Expose the default Handler implementation and allow customizing the error parsing by passing a new parser function. This allows overriding the error handling but with a reasonable default.

Additionally, change ValidationErrorsToFiedErrorResponse signature to accept any map[string]error. This is the underlying type for a validation.Error, so it is backwards compatible, but it is also more flexible and will accept more type aliases for validation errors, making it easier to use.

Signed-off-by: Lucas Roesler <[email protected]>

Author: LucasRoesler

pkg/errors/errors.go

@@ -44,7 +44,7 @@ type ValidationErrors = validation.Errors
 
 // ValidationErrorsToFieldErrorResponse converts validation errors to the format that is
 // served by HTTP handlers
-func ValidationErrorsToFieldErrorResponse(errs ValidationErrors) (resp ErrorResponse) {
+func ValidationErrorsToFieldErrorResponse(errs map[string]error) (resp ErrorResponse) {
 	resp.Errors = make([]APIErrorMessenger, 0, len(errs))
 	for key, fieldErr := range errs {
 		if fieldErr == nil {

pkg/http/handlers/base.go

@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"io"
 	"net/http"
-	"strings"
 
 	cerrors "github.com/contiamo/go-base/v4/pkg/errors"
 	"github.com/contiamo/go-base/v4/pkg/tracing"
@@ -20,6 +19,9 @@ const (
 	Megabyte = 1024 * 1024
 )
 
+// ErrorParser is a function that parses an error into an HTTP status code and response body.
+type ErrorParser = func(ctx context.Context, err error, debug bool) (int, interface{})
+
 // BaseHandler contains all the base functions every handler should have
 type BaseHandler interface {
 	tracing.Tracer
@@ -34,48 +36,105 @@ type BaseHandler interface {
 // NewBaseHandler creates a new base HTTP handler that
 // contains shared logic among all the handlers.
 // The handler supports parsing and writing JSON objects
-// `maxBodyBytes` is the maximal request body size, < 0 means the default Megabyte.
+// `maxBodyBytes` is the maximal request body size, < 0 means the default Megabyte. Using 0 will disable the limit.
 // `componentName` is used for tracing to identify to which
 // component this handler belongs to.
-func NewBaseHandler(componentName string, maxBodyBytes int64, debug bool) BaseHandler {
+func NewBaseHandler(componentName string, maxBodyBytes int64, debug bool) *Handler {
 	if maxBodyBytes < 0 {
 		maxBodyBytes = Megabyte
 	}
-	return &baseHandler{
+	return &Handler{
 		Tracer:       tracing.NewTracer("handlers", componentName),
-		maxBodyBytes: maxBodyBytes,
-		debug:        debug,
+		MaxBodyBytes: maxBodyBytes,
+		Debug:        debug,
 	}
 }
 
 // NewBasehandlerWithTracer create a new base HTTP handler, like NewBaseHandler, but allows
 // the caller to configure the Tracer implementation independently.
-func NewBaseHandlerWithTracer(tracer tracing.Tracer, maxBodyBytes int64, debug bool) BaseHandler {
+//
+// Deprecated: you can now configure/override the default Tracer using
+//
+//    h := NewBaseHandler(componentName, maxBodyBytes, debug)
+//    h.Tracer = tracing.NewTracer("handlers", componentName)
+func NewBaseHandlerWithTracer(tracer tracing.Tracer, maxBodyBytes int64, debug bool) *Handler {
 	if maxBodyBytes < 0 {
 		maxBodyBytes = Megabyte
 	}
-	return &baseHandler{
+	return &Handler{
 		Tracer:       tracer,
-		maxBodyBytes: maxBodyBytes,
-		debug:        debug,
+		MaxBodyBytes: maxBodyBytes,
+		ErrorParser:  DefaultErrorParser,
+		Debug:        debug,
 	}
 }
 
-type baseHandler struct {
+// Handler is the default implementation of BaseHandler and is suitable for use in
+// most REST API implementations.
+type Handler struct {
 	tracing.Tracer
-	maxBodyBytes int64
-	debug        bool
+	// ErrorParser is used to parse error objects into HTTP status codes and response bodies.
+	ErrorParser ErrorParser
+	// MaxBodyBytes is the maximal request body size, < 0 means the default Megabyte.
+	// Using 0 will disable the limit and allow parsing streams.
+	MaxBodyBytes int64
+	// Debug was used to enable/disable Debug mode, when enabled error messages will be included in responses.
+	Debug bool
 }
 
-func (h *baseHandler) Error(ctx context.Context, w http.ResponseWriter, err error) {
+func (h *Handler) Error(ctx context.Context, w http.ResponseWriter, err error) {
 	span, _ := h.StartSpan(ctx, "Error")
 	defer h.FinishSpan(span, nil)
 
-	if err == nil {
-		w.WriteHeader(http.StatusOK)
+	parser := h.ErrorParser
+	if parser == nil {
+		parser = DefaultErrorParser
+	}
+
+	status, resp := parser(ctx, err, h.Debug)
+	h.Write(ctx, w, status, resp)
+}
+
+func (h *Handler) Write(ctx context.Context, w http.ResponseWriter, status int, obj interface{}) {
+	span, _ := h.StartSpan(ctx, "Write")
+	var err error
+	defer func() {
+		h.FinishSpan(span, err)
+	}()
+
+	if obj == nil {
+		w.WriteHeader(status)
 		return
 	}
 
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(status)
+	enc := json.NewEncoder(w)
+	err = enc.Encode(obj)
+}
+
+func (h *Handler) Parse(r *http.Request, out interface{}) error {
+	var body io.Reader = r.Body
+	if h.MaxBodyBytes > 0 {
+		body = io.LimitReader(r.Body, h.MaxBodyBytes)
+	}
+
+	dec := json.NewDecoder(body)
+	err := dec.Decode(out)
+	if err != nil {
+		return &parseError{cause: err}
+	}
+	return nil
+}
+
+// DefaultErrorParser provides a reasonable default error parser that can handle
+// the various sentile errors in go-base as well as ozzo-validation errors.
+func DefaultErrorParser(ctx context.Context, err error, debug bool) (int, interface{}) {
+	// hey what are you doing here?
+	if err == nil {
+		return http.StatusOK, nil
+	}
+
 	genErrResp := cerrors.ErrorResponse{
 		Errors: []cerrors.APIErrorMessenger{
 			&cerrors.GeneralError{
@@ -84,93 +143,68 @@ func (h *baseHandler) Error(ctx context.Context, w http.ResponseWriter, err erro
 			}},
 	}
 
-	// Handler concrete errors:
-	// we can extend this error list in the future if needed
+	// Handle sentinel errors
 	switch err {
-	case cerrors.ErrNotImplemented:
-		h.Write(ctx, w, http.StatusNotImplemented, genErrResp)
-		return
-	case cerrors.ErrAuthorization:
-		h.Write(ctx, w, http.StatusUnauthorized, genErrResp)
-		return
 	case cerrors.ErrPermission:
-		h.Write(ctx, w, http.StatusForbidden, genErrResp)
-		return
-	case cerrors.ErrForm:
-		h.Write(ctx, w, http.StatusUnprocessableEntity, genErrResp)
-		return
-	case sql.ErrNoRows, cerrors.ErrNotFound:
-		h.Write(ctx, w, http.StatusNotFound, genErrResp)
-		return
+		return http.StatusForbidden, genErrResp
+	case cerrors.ErrAuthorization:
+		return http.StatusUnauthorized, genErrResp
+	case cerrors.ErrInternal:
+		return http.StatusInternalServerError, genErrResp
 	case cerrors.ErrInvalidParameters:
-		h.Write(ctx, w, http.StatusBadRequest, genErrResp)
-		return
-	case cerrors.ErrUnsupportedMediaType:
-		h.Write(ctx, w, http.StatusUnsupportedMediaType, genErrResp)
-		return
+		return http.StatusBadRequest, genErrResp
+	case cerrors.ErrUnmarshalling, cerrors.ErrForm:
+		return http.StatusUnprocessableEntity, genErrResp
+	case sql.ErrNoRows, cerrors.ErrNotFound:
+		return http.StatusNotFound, genErrResp
 	case cerrors.ErrNotImplemented:
-		h.Write(ctx, w, http.StatusNotImplemented, genErrResp)
-		return
+		return http.StatusNotImplemented, genErrResp
+	case cerrors.ErrUnsupportedMediaType:
+		return http.StatusUnsupportedMediaType, genErrResp
 	}
 
-	if strings.HasPrefix(err.Error(), cerrors.ErrUnmarshalling.Error()) {
-		h.Write(ctx, w, http.StatusUnprocessableEntity, genErrResp)
-		return
+	// Handle error types that wrap other errors
+	var parseErr parseError
+	if errors.As(err, &parseErr) {
+		return http.StatusUnprocessableEntity, genErrResp
+	}
+
+	var userErr cerrors.UserError
+	if errors.As(err, &userErr) {
+		return http.StatusBadRequest, genErrResp
 	}
 
-	// Handle error types that wrap other errors
 	switch e := err.(type) {
-	// covers ozzo v1 errors and go-base ValidationErrors
 	case cerrors.ValidationErrors:
-		h.Write(
-			ctx,
-			w,
-			http.StatusUnprocessableEntity,
-			cerrors.ValidationErrorsToFieldErrorResponse(e),
-		)
-		return
+		return http.StatusUnprocessableEntity, cerrors.ValidationErrorsToFieldErrorResponse(e)
 	case validation.Errors:
-		h.Write(
-			ctx,
-			w,
-			http.StatusUnprocessableEntity,
-			cerrors.ValidationErrorsToFieldErrorResponse(cerrors.ValidationErrors(e)),
-		)
-		return
-	case cerrors.UserError:
-		h.Write(ctx, w, http.StatusBadRequest, genErrResp)
-		return
+		return http.StatusUnprocessableEntity, cerrors.ValidationErrorsToFieldErrorResponse(e)
 	default:
-		if !h.debug {
+		if !debug {
 			for idx, e := range genErrResp.Errors {
 				genErrResp.Errors[idx] = e.Scrubbed(cerrors.ErrInternal.Error())
 			}
 		}
-		h.Write(ctx, w, http.StatusInternalServerError, genErrResp)
+		return http.StatusInternalServerError, genErrResp
 	}
 }
 
-func (h *baseHandler) Write(ctx context.Context, w http.ResponseWriter, status int, obj interface{}) {
-	span, _ := h.StartSpan(ctx, "Write")
-	var err error
-	defer func() {
-		h.FinishSpan(span, err)
-	}()
+type parseError struct {
+	cause error
+}
 
-	w.Header().Set("Content-Type", "application/json")
-	w.WriteHeader(status)
-	if obj != nil {
-		enc := json.NewEncoder(w)
-		err = enc.Encode(obj)
-	}
+func (e parseError) Error() string {
+	return cerrors.ErrUnmarshalling.Error() + ": " + e.cause.Error()
 }
 
-func (h *baseHandler) Parse(r *http.Request, out interface{}) error {
-	limitedBody := io.LimitReader(r.Body, h.maxBodyBytes)
-	dec := json.NewDecoder(limitedBody)
-	err := dec.Decode(out)
-	if err != nil {
-		return errors.Wrap(err, cerrors.ErrUnmarshalling.Error())
-	}
-	return nil
+func (e parseError) Unwrap() error {
+	return e.cause
+}
+
+func (e parseError) As(target interface{}) bool {
+	return errors.As(e.cause, &target)
+}
+
+func (e parseError) Is(target error) bool {
+	return errors.Is(e.cause, target)
 }

pkg/http/handlers/base_test.go

@@ -70,6 +70,15 @@ func TestWrite(t *testing.T) {
 		require.Equal(t, "application/json", resp.Header().Get("content-type"))
 		require.Equal(t, "{\"Name\":\"some\"}\n", resp.Body.String())
 	})
+
+	t.Run("Write handle nil objects", func(t *testing.T) {
+		h := NewBaseHandler("test", Megabyte, false)
+		resp := httptest.NewRecorder()
+		h.Write(context.Background(), resp, http.StatusCreated, nil)
+		require.Equal(t, http.StatusCreated, resp.Code)
+		require.Equal(t, "", resp.Header().Get("content-type"))
+		require.Equal(t, "", resp.Body.String())
+	})
 }
 
 func TestError(t *testing.T) {
@@ -99,11 +108,8 @@ func TestError(t *testing.T) {
 			expBody:   `{"errors":[{"type":"GeneralError","message":"You don't have required permission to perform this action"}]}`,
 		},
 		{
-			name: "Returns 422 when ErrUnmarshalling",
-			err: errors.Wrap(
-				errors.New("unexpected end of file"),
-				cerrors.ErrUnmarshalling.Error(),
-			),
+			name:      "Returns 422 when ErrUnmarshalling",
+			err:       &parseError{cause: errors.New("unexpected end of file")},
 			expStatus: http.StatusUnprocessableEntity,
 			expBody:   `{"errors":[{"type":"GeneralError","message":"Failed to read JSON from the request body: unexpected end of file"}]}`,
 		},
@@ -133,13 +139,13 @@ func TestError(t *testing.T) {
 		},
 		{
 			name:      "Returns 422 and field errors when validation errors",
-			err:       validation.Errors{"field": errors.New("terrible")},
+			err:       validation.Errors{"field": errors.New("terrible")}.Filter(),
 			expStatus: http.StatusUnprocessableEntity,
 			expBody:   `{"errors":[{"type":"FieldError","message":"terrible","key":"field"}]}`,
 		},
 		{
-			name:      "Returns 422 and field errors when validation errors",
-			err:       validationV1.Errors{"field": errors.New("terrible")},
+			name:      "Returns 422 and field errors when validation v1 errors",
+			err:       validationV1.Errors{"field": errors.New("terrible")}.Filter(),
 			expStatus: http.StatusUnprocessableEntity,
 			expBody:   `{"errors":[{"type":"FieldError","message":"terrible","key":"field"}]}`,
 		},