fix(studio): a thumbnail reader forgets a start's failures after a minute (#6642)

* fix(studio): a thumbnail reader forgets a start's failures after a minute A pooled thumbnail reader lives for as long as clips keep asking it, and each start byte has a retry budget of two. That budget never reset, so two transient failures at one start early in a session made a third, twenty minutes later after the cache evicted it, give up at once: the reader retired and the clip showed an error. A start's failures now count for a minute. One bout of retries (8 s stalls, 0.5 s and 1 s waits) fits inside it, so a start that keeps failing still gives up; one met again much later starts afresh. Clearing the count when a read from that start delivers bytes would not do: a server that ignores Range sends bytes 0 onward again before it breaks at the same byte, and would retry forever. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(studio): the start-failure window says what it is in its name, not a comment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(studio): a start's retry budget resets when a read from it ends cleanly, not after a minute The one-minute window only held while each attempt took under a minute. An attempt has no limit before its answer, and a body that keeps sending never stalls, so a server that ignores Range, sends slowly and breaks past a minute re-downloaded from 0 forever. A start's failures are now forgotten when a read from it ends cleanly: its body finishes, or mediabunny cancels it at its target. A server that breaks before the target never ends a read cleanly, so its budget still runs out, however slowly it fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(studio): a read mediabunny stops at its target ends cleanly and resets its start's budget mediabunny's UrlSource never cancels the body: once a read reaches its target, _runWorker aborts the signal it passed in, and the pending pull fails with an AbortError. So the reset on cancel never fired, and any open bytes=N- read stopped at its target kept its start's failures. The request now listens on its own signal, which follows mediabunny's and is unlinked once the read ends: an abort while live counts as a clean end only when mediabunny's signal is the one aborted, never a stall or a dispose. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-30 09:16:55 +00:00 · 2026-10-28 10:15:28 +00:00
parent 0e32ee9b56
commit a88751d9d6
2 changed files with 114 additions and 2 deletions
@@ -498,6 +498,112 @@ describe("mediaUrlSourceOptions", () => {
    expect(repeated).toEqual([0.5, 1, null]);
  });

+  it("forgets a start's failures once a read from it ends cleanly, so a long-lived reader meeting it again retries", async () => {
+    const headers = { "Content-Range": "bytes 5000-6023/1000000" };
+    vi.stubGlobal(
+      "fetch",
+      vi
+        .fn()
+        .mockRejectedValueOnce(new TypeError("Failed to fetch"))
+        .mockRejectedValueOnce(new TypeError("Failed to fetch"))
+        .mockResolvedValueOnce(new Response(new Uint8Array(1024), { status: 206, headers }))
+        .mockRejectedValue(new TypeError("Failed to fetch")),
+    );
+    const { fetchFn, getRetryDelay } = mediaUrlSourceOptions("thumbnail");
+    const read5000 = () => fetchFn(URL_WITH_PATH, { headers: { Range: "bytes=5000-" } });
+    const failAt5000 = async () => getRetryDelay(1, await read5000().catch((e: unknown) => e));
+    const early = [await failAt5000(), await failAt5000()];
+    await (await read5000()).arrayBuffer();
+    const later = [await failAt5000(), await failAt5000(), await failAt5000()];
+
+    expect(early).toEqual([0.5, 1]);
+    expect(later).toEqual([0.5, 1, null]);
+  });
+
+  it("forgets a start's failures when mediabunny stops a read from it at its target", async () => {
+    const headers = { "Content-Range": "bytes 5000-999999/1000000" };
+    const endless = () =>
+      new ReadableStream<Uint8Array>({ pull: (c) => c.enqueue(new Uint8Array(4096)) });
+    vi.stubGlobal(
+      "fetch",
+      vi
+        .fn()
+        .mockRejectedValueOnce(new TypeError("Failed to fetch"))
+        .mockRejectedValueOnce(new TypeError("Failed to fetch"))
+        .mockResolvedValueOnce(new Response(endless(), { status: 206, headers }))
+        .mockRejectedValue(new TypeError("Failed to fetch")),
+    );
+    const { fetchFn, getRetryDelay } = mediaUrlSourceOptions("thumbnail");
+    const read5000 = (signal?: AbortSignal) =>
+      fetchFn(URL_WITH_PATH, { headers: { Range: "bytes=5000-" }, signal });
+    const failAt5000 = async () => getRetryDelay(1, await read5000().catch((e: unknown) => e));
+    const early = [await failAt5000(), await failAt5000()];
+    // what mediabunny's _runWorker does once a read reaches its target: abort the signal it passed
+    const target = new AbortController();
+    const reader = (await read5000(target.signal)).body!.getReader();
+    await reader.read();
+    target.abort();
+    await reader.read().catch(() => {});
+    const later = [await failAt5000(), await failAt5000(), await failAt5000()];
+
+    expect(early).toEqual([0.5, 1]);
+    expect(later).toEqual([0.5, 1, null]);
+  });
+
+  it("keeps a start's failures when its read broke before mediabunny stopped it", async () => {
+    const headers = { "Content-Range": "bytes 5000-999999/1000000" };
+    let sent = false;
+    const breaking = new ReadableStream<Uint8Array>({
+      pull(controller) {
+        if (sent) return controller.error(new TypeError("network error"));
+        sent = true;
+        controller.enqueue(new Uint8Array(4096));
+      },
+    });
+    vi.stubGlobal(
+      "fetch",
+      vi
+        .fn()
+        .mockResolvedValueOnce(new Response(breaking, { status: 206, headers }))
+        .mockRejectedValue(new TypeError("Failed to fetch")),
+    );
+    const { fetchFn, getRetryDelay } = mediaUrlSourceOptions("thumbnail");
+    const target = new AbortController();
+    const reader = (
+      await fetchFn(URL_WITH_PATH, { headers: { Range: "bytes=5000-" }, signal: target.signal })
+    ).body!.getReader();
+    await reader.read();
+    const broken = await reader.read().catch((e: unknown) => e);
+    const delays = [getRetryDelay(1, broken)];
+    target.abort();
+    for (let i = 0; i < 2; i++) {
+      const failure = await fetchFn(URL_WITH_PATH, { headers: { Range: "bytes=5000-" } }).catch(
+        (e: unknown) => e,
+      );
+      delays.push(getRetryDelay(1, failure));
+    }
+
+    expect(delays).toEqual([0.5, 1, null]);
+  });
+
+  it("keeps counting a start's failures however far apart they come, while no read from it ends", async () => {
+    vi.useFakeTimers({ now: Date.UTC(2026, 9, 28, 9, 0, 0) });
+    vi.stubGlobal("fetch", vi.fn().mockRejectedValue(new TypeError("Failed to fetch")));
+    const { fetchFn, getRetryDelay } = mediaUrlSourceOptions("thumbnail");
+    const delays = [];
+    for (let i = 0; i < 3; i++) {
+      const failure = await fetchFn(URL_WITH_PATH, { headers: { Range: "bytes=5000-" } }).catch(
+        (e: unknown) => e,
+      );
+      delays.push(getRetryDelay(1, failure));
+      // a slow server streaming for minutes before it breaks
+      vi.setSystemTime(Date.now() + 20 * 60_000);
+    }
+    vi.useRealTimers();
+
+    expect(delays).toEqual([0.5, 1, null]);
+  });
+
  it("does not retry a client error status", () => {
    const failure = new MediaFetchError("probe", 404, null);

@@ -71,7 +71,9 @@ const typedFailure = (reader: MediaReader, error: unknown, start: number) =>
 * A stalled body ignores every signal, so a silent one is aborted. The wait for the answer is not
 * timed: it includes Chrome's queue for a connection, which streaming previews can hold for long.
 */
-function watchedRequest(reader: MediaReader, parents: readonly AbortSignal[]) {
+type CleanEnd = { ended: (start: number) => void; atTarget: AbortSignal | null | undefined };
+
+function watchedRequest(reader: MediaReader, parents: readonly AbortSignal[], end?: CleanEnd) {
  const controller = new AbortController();
  let stalled = false;
  let timer: ReturnType<typeof setTimeout> | undefined;
@@ -102,7 +104,7 @@ function watchedRequest(reader: MediaReader, parents: readonly AbortSignal[]) {
    stalled
      ? new MediaFetchError(reader, null, "stalled", start)
      : typedFailure(reader, error, start);
-  return { signal: controller.signal, wait, done, failure };
+  return { signal: controller.signal, wait, done, failure, end };
 }

 type WatchedRequest = ReturnType<typeof watchedRequest>;
@@ -114,12 +116,15 @@ function typedBody(
  request: WatchedRequest,
 ): ReadableStream<Uint8Array> {
  const source = body.getReader();
+  const stopped = () => request.end?.atTarget?.aborted && request.end.ended(start);
+  request.signal.addEventListener("abort", stopped, { once: true });
  return new ReadableStream<Uint8Array>({
    async pull(controller) {
      try {
        const { done, value } = await request.wait(source.read());
        if (!done) return controller.enqueue(value);
        request.done();
+        request.end?.ended(start);
        controller.close();
      } catch (error) {
        request.done();
@@ -178,6 +183,7 @@ export function mediaUrlSourceOptions(reader: MediaReader) {
    const request = watchedRequest(
      reader,
      init?.signal ? [inFlightReads.signal, init.signal] : [inFlightReads.signal],
+      { ended: (start) => failuresByStart.delete(start), atTarget: init?.signal },
    );
    const response = await typedFetch(reader, input, sent ?? {}, asked, request);
    if (ranges !== "hidden") ranges = rangesReadable(sent, response) ? "readable" : "hidden";