diff --git a/internal/jellycompat/streams.go b/internal/jellycompat/streams.go index 6dd30775..0386a399 100644 --- a/internal/jellycompat/streams.go +++ b/internal/jellycompat/streams.go @@ -458,11 +458,8 @@ func (h *PlaybackHandler) HandleHLSSegment(w http.ResponseWriter, r *http.Reques } } if err != nil { - if errors.Is(err, playback.ErrSegmentNotFound) { - writeError(w, http.StatusNotFound, "NotFound", "Segment not found") - return - } - writeError(w, http.StatusInternalServerError, "ServerError", "Failed to load segment") + status, code, message := hlsSegmentErrorResponse(err) + writeError(w, status, code, message) return } @@ -473,6 +470,23 @@ func (h *PlaybackHandler) HandleHLSSegment(w http.ResponseWriter, r *http.Reques http.ServeFile(w, r, segmentPath) } +// hlsSegmentErrorResponse maps a segment-retrieval error to a Jellyfin-faithful +// HTTP status. A segment that is absent (ErrSegmentNotFound) or whose transcode +// process started and then exited non-zero (ErrTranscodeFailed, surfaced by +// WaitForSegment after the recovery/restart path is exhausted) will never +// materialize. Jellyfin serves both as 404: its DynamicHls segment handler falls +// through to a PhysicalFileResult for the missing file, which ASP.NET returns as +// 404, never 500. Reserve 500 for genuinely unexpected errors (e.g. a stat +// failure on a file that does exist). +func hlsSegmentErrorResponse(err error) (status int, code, message string) { + switch { + case errors.Is(err, playback.ErrSegmentNotFound), errors.Is(err, playback.ErrTranscodeFailed): + return http.StatusNotFound, "NotFound", "Segment not found" + default: + return http.StatusInternalServerError, "ServerError", "Failed to load segment" + } +} + // HandleSubtitleStream proxies subtitle requests through the native stream subtitle route. func (h *PlaybackHandler) HandleSubtitleStream(w http.ResponseWriter, r *http.Request) { session := SessionFromContext(r.Context()) diff --git a/internal/jellycompat/streams_segment_error_test.go b/internal/jellycompat/streams_segment_error_test.go new file mode 100644 index 00000000..2d789738 --- /dev/null +++ b/internal/jellycompat/streams_segment_error_test.go @@ -0,0 +1,50 @@ +package jellycompat + +import ( + "errors" + "fmt" + "net/http" + "testing" + + "github.com/Silo-Server/silo-server/internal/playback" +) + +// TestHLSSegmentErrorResponse pins the never-500 contract for the HLS segment +// handler: a segment that will never materialize (absent, or whose transcode +// process started then died) maps to 404 like Jellyfin, and only genuinely +// unexpected errors keep the 500. +func TestHLSSegmentErrorResponse(t *testing.T) { + tests := []struct { + name string + err error + wantStatus int + wantCode string + }{ + {"segment not found", playback.ErrSegmentNotFound, http.StatusNotFound, "NotFound"}, + {"transcode failed", playback.ErrTranscodeFailed, http.StatusNotFound, "NotFound"}, + { + // WaitForSegment wraps the ffmpeg exit error as + // fmt.Errorf("%w: %v", ErrTranscodeFailed, waitErr) — this is exactly the + // error that hit the catch-all 500 in production (Fire TV, seg_00000.ts). + name: "wrapped transcode failed", + err: fmt.Errorf("%w: exit status 1", playback.ErrTranscodeFailed), + wantStatus: http.StatusNotFound, + wantCode: "NotFound", + }, + { + name: "unexpected error stays 500", + err: errors.New("stat segment: permission denied"), + wantStatus: http.StatusInternalServerError, + wantCode: "ServerError", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + status, code, _ := hlsSegmentErrorResponse(tt.err) + if status != tt.wantStatus || code != tt.wantCode { + t.Fatalf("hlsSegmentErrorResponse(%v) = (%d, %q), want (%d, %q)", + tt.err, status, code, tt.wantStatus, tt.wantCode) + } + }) + } +}