From 336e9fb8a5ed90d7629e0583b6faede332138764 Mon Sep 17 00:00:00 2001 From: sudacode Date: Sat, 1 Aug 2026 03:13:48 -0700 Subject: [PATCH] fix(anime): cap outcome polling and upstream stalls by wall clock - watchPlaybackOutcome now tracks a real deadline so slow property reads eat the timeout budget instead of extending it, and a zero probe interval still terminates - stream strip proxy aborts upstream GETs that go silent for 15s instead of hanging mpv on that segment forever - anime browser runtime drops a stream proxy that finishes starting after its bridge already died, instead of leaking a listener pointed at nothing --- src/anime-bridge/playback-outcome.test.ts | 46 +++++++++- src/anime-bridge/playback-outcome.ts | 13 ++- src/anime-bridge/stream-strip-proxy.test.ts | 98 ++++++++++----------- src/anime-bridge/stream-strip-proxy.ts | 14 ++- src/main/runtime/anime-browser-runtime.ts | 7 +- 5 files changed, 120 insertions(+), 58 deletions(-) diff --git a/src/anime-bridge/playback-outcome.test.ts b/src/anime-bridge/playback-outcome.test.ts index 60640907..c56beb0c 100644 --- a/src/anime-bridge/playback-outcome.test.ts +++ b/src/anime-bridge/playback-outcome.test.ts @@ -7,12 +7,25 @@ type Harness = { listenerCount: () => number; setProperty: (name: string, value: unknown) => void; failProperty: (name: string) => void; + /** Virtual milliseconds burned so far, by sleeps and by property reads. */ + elapsed: () => number; }; -function createHarness(overrides?: { timeoutMs?: number; probeIntervalMs?: number }) { +/** + * The clock is virtual and only moves when the code under test sleeps (or, + * with `readCostMs`, when it reads a property), so timeout behaviour is + * asserted without any real waiting. + */ +function createHarness(overrides?: { + timeoutMs?: number; + probeIntervalMs?: number; + readCostMs?: number; +}) { const listeners = new Set<(event: PlaybackEndFileEvent) => void>(); const properties = new Map(); const failing = new Set(); + const readCostMs = overrides?.readCostMs ?? 0; + let clock = 0; const watch = watchPlaybackOutcome({ onEndFile: (listener) => { @@ -20,10 +33,14 @@ function createHarness(overrides?: { timeoutMs?: number; probeIntervalMs?: numbe return () => listeners.delete(listener); }, readProperty: async (name) => { + clock += readCostMs; if (failing.has(name)) throw new Error(`Failed to read MPV property '${name}'`); return properties.get(name); }, - wait: async () => {}, + wait: async (ms) => { + clock += ms; + }, + now: () => clock, timeoutMs: overrides?.timeoutMs ?? 1000, probeIntervalMs: overrides?.probeIntervalMs ?? 100, }); @@ -35,6 +52,7 @@ function createHarness(overrides?: { timeoutMs?: number; probeIntervalMs?: numbe listenerCount: () => listeners.size, setProperty: (name, value) => properties.set(name, value), failProperty: (name) => failing.add(name), + elapsed: () => clock, }; return { watch, harness }; } @@ -68,10 +86,32 @@ test('ignores the end-file fired for the file being replaced', async () => { }); test('times out with a failure when nothing ever starts', async () => { - const { watch } = createHarness({ timeoutMs: 300, probeIntervalMs: 100 }); + const { watch, harness } = createHarness({ timeoutMs: 300, probeIntervalMs: 100 }); const outcome = await watch.wait(); assert.equal(outcome.ok, false); assert.ok(!outcome.ok && outcome.error.length > 0); + assert.equal(harness.elapsed(), 300); + watch.dispose(); +}); + +test('slow property reads eat the budget instead of extending it', async () => { + const { watch, harness } = createHarness({ + timeoutMs: 300, + probeIntervalMs: 100, + readCostMs: 250, + }); + const outcome = await watch.wait(); + assert.equal(outcome.ok, false); + // Two probes: 250 + 50 (the sleep clamped to what was left) then 250 again. + assert.ok(harness.elapsed() >= 300, 'gave up before the timeout'); + assert.ok(harness.elapsed() < 900, 'read delays stretched the timeout'); + watch.dispose(); +}); + +test('a zero probe interval still terminates at the deadline', async () => { + const { watch } = createHarness({ timeoutMs: 200, probeIntervalMs: 0, readCostMs: 50 }); + const outcome = await watch.wait(); + assert.equal(outcome.ok, false); watch.dispose(); }); diff --git a/src/anime-bridge/playback-outcome.ts b/src/anime-bridge/playback-outcome.ts index dd4731e5..c703ca7e 100644 --- a/src/anime-bridge/playback-outcome.ts +++ b/src/anime-bridge/playback-outcome.ts @@ -26,6 +26,8 @@ export interface WatchPlaybackOutcomeDeps { /** One-shot mpv property read; may reject while the file is still loading. */ readProperty: (name: string) => Promise; wait: (ms: number) => Promise; + /** Injectable clock; the timeout is wall-clock, not a probe count. */ + now?: () => number; timeoutMs?: number; probeIntervalMs?: number; } @@ -58,7 +60,11 @@ export function watchPlaybackOutcome(deps: WatchPlaybackOutcomeDeps): PlaybackOu }); async function wait(): Promise { - for (let elapsed = 0; elapsed < timeoutMs; elapsed += probeIntervalMs) { + // Wall-clock, not a probe count: a slow `readProperty` must eat into the + // budget rather than stretch it, and a zero probe interval must still end. + const now = deps.now ?? Date.now; + const deadline = now() + timeoutMs; + while (now() < deadline) { if (failure) return failure; try { if ((await deps.readProperty('vo-configured')) === true) return { ok: true }; @@ -66,7 +72,10 @@ export function watchPlaybackOutcome(deps: WatchPlaybackOutcomeDeps): PlaybackOu // The property is unreadable while mpv is between files; keep polling. } if (failure) return failure; - await deps.wait(probeIntervalMs); + // Sleeping past the deadline would only delay the timeout report. + const remaining = deadline - now(); + if (remaining <= 0) break; + await deps.wait(Math.min(probeIntervalMs, remaining)); } return ( failure ?? { diff --git a/src/anime-bridge/stream-strip-proxy.test.ts b/src/anime-bridge/stream-strip-proxy.test.ts index a0473346..452aebc8 100644 --- a/src/anime-bridge/stream-strip-proxy.test.ts +++ b/src/anime-bridge/stream-strip-proxy.test.ts @@ -64,11 +64,16 @@ test('rewritePlaylistOrigins swaps absolute upstream URLs and keeps relative lin type Route = { status: number; contentType: string; body: Buffer }; +/** Either a static routing table or a handler, for upstreams that need one. */ async function withProxy( - routes: Record, + routes: Record | http.RequestListener, run: (proxyOrigin: string, upstreamOrigin: string) => Promise, ): Promise { const upstream = http.createServer((req, res) => { + if (typeof routes === 'function') { + routes(req, res); + return; + } const route = routes[req.url ?? '']; if (!route) { res.writeHead(404).end('missing'); @@ -133,63 +138,54 @@ test('proxy leaves non-TS bodies alone', async () => { }); test('proxy rewrites absolute upstream playlist entries to its own origin', async () => { - const upstream = http.createServer((req, res) => { - const origin = `http://127.0.0.1:${(upstream.address() as AddressInfo).port}`; - if (req.url === '/video/list.m3u8') { + await withProxy( + (req, res) => { + if (req.url !== '/video/list.m3u8') { + res.writeHead(404).end(); + return; + } + // The proxy rewrites the Host header to the upstream it dialled, so this + // is that origin — the one the playlist must not leak to mpv. res.writeHead(200, { 'content-type': 'application/vnd.apple.mpegurl' }); - res.end(`#EXTM3U\n#EXTINF:6,\n${origin}/video/abs.ts\n`); - return; - } - res.writeHead(404).end(); - }); - await new Promise((resolve) => upstream.listen(0, '127.0.0.1', resolve)); - const upstreamOrigin = `http://127.0.0.1:${(upstream.address() as AddressInfo).port}`; - const proxy = await startStreamStripProxy({ upstreamOrigin: () => upstreamOrigin }); - try { - const { body } = await fetchBytes(`${proxy.origin}/video/list.m3u8`); - const text = body.toString('utf8'); - assert.ok(text.includes(`${proxy.origin}/video/abs.ts`)); - assert.ok(!text.includes(upstreamOrigin)); - } finally { - await proxy.close(); - await new Promise((resolve) => upstream.close(() => resolve())); - } + res.end(`#EXTM3U\n#EXTINF:6,\nhttp://${req.headers.host}/video/abs.ts\n`); + }, + async (proxyOrigin, upstreamOrigin) => { + const { body } = await fetchBytes(`${proxyOrigin}/video/list.m3u8`); + const text = body.toString('utf8'); + assert.ok(text.includes(`${proxyOrigin}/video/abs.ts`)); + assert.ok(!text.includes(upstreamOrigin)); + }, + ); }); test('proxy strips even when the client asks for a byte range', async () => { - // ffmpeg opens every HLS segment with `Range: bytes=0-`; the bridge answers - // some of those with 206, which must not bypass the strip. + // ffmpeg opens every HLS segment with `Range: bytes=0-`. The proxy drops the + // header, so the upstream answers 200 with the whole body and the strip + // applies; a 206 would have been forwarded untouched. const ts = makeTsPackets(8); const disguised = Buffer.concat([PNG_HEADER, ts]); - const upstream = http.createServer((req, res) => { - if (req.headers.range !== undefined) { - res.writeHead(206, { - 'content-type': 'image/png', - 'content-range': `bytes 0-${disguised.length - 1}/${disguised.length}`, - }); + await withProxy( + (req, res) => { + if (req.headers.range !== undefined) { + res.writeHead(206, { + 'content-type': 'image/png', + 'content-range': `bytes 0-${disguised.length - 1}/${disguised.length}`, + }); + res.end(disguised); + return; + } + res.writeHead(200, { 'content-type': 'image/png' }); res.end(disguised); - return; - } - res.writeHead(200, { 'content-type': 'image/png' }); - res.end(disguised); - }); - await new Promise((resolve) => upstream.listen(0, '127.0.0.1', resolve)); - const upstreamOrigin = `http://127.0.0.1:${(upstream.address() as AddressInfo).port}`; - const proxy = await startStreamStripProxy({ - upstreamOrigin: () => upstreamOrigin, - retryDelayMs: 5, - }); - try { - const response = await fetch(`${proxy.origin}/video/seg.ts`, { - headers: { Range: 'bytes=0-' }, - }); - const body = Buffer.from(await response.arrayBuffer()); - assert.equal(response.status, 200); - assert.deepEqual(body, ts); - } finally { - await proxy.close(); - await new Promise((resolve) => upstream.close(() => resolve())); - } + }, + async (proxyOrigin) => { + const response = await fetch(`${proxyOrigin}/video/seg.ts`, { + headers: { Range: 'bytes=0-' }, + }); + const body = Buffer.from(await response.arrayBuffer()); + assert.equal(response.status, 200); + assert.deepEqual(body, ts); + }, + ); }); test('proxy forwards error statuses without touching the body', async () => { diff --git a/src/anime-bridge/stream-strip-proxy.ts b/src/anime-bridge/stream-strip-proxy.ts index 214da2e9..00ab5024 100644 --- a/src/anime-bridge/stream-strip-proxy.ts +++ b/src/anime-bridge/stream-strip-proxy.ts @@ -73,6 +73,12 @@ export interface StreamStripProxyOptions { } const DEFAULT_RETRY_DELAY_MS = 400; +/** + * Socket timeout on the upstream GET, cleared once its headers arrive. Node's + * http client has no deadline of its own, so a host that accepts the + * connection and then says nothing would hang mpv on that segment forever. + */ +const UPSTREAM_TIMEOUT_MS = 15_000; export interface StreamStripProxyHandle { origin: string; @@ -152,8 +158,10 @@ export function startStreamStripProxy( const upstreamRequest = http.request( upstreamUrl, - { method: req.method, headers: requestHeaders }, + { method: req.method, headers: requestHeaders, timeout: UPSTREAM_TIMEOUT_MS }, (upstream) => { + // Body streaming has its own pace; only the wait for headers is capped. + upstreamRequest.setTimeout(0); const status = upstream.statusCode ?? 502; if (status === 404 || status >= 500) { if (mayRetry) { @@ -167,6 +175,10 @@ export function startStreamStripProxy( handleUpstreamResponse(req, res, upstream); }, ); + // Destroying with an error routes the stall through the retry/502 path. + upstreamRequest.on('timeout', () => { + upstreamRequest.destroy(new Error(`upstream silent for ${UPSTREAM_TIMEOUT_MS}ms`)); + }); upstreamRequest.on('error', (error) => { if (mayRetry) { log(`[stream-proxy] upstream request failed: ${String(error)}; retrying once`); diff --git a/src/main/runtime/anime-browser-runtime.ts b/src/main/runtime/anime-browser-runtime.ts index e239af57..6ebc1ef0 100644 --- a/src/main/runtime/anime-browser-runtime.ts +++ b/src/main/runtime/anime-browser-runtime.ts @@ -228,10 +228,15 @@ export function createAnimeBrowserRuntime(deps: AnimeBrowserRuntimeDeps) { }); try { - stripProxy = await startStreamStripProxy({ + const proxy = await startStreamStripProxy({ upstreamOrigin: () => sidecar?.baseUrl ?? handle.baseUrl, log: deps.log, }); + // The bridge can die while the proxy is coming up, in which case onExit + // already ran and cleared `stripProxy`; adopting this one would leak a + // listening server pointed at a dead upstream. + if (sidecar === handle) stripProxy = proxy; + else void proxy.close(); } catch (error) { // Playback still works for undisguised streams; log and carry on. deps.log(`[anime-browser] stream proxy failed to start: ${describeError(error)}`);