diff --git a/internal/api/handlers/stream.go b/internal/api/handlers/stream.go index 83225a27..0c2a4264 100644 --- a/internal/api/handlers/stream.go +++ b/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 } } diff --git a/internal/api/handlers/stream_test.go b/internal/api/handlers/stream_test.go index 5c727692..11d89759 100644 --- a/internal/api/handlers/stream_test.go +++ b/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{ diff --git a/internal/api/handlers/subtitle_search_test.go b/internal/api/handlers/subtitle_search_test.go index 1490e9b6..7e8f4580 100644 --- a/internal/api/handlers/subtitle_search_test.go +++ b/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 }