From 0ec2425f78c60941ddb52b1d8e643d96812f3eef Mon Sep 17 00:00:00 2001 From: Sebastian Date: Tue, 6 Oct 2026 16:21:14 +0200 Subject: [PATCH] Return every error as {"error": "..."} Echo errors, middleware and handlers used three different formats (message object, error object, bare string). One error handler now writes all of them the same way; status codes are unchanged. --- internal/server/errors.go | 47 ++++++++++++++++++++++++ internal/server/middleware/rate_limit.go | 4 +- internal/server/middleware/sync_check.go | 2 +- internal/server/middleware/token_auth.go | 8 ++-- internal/server/music_handler.go | 4 +- internal/server/routes.go | 1 + internal/server/statistics_handler.go | 4 +- internal/server/sync_handler.go | 2 +- internal/server/token_handler.go | 16 ++++---- 9 files changed, 67 insertions(+), 21 deletions(-) create mode 100644 internal/server/errors.go diff --git a/internal/server/errors.go b/internal/server/errors.go new file mode 100644 index 0000000..47059c6 --- /dev/null +++ b/internal/server/errors.go @@ -0,0 +1,47 @@ +package server + +import ( + "errors" + "net/http" + + "music-server/internal/logging" + + "github.com/labstack/echo/v5" + "go.uber.org/zap" +) + +// ErrorResponse is the body of every error response. +type ErrorResponse struct { + Error string `json:"error" example:"Not Found"` +} + +// httpErrorHandler writes every error as an ErrorResponse. +func httpErrorHandler(c *echo.Context, err error) { + if r, _ := echo.UnwrapResponse(c.Response()); r != nil && r.Committed { + return + } + + code := http.StatusInternalServerError + message := http.StatusText(code) + var he *echo.HTTPError + var sc echo.HTTPStatusCoder + switch { + case errors.As(err, &he): + code = he.Code + message = he.Message + case errors.As(err, &sc): + code = sc.StatusCode() + } + if code >= http.StatusInternalServerError && he == nil { + logging.GetLogger().Error("Unhandled error", zap.String("path", c.Request().URL.Path), zap.Error(err)) + } + if message == "" { + message = http.StatusText(code) + } + + if c.Request().Method == http.MethodHead { + _ = c.NoContent(code) + return + } + _ = c.JSON(code, ErrorResponse{Error: message}) +} diff --git a/internal/server/middleware/rate_limit.go b/internal/server/middleware/rate_limit.go index d27e78f..8ecf302 100644 --- a/internal/server/middleware/rate_limit.go +++ b/internal/server/middleware/rate_limit.go @@ -23,9 +23,7 @@ func RateLimit(limit int, window time.Duration) echo.MiddlewareFunc { logging.GetLogger().Warn("Rate limit exceeded", zap.String("ip", identifier), zap.String("path", c.Request().URL.Path)) - return c.JSON(http.StatusTooManyRequests, map[string]string{ - "error": "Rate limit exceeded. Please try again later.", - }) + return echo.NewHTTPError(http.StatusTooManyRequests, "Rate limit exceeded. Please try again later.") } return echoMiddleware.RateLimiterWithConfig(config) } diff --git a/internal/server/middleware/sync_check.go b/internal/server/middleware/sync_check.go index 7b453f4..a9f2235 100644 --- a/internal/server/middleware/sync_check.go +++ b/internal/server/middleware/sync_check.go @@ -14,7 +14,7 @@ func SyncCheck(isSyncing func() bool) echo.MiddlewareFunc { return func(c *echo.Context) error { if isSyncing() { logging.GetLogger().Info("Syncing is in progress - request blocked") - return c.JSON(http.StatusLocked, "Syncing is in progress") + return echo.NewHTTPError(http.StatusLocked, "Syncing is in progress") } return next(c) } diff --git a/internal/server/middleware/token_auth.go b/internal/server/middleware/token_auth.go index dba8cc3..47715d4 100644 --- a/internal/server/middleware/token_auth.go +++ b/internal/server/middleware/token_auth.go @@ -28,11 +28,11 @@ func TokenAuth(q repository.Querier, disabled bool) echo.MiddlewareFunc { } if c.Request().Header.Get("Authorization") == "" { - return c.JSON(http.StatusUnauthorized, map[string]string{"error": "Authorization header required"}) + return echo.NewHTTPError(http.StatusUnauthorized, "Authorization header required") } token, ok := BearerToken(c) if !ok { - return c.JSON(http.StatusUnauthorized, map[string]string{"error": "Invalid authorization format. Use: Bearer "}) + return echo.NewHTTPError(http.StatusUnauthorized, "Invalid authorization format. Use: Bearer ") } ctx := c.Request().Context() @@ -42,7 +42,7 @@ func TokenAuth(q repository.Querier, disabled bool) echo.MiddlewareFunc { zap.String("ip", c.RealIP()), zap.String("error", err.Error()), ) - return c.JSON(http.StatusUnauthorized, map[string]string{"error": "Invalid or expired token"}) + return echo.NewHTTPError(http.StatusUnauthorized, "Invalid or expired token") } if time.Now().After(session.ExpiresAt.Time) { @@ -52,7 +52,7 @@ func TokenAuth(q repository.Querier, disabled bool) echo.MiddlewareFunc { logging.GetLogger().Error("Failed to delete expired session", zap.Error(err)) } }(context.WithoutCancel(ctx)) - return c.JSON(http.StatusUnauthorized, map[string]string{"error": "Token expired"}) + return echo.NewHTTPError(http.StatusUnauthorized, "Token expired") } c.Set("session", session) diff --git a/internal/server/music_handler.go b/internal/server/music_handler.go index 384ec97..b05e703 100644 --- a/internal/server/music_handler.go +++ b/internal/server/music_handler.go @@ -34,7 +34,7 @@ func NewMusicHandler(music *backend.MusicService) *MusicHandler { func (m *MusicHandler) GetSong(ctx *echo.Context) error { song, err := strconv.Atoi(ctx.QueryParam("song")) if err != nil { - return ctx.String(http.StatusBadRequest, "song must be a number") + return echo.NewHTTPError(http.StatusBadRequest, "song must be a number") } path, err := m.music.Song(song) return serveSong(ctx, path, err) @@ -204,7 +204,7 @@ func (m *MusicHandler) GetAllSoundtracksRandom(ctx *echo.Context) error { func (m *MusicHandler) PutPlayed(ctx *echo.Context) error { song, err := strconv.Atoi(ctx.QueryParam("song")) if err != nil { - return ctx.JSON(http.StatusBadRequest, err.Error()) + return echo.NewHTTPError(http.StatusBadRequest, "song must be a number") } logging.GetLogger().Info("Marking song as played", zap.Int("song_id", song)) if err := m.music.SetPlayed(ctx.Request().Context(), song); err != nil { diff --git a/internal/server/routes.go b/internal/server/routes.go index 7df93cc..fe54543 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -59,6 +59,7 @@ func (s *Server) handlers() handlers { func (s *Server) RegisterRoutes() http.Handler { e := echo.New() + e.HTTPErrorHandler = httpErrorHandler if len(s.cfg.TrustedProxies) > 0 { var trust []echo.TrustOption for _, ipNet := range s.cfg.TrustedProxies { diff --git a/internal/server/statistics_handler.go b/internal/server/statistics_handler.go index bf736f0..a527dbe 100644 --- a/internal/server/statistics_handler.go +++ b/internal/server/statistics_handler.go @@ -43,7 +43,7 @@ func parseLimit(ctx *echo.Context) (int32, bool) { func withLimit[T any](ctx *echo.Context, name string, query func(*echo.Context, int32) (T, error)) error { limit, ok := parseLimit(ctx) if !ok { - return ctx.JSON(http.StatusBadRequest, map[string]string{"error": "Invalid limit parameter"}) + return echo.NewHTTPError(http.StatusBadRequest, "Invalid limit parameter") } result, err := query(ctx, limit) return statsResult(ctx, name, result, err) @@ -52,7 +52,7 @@ func withLimit[T any](ctx *echo.Context, name string, query func(*echo.Context, func statsResult[T any](ctx *echo.Context, name string, result T, err error) error { if err != nil { logging.GetLogger().Error("Failed to get statistics", zap.String("statistic", name), zap.Error(err)) - return ctx.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to get statistics"}) + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to get statistics") } return ctx.JSON(http.StatusOK, result) } diff --git a/internal/server/sync_handler.go b/internal/server/sync_handler.go index ba455de..c1e340a 100644 --- a/internal/server/sync_handler.go +++ b/internal/server/sync_handler.go @@ -57,7 +57,7 @@ func (s *SyncHandler) SyncSoundtracksNewFull(ctx *echo.Context) error { func (s *SyncHandler) start(ctx *echo.Context, full bool, message string) error { if !s.sync.Start(full) { - return ctx.JSON(http.StatusLocked, "Syncing is in progress") + return echo.NewHTTPError(http.StatusLocked, "Syncing is in progress") } logging.GetLogger().Info(message) return ctx.JSON(http.StatusOK, message) diff --git a/internal/server/token_handler.go b/internal/server/token_handler.go index 4a93870..ae477e9 100644 --- a/internal/server/token_handler.go +++ b/internal/server/token_handler.go @@ -75,14 +75,14 @@ func (h *TokenHandler) passwordValid(password string) bool { func (h *TokenHandler) CreateTokenHandler(c *echo.Context) error { var req TokenRequest if err := c.Bind(&req); err != nil { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "Invalid request body"}) + return echo.NewHTTPError(http.StatusBadRequest, "Invalid request body") } if !h.passwordValid(req.Password) { logging.GetLogger().Warn("Invalid password attempt for token creation", zap.String("ip", c.RealIP()), zap.String("user_agent", c.Request().UserAgent())) - return c.JSON(http.StatusUnauthorized, map[string]string{"error": "Invalid password"}) + return echo.NewHTTPError(http.StatusUnauthorized, "Invalid password") } if req.ClientType == "" { @@ -92,7 +92,7 @@ func (h *TokenHandler) CreateTokenHandler(c *echo.Context) error { token, err := generateToken() if err != nil { logging.GetLogger().Error("Failed to generate token", zap.Error(err)) - return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to generate token"}) + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to generate token") } session, err := h.q.CreateSession(c.Request().Context(), repository.CreateSessionParams{ @@ -104,7 +104,7 @@ func (h *TokenHandler) CreateTokenHandler(c *echo.Context) error { }) if err != nil { logging.GetLogger().Error("Failed to create session", zap.Error(err)) - return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to create session"}) + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to create session") } clientType := req.ClientType @@ -131,16 +131,16 @@ func (h *TokenHandler) CreateTokenHandler(c *echo.Context) error { // @Router /api/v1/token [delete] func (h *TokenHandler) DeleteTokenHandler(c *echo.Context) error { if c.Request().Header.Get("Authorization") == "" { - return c.JSON(http.StatusUnauthorized, map[string]string{"error": "Authorization header required"}) + return echo.NewHTTPError(http.StatusUnauthorized, "Authorization header required") } token, ok := middleware.BearerToken(c) if !ok { - return c.JSON(http.StatusUnauthorized, map[string]string{"error": "Invalid authorization format"}) + return echo.NewHTTPError(http.StatusUnauthorized, "Invalid authorization format") } if err := h.q.DeleteSession(c.Request().Context(), token); err != nil { logging.GetLogger().Error("Failed to delete session", zap.Error(err)) - return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to invalidate token"}) + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to invalidate token") } return c.JSON(http.StatusOK, map[string]string{"status": "token invalidated"}) } @@ -159,7 +159,7 @@ func (h *TokenHandler) DeleteTokenHandler(c *echo.Context) error { func (h *TokenHandler) CleanupExpiredSessionsHandler(c *echo.Context) error { if err := h.q.DeleteExpiredSessions(c.Request().Context()); err != nil { logging.GetLogger().Error("Failed to cleanup sessions", zap.Error(err)) - return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to cleanup sessions"}) + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to cleanup sessions") } return c.JSON(http.StatusOK, map[string]string{"status": "cleanup complete"}) }