fix(core): bound CDP round-trips so an unanswered command can't hang the CLI (PER-10287)

[pranavz28]

Fixes PER-10287 — "Percy visual tests are stalling intermittently when run in CI pipeline".

Why this exists when PER-10287 already shipped a fix

percy-storybook#1354 bounded the storyRendered wait (PERCY_STORY_RENDER_TIMEOUT) and shipped in @percy/storybook 10.0.1-beta.5 / 10.0.2. The stall still reproduces on 10.0.2 — 3 times in ~19 runs against the reported Storybook build, once on a completely idle machine (frozen at 4 snapshots, same story, polled 4+ minutes).

A CDP trace of a real stall shows the render wait was never the problem:

t=18874ms  probe       armed hero--home-page 30000
t=18979ms  probe       rendered hero--home-page                        <-- rendered
t=18983ms  nav         Page.frameScheduledNavigation reason=reload
t=19041ms  evalSettle  cdpError: Protocol error (Runtime.callFunctionOn):
                       Inspected target navigated or closed            <-- settled
t=19043ms  ctxCleared ... ctxCreated ... navigatedWithinDocument
   ... nothing from this target again, for the remaining ~180s.

The story rendered, the deadline was correctly cleared, and the eval settled with a proper Error. Execution stops after that. Page accounting in the stalled run: 7 pages created, 6 closed — the unclosed one is the story page. Page closed never logs for it, and Retrying Story: never appears — and the retry lives in the enclosing catch, i.e. after the finally that awaits page.close().

Root cause

Browser.send/Session.send register a callback in a map and return a promise settled only when a matching response arrives over the websocket (or when Browser.close() rejects everything). There is no timeout — the only setTimeout in browser.js is spawn()'s launch guard, and Session._handleClose() rejects only session callbacks, not browser-level ones.

withPage finally → Page.close() → Session.close()
                 → await browser.send('Target.closeTarget', { targetId })   // unbounded

So a Target.closeTarget that Chrome never answers — which a target closed mid-reload can produce — blocks forever, inside a finally. No error, no retry, no log line, and the build is never finalized; server-side it is force-closed by the reaper hours later, which is exactly the reported symptom.

Fix

1. One deadline at the chokepoint. Both callback maps now route through a single pendingCommand helper that attaches a deadline, so no protocol round-trip can hang the process — covering Target.closeTarget, Target.disposeBrowserContext, Runtime.callFunctionOn and every future command, for every SDK.

• Default DEFAULT_CDP_TIMEOUT = 300000. Deliberately far above any legitimate command — the longest real waits are page loads and Runtime.callFunctionOn with awaitPromise: true, both tens of seconds — so it only ever converts an infinite hang into an actionable error.
• PERCY_CDP_TIMEOUT overrides it; 0 opts out entirely (single auditable escape hatch).
• Timers are cleared on every settle path (_handleMessage in both classes, Browser.close, Session._handleClose) and unref'd, so they never hold the process open.

2. A tighter bound on cleanup. Target.closeTarget and Target.disposeBrowserContext get DEFAULT_CDP_CLOSE_TIMEOUT = 10000 — these run where blocking is worst and should be near-instant. A failed session close no longer propagates out of Page.close(), so cleanup can never swallow the real error or block the retry.

3. Page.eval no longer throws a raw exception.description. CDP omits that field when a page rejects with a string, undefined, null or false, so eval threw undefined:

in-page rejection	exception.description	old throw
reject(new Error(…))	"Error: …"	string
reject({ … })	"Object"	string
reject(42)	"42"	string
reject("some-id")	absent	undefined
reject(undefined) / null / false	absent	undefined

This is how the reported run surfaced as TypeError: Cannot read properties of undefined (reading 'isExecutionContextDestroyed') — the real cause was destroyed and the run aborted after 3 retries. Storybook emits STORY_MISSING with a bare story-id string when a story's chunk fails to load, which is a rejection with a string. normalizeEvalException now always returns an Error, falling back to exception.value then exceptionDetails.text.

Behaviour change worth flagging

Page.eval now rejects with an Error rather than a string. No in-repo consumer treats it as a string. @percy/storybook's withPage has a typeof error !== 'string' branch that strips stacks from string errors — that branch simply stops being taken, so its messages keep the stack. Cosmetic, but reviewers should see it.

Testing

packages/core/test/unit/cdp-timeout.test.js — 14 specs, 0 failures:

• cdpTimeout — default, explicit override, PERCY_CDP_TIMEOUT, non-numeric env, 0 opt-out
• pendingCommand — resolves on response; rejects an unanswered command with Protocol error (…): Timed out after Nms; does not reject a command that settled before the deadline; registers no timer when disabled
• normalizeEvalException — keeps an error description; and returns a real Error for string / undefined / null / no-details rejections, each of which previously produced throw undefined

Full @percy/core suite left to CI.

Not in this PR

The @percy/storybook side is separate (different repo): optional-chaining the isExecutionContextDestroyed reads, wrapping non-Error channel payloads in evalSetCurrentStory, and a channel listener leak found in the same trace — channel.on('storyRendered', …) is never removed, so with one page serving many stories every prior story's handler fires on each render.

🤖 Generated with Claude Code