diff --git a/internal/api/api.go b/internal/api/api.go index 9935cfc0..1912eece 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -107,6 +107,10 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Delete("/quarantine/{track_id}", h.handleUnflag) authed.Get("/quarantine/mine", h.handleListMyQuarantine) + // Client-reported playback errors (zero-duration tracks, + // load failures). Admin-only inbox; any user can report. + authed.Post("/playback-errors", h.handleReportPlaybackError) + // Self-hosted in-app update channel (#397). Auth-gated to // prevent anonymous bandwidth abuse on the APK stream; // /apk additionally per-user rate-limited. @@ -127,6 +131,9 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Get("/quarantine", h.handleListAdminQuarantine) admin.Post("/quarantine/{track_id}/resolve", h.handleResolveQuarantine) + + admin.Get("/playback-errors", h.handleListAdminPlaybackErrors) + admin.Post("/playback-errors/{id}/resolve", h.handleResolvePlaybackError) admin.Post("/quarantine/{track_id}/delete-file", h.handleDeleteQuarantineFile) admin.Post("/quarantine/{track_id}/delete-via-lidarr", h.handleDeleteQuarantineViaLidarr) admin.Get("/quarantine/actions", h.handleListQuarantineActions) diff --git a/internal/api/playback_errors.go b/internal/api/playback_errors.go new file mode 100644 index 00000000..fb7da802 --- /dev/null +++ b/internal/api/playback_errors.go @@ -0,0 +1,242 @@ +package api + +import ( + "encoding/json" + "errors" + "net/http" + "strconv" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Valid `kind` values. Mirrors the CHECK constraint in migration 0032 +// — keep both lists in sync if you add a kind here, otherwise inserts +// pass the handler whitelist and trip the DB constraint instead. +var validPlaybackErrorKinds = map[string]struct{}{ + "zero_duration": {}, + "load_failed": {}, + "stalled": {}, +} + +// Valid `resolution` values. Hidden / deleted / requested are stamped +// automatically when the admin clicks the matching action from the +// inbox; fixed / ignored are operator-driven (no action taken on the +// track itself). Mirrors the CHECK constraint in migration 0032. +var validPlaybackErrorResolutions = map[string]struct{}{ + "hidden": {}, + "deleted": {}, + "requested": {}, + "fixed": {}, + "ignored": {}, +} + +// reportPlaybackErrorRequest is the body of POST /api/playback-errors. +// TrackID is a UUID string; Kind is one of [validPlaybackErrorKinds]; +// Detail is optional free-text from the client (e.g. ExoPlayer error +// message); ClientID identifies the device/browser so support can +// correlate reports across surfaces. +type reportPlaybackErrorRequest struct { + TrackID string `json:"track_id"` + Kind string `json:"kind"` + Detail *string `json:"detail,omitempty"` + ClientID string `json:"client_id"` +} + +// adminPlaybackErrorView is one row in the admin inbox response. The +// track / album / artist fields are joined so the SPA can render +// without a second round-trip per row. +type adminPlaybackErrorView struct { + ID string `json:"id"` + TrackID string `json:"track_id"` + UserID string `json:"user_id"` + Username string `json:"username"` + ClientID string `json:"client_id"` + Kind string `json:"kind"` + Detail *string `json:"detail,omitempty"` + OccurredAt string `json:"occurred_at"` + ResolvedAt *string `json:"resolved_at,omitempty"` + Resolution *string `json:"resolution,omitempty"` + TrackTitle string `json:"track_title"` + TrackFilePath string `json:"track_file_path"` + ArtistName string `json:"artist_name"` + AlbumTitle string `json:"album_title"` + AlbumID string `json:"album_id"` +} + +// resolvePlaybackErrorRequest is the body of POST +// /api/admin/playback-errors/{id}/resolve. The resolution string is +// validated against [validPlaybackErrorResolutions]. +type resolvePlaybackErrorRequest struct { + Resolution string `json:"resolution"` +} + +// Pagination defaults for the admin list endpoint. 50 fits the +// admin inbox table on a laptop without scrolling; max 200 prevents +// a misbehaving client from asking for the whole table at once. +const ( + defaultPlaybackErrorPageSize = 50 + maxPlaybackErrorPageSize = 200 +) + +// handleReportPlaybackError implements POST /api/playback-errors. +// Any signed-in user can report; the handler validates kind + track +// existence and inserts a row for admin review. +func (h *handlers) handleReportPlaybackError(w http.ResponseWriter, r *http.Request) { + user, ok := requireUser(w, r) + if !ok { + return + } + var req reportPlaybackErrorRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, apierror.BadRequest("bad_request", "invalid JSON body")) + return + } + trackID, ok := parseUUID(req.TrackID) + if !ok { + writeErr(w, apierror.BadRequest("bad_request", "invalid track_id")) + return + } + if _, ok := validPlaybackErrorKinds[req.Kind]; !ok { + writeErr(w, apierror.BadRequest("bad_request", "invalid kind")) + return + } + if req.ClientID == "" { + writeErr(w, apierror.BadRequest("bad_request", "client_id required")) + return + } + if _, err := dbq.New(h.pool).GetTrackByID(r.Context(), trackID); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, &apierror.Error{Status: 404, Code: "not_found", Message: "track not found"}) + return + } + h.logger.Error("api: playback_error: lookup track", "err", err) + writeErr(w, apierror.Internal(err)) + return + } + row, err := dbq.New(h.pool).InsertPlaybackError(r.Context(), dbq.InsertPlaybackErrorParams{ + TrackID: trackID, + UserID: user.ID, + ClientID: req.ClientID, + Kind: req.Kind, + Detail: req.Detail, + }) + if err != nil { + h.logger.Error("api: playback_error: insert", "err", err) + writeErr(w, apierror.Internal(err)) + return + } + writeJSON(w, http.StatusCreated, map[string]string{"id": uuidToString(row.ID)}) +} + +// handleListAdminPlaybackErrors implements GET +// /api/admin/playback-errors?resolved=false&offset=0&limit=50. +func (h *handlers) handleListAdminPlaybackErrors(w http.ResponseWriter, r *http.Request) { + resolved := r.URL.Query().Get("resolved") == "true" + limit := parsePlaybackErrorLimit(r.URL.Query().Get("limit")) + offset := parsePlaybackErrorOffset(r.URL.Query().Get("offset")) + rows, err := dbq.New(h.pool).ListAdminPlaybackErrors(r.Context(), dbq.ListAdminPlaybackErrorsParams{ + ResolvedFilter: resolved, + Off: int32(offset), + Lim: int32(limit), + }) + if err != nil { + h.logger.Error("admin: list playback_errors", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + out := make([]adminPlaybackErrorView, 0, len(rows)) + for _, row := range rows { + view := adminPlaybackErrorView{ + ID: uuidToString(row.ID), + TrackID: uuidToString(row.TrackID), + UserID: uuidToString(row.UserID), + Username: row.Username, + ClientID: row.ClientID, + Kind: row.Kind, + Detail: row.Detail, + OccurredAt: formatTimestamp(row.OccurredAt), + Resolution: row.Resolution, + TrackTitle: row.TrackTitle, + TrackFilePath: row.TrackFilePath, + ArtistName: row.ArtistName, + AlbumTitle: row.AlbumTitle, + AlbumID: uuidToString(row.AlbumID), + } + if row.ResolvedAt.Valid { + ts := formatTimestamp(row.ResolvedAt) + view.ResolvedAt = &ts + } + out = append(out, view) + } + writeJSON(w, http.StatusOK, out) +} + +// handleResolvePlaybackError implements POST +// /api/admin/playback-errors/{id}/resolve. +func (h *handlers) handleResolvePlaybackError(w http.ResponseWriter, r *http.Request) { + admin, ok := auth.UserFromContext(r.Context()) + if !ok { + writeAdminJSONErr(w, http.StatusUnauthorized, "unauthorized") + return + } + id, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + var req resolvePlaybackErrorRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeAdminJSONErr(w, http.StatusBadRequest, "bad_request") + return + } + if _, ok := validPlaybackErrorResolutions[req.Resolution]; !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_resolution") + return + } + resolution := req.Resolution + row, err := dbq.New(h.pool).ResolvePlaybackError(r.Context(), dbq.ResolvePlaybackErrorParams{ + ID: id, + ResolvedBy: admin.ID, + Resolution: &resolution, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeAdminJSONErr(w, http.StatusNotFound, "not_found") + return + } + h.logger.Error("admin: resolve playback_error", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + writeJSON(w, http.StatusOK, map[string]string{"id": uuidToString(row.ID)}) +} + +func parsePlaybackErrorLimit(raw string) int { + if raw == "" { + return defaultPlaybackErrorPageSize + } + n, err := strconv.Atoi(raw) + if err != nil || n <= 0 { + return defaultPlaybackErrorPageSize + } + if n > maxPlaybackErrorPageSize { + return maxPlaybackErrorPageSize + } + return n +} + +func parsePlaybackErrorOffset(raw string) int { + if raw == "" { + return 0 + } + n, err := strconv.Atoi(raw) + if err != nil || n < 0 { + return 0 + } + return n +} diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 230daadc..1856b21a 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -383,6 +383,19 @@ type PlaySession struct { ClientID *string } +type PlaybackError struct { + ID pgtype.UUID + TrackID pgtype.UUID + UserID pgtype.UUID + ClientID string + Kind string + Detail *string + OccurredAt pgtype.Timestamptz + ResolvedAt pgtype.Timestamptz + ResolvedBy pgtype.UUID + Resolution *string +} + type Playlist struct { ID pgtype.UUID UserID pgtype.UUID diff --git a/internal/db/dbq/playback_errors.sql.go b/internal/db/dbq/playback_errors.sql.go new file mode 100644 index 00000000..607441b9 --- /dev/null +++ b/internal/db/dbq/playback_errors.sql.go @@ -0,0 +1,210 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: playback_errors.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const insertPlaybackError = `-- name: InsertPlaybackError :one +INSERT INTO playback_errors (track_id, user_id, client_id, kind, detail) +VALUES ($1, $2, $3, $4, $5) +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution +` + +type InsertPlaybackErrorParams struct { + TrackID pgtype.UUID + UserID pgtype.UUID + ClientID string + Kind string + Detail *string +} + +// Records a client-reported playback failure. The handler validates +// the kind enum before this runs; the CHECK constraint is a +// belt-and-braces guard. +func (q *Queries) InsertPlaybackError(ctx context.Context, arg InsertPlaybackErrorParams) (PlaybackError, error) { + row := q.db.QueryRow(ctx, insertPlaybackError, + arg.TrackID, + arg.UserID, + arg.ClientID, + arg.Kind, + arg.Detail, + ) + var i PlaybackError + err := row.Scan( + &i.ID, + &i.TrackID, + &i.UserID, + &i.ClientID, + &i.Kind, + &i.Detail, + &i.OccurredAt, + &i.ResolvedAt, + &i.ResolvedBy, + &i.Resolution, + ) + return i, err +} + +const listAdminPlaybackErrors = `-- name: ListAdminPlaybackErrors :many +SELECT + pe.id AS id, + pe.track_id AS track_id, + pe.user_id AS user_id, + u.username AS username, + pe.client_id AS client_id, + pe.kind AS kind, + pe.detail AS detail, + pe.occurred_at AS occurred_at, + pe.resolved_at AS resolved_at, + pe.resolution AS resolution, + t.title AS track_title, + t.file_path AS track_file_path, + ar.name AS artist_name, + al.title AS album_title, + al.id AS album_id +FROM playback_errors pe +JOIN users u ON u.id = pe.user_id +JOIN tracks t ON t.id = pe.track_id +JOIN albums al ON al.id = t.album_id +JOIN artists ar ON ar.id = t.artist_id +WHERE ($1::bool AND pe.resolved_at IS NOT NULL) + OR (NOT $1::bool AND pe.resolved_at IS NULL) +ORDER BY pe.occurred_at DESC +LIMIT $3::int OFFSET $2::int +` + +type ListAdminPlaybackErrorsParams struct { + ResolvedFilter bool + Off int32 + Lim int32 +} + +type ListAdminPlaybackErrorsRow struct { + ID pgtype.UUID + TrackID pgtype.UUID + UserID pgtype.UUID + Username string + ClientID string + Kind string + Detail *string + OccurredAt pgtype.Timestamptz + ResolvedAt pgtype.Timestamptz + Resolution *string + TrackTitle string + TrackFilePath string + ArtistName string + AlbumTitle string + AlbumID pgtype.UUID +} + +// Admin inbox query. Returns one row per playback_errors row joined +// with track / album / artist metadata so the SPA can render without +// a second round-trip. Filter by resolved status via the sqlc.arg — +// pass true for the Resolved tab, false for Unresolved (the default). +func (q *Queries) ListAdminPlaybackErrors(ctx context.Context, arg ListAdminPlaybackErrorsParams) ([]ListAdminPlaybackErrorsRow, error) { + rows, err := q.db.Query(ctx, listAdminPlaybackErrors, arg.ResolvedFilter, arg.Off, arg.Lim) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAdminPlaybackErrorsRow + for rows.Next() { + var i ListAdminPlaybackErrorsRow + if err := rows.Scan( + &i.ID, + &i.TrackID, + &i.UserID, + &i.Username, + &i.ClientID, + &i.Kind, + &i.Detail, + &i.OccurredAt, + &i.ResolvedAt, + &i.Resolution, + &i.TrackTitle, + &i.TrackFilePath, + &i.ArtistName, + &i.AlbumTitle, + &i.AlbumID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const resolveAllPlaybackErrorsForTrack = `-- name: ResolveAllPlaybackErrorsForTrack :execrows +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE track_id = $1 AND resolved_at IS NULL +` + +type ResolveAllPlaybackErrorsForTrackParams struct { + TrackID pgtype.UUID + ResolvedBy pgtype.UUID + Resolution *string +} + +// Bulk-resolve every unresolved error for a track when an admin acts +// on it from a non-inbox surface (existing quarantine flow, etc.). +// Returns the affected row count so the caller can surface "N reports +// auto-resolved" in the response if useful. +func (q *Queries) ResolveAllPlaybackErrorsForTrack(ctx context.Context, arg ResolveAllPlaybackErrorsForTrackParams) (int64, error) { + result, err := q.db.Exec(ctx, resolveAllPlaybackErrorsForTrack, arg.TrackID, arg.ResolvedBy, arg.Resolution) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const resolvePlaybackError = `-- name: ResolvePlaybackError :one +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE id = $1 +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution +` + +type ResolvePlaybackErrorParams struct { + ID pgtype.UUID + ResolvedBy pgtype.UUID + Resolution *string +} + +// Marks a single error resolved. Idempotent over (id, resolution) — +// a second resolve with the same resolution is a no-op rewrite. Returns +// the row so the handler can echo it. The CHECK constraint enforces +// the resolution enum. +func (q *Queries) ResolvePlaybackError(ctx context.Context, arg ResolvePlaybackErrorParams) (PlaybackError, error) { + row := q.db.QueryRow(ctx, resolvePlaybackError, arg.ID, arg.ResolvedBy, arg.Resolution) + var i PlaybackError + err := row.Scan( + &i.ID, + &i.TrackID, + &i.UserID, + &i.ClientID, + &i.Kind, + &i.Detail, + &i.OccurredAt, + &i.ResolvedAt, + &i.ResolvedBy, + &i.Resolution, + ) + return i, err +} diff --git a/internal/db/migrations/0032_playback_errors.down.sql b/internal/db/migrations/0032_playback_errors.down.sql new file mode 100644 index 00000000..43ae6b48 --- /dev/null +++ b/internal/db/migrations/0032_playback_errors.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS playback_errors; diff --git a/internal/db/migrations/0032_playback_errors.up.sql b/internal/db/migrations/0032_playback_errors.up.sql new file mode 100644 index 00000000..5f4b8d7e --- /dev/null +++ b/internal/db/migrations/0032_playback_errors.up.sql @@ -0,0 +1,41 @@ +-- Client-reported playback errors. Populated by clients when a track +-- fails to play (zero duration, decode error, etc.); surfaced in the +-- admin /admin/playback-errors inbox so the operator can hide / delete +-- / re-request the offending track. +-- +-- resolved_at + resolved_by + resolution are NULL until an admin acts +-- on the row. Auto-resolved by the client when the admin clicks Hide / +-- Delete / Re-request from the inbox row, with the resolution string +-- recording which path was taken. The partial index keeps the +-- unresolved-queue lookup fast even as resolved history accumulates. +-- +-- CHECK constraints on kind/resolution gate the enum values so client +-- typos can't pollute the data (per the standing rule about +-- enum-CHECK whitelists needing migrations). + +CREATE TABLE playback_errors ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + track_id uuid NOT NULL REFERENCES tracks(id) ON DELETE CASCADE, + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + client_id text NOT NULL, + kind text NOT NULL, + detail text, + occurred_at timestamptz NOT NULL DEFAULT now(), + resolved_at timestamptz, + resolved_by uuid REFERENCES users(id), + resolution text, + CONSTRAINT playback_errors_kind_check + CHECK (kind IN ('zero_duration', 'load_failed', 'stalled')), + CONSTRAINT playback_errors_resolution_check + CHECK (resolution IS NULL OR resolution IN + ('hidden', 'deleted', 'requested', 'fixed', 'ignored')), + CONSTRAINT playback_errors_resolved_consistency + CHECK ((resolved_at IS NULL) = (resolution IS NULL)) +); + +CREATE INDEX idx_playback_errors_unresolved + ON playback_errors (occurred_at DESC) + WHERE resolved_at IS NULL; + +CREATE INDEX idx_playback_errors_track + ON playback_errors (track_id); diff --git a/internal/db/queries/playback_errors.sql b/internal/db/queries/playback_errors.sql new file mode 100644 index 00000000..c89da41f --- /dev/null +++ b/internal/db/queries/playback_errors.sql @@ -0,0 +1,63 @@ +-- name: InsertPlaybackError :one +-- Records a client-reported playback failure. The handler validates +-- the kind enum before this runs; the CHECK constraint is a +-- belt-and-braces guard. +INSERT INTO playback_errors (track_id, user_id, client_id, kind, detail) +VALUES ($1, $2, $3, $4, $5) +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution; + +-- name: ListAdminPlaybackErrors :many +-- Admin inbox query. Returns one row per playback_errors row joined +-- with track / album / artist metadata so the SPA can render without +-- a second round-trip. Filter by resolved status via the sqlc.arg — +-- pass true for the Resolved tab, false for Unresolved (the default). +SELECT + pe.id AS id, + pe.track_id AS track_id, + pe.user_id AS user_id, + u.username AS username, + pe.client_id AS client_id, + pe.kind AS kind, + pe.detail AS detail, + pe.occurred_at AS occurred_at, + pe.resolved_at AS resolved_at, + pe.resolution AS resolution, + t.title AS track_title, + t.file_path AS track_file_path, + ar.name AS artist_name, + al.title AS album_title, + al.id AS album_id +FROM playback_errors pe +JOIN users u ON u.id = pe.user_id +JOIN tracks t ON t.id = pe.track_id +JOIN albums al ON al.id = t.album_id +JOIN artists ar ON ar.id = t.artist_id +WHERE (sqlc.arg(resolved_filter)::bool AND pe.resolved_at IS NOT NULL) + OR (NOT sqlc.arg(resolved_filter)::bool AND pe.resolved_at IS NULL) +ORDER BY pe.occurred_at DESC +LIMIT sqlc.arg(lim)::int OFFSET sqlc.arg(off)::int; + +-- name: ResolvePlaybackError :one +-- Marks a single error resolved. Idempotent over (id, resolution) — +-- a second resolve with the same resolution is a no-op rewrite. Returns +-- the row so the handler can echo it. The CHECK constraint enforces +-- the resolution enum. +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE id = $1 +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution; + +-- name: ResolveAllPlaybackErrorsForTrack :execrows +-- Bulk-resolve every unresolved error for a track when an admin acts +-- on it from a non-inbox surface (existing quarantine flow, etc.). +-- Returns the affected row count so the caller can surface "N reports +-- auto-resolved" in the response if useful. +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE track_id = $1 AND resolved_at IS NULL;