Skip to content

fix(cli): a page whose renderer crashes fails at once, named as the crash - #6598

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/cli-page-crash
Oct 27, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/cli-page-crash

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

What changes

When a page's renderer crashes (most often out of memory, for example a composition with hundreds of video clips), Puppeteer emits a page error event and nothing else. Every call still pending on that page waits for the protocol timeout (180 s by default). The command then fails with Runtime.callFunctionOn timed out, which says nothing about the crash.

Now:

  • Every page a CLI browser opens closes itself when its renderer crashes, so pending calls fail at once. This happens in launchOwnedBrowser, the one owner of CLI browsers, which wraps browser.newPage the same way launchCaptureBrowser already does. snapshot, check, layout, validate and capture all get it.
  • After a crash, normalizeErrorMessage names the follow-on close errors (Target closed, detached Frame, Session closed):
✗ Snapshot failed: The browser page crashed, most likely out of memory: Protocol error (Runtime.callFunctionOn): Target closed

On a composition with 400 clips cut from one long 720p source, the renderer grows past 7 GB and crashes about 15 s in. snapshot used to fail after 180 s or more with the timeout message. With this change it fails with the crash named, in under 30 s.

Limits

  • This names the crash. It does not stop large compositions from running out of memory.
  • The crash flag lasts for the whole process. A long-lived process that sees one crash will label later page-close errors as that crash too.

Tests

  • New captureCompositionFrame.pageCrash.browser.test.ts. It opens a real composition page through openSettledCompositionPage (the path snapshot and check use), leaves a call pending, crashes the renderer with Page.crash and expects a named rejection within 10 s.
    • On Linux it passed 6 runs in a row (2 to 7 s each).
    • With launch.ts and errorMessage.ts reverted to main, it times out at 60 s.
  • The full CLI unit project passed: 418 files, 5810 tests.
    • The fake page in browserFallback.test.ts gains once.

Why under 100 lines

P0 crash fix, ships alone so the Desktop pin can take it today.

@github-actions

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 116 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

  • seqplainkeys-none-center-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@heygengenesis heygengenesis 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.

Verdict: APPROVE · 0 blocker · 0 important · 0 nits · head 1ec5660f

I was asked to review 8f8126c1, but that head failed Comments (the comment-share ratchet on launch.ts and errorMessage.ts). The new head 1ec5660f removes those comments and inlines messageOf, which only names the crash for Error instances (Puppeteer rejects with those). I reviewed that delta, and this approval covers 1ec5660f only.

Scope and checks
  • Every CLI page is created through launchOwnedBrowser's newPage; no CLI code creates pages through createBrowserContext or pages(). The failWhenPageDies handler in checkBrowser is registered after closeOnRendererCrash, so check still rejects with "The page crashed" from the same emit. No caller matches on Target closed text, so the prefix breaks nothing. The process-wide flag is called out in the body, and the only long-lived CLI process (warm check) serves one request.
  • Ran locally: the new crash test passed 3 of 3 runs together with browserFallback. With launch.ts reverted to main, it times out at 60 s.
  • CI on 1ec5660f is all green: CI, Windows render verification, Comments, regression, preview-regression, PR captures and CodeQL.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 27, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 439d685 Oct 27, 2026
117 checks passed
@miguel-heygen
miguel-heygen deleted the fix/cli-page-crash branch October 27, 2026 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant