From 2abd4ffcb66b106981e2e72259f6ef77736dbd1d Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Sun, 27 Sep 2026 20:22:43 +0200
Subject: [PATCH] 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.
---
 .../components/PlexPlayer.hls.dom.test.tsx    | 31 ++++++++++++++++---
 frontend/src/components/PlexPlayer.tsx        | 29 +++++++++++------
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/frontend/src/components/PlexPlayer.hls.dom.test.tsx b/frontend/src/components/PlexPlayer.hls.dom.test.tsx
index 1e83fb9e..7377fa7a 100644
--- a/frontend/src/components/PlexPlayer.hls.dom.test.tsx
+++ b/frontend/src/components/PlexPlayer.hls.dom.test.tsx
@@ -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);
   });
 });
diff --git a/frontend/src/components/PlexPlayer.tsx b/frontend/src/components/PlexPlayer.tsx
index 1acf94aa..2d1fbea6 100644
--- a/frontend/src/components/PlexPlayer.tsx
+++ b/frontend/src/components/PlexPlayer.tsx
@@ -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;
             }
           }