From dfd2028f16381c1c52c8d8e6483b92ec8c2ca5fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Mon, 26 Oct 2026 21:08:46 +0000 Subject: [PATCH] fix(studio): a clip that cannot be read gives up after two retries with one typed line (#6477) * fix(studio): a clip that cannot be read gives up after two retries with one typed line * fix(studio): a clip whose stream breaks mid-read also fails with the typed line --- .../studio/src/player/lib/mediaFetch.test.ts | 163 ++++++++++++++++++ packages/studio/src/player/lib/mediaFetch.ts | 98 +++++++++++ .../studio/src/player/lib/mediaProbe.test.ts | 22 ++- packages/studio/src/player/lib/mediaProbe.ts | 6 +- .../player/lib/thumbnailVideoDecoder.test.ts | 25 ++- .../src/player/lib/thumbnailVideoDecoder.ts | 4 +- 6 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 packages/studio/src/player/lib/mediaFetch.test.ts create mode 100644 packages/studio/src/player/lib/mediaFetch.ts diff --git a/packages/studio/src/player/lib/mediaFetch.test.ts b/packages/studio/src/player/lib/mediaFetch.test.ts new file mode 100644 index 0000000000..0028ac8939 --- /dev/null +++ b/packages/studio/src/player/lib/mediaFetch.test.ts @@ -0,0 +1,163 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { MediaFetchError, mediaUrlSourceOptions, reportMediaFetchFailure } from "./mediaFetch"; + +const URL_WITH_PATH = "http://127.0.0.1:5190/api/projects/demo/preview/clips/take-1.mp4"; + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("mediaUrlSourceOptions", () => { + it("turns a network failure into a typed error without the URL", async () => { + vi.stubGlobal("fetch", vi.fn().mockRejectedValue(new TypeError("Failed to fetch"))); + const { fetchFn } = mediaUrlSourceOptions("probe"); + + const error = await fetchFn(URL_WITH_PATH).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(MediaFetchError); + expect(error).toMatchObject({ reader: "probe", status: null, code: "TypeError" }); + expect(String((error as Error).message)).not.toContain("take-1"); + }); + + it("names the network code when the failure carries one", async () => { + const failure = Object.assign(new TypeError("fetch failed"), { cause: { code: "EBUSY" } }); + vi.stubGlobal("fetch", vi.fn().mockRejectedValue(failure)); + + const error = await mediaUrlSourceOptions("thumbnail") + .fetchFn(URL_WITH_PATH) + .catch((e) => e); + + expect(error).toMatchObject({ reader: "thumbnail", code: "EBUSY" }); + }); + + it("turns an HTTP error status into a typed error", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response("", { status: 403 }))); + + const error = await mediaUrlSourceOptions("probe") + .fetchFn(URL_WITH_PATH) + .catch((e) => e); + + expect(error).toMatchObject({ + status: 403, + message: "probe range fetch failed: HTTP 403 after 0 retries", + }); + }); + + it("lets a cancellation through untouched", async () => { + const abort = new DOMException("Aborted", "AbortError"); + vi.stubGlobal("fetch", vi.fn().mockRejectedValue(abort)); + + await expect(mediaUrlSourceOptions("probe").fetchFn(URL_WITH_PATH)).rejects.toBe(abort); + }); + + it("retries a network failure twice, then gives up and records the retries", () => { + const { getRetryDelay } = mediaUrlSourceOptions("probe"); + const failure = new MediaFetchError("probe", null, "TypeError"); + + expect([1, 2, 3].map((attempt) => getRetryDelay(attempt, failure))).toEqual([0.5, 1, null]); + expect(failure).toMatchObject({ + retries: 2, + message: "probe range fetch failed: network TypeError after 2 retries", + }); + }); + + it("gives up on a body that breaks mid-read with the typed error, through a real mediabunny read", async () => { + const brokenBody = () => + new Response( + new ReadableStream({ + pull: (controller) => controller.error(new TypeError("network error")), + }), + { + status: 206, + headers: { "Content-Range": "bytes 0-65535/1000000", "Content-Length": "65536" }, + }, + ); + vi.stubGlobal( + "fetch", + vi.fn(async () => brokenBody()), + ); + vi.useFakeTimers(); + const mb = await import("mediabunny"); + const runnerListeners = process.listeners("unhandledRejection"); + const unhandled: unknown[] = []; + process.removeAllListeners("unhandledRejection"); + process.on("unhandledRejection", (reason) => unhandled.push(reason)); + try { + const input = new mb.Input({ + source: new mb.UrlSource(URL_WITH_PATH, mediaUrlSourceOptions("thumbnail")), + formats: mb.ALL_FORMATS, + }); + const read = input.getDurationFromMetadata().catch((e: unknown) => e); + await vi.runAllTimersAsync(); + const error = await read; + input.dispose(); + await vi.runAllTimersAsync(); + + expect(error).toMatchObject({ + name: "MediaFetchError", + message: "thumbnail range fetch failed: network TypeError after 2 retries", + }); + for (const reason of unhandled) expect(reason).toBeInstanceOf(MediaFetchError); + } finally { + vi.useRealTimers(); + process.removeAllListeners("unhandledRejection"); + for (const listener of runnerListeners) process.on("unhandledRejection", listener); + } + }); + + it("counts mid-stream read failures, which always arrive as attempt 1", () => { + const { getRetryDelay } = mediaUrlSourceOptions("thumbnail"); + const readError = new TypeError("network error"); + + expect([1, 1, 1].map((attempt) => getRetryDelay(attempt, readError))).toEqual([0.5, 1, null]); + }); + + it("does not retry a client error status", () => { + const failure = new MediaFetchError("probe", 404, null); + + expect(mediaUrlSourceOptions("probe").getRetryDelay(1, failure)).toBeNull(); + expect(failure.retries).toBe(0); + }); + + // mediabunny rethrows a failed read-ahead nobody awaits "so it doesn't get swallowed": that copy is typed too. + it("fails a real mediabunny read, and its unawaited read-ahead, with the typed error", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response("", { status: 404 }))); + const mb = await import("mediabunny"); + const runnerListeners = process.listeners("unhandledRejection"); + const unhandled: unknown[] = []; + process.removeAllListeners("unhandledRejection"); + process.on("unhandledRejection", (reason) => unhandled.push(reason)); + try { + const input = new mb.Input({ + source: new mb.UrlSource(URL_WITH_PATH, mediaUrlSourceOptions("probe")), + formats: mb.ALL_FORMATS, + }); + const error = await input.getDurationFromMetadata().catch((e: unknown) => e); + input.dispose(); + await new Promise(setImmediate); + + expect(error).toMatchObject({ name: "MediaFetchError", status: 404, retries: 0 }); + expect(unhandled.length).toBeGreaterThan(0); + for (const reason of unhandled) expect(reason).toBeInstanceOf(MediaFetchError); + } finally { + process.removeAllListeners("unhandledRejection"); + for (const listener of runnerListeners) process.on("unhandledRejection", listener); + } + }); +}); + +describe("reportMediaFetchFailure", () => { + it("writes one scrubbed line for a typed failure and nothing for anything else", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const failure = new MediaFetchError("thumbnail", null, "TypeError"); + failure.setRetries(2); + + reportMediaFetchFailure(failure); + reportMediaFetchFailure(new Error("Video source has no decodable video track")); + + expect(warn.mock.calls).toEqual([ + ["[Studio] thumbnail range fetch failed: network TypeError after 2 retries"], + ]); + }); +}); diff --git a/packages/studio/src/player/lib/mediaFetch.ts b/packages/studio/src/player/lib/mediaFetch.ts new file mode 100644 index 0000000000..99cbe4dda0 --- /dev/null +++ b/packages/studio/src/player/lib/mediaFetch.ts @@ -0,0 +1,98 @@ +export type MediaReader = "probe" | "thumbnail"; + +const describe = ( + reader: MediaReader, + status: number | null, + code: string | null, + retries: number, +) => + `${reader} range fetch failed: ${status === null ? `network ${code}` : `HTTP ${status}`} after ${retries} retries`; + +/** A media range fetch that gave up: which reader, the HTTP status or network code, and the retries. Never the URL. */ +export class MediaFetchError extends Error { + override readonly name = "MediaFetchError"; + retries = 0; + + constructor( + readonly reader: MediaReader, + readonly status: number | null, + readonly code: string | null, + ) { + super(describe(reader, status, code, 0)); + } + + setRetries(retries: number): void { + this.retries = retries; + this.message = describe(this.reader, this.status, this.code, retries); + } +} + +const MAX_RETRIES = 2; + +const isAbort = (error: unknown) => (error as { name?: unknown } | null)?.name === "AbortError"; + +function networkCode(error: unknown): string { + const cause = (error as { cause?: { code?: unknown } } | null)?.cause; + if (typeof cause?.code === "string") return cause.code; + const name = (error as { name?: unknown } | null)?.name; + return typeof name === "string" ? name : "unknown"; +} + +const typedFailure = (reader: MediaReader, error: unknown) => + isAbort(error) ? error : new MediaFetchError(reader, null, networkCode(error)); + +/** The body, failing a broken read with a MediaFetchError: mediabunny rethrows a body error as-is. */ +function typedBody( + reader: MediaReader, + body: ReadableStream, +): ReadableStream { + const source = body.getReader(); + return new ReadableStream({ + async pull(controller) { + try { + const { done, value } = await source.read(); + if (done) controller.close(); + else controller.enqueue(value); + } catch (error) { + controller.error(typedFailure(reader, error)); + } + }, + cancel: (reason) => source.cancel(reason), + }); +} + +/** + * Options for a mediabunny `UrlSource`. Its default retries a same-origin network failure + * forever and logs a bare `TypeError` each time; these retry twice and fail with a MediaFetchError. + */ +export function mediaUrlSourceOptions(reader: MediaReader) { + let failures = 0; + const fetchFn = async (input: RequestInfo | URL, init?: RequestInit): Promise => { + let response: Response; + try { + // oxlint-disable-next-line no-restricted-globals -- a media read, on the media's sockets like mediabunny's own + response = await fetch(input, init); + } catch (error) { + throw typedFailure(reader, error); + } + if (!response.ok) throw new MediaFetchError(reader, response.status, null); + if (!response.body) return response; + const { status, statusText, headers } = response; + return new Response(typedBody(reader, response.body), { status, statusText, headers }); + }; + // Counted per source: mediabunny passes 1 for every failed mid-stream read. + const getRetryDelay = (_previousAttempts: number, error: unknown): number | null => { + const clientError = + error instanceof MediaFetchError && error.status !== null && error.status < 500; + if (error instanceof MediaFetchError) error.setRetries(failures); + if (clientError || failures >= MAX_RETRIES) return null; + failures++; + return 0.5 * failures; + }; + return { fetchFn, getRetryDelay }; +} + +export function reportMediaFetchFailure(error: unknown): void { + if (!(error instanceof MediaFetchError)) return; + console.warn(`[Studio] ${error.message}`); +} diff --git a/packages/studio/src/player/lib/mediaProbe.test.ts b/packages/studio/src/player/lib/mediaProbe.test.ts index 3f5cb83067..8f04af7bed 100644 --- a/packages/studio/src/player/lib/mediaProbe.test.ts +++ b/packages/studio/src/player/lib/mediaProbe.test.ts @@ -8,16 +8,22 @@ import { probeMissingSourceDurations, resetMediaProbeRegistry, } from "./mediaProbe"; +import { MediaFetchError } from "./mediaFetch"; const dispose = vi.fn(); const getDurationFromMetadata = vi.fn(async () => 5); const requestedSources: string[] = []; +const sourceRetryRules: string[] = []; vi.mock("mediabunny", () => ({ ALL_FORMATS: {}, UrlSource: class { - constructor(readonly url: string) { + constructor( + readonly url: string, + readonly options?: { getRetryDelay?: unknown }, + ) { requestedSources.push(url); + sourceRetryRules.push(typeof options?.getRetryDelay); } }, Input: class { @@ -32,6 +38,7 @@ beforeEach(() => { resetMediaProbeRegistry(); vi.clearAllMocks(); requestedSources.length = 0; + sourceRetryRules.length = 0; getDurationFromMetadata.mockResolvedValue(5); }); @@ -40,6 +47,19 @@ afterEach(() => { }); describe("media probe registry", () => { + it("reports a failed range fetch as one typed line and records the probe as failed", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const failure = new MediaFetchError("probe", 403, null); + getDurationFromMetadata.mockRejectedValue(failure); + + await expect(probeMediaUrl("/locked.mp4")).resolves.toBeNull(); + expect(sourceRetryRules).toEqual(["function"]); + expect(warn.mock.calls).toEqual([ + ["[Studio] probe range fetch failed: HTTP 403 after 0 retries"], + ]); + warn.mockRestore(); + }); + it("deduplicates and caches successful probes", async () => { const [first, second] = await Promise.all([ probeMediaUrl("/video.mp4"), diff --git a/packages/studio/src/player/lib/mediaProbe.ts b/packages/studio/src/player/lib/mediaProbe.ts index ec9f015716..9c92498505 100644 --- a/packages/studio/src/player/lib/mediaProbe.ts +++ b/packages/studio/src/player/lib/mediaProbe.ts @@ -1,5 +1,6 @@ import { authoredSrcPath, resolveMediaPreviewUrl } from "../components/thumbnailUtils"; import { TIMELINE_VIEWPORT_BUDGETS } from "./timelineViewportBudgets"; +import { mediaUrlSourceOptions, reportMediaFetchFailure } from "./mediaFetch"; export interface MediaProbeResult { duration: number; @@ -55,7 +56,7 @@ async function probeOne(url: string): Promise { if (!mb) return null; const input = new mb.Input({ - source: new mb.UrlSource(url), + source: new mb.UrlSource(url, mediaUrlSourceOptions("probe")), formats: mb.ALL_FORMATS, }); try { @@ -73,7 +74,8 @@ async function probeOne(url: string): Promise { hasAudio: audioTracks.length > 0, }; return result; - } catch { + } catch (error) { + reportMediaFetchFailure(error); return null; } finally { input.dispose(); diff --git a/packages/studio/src/player/lib/thumbnailVideoDecoder.test.ts b/packages/studio/src/player/lib/thumbnailVideoDecoder.test.ts index cbac2487a4..425f80cfbe 100644 --- a/packages/studio/src/player/lib/thumbnailVideoDecoder.test.ts +++ b/packages/studio/src/player/lib/thumbnailVideoDecoder.test.ts @@ -2,8 +2,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { decodeVideoThumbnail, videoThumbnailTimestamps } from "./thumbnailVideoDecoder"; +import { MediaFetchError } from "./mediaFetch"; const dispose = vi.fn(); +const sourceRetryRules: string[] = []; const canvasesAtTimestamps = vi.fn(); const getKeyPacket = vi.fn(async (_time: number) => null as { timestamp: number } | null); const input = { @@ -14,7 +16,12 @@ const input = { vi.mock("mediabunny", () => ({ ALL_FORMATS: {}, UrlSource: class { - constructor(readonly url: string) {} + constructor( + readonly url: string, + readonly options?: { getRetryDelay?: unknown }, + ) { + sourceRetryRules.push(typeof options?.getRetryDelay); + } }, Input: class { getPrimaryVideoTrack = input.getPrimaryVideoTrack; @@ -371,6 +378,22 @@ describe("decodeVideoThumbnail", () => { expect(URL.revokeObjectURL).toHaveBeenCalledTimes(5); }); + it("reports a failed range fetch as one typed line and rethrows it", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + sourceRetryRules.length = 0; + input.getPrimaryVideoTrack.mockRejectedValue( + new MediaFetchError("thumbnail", null, "TypeError"), + ); + await expect( + decodeVideoThumbnail({ source: "/locked.mp4", frameCount: 1 }, new AbortController().signal), + ).rejects.toBeInstanceOf(MediaFetchError); + expect(sourceRetryRules).toEqual(["function"]); + expect(warn.mock.calls).toEqual([ + ["[Studio] thumbnail range fetch failed: network TypeError after 0 retries"], + ]); + warn.mockRestore(); + }); + it("releases input and degrades when the source has no video track", async () => { input.getPrimaryVideoTrack.mockResolvedValue(null); await expect( diff --git a/packages/studio/src/player/lib/thumbnailVideoDecoder.ts b/packages/studio/src/player/lib/thumbnailVideoDecoder.ts index 0bf417dd56..86b3accb06 100644 --- a/packages/studio/src/player/lib/thumbnailVideoDecoder.ts +++ b/packages/studio/src/player/lib/thumbnailVideoDecoder.ts @@ -4,6 +4,7 @@ import { type ThumbnailLoadedResult, type ThumbnailValue, } from "./thumbnailScheduler"; +import { mediaUrlSourceOptions, reportMediaFetchFailure } from "./mediaFetch"; export interface VideoThumbnailDecodeRequest { source: string; @@ -260,6 +261,7 @@ export async function decodeVideoThumbnail( return await decodeMissingFrames(request, signal, budgets, keyOf, resources); } catch (error) { releaseDecodedResources(resources); + reportMediaFetchFailure(error); throw error; } } @@ -275,7 +277,7 @@ async function decodeMissingFrames( throwIfAborted(signal); const input = new mediabunny.Input({ - source: new mediabunny.UrlSource(request.source), + source: new mediabunny.UrlSource(request.source, mediaUrlSourceOptions("thumbnail")), formats: mediabunny.ALL_FORMATS, }); try {