fix(plex): the reclaim budget bounds what it says it bounds, and a refused finish is not performed anyway
`MAX_SESSION_RECLAIMS` was cleared by the first fragment of any session, and a reclaimed session essentially always delivers one — `start_session` does not return until its first segment exists. So the budget only ever bit where a session parsed its manifest and then died before a single fragment, which is not either case its own comment names. Measured: 40 loss cycles produced 41 sessions and never an error. A transcode falling behind realtime, or two viewers of one rating_key taking turns, respawned ffmpeg indefinitely — the round-5 behaviour, still there behind the guard added to stop it. It counts within a rolling window now. Two losses inside two minutes is a session that keeps being lost; one an hour apart is not. And the window is reset per item, because the player stays mounted across a queue advance — otherwise a budget spent on one episode would deny the next its first legitimate reclaim. Separately: `finish`'s rollback re-performed the write it was taking back. On the play-to-the-end path the position it restores IS the end of the media, and the backend reads a position within its finish margin as a finish — so the branch named "take the finish back" set watched, cleared the position and scrobbled to Plex through a second route, while the caller's error toast was already on screen saying the mark had failed. There is no resume point at the end of the media to restore; the cache drop is the whole of the rollback there. The credits-skip caller captures a mid-episode position and still restores it, which is the case that needs it. 7 tests, mutation-checked: the FRAG_LOADED reset, the missing window, and the unguarded rollback each red on the test that names them.
This commit is contained in:
@@ -174,16 +174,19 @@ describe("an HLS session the server no longer has", () => {
|
||||
expect(session).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("starts the budget over once a fragment actually loads", async () => {
|
||||
// A session lost once an hour is not a session that cannot be served; the only proof a reclaim
|
||||
// worked is a fragment arriving.
|
||||
it("keeps giving up even though every reclaimed session plays a fragment first", async () => {
|
||||
// The budget used to be cleared by the first FRAG_LOADED of any session — and a reclaimed
|
||||
// session essentially always delivers one, because `start_session` does not return until its
|
||||
// first segment exists. So it bounded NEITHER case its own comment named: measured at 40 loss
|
||||
// cycles, the player opened 41 sessions and never showed an error (deep-review round 6). A
|
||||
// fragment is not proof the session can be served; surviving the window is.
|
||||
const session = mount();
|
||||
for (let i = 0; i < 4; i++) {
|
||||
const v = () => document.querySelector("video") as Media;
|
||||
for (let i = 0; i < 3; i++) {
|
||||
await waitFor(() => expect(hlsInstances).toHaveLength(i + 1));
|
||||
act(() => hlsInstances[i]!.emit(Hls.Events.MANIFEST_PARSED));
|
||||
const v = document.querySelector("video") as Media;
|
||||
v._t = 700;
|
||||
act(() => void v.dispatchEvent(new Event("timeupdate")));
|
||||
v()._t = 700;
|
||||
act(() => void v().dispatchEvent(new Event("timeupdate")));
|
||||
act(() =>
|
||||
hlsInstances[i]!.emit(Hls.Events.FRAG_LOADED, {
|
||||
frag: { type: "main", duration: 6, stats: { total: 1000 } },
|
||||
@@ -191,8 +194,30 @@ describe("an HLS session the server no longer has", () => {
|
||||
);
|
||||
act(() => hlsInstances[i]!.emit(Hls.Events.ERROR, lost404));
|
||||
}
|
||||
expect(screen.queryByText("plex.player.errGeneric")).toBeNull();
|
||||
expect(session).toHaveBeenCalledTimes(5);
|
||||
await screen.findByText("plex.player.errGeneric");
|
||||
expect(session).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("starts the budget over for a loss long after the last one", async () => {
|
||||
// The other half: a session reaped once an hour is not a session that cannot be served. The
|
||||
// window is what separates the two, so a spent budget must not follow the viewer around.
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true, toFake: ["Date"] });
|
||||
try {
|
||||
const session = mount();
|
||||
const v = () => document.querySelector("video") as Media;
|
||||
for (let i = 0; i < 4; i++) {
|
||||
await waitFor(() => expect(hlsInstances).toHaveLength(i + 1));
|
||||
act(() => hlsInstances[i]!.emit(Hls.Events.MANIFEST_PARSED));
|
||||
v()._t = 700;
|
||||
act(() => void v().dispatchEvent(new Event("timeupdate")));
|
||||
act(() => hlsInstances[i]!.emit(Hls.Events.ERROR, lost404));
|
||||
act(() => void vi.setSystemTime(Date.now() + 10 * 60_000));
|
||||
}
|
||||
expect(screen.queryByText("plex.player.errGeneric")).toBeNull();
|
||||
expect(session).toHaveBeenCalledTimes(5);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("is a failure, not a restart, when the session never played", async () => {
|
||||
|
||||
@@ -371,14 +371,39 @@ describe("leaving an item that was played through", () => {
|
||||
endMedia();
|
||||
await screen.findByText("Title B");
|
||||
await tick();
|
||||
// The position the viewer actually reached is reported, not suppressed by a finish that the
|
||||
// server never confirmed…
|
||||
expect(progress.mock.calls.filter(([id, , , final]) => id === "A" && final)).toEqual([
|
||||
["A", 2995, 3000, true],
|
||||
]);
|
||||
// …and the cached detail is not left claiming watched: only the server is honest now.
|
||||
// The cached detail is not left claiming watched: only the server is honest now.
|
||||
const cached = qc.getQueryData<PlexItemDetail>(qk.plexItem("A", "en"));
|
||||
expect(cached?.status).not.toBe("watched");
|
||||
// And the rollback does NOT write the position back. 2995 of 3000 is inside the backend's
|
||||
// finish margin, so that checkpoint would have set watched / position 0 and scrobbled to Plex
|
||||
// — performing, through a second route, the very write that was just refused, while the error
|
||||
// toast said it had failed (round 6). There is no resume point at the end of the media to
|
||||
// restore; the cache drop is the whole of the rollback here.
|
||||
expect(progress.mock.calls.filter(([id, , , final]) => id === "A" && final)).toEqual([]);
|
||||
});
|
||||
|
||||
it("does restore the position when the refused finish came from the credits skip", async () => {
|
||||
// The other caller of the same helper captures a MID-episode position, and there the rollback
|
||||
// is the point: without it the viewer loses the stretch they watched. One helper, two callers,
|
||||
// and only the near-end one must hold back.
|
||||
prefs({ upNext: false, autoSkipCredits: true, autoSkipDelay: 0 });
|
||||
const withCredits = {
|
||||
...item("A"),
|
||||
markers: [{ type: "credits", start_s: 2000, end_s: 2990 }],
|
||||
} as unknown as PlexItemDetail;
|
||||
const { qc, progress } = mount({
|
||||
details: { A: withCredits, B: item("B") },
|
||||
sessions: { A: () => Promise.resolve(direct("A")), B: () => new Promise(() => {}) },
|
||||
queue: ["A", "B"],
|
||||
});
|
||||
vi.spyOn(api, "plexSetState").mockRejectedValue(new Error("offline"));
|
||||
await playingAt("/media/A", 2010); // inside the credits → auto-skip advances and marks watched
|
||||
await screen.findByText("Title B");
|
||||
await tick();
|
||||
expect(progress.mock.calls.filter(([id, , , final]) => id === "A" && final)).toEqual([
|
||||
["A", 2010, 3000, true],
|
||||
]);
|
||||
expect(qc.getQueryData<PlexItemDetail>(qk.plexItem("A", "en"))?.status).not.toBe("watched");
|
||||
});
|
||||
|
||||
it("records a second viewing when the viewer plays on from the finished frame", async () => {
|
||||
|
||||
@@ -75,10 +75,16 @@ import { pushLayer, removeLayer, anyLayerAbove, escapeHandled } from "../lib/lay
|
||||
// The rich info overlay (poster/cast/ratings) reuses the same component as the card's info page.
|
||||
const PlexInfo = lazy(() => import("./PlexInfo"));
|
||||
|
||||
// Consecutive reclaims of a 404'd HLS session before the player stops and says so. Two covers the
|
||||
// real case (the session was reaped, once) without letting a server that cannot serve the stream
|
||||
// keep the viewer on a spinner (see the ERROR handler).
|
||||
// Reclaims of a 404'd HLS session, within `RECLAIM_WINDOW_MS`, before the player stops and says so.
|
||||
// Two covers the real case (the session was reaped, once) without letting a server that cannot serve
|
||||
// the stream keep the viewer on a spinner (see the ERROR handler).
|
||||
const MAX_SESSION_RECLAIMS = 2;
|
||||
// A ROLLING window, because "consecutive" could not bound anything: the budget used to be cleared by
|
||||
// the first fragment of any session, and a reclaimed session essentially always delivers one —
|
||||
// `start_session` does not return until its first segment exists. So both cases the ERROR handler
|
||||
// names restarted ffmpeg forever anyway (measured: 40 loss cycles → 41 sessions, never an error).
|
||||
// Two losses inside two minutes is a session that keeps being lost; one an hour apart is not.
|
||||
const RECLAIM_WINDOW_MS = 120_000;
|
||||
|
||||
// The Plex module's rich, full-page player. Plays the LOCAL file: direct-playable files stream raw
|
||||
// (native <video>), remux files stream via the seek-restart HLS session (hls.js). The visible
|
||||
@@ -160,9 +166,10 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
|
||||
// Where the current session was ASKED to start — set before the request goes out, so the subtitle
|
||||
// poll has a position before `sessionK` (the session's answer) and before the first timeupdate.
|
||||
const loadStartRef = useRef(0);
|
||||
// How many times in a row this player may reclaim a session the server answered 404 for, before it
|
||||
// stops and says playback failed. Reset by the first fragment that loads (see FRAG_LOADED).
|
||||
const restartsRef = useRef({ n: 0 });
|
||||
// How often this player may reclaim a session the server answered 404 for, before it stops and
|
||||
// says playback failed. `at` is when the window started; the item-start effect zeroes it, so a
|
||||
// budget spent on one episode cannot deny the next one its first legitimate reclaim.
|
||||
const restartsRef = useRef({ n: 0, at: 0 });
|
||||
const [seekableEnd, setSeekableEnd] = useState(0); // absolute end of the loaded region
|
||||
const { fs, toggleFs } = useFullscreen(wrapRef);
|
||||
const [ready, setReady] = useState(false);
|
||||
@@ -376,9 +383,6 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
|
||||
// rolling window starts empty rather than carrying the previous session's numbers over.
|
||||
fragMeterRef.current.reset();
|
||||
hls.on(Hls.Events.FRAG_LOADED, (_e, data) => {
|
||||
// A fragment actually arriving is the only proof a reclaimed session works; the budget is
|
||||
// for a session that keeps being lost, not for one that was lost once an hour.
|
||||
restartsRef.current.n = 0;
|
||||
fragMeterRef.current.add({
|
||||
type: data.frag.type,
|
||||
bytes: data.frag.stats?.total || data.frag.stats?.loaded || 0,
|
||||
@@ -429,6 +433,11 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
|
||||
hls.on(Hls.Events.ERROR, (_e, data) => {
|
||||
if (!data.fatal || !current()) return;
|
||||
if (parsed && 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.
|
||||
if (now - restartsRef.current.at > RECLAIM_WINDOW_MS)
|
||||
restartsRef.current = { n: 0, at: now };
|
||||
if (restartsRef.current.n < MAX_SESSION_RECLAIMS) {
|
||||
restartsRef.current.n += 1;
|
||||
loadSession(playhead(absRef, loadStartRef), !video.paused);
|
||||
@@ -504,6 +513,9 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
|
||||
useEffect(() => {
|
||||
if (!detail || detail.id !== id || startedForRef.current === id) return;
|
||||
startedForRef.current = id;
|
||||
// A fresh item is a fresh incident: the reclaim budget must not carry over from the previous
|
||||
// one (the player stays mounted across a queue advance — `go()` only changes `id`).
|
||||
restartsRef.current = { n: 0, at: 0 };
|
||||
// Restore the per-account audio/subtitle language choice, resolved to THIS item's tracks. Set the
|
||||
// audio ref synchronously so the loadSession below picks the right track on the first request.
|
||||
const aOrd = audioOrdForLang(detail.audio_streams ?? [], prefs.audioLang);
|
||||
|
||||
@@ -21,6 +21,13 @@ import { plexLang } from "../i18n";
|
||||
import { type PlexPlayerPrefs } from "./plexPlayerPrefs";
|
||||
import { PLEX_SCALE, normalizeVolumeIn, stepVolumeIn } from "./playerVolume";
|
||||
|
||||
// How close to the end the backend counts as "finished" (`_FINISH_MARGIN_S` in routes/plex/playback):
|
||||
// a progress checkpoint at or past `duration - this` marks the item watched and clears the position.
|
||||
// Mirrored here because `finish`'s rollback must not accidentally drive that branch. It is a
|
||||
// one-way safety margin, not a shared contract — if the two ever diverge, the worst case is a
|
||||
// rollback that is skipped or a resume point set a few seconds from the end.
|
||||
const FINISH_MARGIN_S = 30;
|
||||
|
||||
// A live wall clock for the top bar, re-rendered every 15s (minute resolution is enough for the
|
||||
// HH:MM display, and a coarse tick keeps the whole player from re-rendering each second).
|
||||
export function useWallClock(): Date {
|
||||
@@ -186,7 +193,15 @@ export function usePlexProgress(opts: {
|
||||
const dur = durationRef.current;
|
||||
void confirmed?.catch(() => {
|
||||
finished = false;
|
||||
if (pos > 0 && dur > 0) api.plexProgress(id, pos, dur, true).catch(() => {});
|
||||
// Restore the resume point the finish replaced — but ONLY where there is one to restore.
|
||||
// On the play-to-the-end path `pos` IS the end of the media, and the backend reads a
|
||||
// position within `_FINISH_MARGIN_S` of the duration as a finish: it would set watched,
|
||||
// clear the position and scrobble to Plex. So the branch named "take the finish back"
|
||||
// performed, through a second route, exactly the write that had just been refused — while
|
||||
// the caller's error toast was already on screen saying it had failed (round 6). The
|
||||
// credits-skip caller captures a mid-episode position and is unaffected.
|
||||
if (pos > 0 && dur > 0 && pos <= dur - FINISH_MARGIN_S)
|
||||
api.plexProgress(id, pos, dur, true).catch(() => {});
|
||||
// `settle` may already have patched the detail to watched / 0 on the way out. Dropping that
|
||||
// entry rather than invalidating it: an invalidated query with no observer keeps serving its
|
||||
// stale data, and this one is a lie — no data is better, the next open fetches the truth.
|
||||
|
||||
Reference in New Issue
Block a user