fix(api): surface ListDownloadedSubtitles errors instead of masking as 404 (#256)

The web subtitle handler wrapped the downloaded-subtitle lookup in
`if err == nil { ... }`, so a real DB/backing-store failure from
ListDownloadedSubtitles was silently swallowed and control fell through
to a generic 404 ("Subtitle track not found") with no server-side signal.
This made genuine internal failures look like an intermittent client-side
"subtitles won't render" bug and left nothing in the logs to diagnose.

On error, log at ERROR (with file_id/track/error, matching the sibling
font-extraction path) and return 500 — mirroring the neighbouring error
handling in the same branch (S3 GetObject -> 502, ConvertToVTT -> 500).
The genuine not-found fall-through is preserved for the case where the
listing succeeds but the requested index is out of range.

Closes #248

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

internal/api/handlers/stream.go

@@ -240,37 +240,49 @@ func (h *StreamHandler) HandleSubtitle(w http.ResponseWriter, r *http.Request) {
 	// Check downloaded subtitles (from S3).
 	if h.SubtitleRepo != nil && h.S3Client != nil {
 		downloaded, err := h.SubtitleRepo.ListDownloadedSubtitles(r.Context(), file.ID)
-		if err == nil {
-			downloadedIndex := embeddedIndex - len(file.SubtitleTracks)
-			if downloadedIndex >= 0 && downloadedIndex < len(downloaded) {
-				dl := downloaded[downloadedIndex]
-				data, err := h.S3Client.GetObject(r.Context(), h.S3Bucket, dl.S3Key)
-				if err != nil {
-					writeError(w, http.StatusBadGateway, "s3_error", "Failed to load subtitle from storage")
-					return
-				}
+		if err != nil {
+			// A DB failure here must not masquerade as "track not found":
+			// surface it as an internal error (with a server-side signal)
+			// so the real failure is diagnosable instead of looking like an
+			// intermittent 404 to the client.
+			slog.ErrorContext(r.Context(), "list downloaded subtitles failed",
+				"file_id", file.ID,
+				"track", trackIndex,
+				"error", err,
+			)
+			writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list downloaded subtitles")
+			return
+		}
 
-				// Serve ASS/SSA downloaded subtitles as raw data.
-				if playback.IsASS(string(dl.Format)) {
-					playback.ServeSubtitle(w, data, "ass")
-					return
-				}
-
-				// If the subtitle is already VTT, serve directly.
-				if dl.Format == subtitles.FormatVTT {
-					playback.ServeSubtitle(w, data, "vtt")
-					return
-				}
-
-				// Convert to VTT using the playback conversion pipeline.
-				vttData, err := playback.ConvertToVTT(data, string(dl.Format))
-				if err != nil {
-					writeError(w, http.StatusInternalServerError, "convert_error", "Failed to convert subtitle")
-					return
-				}
-				playback.ServeSubtitle(w, vttData, "vtt")
+		downloadedIndex := embeddedIndex - len(file.SubtitleTracks)
+		if downloadedIndex >= 0 && downloadedIndex < len(downloaded) {
+			dl := downloaded[downloadedIndex]
+			data, err := h.S3Client.GetObject(r.Context(), h.S3Bucket, dl.S3Key)
+			if err != nil {
+				writeError(w, http.StatusBadGateway, "s3_error", "Failed to load subtitle from storage")
 				return
 			}
+
+			// Serve ASS/SSA downloaded subtitles as raw data.
+			if playback.IsASS(string(dl.Format)) {
+				playback.ServeSubtitle(w, data, "ass")
+				return
+			}
+
+			// If the subtitle is already VTT, serve directly.
+			if dl.Format == subtitles.FormatVTT {
+				playback.ServeSubtitle(w, data, "vtt")
+				return
+			}
+
+			// Convert to VTT using the playback conversion pipeline.
+			vttData, err := playback.ConvertToVTT(data, string(dl.Format))
+			if err != nil {
+				writeError(w, http.StatusInternalServerError, "convert_error", "Failed to convert subtitle")
+				return
+			}
+			playback.ServeSubtitle(w, vttData, "vtt")
+			return
 		}
 	}
 

internal/api/handlers/stream_test.go

@@ -2,12 +2,15 @@ package handlers
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"testing"
 
+	"github.com/go-chi/chi/v5"
+
 	"github.com/Silo-Server/silo-server/internal/models"
 	"github.com/Silo-Server/silo-server/internal/playback"
 )
@@ -121,6 +124,55 @@ func TestHandleStream_KeepsSessionWhenLookupFailsForNonMissingReason(t *testing.
 	}
 }
 
+// TestHandleSubtitle_ListDownloadedSubtitlesErrorReturns500 pins the fix for
+// issue #248: a failure listing downloaded subtitles must surface as a 500 with
+// an "internal_error" code, not be swallowed and reported to the client as a
+// generic "Subtitle track not found" 404 (which made a real backing-store
+// failure look like an intermittent client-side subtitle bug).
+func TestHandleSubtitle_ListDownloadedSubtitlesErrorReturns500(t *testing.T) {
+	// No external or embedded tracks, so track index 0 falls through to the
+	// downloaded-subtitle branch that queries the repository.
+	file := &models.MediaFile{
+		ID:        42,
+		ContentID: "movie-1",
+		FilePath:  "/tmp/movie.mkv",
+		Duration:  3600,
+	}
+	baseMgr := playback.NewSessionManager(0, 0)
+	session, err := baseMgr.StartSession(1, "profile-1", 42, playback.PlayDirect, false)
+	if err != nil {
+		t.Fatalf("StartSession: %v", err)
+	}
+
+	handler := NewStreamHandler(baseMgr, testPlaybackFileResolver{file: file})
+	handler.SubtitleRepo = &handlerMockSubtitleRepo{listErr: errors.New("db unavailable")}
+	handler.S3Client = newMockS3ClientForHandler()
+	handler.S3Bucket = "test-bucket"
+
+	req := httptest.NewRequest(http.MethodGet, "/api/v1/stream/"+session.ID+"/subtitles/0.vtt", nil)
+	req = req.WithContext(newAuthorizedPlaybackContext())
+	routeCtx := chi.NewRouteContext()
+	routeCtx.URLParams.Add("session_id", session.ID)
+	routeCtx.URLParams.Add("track", "0.vtt")
+	req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, routeCtx))
+
+	rr := httptest.NewRecorder()
+	handler.HandleSubtitle(rr, req)
+
+	if rr.Code != http.StatusInternalServerError {
+		t.Fatalf("status = %d, body = %s", rr.Code, rr.Body.String())
+	}
+	var body struct {
+		Error string `json:"error"`
+	}
+	if err := json.Unmarshal(rr.Body.Bytes(), &body); err != nil {
+		t.Fatalf("decode error body: %v (body = %s)", err, rr.Body.String())
+	}
+	if body.Error != "internal_error" {
+		t.Fatalf("error code = %q, want %q (body = %s)", body.Error, "internal_error", rr.Body.String())
+	}
+}
+
 func TestHandleTransportStartFailure_KeepsSessionForNonMissingError(t *testing.T) {
 	filePath := writePlaybackTestMediaFile(t, "movie.mkv")
 	file := &models.MediaFile{

internal/api/handlers/subtitle_search_test.go

@@ -239,6 +239,9 @@ type handlerMockSubtitleRepo struct {
 	subtitles map[int]*subtitles.DownloadedSubtitle
 	nextID    int
 	byKey     map[string]*subtitles.DownloadedSubtitle
+	// listErr, when set, is returned by ListDownloadedSubtitles to simulate a
+	// backing-store failure.
+	listErr error
 }
 
 func newMockSubtitleRepoForHandler() *handlerMockSubtitleRepo {
@@ -265,6 +268,9 @@ func (m *handlerMockSubtitleRepo) GetDownloadedSubtitle(_ context.Context, id in
 }
 
 func (m *handlerMockSubtitleRepo) ListDownloadedSubtitles(context.Context, int) ([]subtitles.DownloadedSubtitle, error) {
+	if m.listErr != nil {
+		return nil, m.listErr
+	}
 	return nil, nil
 }