fix(player): a session superseded before it played is reclaimed, not "couldn't start"
Two session requests for one item in flight at once (a quick double seek; the StrictMode replay of the start effect) let the server's last spawn be the older request's. The newer request's answer then named a session already gone; its playlist 404'd before a manifest parsed, and the player stopped with "Playback couldn't start". The 404 reclaim now covers a session that never played too, bounded by the same budget, and keeps the start's own autoplay.
This commit is contained in:
@@ -248,12 +248,35 @@ describe("an HLS session the server no longer has", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("is a failure, not a restart, when the session never played", async () => {
|
||||
// Otherwise a session that 404s from the start would restart itself in a loop.
|
||||
it("is reclaimed at the asked position, and still starts, when it was lost before it played", async () => {
|
||||
// Two session requests for one item in flight at once (a seek twice in quick succession; the
|
||||
// dev StrictMode replay of the start effect): the server's LAST spawn wins, and it may be the
|
||||
// older request's. The newer request's answer then names a session that is already gone, and
|
||||
// its playlist 404s before a single manifest parsed — which ended playback with "couldn't
|
||||
// start" (Arrow S03E14 on localdev, 2026-09-27). The video is still paused then, so the
|
||||
// reclaim must keep the start's own "play once ready", not read it off the paused element.
|
||||
const play = vi.spyOn(HTMLMediaElement.prototype, "play");
|
||||
const session = mount();
|
||||
await waitFor(() => expect(hlsInstances).toHaveLength(1));
|
||||
act(() => hlsInstances[0]!.emit(Hls.Events.ERROR, lost404));
|
||||
act(() =>
|
||||
hlsInstances[0]!.emit(Hls.Events.ERROR, { ...lost404, details: "manifestLoadError" }),
|
||||
);
|
||||
await waitFor(() => expect(hlsInstances).toHaveLength(2));
|
||||
expect(session).toHaveBeenCalledTimes(2);
|
||||
expect(session.mock.calls[1]![1]).toBe(600);
|
||||
act(() => hlsInstances[1]!.emit(Hls.Events.MANIFEST_PARSED));
|
||||
expect(play).toHaveBeenCalled();
|
||||
expect(screen.queryByText("plex.player.errGeneric")).toBeNull();
|
||||
});
|
||||
|
||||
it("gives up and says so when a session that never played keeps 404ing", async () => {
|
||||
// The reclaim budget is what bounds a server that 404s every session from the start.
|
||||
const session = mount();
|
||||
for (let i = 0; i < 3; i++) {
|
||||
await waitFor(() => expect(hlsInstances).toHaveLength(i + 1));
|
||||
act(() => hlsInstances[i]!.emit(Hls.Events.ERROR, lost404));
|
||||
}
|
||||
await screen.findByText("plex.player.errGeneric");
|
||||
expect(session).toHaveBeenCalledTimes(1);
|
||||
expect(session).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -483,8 +483,8 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
hls.loadSource(`${sess.url}?t=${Math.floor(startAt)}`);
|
||||
hls.attachMedia(video);
|
||||
// Subtitles are native <track>s (not muxed into the HLS), so nothing subtitle-related here.
|
||||
// Whether this session has ever played. Only such a session can be "lost" (below): one that
|
||||
// fails before its manifest is simply a failure, and must not restart itself in a loop.
|
||||
// Whether this session has ever played. A reclaim of one that has not (below) keeps the
|
||||
// start's own `resumePlay`: its <video> is still paused, and that is not the viewer's pause.
|
||||
let parsed = false;
|
||||
hls.on(Hls.Events.MANIFEST_PARSED, () => {
|
||||
parsed = true;
|
||||
@@ -509,13 +509,19 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
// A fatal HLS error (e.g. the remux session died / the file became unreadable) would
|
||||
// otherwise leave the spinner up forever — surface it instead.
|
||||
//
|
||||
// Except a 404 on a session that WAS playing: the server has reaped it (a pause longer than
|
||||
// its idle limit, a deploy) and the stream is simply gone, not broken. hls.js never retries a
|
||||
// 4xx, so this used to end playback with "couldn't play" after the buffer ran out; a new
|
||||
// session at the playhead is what closing and reopening the player did by hand.
|
||||
// Except a 404: the session is gone, not broken. Either the server reaped it (a pause longer
|
||||
// than its idle limit, a deploy) — hls.js never retries a 4xx, so this used to end playback
|
||||
// with "couldn't play" after the buffer ran out, and a new session at the playhead is what
|
||||
// closing and reopening the player did by hand — or it was superseded before it ever played:
|
||||
// two requests for one item in flight at once (a seek twice in quick succession; StrictMode
|
||||
// replaying the start effect in development) and the server's LAST spawn wins, which may be
|
||||
// the older request's. The newer request's answer then names a session that is already
|
||||
// gone, and its playlist 404s before a manifest parsed — "couldn't start" on a file that
|
||||
// plays (Arrow S03E14, 2026-09-27). The server answers a superseded session exactly so the
|
||||
// client asks again; the reclaim is that asking.
|
||||
//
|
||||
// Bounded, because `parsed` cannot bound it: it belongs to ONE hls instance and every restart
|
||||
// builds a fresh one, so it only ever blocks a session that died before its manifest. The
|
||||
// Bounded by the budget below, not by whether the session parsed: `parsed` belongs to ONE hls
|
||||
// instance and every restart builds a fresh one, so it could never bound a loop. The
|
||||
// server also answers 404 for "that segment is not there YET" — indistinguishable from "the
|
||||
// session is gone" on the wire — so a transcode that falls behind realtime, or two viewers of
|
||||
// one item taking turns reclaiming its session, restarted ffmpeg forever behind a spinner
|
||||
@@ -539,7 +545,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
);
|
||||
}
|
||||
if (!data.fatal || !current()) return;
|
||||
if (parsed && data.type === Hls.ErrorTypes.NETWORK_ERROR && data.response?.code === 404) {
|
||||
if (data.type === Hls.ErrorTypes.NETWORK_ERROR && data.response?.code === 404) {
|
||||
const now = Date.now();
|
||||
// A new window: the last loss was long enough ago that this is a fresh incident, not the
|
||||
// same session being lost over and over. The anchor moves with EVERY loss, because with
|
||||
@@ -557,7 +563,10 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
|
||||
// useCallback, which the checker rightly reads as "accessed before it is declared".
|
||||
// The ref (assigned right below the callback) is the file's existing answer to that —
|
||||
// detachSessionRef and cancelAutoSkipRef do the same for the same reason.
|
||||
void loadSessionRef.current(playhead(absRef, loadStartRef), !video.paused);
|
||||
void loadSessionRef.current(
|
||||
playhead(absRef, loadStartRef),
|
||||
parsed ? !video.paused : resumePlay,
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user