Handle API errors better

server/internal/api/api.go

@@ -3,15 +3,32 @@ package api
 import (
 	"github.com/gin-gonic/gin"
 	"github.com/shroff/phylum/server/internal/api/auth"
+	"github.com/shroff/phylum/server/internal/api/errors"
+	"github.com/shroff/phylum/server/internal/api/routes"
 	"github.com/shroff/phylum/server/internal/app"
 )
 
 func Setup(r *gin.RouterGroup, a *app.App) {
+	r.Use(handleApiError)
+
 	// Unauthenticated routes
-	setupAuthRoutes(r, a)
+	routes.SetupAuthRoutes(r, a)
 
 	// Authenticated routes
 	r.Use(auth.CreateBearerAuthHandler(a))
-	setupSiloRoutes(r, a)
-	setupResourceRoutes(r, a)
+	routes.SetupSiloRoutes(r, a)
+	routes.SetupResourceRoutes(r, a)
+}
+
+func handleApiError(c *gin.Context) {
+	defer func() {
+		if err := recover(); err != nil {
+			if e, ok := err.(errors.Err); ok {
+				c.AbortWithStatusJSON(e.Status, e)
+			} else {
+				panic(err)
+			}
+		}
+	}()
+	c.Next()
 }

server/internal/api/auth/auth_basic.go

@@ -14,24 +14,24 @@ func CreateBasicAuthHandler(app *app.App) func(c *gin.Context) {
 		username, pass, ok := c.Request.BasicAuth()
 		if !ok {
 			c.Header("WWW-Authenticate", "Basic realm=\"Phylum WebDAV\"")
-			c.AbortWithStatus(http.StatusUnauthorized)
+			c.Status(http.StatusUnauthorized)
 			return
 		}
 		user, err := app.FindUser(context.Background(), username)
 		if err != nil {
 			c.Header("WWW-Authenticate", "Basic realm=\"Phylum WebDAV\"")
-			c.AbortWithStatus(http.StatusUnauthorized)
+			c.Status(http.StatusUnauthorized)
 			return
 		}
 		ok, err = cryptutil.VerifyPassword(pass, user.PasswordHash)
 		if err != nil {
 			c.Header("WWW-Authenticate", "Basic realm=\"Phylum WebDAV\"")
-			c.AbortWithStatus(http.StatusUnauthorized)
+			c.Status(http.StatusUnauthorized)
 			return
 		}
 		if !ok {
 			c.Header("WWW-Authenticate", "Basic realm=\"Phylum WebDAV\"")
-			c.AbortWithStatus(http.StatusUnauthorized)
+			c.Status(http.StatusUnauthorized)
 			return
 		}
 		c.Set(keyUsername, user)

server/internal/api/auth/auth_bearer.go

@@ -1,51 +1,36 @@
 package auth
 
 import (
-	"errors"
 	"strings"
 
 	"github.com/gin-gonic/gin"
+	"github.com/shroff/phylum/server/internal/api/errors"
 	"github.com/shroff/phylum/server/internal/app"
-	"github.com/sirupsen/logrus"
 )
 
+const errCodeAuthRequred = "auth_required"
+const errCodeTokenInvalid = "token_invalid"
+
 func CreateBearerAuthHandler(a *app.App) func(c *gin.Context) {
 	return func(c *gin.Context) {
 		authHeader := c.GetHeader("Authorization")
 		if authHeader == "" {
-			c.AbortWithStatusJSON(401, gin.H{
-				"ERR_CODE": "auth_required",
-			})
-			return
+			panic(errors.Err{Status: 401, Code: errCodeAuthRequred})
 		}
 		authParts := strings.Split(authHeader, " ")
 		if len(authParts) != 2 {
-			c.AbortWithStatusJSON(401, gin.H{
-				"ERR_CODE": "auth_malformed",
-			})
-			return
+			panic(errors.Err{Status: 401, Code: errCodeAuthRequred})
 		}
 		if authParts[0] != "bearer" {
-			c.AbortWithStatusJSON(401, gin.H{
-				"ERR_CODE": "auth_scheme_not_recognized",
-			})
-			return
+			panic(errors.Err{Status: 401, Code: errCodeAuthRequred})
 		}
 
 		username, err := a.VerifyAccessToken(authParts[1])
 		if err != nil {
-			if errors.Is(err, app.ErrAccessTokenExpired) || errors.Is(err, app.ErrAccessTokenInvalid) {
-				c.AbortWithStatusJSON(401, gin.H{
-					"ERR_CODE": "access_token_invalid",
-				})
-			} else {
-				logrus.Warn(err.Error())
-				c.AbortWithStatusJSON(500, gin.H{
-					"ERR_CODE":    "unknown_error",
-					"ERR_DETAILS": err.Error(),
-				})
+			if errors.Is(err, app.ErrTokenExpired) || errors.Is(err, app.ErrTokenInvalid) {
+				panic(errors.Err{Status: 401, Code: errCodeTokenInvalid})
 			}
-			return
+			panic(err)
 		}
 		c.Set(keyUsername, username)
 	}

server/internal/api/errors/errors.go

@@ -0,0 +1,28 @@
+package errors
+
+import (
+	"errors"
+	"fmt"
+)
+
+type Err struct {
+	Status  int    `json:"status"`
+	Code    string `json:"code"`
+	Details string `json:"details"`
+}
+
+func New(httpStatus int, code, details string) error {
+	return &Err{
+		Status:  httpStatus,
+		Code:    code,
+		Details: details,
+	}
+}
+
+func (e *Err) Error() string {
+	return fmt.Sprintf("%d %s (%s)", e.Status, e.Code, e.Details)
+}
+
+func Is(err, target error) bool {
+	return errors.Is(err, target)
+}

server/internal/api/routes/auth.go

@@ -1,13 +1,14 @@
-package api
+package routes
 
 import (
-	"errors"
+	"net/http"
 
 	"github.com/gin-gonic/gin"
+	"github.com/shroff/phylum/server/internal/api/errors"
 	"github.com/shroff/phylum/server/internal/app"
 )
 
-func setupAuthRoutes(r *gin.RouterGroup, a *app.App) {
+func SetupAuthRoutes(r *gin.RouterGroup, a *app.App) {
 	group := r.Group("/auth")
 	group.POST("/login", createLoginRouteHandler(a))
 }
@@ -16,31 +17,18 @@ func createLoginRouteHandler(a *app.App) func(c *gin.Context) {
 	return func(c *gin.Context) {
 		username, ok := c.GetQuery("username")
 		if !ok {
-			c.AbortWithStatusJSON(401, gin.H{
-				"ERR_CODE":    "missing_params",
-				"ERR_DETAILS": "Missing username input",
-			})
-			return
+			panic(errors.New(http.StatusBadRequest, "missing_username", ""))
 		}
 		password, ok := c.GetQuery("password")
 		if !ok {
-			c.AbortWithStatusJSON(401, gin.H{
-				"ERR_CODE":    "missing_params",
-				"ERR_DETAILS": "Missing password input",
-			})
-			return
+			panic(errors.New(http.StatusBadRequest, "missing_password", ""))
 		}
 
 		if token, err := a.CreateAccessToken(username, password); err != nil {
 			if errors.Is(err, app.ErrCredentialsInvalid) {
-				c.AbortWithStatusJSON(401, gin.H{
-					"ERR_CODE": "invalid_username_or_password",
-				})
-			} else {
-				c.AbortWithStatusJSON(401, gin.H{
-					"ERR_DETAILS": err.Error(),
-				})
+				panic(errors.New(http.StatusUnauthorized, "credentials_invalid", ""))
 			}
+			panic(err)
 		} else {
 			c.JSON(200, gin.H{
 				"access_token": token.ID,

server/internal/api/routes/resources.go

@@ -1,19 +1,22 @@
-package api
+package routes
 
 import (
 	"context"
-	"errors"
 	"net/http"
 	"time"
 
 	"github.com/gin-gonic/gin"
 	"github.com/google/uuid"
 	"github.com/jackc/pgx/v5"
+	"github.com/shroff/phylum/server/internal/api/errors"
 	"github.com/shroff/phylum/server/internal/app"
 	"github.com/shroff/phylum/server/internal/sql"
-	"github.com/sirupsen/logrus"
 )
 
+const errCodeResourceNotFound = "resource_not_found"
+const errCodeResourceIdInvalid = "resource_id_invalid"
+const errCodeResourceNotCollection = "resource_not_collection"
+
 type resourceResponse struct {
 	ID       uuid.UUID  `json:"id"`
 	Parent   *uuid.UUID `json:"parent"`
@@ -26,35 +29,25 @@ type resourceResponse struct {
 	Etag     string     `json:"etag"`
 }
 
-func setupResourceRoutes(r *gin.RouterGroup, a *app.App) {
+func SetupResourceRoutes(r *gin.RouterGroup, a *app.App) {
 	group := r.Group("/resources")
 	group.GET("/:id/ls", createResourceLsRouteHandler(a))
 	group.GET("/:id/metadata", createResourceMetadataRouteHandler(a))
 }
+
 func createResourceMetadataRouteHandler(a *app.App) func(c *gin.Context) {
 	return func(c *gin.Context) {
 		resourceId, err := uuid.Parse(c.Param("id"))
 		if err != nil {
-			c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
-				"ERR_CODE": "resource_not_found",
-			})
-			return
+			panic(errors.New(http.StatusBadRequest, errCodeResourceIdInvalid, ""))
 		}
 
 		resource, err := a.Db.Queries().ResourceById(context.Background(), resourceId)
 		if err != nil {
 			if errors.Is(err, pgx.ErrNoRows) {
-				c.JSON(http.StatusNotFound, gin.H{
-					"ERR_CODE": "resource_not_found",
-				})
-			} else {
-				logrus.Warn(err)
-				c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{
-					"ERR_CODE":    "unknown_error",
-					"ERR_DETAILS": err.Error(),
-				})
+				panic(errors.New(http.StatusNotFound, errCodeResourceNotFound, ""))
 			}
-			return
+			panic(err)
 		}
 
 		deleted := &resource.Deleted.Time
@@ -80,40 +73,21 @@ func createResourceLsRouteHandler(a *app.App) func(c *gin.Context) {
 	return func(c *gin.Context) {
 		resourceId, err := uuid.Parse(c.Param("id"))
 		if err != nil {
-			c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
-				"ERR_CODE": "resource_not_found",
-			})
-			return
+			panic(errors.New(http.StatusBadRequest, errCodeResourceIdInvalid, ""))
 		}
 		resource, err := a.Db.Queries().ResourceById(context.Background(), resourceId)
 		if err != nil {
 			if errors.Is(err, pgx.ErrNoRows) {
-				c.JSON(http.StatusNotFound, gin.H{
-					"ERR_CODE": "resource_not_found",
-				})
-			} else {
-				logrus.Warn(err)
-				c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{
-					"ERR_CODE":    "unknown_error",
-					"ERR_DETAILS": err.Error(),
-				})
+				panic(errors.New(http.StatusNotFound, errCodeResourceNotFound, ""))
 			}
-			return
+			panic(err)
 		}
 		if !resource.Dir {
-			c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
-				"ERR_CODE": "resource_not_collection",
-			})
-			return
+			panic(errors.New(http.StatusBadRequest, errCodeResourceNotCollection, ""))
 		}
 		children, err := a.Db.Queries().ReadDir(context.Background(), sql.ReadDirParams{IncludeRoot: false, ID: resource.ID, Recursive: false})
 		if err != nil {
-			logrus.Warn(err)
-			c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{
-				"ERR_CODE":    "unknown_error",
-				"ERR_DETAILS": err.Error(),
-			})
-			return
+			panic(err)
 		}
 		results := make([]resourceResponse, len(children))
 		for i, c := range children {

server/internal/api/routes/silos.go

@@ -1,4 +1,4 @@
-package api
+package routes
 
 import (
 	"context"
@@ -15,7 +15,7 @@ type siloResponse struct {
 	Storage string    `json:"storage"`
 }
 
-func setupSiloRoutes(r *gin.RouterGroup, a *app.App) {
+func SetupSiloRoutes(r *gin.RouterGroup, a *app.App) {
 	group := r.Group("/silos")
 	group.GET("/list", createSiloListRouteHandler(a))
 }
@@ -24,10 +24,7 @@ func createSiloListRouteHandler(a *app.App) func(c *gin.Context) {
 	return func(c *gin.Context) {
 		silos, err := a.ListSilos(context.Background())
 		if err != nil {
-			c.AbortWithStatusJSON(500, gin.H{
-				"ERR_DETAILS": err.Error(),
-			})
-			return
+			panic(err)
 		}
 		results := make([]siloResponse, len(silos))
 		for i, s := range silos {

server/internal/app/auth.go

@@ -22,19 +22,19 @@ var accessTokenValiditiy = pgtype.Interval{
 }
 
 var ErrCredentialsInvalid = errors.New("credentials invalid")
-var ErrAccessTokenInvalid = errors.New("access token invalid")
-var ErrAccessTokenExpired = errors.New("access token expired")
+var ErrTokenInvalid = errors.New("token invalid")
+var ErrTokenExpired = errors.New("token expired")
 
 func (a App) CreateAccessToken(username, password string) (sql.AccessToken, error) {
 	if user, err := a.FindUser(context.Background(), username); err != nil {
-		if err == pgx.ErrNoRows {
+		if errors.Is(err, pgx.ErrNoRows) {
 			return sql.AccessToken{}, ErrCredentialsInvalid
 		}
 		logrus.Info(err)
 		return sql.AccessToken{}, err
 	} else {
 		if b, err := cryptutil.VerifyPassword(password, user.PasswordHash); err != nil {
-			logrus.Info(err.Error())
+			logrus.Warn(err.Error())
 			return sql.AccessToken{}, err
 		} else if !b {
 			return sql.AccessToken{}, ErrCredentialsInvalid
@@ -54,12 +54,12 @@ func (a App) CreateAccessToken(username, password string) (sql.AccessToken, erro
 func (a App) VerifyAccessToken(accessToken string) (string, error) {
 	token, err := a.Db.Queries().AccessTokenById(context.Background(), accessToken)
 	if errors.Is(err, pgx.ErrNoRows) {
-		return "", ErrAccessTokenInvalid
+		return "", ErrTokenInvalid
 	} else if err != nil {
 		return "", err
 	}
 	if time.Now().After(token.Expires.Time) {
-		return "", ErrAccessTokenExpired
+		return "", ErrTokenExpired
 	}
 	return token.Username, nil
 }