Skip to content

@remotion/media: Resume browser exports after offline fetch failures - #12021

Open
alec-watts wants to merge 15 commits into
remotion-dev:mainfrom
alec-watts:fix/offline-media-fetch-recovery
Open

alec-watts wants to merge 15 commits into
remotion-dev:mainfrom
alec-watts:fix/offline-media-fetch-recovery

Conversation

@alec-watts

@alec-watts alec-watts commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Browser exports can exhaust their media and readiness deadlines during an outage, preventing the same export from continuing after reconnection. This change waits for reconnection when a media fetch rejects with TypeError and the browser explicitly reports offline.

Maintainer requests addressed

  • Render scope: the existing render-local media cache tracks video/audio extraction handles using their actual DelayRenderContextType scope. Offline waits suspend those handles; the web renderer subtracts that scope's suspended time from its readiness deadline.
  • Disposal: each input factory owns an abort controller and aborts fetch/reconnect waits before disposing its Mediabunny input. Completion, unmount, cancellation, and cache disposal release handle tracking.
  • Package ownership: fetch recovery and connectivity policy live in @remotion/media. Core exposes only generic nested timeout suspension. There is no window-global offline counter or media-label matching.

Merged upstream main at f29e9f3, preserving the current video effectsOutputSize API. The old caption-inspector selector repair is already upstream and is no longer part of this diff.

The follow-up review fixes remove the need for AbortSignal.any and throwIfAborted. Request aborts still reach response-body reads; input-lifetime forwarding is removed after fetch settles, so completed requests do not accumulate lifetime listeners. Offline wait notifications are paired inside try/finally. A regression disabling the newer AbortSignal methods passes with the fix and fails against the previous implementation.

Verification on the refreshed branch

  • Production package builds and TypeScript checks for core, media, player, and web renderer pass. Core/media/web-renderer lint and formatting pass, with existing warnings.
  • All 726 core tests pass.
  • Full local Chromium media suite: 165 tests pass across 48 files on the PR; the clean upstream-main control passes 163 tests across the same 48 files. Focused shared-input/keyframe-bank/iterator checks also pass.
  • Across configured browsers, web-renderer recovery/cancellation/frame-range checks pass: 33 tests pass, 15 existing/explicit browser conditions skip across 9 files. The recovery regression passes in Chrome and Firefox; WebKit is explicitly skipped.
  • Actual network outage, production React: load the harness, disable Chrome networking, and verify navigator.onLine is false. With a 2.5-second render timeout (500 ms extraction deadline; 5.5-second readiness deadline), the export remains pending after 6.5 seconds offline. Reconnection completes the same 30-frame video/audio export. FFmpeg decodes all 30 frames; decoded RGBA video and audio PCM match the uninterrupted online control byte for byte. This run uses VP8/Opus in WebM.
  • Added committed regressions for nested/scope-isolated suspension, clear-before-resume cleanup, abort-before-input-dispose ordering, and a full export surviving a fault-injected 6.5-second outage and then cancelling another offline export. The CI fixture explicitly allows 3 seconds for extraction after reconnecting, while retaining a 5.5-second readiness deadline; the outage exceeds both. The separate actual-network harness above verified the tighter 500 ms extraction deadline.
  • Mutation checks confirm those regressions fail when suspension, cleared-handle protection, or the input-owned disposal abort is removed. Production code and bundles were restored afterward.
  • Diff cleanup finds no formatting bloat, unused mechanisms, or parallel request/decoder/cache pipelines. git diff --check passes.
  • Final head 6be88bb: upstream Install and Test and Required CI pass, including macOS/Windows/Ubuntu builds/tests, full web-renderer/Browser Studio, browser E2E, SSR, templates, monorepo, and lint/formatting. Both Vercel previews pass, and there are zero unresolved review threads. A prior attempt hit the existing nested playback-rate frame assertion on macOS; the exact test and full media suites passed locally on clean main and the PR, and the final CI recheck passed with an identical file tree. The recheck commit contains no code changes.

The browser tests inject offline fetch failure and navigator state; the separate production harness disables actual Chrome networking.

Limits

navigator.onLine is a connectivity hint: failures while it stays true retain Mediabunny's retry policy. Recovery covers fetch rejection before a response, not failure reading an already-open response body. Offline exports may wait until reconnection or cancellation. Unrelated render scopes and authored handles retain their deadlines. Server-render timeout policy is unchanged.

Why each file changes

File Reason
packages/core/src/delay-render.ts Add nested per-handle suspension, cleanup, and scope-local elapsed accounting to the existing timeout owner.
packages/core/src/internals.ts Expose the two generic timeout operations through the existing internal API.
packages/core/src/test/ready-manager.test.ts Pin nested suspension, scope isolation, deadline resumption, and clear-before-resume cleanup for #12021.
packages/media/src/offline-media-fetch.ts Own abortable reconnect waits and narrowly scoped fetch recovery in the media package.
packages/media/src/get-shared-input.ts Attach recovery and cancellation to the shared-input lifetime.
packages/media/src/video-extraction/get-frames-since-keyframe.ts Attach recovery and cancellation to extraction-input lifetime.
packages/media/src/get-sink.ts Forward offline wait transitions through existing input factories.
packages/media/src/caches.ts Attribute waits and extraction handles to the existing render-local cache.
packages/media/src/video/video-for-rendering.tsx Register/release video extraction handles on their actual render scope.
packages/media/src/audio/audio-for-rendering.tsx Register/release audio extraction handles on their actual render scope.
packages/media/src/test/shared-input.test.ts Verify input-owned abort happens before disposal and releases the offline wait without reconnecting.
packages/web-renderer/src/wait-for-ready.ts Exclude scope-local suspension from the readiness deadline.
packages/web-renderer/src/test/offline-recovery.test.tsx Exercise real export recovery and cancellation under injected offline fetch failure beyond both deadlines.

Current main integration

Merged upstream bbe11edd85 in 8e50119b54. 4 shared-input browser tests and the offline export recovery browser test passed. Affected-package builds/typechecks passed; Upstream CI passed on 8e50119b54; GitHub reports no merge conflicts.

Main refresh — 2026-10-23

Merged upstream 2bd172f29a into this branch as e6b17e3004. The merge required no manual conflict resolutions. Diff cleanup and whitespace checks passed; retained file changes are scoped to this proposal. Fresh upstream CI passed on this head, including Required CI. Earlier local validation above applies to the commits identified there.

Local verification on this refreshed head: core TypeScript declaration build passed; all 726 core tests passed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
bugs Ready Ready Preview Oct 23, 2026 10:04am UTC
remotion Ready Ready Preview Oct 23, 2026 10:04am UTC

Request Review

Co-authored-by: Cursor <cursoragent@cursor.com>
@alec-watts

Copy link
Copy Markdown
Contributor Author

Follow-up from downstream review in https://github.com/kino-ai/kino/pull/3936:

  • Added a cleared-handle guard at the start of the delayRender timeout poll. A queued callback must not reschedule or cancel a render after continueRender has removed its timeout entry. An external probe executing the actual callback verified both positive and exhausted budgets are no-ops for cleared handles, while an active handle still reschedules.
  • Keep retry budgets owned by Mediabunny per request. A previous downstream implementation tracked attempt counts in a callback shared by UrlSource; interleaved concurrent reads could reset that shared budget. This upstream PR already avoids that design: recovery lives within each fetchFn invocation and leaves Mediabunny’s default online retry policy unchanged.
  • Prefer behavioral fetch recovery/abort coverage over mocked UI transport assertions. The existing external helper probe covers reconnecting the original request, abort cleanup, and online-error propagation; no Kino-only tests or UI changes were copied here.

The downstream PR is now merge-ready with unit CI, Vercel, and Bugbot passing. That is not a substitute for an end-to-end browser disconnect/reconnect render in this upstream checkout, which remains unverified.

Co-authored-by: Cursor <cursoragent@cursor.com>
@alec-watts

Copy link
Copy Markdown
Contributor Author

Fixed and pushed the CI failures caused by this change:

  • Avoid reading global fetch at module initialization when it is absent (Node 16 / SSR). Preserve Bun fetch properties when fetch is available.
  • Resolve declaration-order lint by grouping temporary event listeners under an AbortController, with cleanup on reconnect or request abort.
  • Match the internal blank-line rules in delay-render and wait-for-ready.

Local verification: core/media/web-renderer lint and formatting passed; core built and all 717 tests passed; the full SSR suite passed; built CJS and ESM imports passed on actual Node 16.20.2 without global fetch; recovery/reconnect/abort and cleared-timeout probes passed. Full local monorepo integration was blocked by missing language tooling (python, PHP, Go and Ruby bundle setup), but monorepo CI now passes.

The new CI run has passed Linting + Formatting, SSR tests, and Monorepo tests. A remaining Windows Node 16 job failed on the unrelated bundler test "emits a zero-delay Rspack React Refresh runtime" hitting its 5000 ms timeout; no fetch import error remains in that job. Other integration jobs are still running.

@JonnyBurger JonnyBurger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UrlSource.fetchFn integration is a sensible direction, but I am requesting changes for the timeout scope wiring, disposal cleanup, and package ownership described below.

I reproduced both behavioral issues with targeted runtime probes against this head, using the pinned Mediabunny 1.56.1. A complete browser disconnect/reconnect export has not been verified.

Comment thread packages/core/src/delay-render.ts Resolved
Comment thread packages/core/src/internals.ts Resolved

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

AbortSignal.any in the new recovery wrapper raises the browser/Node floor for @remotion/media above what WebCodecs requires, and canRenderMediaOnWeb() will not warn about it. Details in the inline comments.

Reviewed changes

Reviewed the full diff of PR #12021 (13 files) at fefbd19d, which adds a scope-local delayRender timeout-suspension mechanism to core and wires @remotion/media browser fetch recovery to it. I also verified that the three concerns from the prior maintainer review are now addressed: recovery targets the real DelayRenderContextType scope, the policy lives in @remotion/media (core exposes only a generic suspension mechanism), and an input-owned abort controller releases reconnect waits before disposal.

  • Core nested timeout suspension (packages/core/src/delay-render.ts): a per-DelayRenderScope WeakMap adds suspendDelayRenderTimeout / getDelayRenderSuspendedTime, with nested suspends, clear-before-resume protection, and scope-local elapsed accounting.
  • Internal API exposure (packages/core/src/internals.ts): exposes the two generic suspension operations.
  • Offline fetch recovery (packages/media/src/offline-media-fetch.ts, new): wraps fetch for Mediabunny's UrlSource, waiting for the online event when a fetch rejects with TypeError while navigator.onLine === false, backed by an input-owned abort controller.
  • Render-local tracking (packages/media/src/caches.ts, get-sink.ts, get-frames-since-keyframe.ts, get-shared-input.ts): the media cache suspends tracked handles while any sink waits offline, and aborts recovery before disposing its input.
  • Handle registration (video-for-rendering.tsx, audio-for-rendering.tsx): registers the in-flight delay handle against the actual render scope and releases it on completion and cleanup.
  • Readiness deadline (packages/web-renderer/src/wait-for-ready.ts): subtracts scope-local suspended time from the timeout + 3000ms deadline.
  • Tests: core suspension regressions, an input-disposal abort test, and an end-to-end web-renderer outage/cancellation test.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The incremental delta is clean — no new issues. The two suggestions from the first review are still open.

Reviewed changes

Reviewed the delta pushed after the previous review (9195cd71, 1 file). It is test-only and introduces no new findings.

  • Offline-recovery test timing (packages/web-renderer/src/test/offline-recovery.test.tsx): the <Video> and <Audio> under test now pass delayRenderTimeoutInMilliseconds={5000}, leaving 3 seconds for extraction after reconnecting on a shared CI runner; the 6.5-second outage still exceeds both that budget and the 5.5-second readiness deadline.

The two suggestions from the previous review (the AbortSignal.any environment floor and the unconditional onWaitChanged true/false pairing) remain open and are unaffected by this commit, so I am intentionally not approving.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash | 𝕏

# Conflicts:
#	packages/core/src/delay-render.ts

This branch was successfully deployed

2 active deployments
Preview – remotion — e6b17e30 Deployed Oct 23, 2026 by vercel[bot]
Preview – bugs — e6b17e30 Deployed Oct 23, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants