fix(cli): stop chrome-headless-shell download hangs from stalling forever - #3883
Open
miga-heygen wants to merge 1 commit into
Open
fix(cli): stop chrome-headless-shell download hangs from stalling forever#3883miga-heygen wants to merge 1 commit into
miga-heygen wants to merge 1 commit into
Conversation
…ever @puppeteer/browsers' download path has no AbortController/timeout anywhere in its chain, and its proxy support is inert (proxy-agent is only an optional peer dependency hyperframes doesn't install) — so on a network that requires a proxy, the download attempts a direct connection and hangs at "Downloading Chrome... 0%" forever instead of failing. Add a stall watchdog (resets on every progress tick, not a total-duration cap, so a slow-but-live download is unaffected) around the install() call, and name HTTP(S)_PROXY as the likely cause in the existing HYPERFRAMES_BROWSER_PATH remediation hint when one is configured. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PRINFRA-682:
hyperframes render/browser ensurehangs at "Downloading Chrome... 0%" forever on a network that requires a proxy, with no timeout and no fallback signal.Investigated (against current
origin/main, not the older npm-tarball source the original vault finding traced) and confirmed both parts of the root cause independently:@puppeteer/browsers'install()→downloadFile()→httpRequest()(lib/httpUtil.jsin the resolved3.2.1) opens a barenode:http(s)request with noAbortController/timeout at all — confirmed by reading the actual resolved package source in a fresh worktree install.httpRequest()does tryawait import('proxy-agent'), butproxy-agentis only an optional peer dependency of@puppeteer/browsersthat hyperframes never installs — the import throws, is caught silently, and the request falls through to a bare Node agent that never readsHTTP_PROXY/HTTPS_PROXY. On a proxy-only network, that direct-connection attempt doesn't fail fast; it hangs.What I did NOT ship, and why: the referenced vault finding also suggested dropping
preferManagedChrome: truefromrender'sensureBrowser()call (or falling back tofindSystemBrowser()) so render could use an already-working system Chrome instead of downloading. Source investigation shows this is a deliberate design constraint, not an oversight —manager.ts'sEnsureBrowserOptions.preferManagedChromedoc comment andmacosOldChromeCrash.tsboth document that render needs the exact pinned Chrome build forcanvas.drawElementImagesupport (Stable-channel system Chrome doesn't have it and used to crash drawElement-eligible renders outright, HF#2060), and that the established remediation for a broken managed-download path is surfacingHYPERFRAMES_BROWSER_PATH(already implemented viawrapDownloadFailureWithBrowserPathHint), not silently falling back to system Chrome. Reporting this back to cli-feedback rather than implementing it — it would reintroduce a known crash class.Fix implemented (the timeout/proxy half only):
packages/cli/src/browser/manager.ts: new exportedwithDownloadStallGuard()— a watchdog that racesinstall()against a "no progress" timer. It resets on every progress tick (and once before the first one), so it's a stall guard, not a total-duration cap — a slow-but-actually-progressing download is never affected, only a connection that stops producing bytes entirely.downloadBrowser()'srunInstall, so a stalled attempt surfaces through the existingwrapDownloadFailureWithBrowserPathHinterror-rewrap path (same one that already handles "all CDN providers failed") instead of hanging.proxyDownloadStallHint(): whenHTTP_PROXY/HTTPS_PROXYis actually set in the environment, appends a short clause explaining why it likely stalled (the proxy isn't honored) on top of the existingHYPERFRAMES_BROWSER_PATHremediation — turning "download stalled, no idea why" into an actionable diagnosis without adding a new dependency (proxy-agentpulls in several sub-agents and is deliberately optional upstream; this mirrors the dependency-freeNODE_USE_ENV_PROXYhint pattern already used inpublishProject.ts'sproxySupportHint()).@puppeteer/browsersitself, does not addproxy-agentas a dependency, does not changerender's browser-selection policy.Test plan
withDownloadStallGuardunit tests (6): resolves normally before any stall, rejects when no progress ever arrives, resets on every progress tick so a slow-but-live download is unaffected, still trips on a mid-download stall, propagates a genuine rejection unchanged (not mislabeled as a stall), forwards progress ticks to the caller.downloadBrowser/ensureBrowserwiring tests (3): proxy hint appended whenHTTPS_PROXYis set, omitted when it isn't, and a full end-to-end case (via fake timers) proving a download that never responds surfaces"Download stalled"+ theHYPERFRAMES_BROWSER_PATHhint instead of hanging.manager.test.tspass (42 pre-existing + 9 new), no regressions.bunx tsc --noEmit -p packages/cliclean.bunx oxlint/oxfmt --writeclean.bunx fallow audit --base origin/main --fail-on-issuesclean (had to reword two JSDoc comments that literally spelledimport('proxy-agent')as prose — fallow's dependency scanner flagged that string as a real unlisted import).packages/cli/src/commands/rendertest suite (95 tests) — no regressions.🤖 Generated with Claude Code