[wrangler] Exit quietly when the output pipe is closed early - #15416
[wrangler] Exit quietly when the output pipe is closed early#15416jkubo wants to merge 2 commits into
Conversation
Running a Wrangler command with both stdout and stderr going to a reader that stops early (`wrangler whoami 2>&1 | head`, or an agent/CI runner that captures combined output and times out) made Wrangler abort with a JavaScript heap out-of-memory error instead of exiting. Node ignores SIGPIPE, so the failed write arrives as an unhandled EPIPE error event. That became an uncaught exception, and Sentry's handler for those logs the error with console.error before deferring process.exit behind a transport flush of up to two seconds. The console.error went to the stream that had just failed, raising another EPIPE that re-entered the handler well before the process was allowed to exit. Each turn allocated another Error with a captured stack, so Wrangler exhausted the heap and aborted. Attach an error listener to both stdio streams when Wrangler runs as a CLI, so a broken pipe stops output and exits cleanly instead of ever becoming an uncaught exception. Other stdio errors are still surfaced, and the programmatic API is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 05fa7d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
`bin/cf-wrangler.js` requires `wrangler-dist/cli.js` and runs delegate verbs in-process rather than re-spawning, so `require.main === module` is false and the guard installed for the `wrangler` binary never ran for it. cf-wrangler never calls `main()`, so it does not initialise Sentry and cannot hit the heap-exhaustion loop; an unhandled EPIPE there is a noisy but bounded crash. Guarding it anyway keeps the two binaries consistent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks — good catch, and confirmed: Fixed in the follow-up commit: One correction to the severity, for the record: On test coverage for the delegate specifically: the existing |
|
CI note: The single failure is Every other check is green (51 pass). I can't re-run the job from a fork; a maintainer re-run should clear it. |
Fixes #15415.
When a Wrangler command has both stdout and stderr connected to a reader that stops early, Wrangler aborts with a JavaScript heap out-of-memory error instead of exiting:
On Linux with
systemd-coredumpenabled that leaves a ~300 MB core dump per occurrence, containing the account data and OAuth/API token material the process was holding. It mainly affects agent and CI runners, since they capture combined output and close the pipe on timeout; an interactive TTY never yieldsEPIPE.Cause
Node ignores
SIGPIPE, so the failed write arrives as an asynchronousEPIPEerror event. Nothing listens for it on the stdio streams, so it becomes an uncaught exception — and in released builds that path is Sentry'slogAndExitProcess, whichconsole.errors the error (writing to the stream that just failed, raising anotherEPIPE) and only then callsprocess.exit, deferred behind a transport flush of up to 2s. The secondEPIPEre-enters the handler well before the process can die, and each turn allocates anotherErrorwith a captured stack, so the heap is exhausted rather than the loop terminating.The core dump contains ~787,000 copies of the same
console.error→Socket._writeGeneric→afterWriteDispatchedstack and 1.57M occurrences ofEPIPE.Fix
Attach an
errorlistener toprocess.stdoutandprocess.stderrso a broken pipe stops output and exits cleanly, and never becomes an uncaught exception in the first place.Deliberate scoping:
src/cli.tsonly inside therequire.main === modulebranch, so the programmatic API (unstable_devand friends) does not get process-wide exit handlers installed on its embedder's behalf.EPIPEandERR_STREAM_DESTROYEDare treated as a broken pipe. Any other stdio error is re-thrown, preserving today's behaviour rather than silently swallowing it.Verification
Built from source with
SENTRY_DSNset (a dead local address), since a plainpnpm buildleaves Sentry uninitialised and does not reproduce:wrangler whoami 2>&1 | head -c 50SIGABRT, heap OOM, core dumpedUnit tests added for the guard, and
sentry.test.ts,cli.test.tsandindex.test.tspass.oxlint,oxfmt --checkandtsc --noEmitare clean.Note
This is a contribution from an AI agent: Claude Code, Claude Opus 5.