fix(jellycompat): return 404 for HLS segments of a failed transcode (#163)
HandleHLSSegment mapped every non-ErrSegmentNotFound error from the segment-retrieval/recovery path to a generic 500 "Failed to load segment". When a transcode process starts and then exits non-zero, WaitForSegment returns a wrapped playback.ErrTranscodeFailed, which fell through to that 500 — observed in production as repeated 500s on seg_00000.ts that drove an 8x client retry storm and crash-log uploads. The segment will never materialize once its transcode has died, so this is a not-found condition: Jellyfin's DynamicHls handler falls through to a PhysicalFileResult for the absent file, which ASP.NET serves as 404, never 500. Map ErrTranscodeFailed to 404 alongside ErrSegmentNotFound via a small extracted hlsSegmentErrorResponse helper, reserving 500 for genuinely unexpected errors. Adds a unit test pinning the mapping.
This commit is contained in:
@@ -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())
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user