fix(server/api): preserve 500-class messages via apierror.InternalMsg

The PR1-T2 admin handler migration replaced bespoke 500-class messages
("lookup failed", "refetch failed", "trigger failed", "bump failed",
"remove failed", "update failed", "schedule row missing", "re-read
failed", "read failed") with apierror.Internal(err), which forces the
wire Message to "internal server error". That violates the PR1
contract that errEnvelope{Code, Message} must remain byte-identical
for existing clients.

Add apierror.InternalMsg(message, cause) — a 500-class constructor
that preserves the call site's user-facing message while keeping the
cause attached for logging and errors.Is. Re-migrate the 12 admin
sites whose pre-1cc7eb6 source had a non-empty bespoke Message so
they restore the original wire string. Sites whose original Message
was empty stay on Internal(err) (humanization to "internal server
error" is a tolerable improvement, not a regression).

admin_smtp.go's send_failed site (line 129) was already preserved
via a raw &apierror.Error literal in 1cc7eb6 and needs no fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-07 20:10:18 -04:00
parent 1cc7eb6cad
commit 58766dbebe
9 changed files with 37 additions and 12 deletions
+1 -1
View File
@@ -90,7 +90,7 @@ func (h *handlers) handleUpdateCoverSource(w http.ResponseWriter, r *http.Reques
return return
} }
h.logger.Error("admin: update cover source", "id", id, "err", err) h.logger.Error("admin: update cover source", "id", id, "err", err)
writeErr(w, apierror.Internal(err)) writeErr(w, apierror.InternalMsg("update failed", err))
return return
} }
+1 -1
View File
@@ -25,7 +25,7 @@ func (h *handlers) handleGetLibraryCoverage(w http.ResponseWriter, r *http.Reque
q := dbq.New(h.pool) q := dbq.New(h.pool)
row, err := q.GetAlbumCoverageRollup(r.Context()) row, err := q.GetAlbumCoverageRollup(r.Context())
if err != nil { if err != nil {
writeErrWithLog(w, h.logger, "admin: get coverage rollup", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: get coverage rollup", apierror.InternalMsg("lookup failed", err))
return return
} }
writeJSON(w, http.StatusOK, coverageRollupResp{ writeJSON(w, http.StatusOK, coverageRollupResp{
+1 -1
View File
@@ -18,7 +18,7 @@ import (
func (h *handlers) handleResearchMissingArt(w http.ResponseWriter, r *http.Request) { func (h *handlers) handleResearchMissingArt(w http.ResponseWriter, r *http.Request) {
newVer, err := h.coverSettings.BumpVersion(r.Context()) newVer, err := h.coverSettings.BumpVersion(r.Context())
if err != nil { if err != nil {
writeErrWithLog(w, h.logger, "admin: research missing art bump failed", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: research missing art bump failed", apierror.InternalMsg("bump failed", err))
return return
} }
writeJSON(w, http.StatusOK, map[string]any{"version": newVer}) writeJSON(w, http.StatusOK, map[string]any{"version": newVer})
+2 -2
View File
@@ -32,12 +32,12 @@ func (h *handlers) handleAdminAlbumRefetchCover(w http.ResponseWriter, r *http.R
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "album not found"}) writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "album not found"})
return return
} }
writeErrWithLog(w, h.logger, "admin: cover refetch failed", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: cover refetch failed", apierror.InternalMsg("refetch failed", err))
return return
} }
row, err := dbq.New(h.pool).GetAlbumWithFirstTrackPath(r.Context(), albumID) row, err := dbq.New(h.pool).GetAlbumWithFirstTrackPath(r.Context(), albumID)
if err != nil { if err != nil {
writeErrWithLog(w, h.logger, "admin: post-refetch read failed", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: post-refetch read failed", apierror.InternalMsg("read failed", err))
return return
} }
writeJSON(w, http.StatusOK, adminCoverRefetchResp{ writeJSON(w, http.StatusOK, adminCoverRefetchResp{
+2 -2
View File
@@ -40,7 +40,7 @@ func (h *handlers) handleGetScanStatus(w http.ResponseWriter, r *http.Request) {
writeJSON(w, http.StatusOK, scanStatusResp{InFlight: false}) writeJSON(w, http.StatusOK, scanStatusResp{InFlight: false})
return return
} }
writeErrWithLog(w, h.logger, "admin: get scan status", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: get scan status", apierror.InternalMsg("lookup failed", err))
return return
} }
@@ -85,7 +85,7 @@ func (h *handlers) handleTriggerScan(w http.ResponseWriter, _ *http.Request) {
h.logger.With("source", "manual"), h.scanCfg, h.logger.With("source", "manual"), h.scanCfg,
) )
if err != nil { if err != nil {
writeErrWithLog(w, h.logger, "admin: trigger scan failed", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: trigger scan failed", apierror.InternalMsg("trigger failed", err))
return return
} }
if !started { if !started {
+4 -4
View File
@@ -35,10 +35,10 @@ func (h *handlers) handleGetScanSchedule(w http.ResponseWriter, r *http.Request)
if err != nil { if err != nil {
if errors.Is(err, pgx.ErrNoRows) { if errors.Is(err, pgx.ErrNoRows) {
h.logger.Error("admin: scan_schedule singleton row missing") h.logger.Error("admin: scan_schedule singleton row missing")
writeErr(w, apierror.Internal(errors.New("schedule row missing"))) writeErr(w, apierror.InternalMsg("schedule row missing", errors.New("schedule row missing")))
return return
} }
writeErrWithLog(w, h.logger, "admin: get scan schedule", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: get scan schedule", apierror.InternalMsg("lookup failed", err))
return return
} }
@@ -64,7 +64,7 @@ func (h *handlers) handlePatchScanSchedule(w http.ResponseWriter, r *http.Reques
TimeOfDay: req.TimeOfDay, TimeOfDay: req.TimeOfDay,
WeeklyDay: pgInt32Ptr(req.WeeklyDay), WeeklyDay: pgInt32Ptr(req.WeeklyDay),
}); err != nil { }); err != nil {
writeErrWithLog(w, h.logger, "admin: update scan schedule", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: update scan schedule", apierror.InternalMsg("update failed", err))
return return
} }
@@ -72,7 +72,7 @@ func (h *handlers) handlePatchScanSchedule(w http.ResponseWriter, r *http.Reques
row, err := q.GetScanSchedule(r.Context()) row, err := q.GetScanSchedule(r.Context())
if err != nil { if err != nil {
writeErrWithLog(w, h.logger, "admin: re-read scan schedule", apierror.Internal(err)) writeErrWithLog(w, h.logger, "admin: re-read scan schedule", apierror.InternalMsg("re-read failed", err))
return return
} }
cfg := library.ScheduleConfig{Mode: row.Mode} cfg := library.ScheduleConfig{Mode: row.Mode}
+1 -1
View File
@@ -69,7 +69,7 @@ func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
return return
} }
h.logger.Error("api: remove track failed", "err", err, "track_id", idStr) h.logger.Error("api: remove track failed", "err", err, "track_id", idStr)
writeErr(w, apierror.Internal(err)) writeErr(w, apierror.InternalMsg("remove failed", err))
return return
} }
+8
View File
@@ -58,6 +58,14 @@ func Internal(cause error) *Error {
return &Error{Status: 500, Code: "server_error", Message: "internal server error", Cause: cause} return &Error{Status: 500, Code: "server_error", Message: "internal server error", Cause: cause}
} }
// InternalMsg returns a 500 Error with a custom user-facing message
// and a cause for logging. Use when the call site has a meaningful
// message that should appear on the wire (e.g. "lookup failed") rather
// than the generic "internal server error" from Internal.
func InternalMsg(message string, cause error) *Error {
return &Error{Status: 500, Code: "server_error", Message: message, Cause: cause}
}
var ( var (
ErrNotFound = &Error{Status: 404, Code: "not_found", Message: "not found"} ErrNotFound = &Error{Status: 404, Code: "not_found", Message: "not found"}
ErrForbidden = &Error{Status: 403, Code: "forbidden", Message: "forbidden"} ErrForbidden = &Error{Status: 403, Code: "forbidden", Message: "forbidden"}
+17
View File
@@ -143,6 +143,23 @@ func TestConstructors_FieldsCorrect(t *testing.T) {
} }
} }
func TestInternalMsg(t *testing.T) {
cause := errors.New("db down")
e := InternalMsg("lookup failed", cause)
if e.Status != 500 {
t.Errorf("Status = %d, want 500", e.Status)
}
if e.Code != "server_error" {
t.Errorf("Code = %q, want server_error", e.Code)
}
if e.Message != "lookup failed" {
t.Errorf("Message = %q, want lookup failed", e.Message)
}
if !errors.Is(e, cause) {
t.Errorf("expected Is(cause) to be true")
}
}
func TestSentinels_Identity(t *testing.T) { func TestSentinels_Identity(t *testing.T) {
wrapped := fmt.Errorf("ctx: %w", ErrNotFound) wrapped := fmt.Errorf("ctx: %w", ErrNotFound)