Improve error handling

controller/stories/stories.go

@@ -40,8 +40,9 @@ func HandleRead(w http.ResponseWriter, r *http.Request) error {
 	storyIDStr := chi.URLParam(r, "storyID")
 	storyID, err := strconv.Atoi(storyIDStr)
 	if err != nil {
-		http.Error(w, "Invalid storyID", http.StatusBadRequest)
-		return err
+		return apierrors.ClientBadRequestError{
+			Message: fmt.Sprintf("Invalid storyID: %v", err),
+		}
 	}
 
 	// Get DB instance
@@ -66,7 +67,9 @@ func HandleCreate(w http.ResponseWriter, r *http.Request) error {
 		e, ok := err.(*json.UnmarshalTypeError)
 		if !ok {
 			logrus.Error(err)
-			return err
+			return apierrors.ClientBadRequestError{
+				Message: fmt.Sprintf("Bad JSON parsing: %v", err),
+			}
 		}
 
 		// TODO: Investigate if we should use errors.Wrap instead

controller/users/users.go

@@ -40,8 +40,9 @@ func HandleRead(w http.ResponseWriter, r *http.Request) error {
 	userIDStr := chi.URLParam(r, "userID")
 	userID, err := strconv.Atoi(userIDStr)
 	if err != nil {
-		http.Error(w, "Invalid userID", http.StatusBadRequest)
-		return err
+		return apierrors.ClientBadRequestError{
+			Message: fmt.Sprintf("Invalid userID: %v", err),
+		}
 	}
 
 	// Get DB instance
@@ -66,18 +67,23 @@ func HandleCreate(w http.ResponseWriter, r *http.Request) error {
 		e, ok := err.(*json.UnmarshalTypeError)
 		if !ok {
 			logrus.Error(err)
-			return err
+			return apierrors.ClientBadRequestError{
+				Message: fmt.Sprintf("Bad JSON parsing: %v", err),
+			}
 		}
 
 		// TODO: Investigate if we should use errors.Wrap instead
-		return apierrors.ClientBadRequestError{
+		return apierrors.ClientUnprocessableEntityError{
 			Message: fmt.Sprintf("Invalid JSON format: %s should be a %s.", e.Field, e.Type),
 		}
 	}
 
 	err := params.Validate()
 	if err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
+		logrus.Error(err)
+		return apierrors.ClientUnprocessableEntityError{
+			Message: fmt.Sprintf("JSON validation failed: %v", err),
+		}
 	}
 
 	userModel := *params.ToModel()

internal/auth/middleware.go

@@ -7,6 +7,7 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwa"
 	"github.com/lestrrat-go/jwx/v2/jwt"
 	"github.com/source-academy/stories-backend/internal/config"
+	apierrors "github.com/source-academy/stories-backend/internal/errors"
 	envutils "github.com/source-academy/stories-backend/internal/utils/env"
 )
 
@@ -22,6 +23,7 @@ func MakeMiddlewareFrom(conf *config.Config) func(http.Handler) http.Handler {
 	key, ok := keySet.Key(0)
 	if !ok {
 		// Block all access if JWKS source is down, since we can't verify JWTs
+		// TODO: Investigate if 500 is appropriate
 		return func(next http.Handler) http.Handler {
 			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
@@ -34,15 +36,19 @@ func MakeMiddlewareFrom(conf *config.Config) func(http.Handler) http.Handler {
 			// Get JWT from request
 			authHeader := r.Header.Get("Authorization")
 			if authHeader == "" {
-				http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+				apierrors.ServeHTTP(w, r, apierrors.ClientUnauthorizedError{
+					Message: "Missing Authorization header",
+				})
 				return
 			}
 
 			// Verify JWT
 			toParse := authHeader[len("Bearer "):]
 			token, err := jwt.ParseString(toParse, jwt.WithKey(jwa.RS256, key))
 			if err != nil {
-				fmt.Printf("Failed to verify JWS: %s\n", err)
+				apierrors.ServeHTTP(w, r, apierrors.ClientForbiddenError{
+					Message: fmt.Sprintf("Failed to verify JWT: %s\n", err),
+				})
 				return
 			}
 

internal/errors/401.go

@@ -0,0 +1,17 @@
+package apierrors
+
+import (
+	"net/http"
+)
+
+type ClientUnauthorizedError struct {
+	Message string
+}
+
+func (e ClientUnauthorizedError) Error() string {
+	return e.Message
+}
+
+func (e ClientUnauthorizedError) HTTPStatusCode() int {
+	return http.StatusUnauthorized
+}

internal/errors/403.go

@@ -0,0 +1,17 @@
+package apierrors
+
+import (
+	"net/http"
+)
+
+type ClientForbiddenError struct {
+	Message string
+}
+
+func (e ClientForbiddenError) Error() string {
+	return e.Message
+}
+
+func (e ClientForbiddenError) HTTPStatusCode() int {
+	return http.StatusForbidden
+}

internal/errors/errors.go

@@ -1,8 +1,25 @@
 package apierrors
 
+import (
+	"errors"
+	"net/http"
+)
+
 // ClientError is an interface for errors that should be returned to the client.
 // They generally start with a 4xx HTTP status code.
 type ClientError interface {
 	error
 	HTTPStatusCode() int
 }
+
+func ServeHTTP(w http.ResponseWriter, r *http.Request, err error) {
+	var clientError ClientError
+	if errors.As(err, &clientError) {
+		// Client error (status 4xx), write error message and status code
+		http.Error(w, clientError.Error(), clientError.HTTPStatusCode())
+		return
+	}
+
+	// 500 Internal Server Error as a catch-all
+	http.Error(w, err.Error(), http.StatusInternalServerError)
+}

internal/router/errors.go

@@ -1,7 +1,6 @@
 package router
 
 import (
-	"errors"
 	"net/http"
 
 	apierrors "github.com/source-academy/stories-backend/internal/errors"
@@ -15,14 +14,7 @@ func handleAPIError(handler func(w http.ResponseWriter, r *http.Request) error)
 			return
 		}
 
-		var clientError apierrors.ClientError
-		if errors.As(err, &clientError) {
-			// Client error (status 4xx), write error message and status code
-			http.Error(w, clientError.Error(), clientError.HTTPStatusCode())
-			return
-		}
-
-		// 500 Internal Server Error as a catch-all
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		// Error, write error message and status code
+		apierrors.ServeHTTP(w, r, err)
 	}
 }