refactor(server/api): writeErr accepts error; migrate admin handlers (1/3)
Rewrite writeErr(w, err) to wrap *apierror.Error via apierror.From,
preserving the existing {"error": {"code", "message"}} wire envelope.
Add writeErrWithLog helper for 500-class errors that need an operator
log line. Migrate all 13 admin_*.go handler files (~76 call sites) to
the new signature; T3 will sweep the remaining api package.
The old 4-arg writeErr is removed, so non-admin call sites in
internal/api will not compile until T3 lands. This is by design — T2
and T3 are paired.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,7 @@ import (
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
|
||||
)
|
||||
|
||||
@@ -75,7 +76,7 @@ func (h *handlers) handleUpdateCoverSource(w http.ResponseWriter, r *http.Reques
|
||||
|
||||
var body updateCoverSourceReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "bad_body", "invalid JSON")
|
||||
writeErr(w, apierror.BadRequest("bad_body", "invalid JSON"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -85,11 +86,11 @@ func (h *handlers) handleUpdateCoverSource(w http.ResponseWriter, r *http.Reques
|
||||
})
|
||||
if err != nil {
|
||||
if errors.Is(err, coverart.ErrProviderNotFound) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "no such provider")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "no such provider"})
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin: update cover source", "id", id, "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "update failed")
|
||||
writeErr(w, apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -141,7 +142,7 @@ func (h *handlers) handleTestCoverSource(w http.ResponseWriter, r *http.Request)
|
||||
}
|
||||
|
||||
if errors.Is(err, coverart.ErrProviderNotFound) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "no such provider")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "no such provider"})
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ package api
|
||||
import (
|
||||
"net/http"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
)
|
||||
|
||||
@@ -24,8 +25,7 @@ func (h *handlers) handleGetLibraryCoverage(w http.ResponseWriter, r *http.Reque
|
||||
q := dbq.New(h.pool)
|
||||
row, err := q.GetAlbumCoverageRollup(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin: get coverage rollup", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed")
|
||||
writeErrWithLog(w, h.logger, "admin: get coverage rollup", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, coverageRollupResp{
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
package api
|
||||
|
||||
import "net/http"
|
||||
import (
|
||||
"net/http"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
)
|
||||
|
||||
// handleResearchMissingArt bumps the cover-art version unconditionally
|
||||
// so every 'none' row becomes eligible at the next enrichment pass.
|
||||
@@ -14,8 +18,7 @@ import "net/http"
|
||||
func (h *handlers) handleResearchMissingArt(w http.ResponseWriter, r *http.Request) {
|
||||
newVer, err := h.coverSettings.BumpVersion(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin: research missing art bump failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "bump failed")
|
||||
writeErrWithLog(w, h.logger, "admin: research missing art bump failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]any{"version": newVer})
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/jackc/pgx/v5"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
)
|
||||
@@ -23,22 +24,20 @@ type adminCoverRefetchResp struct {
|
||||
func (h *handlers) handleAdminAlbumRefetchCover(w http.ResponseWriter, r *http.Request) {
|
||||
albumID, ok := parseUUID(chi.URLParam(r, "id"))
|
||||
if !ok {
|
||||
writeErr(w, http.StatusBadRequest, "bad_request", "invalid album id")
|
||||
writeErr(w, apierror.BadRequest("bad_request", "invalid album id"))
|
||||
return
|
||||
}
|
||||
if err := h.coverart.RetryAlbum(r.Context(), albumID); err != nil {
|
||||
if errors.Is(err, coverart.ErrNotFound) || errors.Is(err, pgx.ErrNoRows) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "album not found")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "album not found"})
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin: cover refetch failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "refetch failed")
|
||||
writeErrWithLog(w, h.logger, "admin: cover refetch failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
row, err := dbq.New(h.pool).GetAlbumWithFirstTrackPath(r.Context(), albumID)
|
||||
if err != nil {
|
||||
h.logger.Error("admin: post-refetch read failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "read failed")
|
||||
writeErrWithLog(w, h.logger, "admin: post-refetch read failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, adminCoverRefetchResp{
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
@@ -41,8 +42,7 @@ func (h *handlers) handleListInvites(w http.ResponseWriter, r *http.Request) {
|
||||
q := dbq.New(h.pool)
|
||||
rows, err := q.ListInvitesActive(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin: list invites failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: list invites failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
out := make([]inviteResp, 0, len(rows))
|
||||
@@ -59,7 +59,7 @@ type createInviteReq struct {
|
||||
func (h *handlers) handleCreateInvite(w http.ResponseWriter, r *http.Request) {
|
||||
user, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
var req createInviteReq
|
||||
@@ -70,8 +70,7 @@ func (h *handlers) handleCreateInvite(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
tokenBytes := make([]byte, 16)
|
||||
if _, err := rand.Read(tokenBytes); err != nil {
|
||||
h.logger.Error("admin: invite token gen failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: invite token gen failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
token := hex.EncodeToString(tokenBytes)
|
||||
@@ -90,8 +89,7 @@ func (h *handlers) handleCreateInvite(w http.ResponseWriter, r *http.Request) {
|
||||
ExpiresAt: pgtype.Timestamptz{Time: expiresAt, Valid: true},
|
||||
})
|
||||
if err != nil {
|
||||
h.logger.Error("admin: create invite failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: create invite failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -106,12 +104,12 @@ func (h *handlers) handleCreateInvite(w http.ResponseWriter, r *http.Request) {
|
||||
func (h *handlers) handleDeleteInvite(w http.ResponseWriter, r *http.Request) {
|
||||
user, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
token := chi.URLParam(r, "token")
|
||||
if token == "" {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_token", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_token", ""))
|
||||
return
|
||||
}
|
||||
q := dbq.New(h.pool)
|
||||
@@ -119,16 +117,14 @@ func (h *handlers) handleDeleteInvite(w http.ResponseWriter, r *http.Request) {
|
||||
// return a useful 404 vs 200.
|
||||
if _, err := q.GetInviteByToken(r.Context(), token); err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: ""})
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin: get invite failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: get invite failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
if err := q.DeleteInvite(r.Context(), token); err != nil {
|
||||
h.logger.Error("admin: delete invite failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: delete invite failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
if err := audit.Write(r.Context(), h.pool, user.ID, pgtype.UUID{}, audit.ActionInviteRevoke,
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
|
||||
)
|
||||
@@ -39,8 +40,7 @@ func (h *handlers) handleGetScanStatus(w http.ResponseWriter, r *http.Request) {
|
||||
writeJSON(w, http.StatusOK, scanStatusResp{InFlight: false})
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin: get scan status", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed")
|
||||
writeErrWithLog(w, h.logger, "admin: get scan status", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -85,8 +85,7 @@ func (h *handlers) handleTriggerScan(w http.ResponseWriter, _ *http.Request) {
|
||||
h.logger.With("source", "manual"), h.scanCfg,
|
||||
)
|
||||
if err != nil {
|
||||
h.logger.Error("admin: trigger scan failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "trigger failed")
|
||||
writeErrWithLog(w, h.logger, "admin: trigger scan failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
if !started {
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
|
||||
)
|
||||
@@ -34,11 +35,10 @@ func (h *handlers) handleGetScanSchedule(w http.ResponseWriter, r *http.Request)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
h.logger.Error("admin: scan_schedule singleton row missing")
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "schedule row missing")
|
||||
writeErr(w, apierror.Internal(errors.New("schedule row missing")))
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin: get scan schedule", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed")
|
||||
writeErrWithLog(w, h.logger, "admin: get scan schedule", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -48,12 +48,12 @@ func (h *handlers) handleGetScanSchedule(w http.ResponseWriter, r *http.Request)
|
||||
func (h *handlers) handlePatchScanSchedule(w http.ResponseWriter, r *http.Request) {
|
||||
var req scanScheduleReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "bad_body", "invalid JSON")
|
||||
writeErr(w, apierror.BadRequest("bad_body", "invalid JSON"))
|
||||
return
|
||||
}
|
||||
|
||||
if err := validateScheduleReq(&req); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "validation", err.Error())
|
||||
writeErr(w, apierror.BadRequest("validation", err.Error()))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -64,8 +64,7 @@ func (h *handlers) handlePatchScanSchedule(w http.ResponseWriter, r *http.Reques
|
||||
TimeOfDay: req.TimeOfDay,
|
||||
WeeklyDay: pgInt32Ptr(req.WeeklyDay),
|
||||
}); err != nil {
|
||||
h.logger.Error("admin: update scan schedule", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "update failed")
|
||||
writeErrWithLog(w, h.logger, "admin: update scan schedule", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -73,8 +72,7 @@ func (h *handlers) handlePatchScanSchedule(w http.ResponseWriter, r *http.Reques
|
||||
|
||||
row, err := q.GetScanSchedule(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin: re-read scan schedule", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "re-read failed")
|
||||
writeErrWithLog(w, h.logger, "admin: re-read scan schedule", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
cfg := library.ScheduleConfig{Mode: row.Mode}
|
||||
|
||||
+13
-15
@@ -7,6 +7,7 @@ import (
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/mailer"
|
||||
@@ -38,8 +39,7 @@ func (h *handlers) handleGetSMTPConfig(w http.ResponseWriter, r *http.Request) {
|
||||
q := dbq.New(h.pool)
|
||||
cfg, err := q.GetSMTPConfig(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin smtp: get failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin smtp: get failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
masked := ""
|
||||
@@ -61,21 +61,20 @@ func (h *handlers) handleGetSMTPConfig(w http.ResponseWriter, r *http.Request) {
|
||||
func (h *handlers) handleUpdateSMTPConfig(w http.ResponseWriter, r *http.Request) {
|
||||
var req updateSMTPConfigReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_body", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_body", ""))
|
||||
return
|
||||
}
|
||||
if req.Enabled {
|
||||
if req.Host == "" || req.FromAddress == "" {
|
||||
writeErr(w, http.StatusBadRequest, "missing_fields",
|
||||
"host and from_address are required when enabled")
|
||||
writeErr(w, apierror.BadRequest("missing_fields",
|
||||
"host and from_address are required when enabled"))
|
||||
return
|
||||
}
|
||||
}
|
||||
q := dbq.New(h.pool)
|
||||
current, err := q.GetSMTPConfig(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin smtp: get failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin smtp: get failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
// Preserve existing password when the request sends "" (so the
|
||||
@@ -94,8 +93,7 @@ func (h *handlers) handleUpdateSMTPConfig(w http.ResponseWriter, r *http.Request
|
||||
FromName: req.FromName,
|
||||
UseTls: req.UseTLS,
|
||||
}); err != nil {
|
||||
h.logger.Error("admin smtp: update failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin smtp: update failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
@@ -104,12 +102,12 @@ func (h *handlers) handleUpdateSMTPConfig(w http.ResponseWriter, r *http.Request
|
||||
func (h *handlers) handleTestSMTPConfig(w http.ResponseWriter, r *http.Request) {
|
||||
caller, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
if caller.Email == nil || *caller.Email == "" {
|
||||
writeErr(w, http.StatusBadRequest, "no_email_on_file",
|
||||
"set your email in /settings before testing SMTP")
|
||||
writeErr(w, apierror.BadRequest("no_email_on_file",
|
||||
"set your email in /settings before testing SMTP"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -123,12 +121,12 @@ func (h *handlers) handleTestSMTPConfig(w http.ResponseWriter, r *http.Request)
|
||||
htmlBody := "<p>This is a test message from your Minstrel server.</p><p>If you got this, your SMTP config is working.</p><p>— Minstrel</p>"
|
||||
if err := sender.Send(ctx, *caller.Email, "Minstrel SMTP test", textBody, htmlBody); err != nil {
|
||||
if errors.Is(err, mailer.ErrNotConfigured) {
|
||||
writeErr(w, http.StatusBadRequest, "not_configured",
|
||||
"SMTP is not enabled or required fields are empty")
|
||||
writeErr(w, apierror.BadRequest("not_configured",
|
||||
"SMTP is not enabled or required fields are empty"))
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin smtp: test send failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "send_failed", err.Error())
|
||||
writeErr(w, &apierror.Error{Status: http.StatusInternalServerError, Code: "send_failed", Message: err.Error()})
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
|
||||
)
|
||||
@@ -40,7 +41,7 @@ func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
|
||||
// the spec's error surface — collapse both to 404 not_found so
|
||||
// the client doesn't have to handle a separate bad_request branch
|
||||
// for a code path that's only reachable via UI bugs.
|
||||
writeErr(w, http.StatusNotFound, "not_found", "track not found")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "track not found"})
|
||||
return
|
||||
}
|
||||
|
||||
@@ -55,7 +56,7 @@ func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
|
||||
if !ok {
|
||||
// Defensive: RequireUser+RequireAdmin should have run upstream.
|
||||
// If we got here without a user in context the routing is broken.
|
||||
writeErr(w, http.StatusUnauthorized, "unauthenticated", "no session")
|
||||
writeErr(w, apierror.Unauthorized("unauthenticated", "no session"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -64,11 +65,11 @@ func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
|
||||
)
|
||||
if err != nil {
|
||||
if errors.Is(err, tracks.ErrNotFound) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "track not found")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "track not found"})
|
||||
return
|
||||
}
|
||||
h.logger.Error("api: remove track failed", "err", err, "track_id", idStr)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "remove failed")
|
||||
writeErr(w, apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
+39
-52
@@ -12,6 +12,7 @@ import (
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
@@ -34,8 +35,7 @@ func (h *handlers) handleAdminListUsers(w http.ResponseWriter, r *http.Request)
|
||||
q := dbq.New(h.pool)
|
||||
rows, err := q.ListUsers(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin: list users failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: list users failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
out := make([]adminUserView, 0, len(rows))
|
||||
@@ -60,17 +60,17 @@ type updateUserAdminReq struct {
|
||||
func (h *handlers) handleUpdateUserAdmin(w http.ResponseWriter, r *http.Request) {
|
||||
caller, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
targetID, ok := parseUUID(chi.URLParam(r, "id"))
|
||||
if !ok {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_id", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_id", ""))
|
||||
return
|
||||
}
|
||||
var req updateUserAdminReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_body", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_body", ""))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -86,8 +86,7 @@ func (h *handlers) handleUpdateUserAdmin(w http.ResponseWriter, r *http.Request)
|
||||
if !req.IsAdmin {
|
||||
count, err := q.CountAdmins(r.Context())
|
||||
if err != nil {
|
||||
h.logger.Error("admin: count admins failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: count admins failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
// We need to know whether the target is currently an admin.
|
||||
@@ -95,13 +94,12 @@ func (h *handlers) handleUpdateUserAdmin(w http.ResponseWriter, r *http.Request)
|
||||
// allow it). If they ARE admin and they're the last one, refuse.
|
||||
current, err := q.GetUserByID(r.Context(), targetID)
|
||||
if err != nil {
|
||||
h.logger.Error("admin: get user failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: get user failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
if current.IsAdmin && count <= 1 {
|
||||
writeErr(w, http.StatusConflict, "last_admin",
|
||||
"can't remove the last admin — promote someone else first")
|
||||
writeErr(w, apierror.Conflict("last_admin",
|
||||
"can't remove the last admin — promote someone else first"))
|
||||
return
|
||||
}
|
||||
}
|
||||
@@ -111,8 +109,7 @@ func (h *handlers) handleUpdateUserAdmin(w http.ResponseWriter, r *http.Request)
|
||||
IsAdmin: req.IsAdmin,
|
||||
})
|
||||
if err != nil {
|
||||
h.logger.Error("admin: update user admin failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin: update user admin failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -146,33 +143,31 @@ type adminCreateUserReq struct {
|
||||
func (h *handlers) handleAdminCreateUser(w http.ResponseWriter, r *http.Request) {
|
||||
caller, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
var req adminCreateUserReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_body", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_body", ""))
|
||||
return
|
||||
}
|
||||
if !usernameRe.MatchString(req.Username) {
|
||||
writeErr(w, http.StatusBadRequest, "username_invalid", "username must be 3-32 chars, alphanumeric + underscore + hyphen")
|
||||
writeErr(w, apierror.BadRequest("username_invalid", "username must be 3-32 chars, alphanumeric + underscore + hyphen"))
|
||||
return
|
||||
}
|
||||
if len(req.Password) < minPasswordLength {
|
||||
writeErr(w, http.StatusBadRequest, "password_too_short", "password must be at least 8 chars")
|
||||
writeErr(w, apierror.BadRequest("password_too_short", "password must be at least 8 chars"))
|
||||
return
|
||||
}
|
||||
|
||||
hash, err := bcrypt.GenerateFromPassword([]byte(req.Password), bcrypt.DefaultCost)
|
||||
if err != nil {
|
||||
h.logger.Error("admin create user: hash failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin create user: hash failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
apiToken, err := auth.MintSessionToken()
|
||||
if err != nil {
|
||||
h.logger.Error("admin create user: mint token failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin create user: mint token failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -187,11 +182,10 @@ func (h *handlers) handleAdminCreateUser(w http.ResponseWriter, r *http.Request)
|
||||
if err != nil {
|
||||
var pgErr *pgconn.PgError
|
||||
if errors.As(err, &pgErr) && pgErr.Code == pgerrcode.UniqueViolation {
|
||||
writeErr(w, http.StatusConflict, "username_taken", "")
|
||||
writeErr(w, apierror.Conflict("username_taken", ""))
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin create user: insert failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin create user: insert failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -215,12 +209,12 @@ func (h *handlers) handleAdminCreateUser(w http.ResponseWriter, r *http.Request)
|
||||
func (h *handlers) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request) {
|
||||
caller, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
targetID, ok := parseUUID(chi.URLParam(r, "id"))
|
||||
if !ok {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_id", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_id", ""))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -228,11 +222,10 @@ func (h *handlers) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request)
|
||||
target, err := q.GetUserByID(r.Context(), targetID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: ""})
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin delete user: lookup failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin delete user: lookup failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -240,20 +233,18 @@ func (h *handlers) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request)
|
||||
if target.IsAdmin {
|
||||
count, cerr := q.CountAdmins(r.Context())
|
||||
if cerr != nil {
|
||||
h.logger.Error("admin delete user: count admins failed", "err", cerr)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin delete user: count admins failed", apierror.Internal(cerr))
|
||||
return
|
||||
}
|
||||
if count <= 1 {
|
||||
writeErr(w, http.StatusConflict, "last_admin",
|
||||
"can't delete the last admin — promote someone else first")
|
||||
writeErr(w, apierror.Conflict("last_admin",
|
||||
"can't delete the last admin — promote someone else first"))
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if err := q.DeleteUser(r.Context(), targetID); err != nil {
|
||||
h.logger.Error("admin delete user: delete failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin delete user: delete failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -274,21 +265,21 @@ type adminResetPasswordReq struct {
|
||||
func (h *handlers) handleAdminResetPassword(w http.ResponseWriter, r *http.Request) {
|
||||
caller, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
targetID, ok := parseUUID(chi.URLParam(r, "id"))
|
||||
if !ok {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_id", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_id", ""))
|
||||
return
|
||||
}
|
||||
var req adminResetPasswordReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_body", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_body", ""))
|
||||
return
|
||||
}
|
||||
if len(req.Password) < minPasswordLength {
|
||||
writeErr(w, http.StatusBadRequest, "password_too_short", "password must be at least 8 chars")
|
||||
writeErr(w, apierror.BadRequest("password_too_short", "password must be at least 8 chars"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -296,26 +287,23 @@ func (h *handlers) handleAdminResetPassword(w http.ResponseWriter, r *http.Reque
|
||||
// Verify target exists for a useful 404.
|
||||
if _, err := q.GetUserByID(r.Context(), targetID); err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: ""})
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin reset password: lookup failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin reset password: lookup failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
hash, err := bcrypt.GenerateFromPassword([]byte(req.Password), bcrypt.DefaultCost)
|
||||
if err != nil {
|
||||
h.logger.Error("admin reset password: hash failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin reset password: hash failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
if err := q.ResetUserPassword(r.Context(), dbq.ResetUserPasswordParams{
|
||||
ID: targetID,
|
||||
PasswordHash: string(hash),
|
||||
}); err != nil {
|
||||
h.logger.Error("admin reset password: update failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin reset password: update failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -335,17 +323,17 @@ type adminAutoApproveReq struct {
|
||||
func (h *handlers) handleAdminAutoApproveToggle(w http.ResponseWriter, r *http.Request) {
|
||||
caller, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "auth_required", "")
|
||||
writeErr(w, apierror.Unauthorized("auth_required", ""))
|
||||
return
|
||||
}
|
||||
targetID, ok := parseUUID(chi.URLParam(r, "id"))
|
||||
if !ok {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_id", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_id", ""))
|
||||
return
|
||||
}
|
||||
var req adminAutoApproveReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeErr(w, http.StatusBadRequest, "invalid_body", "")
|
||||
writeErr(w, apierror.BadRequest("invalid_body", ""))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -356,11 +344,10 @@ func (h *handlers) handleAdminAutoApproveToggle(w http.ResponseWriter, r *http.R
|
||||
})
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
writeErr(w, http.StatusNotFound, "not_found", "")
|
||||
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: ""})
|
||||
return
|
||||
}
|
||||
h.logger.Error("admin auto-approve: update failed", "err", err)
|
||||
writeErr(w, http.StatusInternalServerError, "server_error", "")
|
||||
writeErrWithLog(w, h.logger, "admin auto-approve: update failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
+28
-7
@@ -2,11 +2,18 @@ package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
)
|
||||
|
||||
// errorBody is the JSON envelope for failures on /api/*. Kept separate from
|
||||
// the Subsonic envelope so native clients don't have to parse two shapes.
|
||||
// errorBody / errorPayload preserve the existing wire envelope:
|
||||
//
|
||||
// {"error": {"code": "...", "message": "..."}}
|
||||
//
|
||||
// Kept separate from the Subsonic envelope so native clients don't have
|
||||
// to parse two shapes.
|
||||
type errorBody struct {
|
||||
Error errorPayload `json:"error"`
|
||||
}
|
||||
@@ -16,12 +23,26 @@ type errorPayload struct {
|
||||
Message string `json:"message"`
|
||||
}
|
||||
|
||||
// writeErr is the canonical way to emit a failure. Code is a stable
|
||||
// short-string clients can switch on; message is human-readable.
|
||||
func writeErr(w http.ResponseWriter, status int, code, message string) {
|
||||
// writeErr emits the failure envelope for err. If err is or wraps
|
||||
// *apierror.Error, its Status/Code/Message drive the response;
|
||||
// otherwise it's mapped to a 500 server_error.
|
||||
func writeErr(w http.ResponseWriter, err error) {
|
||||
apiErr := apierror.From(err)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(status)
|
||||
_ = json.NewEncoder(w).Encode(errorBody{Error: errorPayload{Code: code, Message: message}})
|
||||
w.WriteHeader(apiErr.Status)
|
||||
_ = json.NewEncoder(w).Encode(errorBody{Error: errorPayload{
|
||||
Code: apiErr.Code,
|
||||
Message: apiErr.Message,
|
||||
}})
|
||||
}
|
||||
|
||||
// writeErrWithLog logs the error at Error level and writes the response.
|
||||
// Use for 500-class errors where the operator needs the cause in logs.
|
||||
func writeErrWithLog(w http.ResponseWriter, logger *slog.Logger, msg string, err error) {
|
||||
if logger != nil {
|
||||
logger.Error(msg, "err", err)
|
||||
}
|
||||
writeErr(w, err)
|
||||
}
|
||||
|
||||
// writeJSON emits a 2xx response with the given body JSON-encoded. Mirrors
|
||||
|
||||
+106
-19
@@ -2,31 +2,118 @@ package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
)
|
||||
|
||||
func TestWriteErr_EmitsJSONEnvelope(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
writeErr(w, http.StatusBadRequest, "bad_request", "field x required")
|
||||
func decodeEnvelope(t *testing.T, body []byte) errorBody {
|
||||
t.Helper()
|
||||
var env errorBody
|
||||
if err := json.Unmarshal(body, &env); err != nil {
|
||||
t.Fatalf("decode envelope: %v (body=%s)", err, string(body))
|
||||
}
|
||||
return env
|
||||
}
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("status = %d, want 400", w.Code)
|
||||
func TestWriteErr_APIError(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
writeErr(rec, apierror.NotFound("track"))
|
||||
|
||||
if rec.Code != 404 {
|
||||
t.Fatalf("status: want 404, got %d", rec.Code)
|
||||
}
|
||||
if got := w.Header().Get("Content-Type"); got != "application/json" {
|
||||
t.Errorf("content-type = %q, want application/json", got)
|
||||
env := decodeEnvelope(t, rec.Body.Bytes())
|
||||
if env.Error.Code != "track_not_found" {
|
||||
t.Errorf("code: want track_not_found, got %q", env.Error.Code)
|
||||
}
|
||||
var body struct {
|
||||
Error struct {
|
||||
Code string `json:"code"`
|
||||
Message string `json:"message"`
|
||||
} `json:"error"`
|
||||
}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("decode: %v\nbody=%s", err, w.Body.String())
|
||||
}
|
||||
if body.Error.Code != "bad_request" || body.Error.Message != "field x required" {
|
||||
t.Errorf("body = %+v", body)
|
||||
if env.Error.Message != "track not found" {
|
||||
t.Errorf("message: want %q, got %q", "track not found", env.Error.Message)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteErr_Sentinel(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
writeErr(rec, apierror.ErrUnauthorized)
|
||||
|
||||
if rec.Code != 401 {
|
||||
t.Fatalf("status: want 401, got %d", rec.Code)
|
||||
}
|
||||
env := decodeEnvelope(t, rec.Body.Bytes())
|
||||
if env.Error.Code != "unauthorized" {
|
||||
t.Errorf("code: want unauthorized, got %q", env.Error.Code)
|
||||
}
|
||||
if env.Error.Message != "authentication required" {
|
||||
t.Errorf("message: want %q, got %q", "authentication required", env.Error.Message)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteErr_PlainError(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
writeErr(rec, errors.New("boom"))
|
||||
|
||||
if rec.Code != 500 {
|
||||
t.Fatalf("status: want 500, got %d", rec.Code)
|
||||
}
|
||||
env := decodeEnvelope(t, rec.Body.Bytes())
|
||||
if env.Error.Code != "server_error" {
|
||||
t.Errorf("code: want server_error, got %q", env.Error.Code)
|
||||
}
|
||||
if env.Error.Message != "internal server error" {
|
||||
t.Errorf("message: want %q, got %q", "internal server error", env.Error.Message)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteErr_WrappedAPIError(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
writeErr(rec, fmt.Errorf("ctx: %w", apierror.BadRequest("bad", "no good")))
|
||||
|
||||
if rec.Code != 400 {
|
||||
t.Fatalf("status: want 400, got %d", rec.Code)
|
||||
}
|
||||
env := decodeEnvelope(t, rec.Body.Bytes())
|
||||
if env.Error.Code != "bad" {
|
||||
t.Errorf("code: want bad, got %q", env.Error.Code)
|
||||
}
|
||||
if env.Error.Message != "no good" {
|
||||
t.Errorf("message: want %q, got %q", "no good", env.Error.Message)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteErr_EnvelopeShape(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
writeErr(rec, apierror.BadRequest("x", "y"))
|
||||
|
||||
var raw map[string]any
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &raw); err != nil {
|
||||
t.Fatalf("decode raw: %v", err)
|
||||
}
|
||||
if len(raw) != 1 {
|
||||
t.Fatalf("top-level keys: want 1, got %d (%v)", len(raw), raw)
|
||||
}
|
||||
inner, ok := raw["error"].(map[string]any)
|
||||
if !ok {
|
||||
t.Fatalf("error: want object, got %T", raw["error"])
|
||||
}
|
||||
if len(inner) != 2 {
|
||||
t.Fatalf("inner keys: want 2, got %d (%v)", len(inner), inner)
|
||||
}
|
||||
if _, ok := inner["code"]; !ok {
|
||||
t.Errorf("inner.code missing")
|
||||
}
|
||||
if _, ok := inner["message"]; !ok {
|
||||
t.Errorf("inner.message missing")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteErr_ContentType(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
writeErr(rec, apierror.BadRequest("x", "y"))
|
||||
|
||||
if got := rec.Header().Get("Content-Type"); got != "application/json" {
|
||||
t.Errorf("Content-Type: want application/json, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user