Skip to content

Commit b3ef687

Browse files
claude[bot]claudeos-sales
authored
fix(cli): make a failed stderr write non-fatal on the published entry point (#16686)
* fix(cli): make a failed stderr write non-fatal on the published entry `bin/run.js` writes to `process.stderr` with no `error` listener, so an `error` event there is an uncaught exception. #15564 was filed NOT REPRODUCED, and its fence was explicit: symmetry with `bin/run-dev.js` is not evidence, establish reachability first. Both of the card's probes were re-run before anything was written here and both still read clean — exit 2, no `uncaughtException`, 3/3 each. They were not a guard, they were the wrong lifecycle: everything a failing invocation puts on stderr is written after `run()` has settled, by `handle()`, which exits on top of its own report, and a failing write reports through libuv's completion callback that a synchronous exit is never told about. Widening to a lifecycle that outlives its first failed write reaches it. `os serve` on `examples/app-todo`, read end destroyed: uncaughtException code=EPIPE msg=write EPIPE at afterWriteDispatched (node:internal/stream_base_commons:159:15) exit code=1 3 of 3 runs, 3049-3433 ms in — the same frame and status #14858 traced on the dev shim. The same child read by a draining parent boots, serves, and exits 0 after 7926 bytes over 16.6 s. Pinned with the hazard manufactured inside the published binary's own process, alongside a live positive control that removes the listener in the child and re-crashes it, so the guarded arm's silence is a reading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ * fix(cli): name the stderr error listener so the pin can see it The first ablation of this change caught a phantom check in its own pin: a probe polling `process.stderr.listenerCount('error') > 0` reported `LISTENER ATTACHED after 20 ms` against a tree with the whole listener block deleted. Node parks an anonymous `once('error', noop)` on this stream for the duration of every `console.error` (`ignoreErrors`), so a count cannot tell this listener apart from that one, and the poll doubled as the probe's only synchronisation point — a slow box would have written before the attach and red a working tree. The listener is named, the probe waits for that name, and a parity case holds the probe's mirror equal to the entry's spelling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ * test(cli): pin the attach ORDER, the real writer, and a named control Three review-adopted fixes to the #15564 pin, none of which change `bin/run.js`'s behaviour. F1 — the order case pinned PRESENCE, not order. `LISTENER ATTACHED after N ms` is true for any N inside the probe's 15 s wait, so an attach moved below `await run(…)` would keep every case green while the entry's own claim ("BEFORE `run()`, and that order is the whole point") had stopped being true. The runtime cases cannot see it — one process, `--version` settles oclif in a few hundred ms, both have happened by the time the poll looks — so the order is now read STRUCTURALLY from the entry's comment-masked source, in a case of its own, and the presence case is renamed to claim only what it pins. F2 — the premise case asserted that `serve.ts` contains `process.stderr.write(`. That file holds about a dozen such sites, so it stayed green even if `printDiagnostic` — the one writer the reproduction ran through (#7915) — moved to `console.error` and stopped being able to crash anything. It now anchors on `printDiagnostic`'s own body, located by symbol and brace-matched, with a length bound so a desynchronised match reds instead of reporting green about some other writer. F3 — the unguarded (positive-control) arm called `removeAllListeners('error')`. Equivalent today, but it measures "no listener at all" rather than "the entry's listener absent"; it now removes the guard BY NAME, and marks the resulting `guard=` reading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ * test(cli): run the stderr-error pin per-PR, without a borrowed dist gate The pin drives `bin/run.js --version` under a `--import` probe and reads the entry's source; nothing it measures lives in `dist`. Measured with `packages/cli/dist` moved aside: `--version` printed `@objectstack/cli/17.3.0 linux-x64 node-v22.22.2`, and all six cases passed — guarded arm `LISTENER ATTACHED … WROTE … SURVIVED … EXIT code=7`, unguarded arm `UNCAUGHT code=EPIPE … EXIT code=1`. So `PUBLISHED_ENTRY_NEEDS_DIST` was a sentence borrowed from `published-entry-stderr-nonblocking.e2e.test.ts` (whose guard genuinely IS the compiled `../dist/utils/stderr-nonblocking.js` import) without the reason coming with it — the exact failure the helper's docblock above `RUN_JS_RESOLVES_FROM_DIST` names. Removed, and replaced by a note that states the measurement and the difference from that neighbour. With no dist dependency the file has no reason to sit in the nightly `.e2e` tier: renamed out of it, so it runs per-PR in `--project integration` (34 → 35 files). Both neighbouring `.e2e` suites are untouched (#16455). Also corrects a wrong count this branch introduced: `serve.ts` holds 2 code-position `process.stderr.write(` sites (`printDiagnostic` and the artifact `warn:` adapter), not "about a dozen". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <sales@objectstack.ai>
1 parent 7797102 commit b3ef687

4 files changed

Lines changed: 617 additions & 0 deletions

File tree

.changeset/tidy-cars-repeat.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
Stop the published CLI from dying of an uncaught `write EPIPE` when its caller's stderr read end is gone.
6+
7+
`bin/run.js` — the file `bin.objectstack` / `bin.os` point at, and the only thing under `bin/` npm packs — now attaches the same no-op `error` listener to `process.stderr` that the in-repo dev shim has carried since the original finding. `process.stderr` is an `EventEmitter`, so an `error` event with nothing listening is an uncaught exception.
8+
9+
Measured on the published entry with the read end destroyed (`stdio: ['ignore','ignore','pipe']`, then `child.stderr.destroy()`), traced with an observer that installs no listener and wraps no write:
10+
11+
```
12+
uncaughtException code=EPIPE msg=write EPIPE
13+
at afterWriteDispatched (node:internal/stream_base_commons:159:15)
14+
exit code=1
15+
```
16+
17+
3 of 3 runs, 3049-3433 ms in, on `os serve` over `examples/app-todo`. Read by a draining parent the same child boots and serves and exits 0, having written 7926 bytes over 16.6 s — so the crash was costing the run at its first diagnostic line and 20 of its 21 stderr writes. Failing invocations do not reach it: everything they put on stderr is written after `run()` has settled, by a handler that exits on top of its own report.
18+
19+
Behaviour change worth knowing about: a long-running command (`os serve`, `os dev`, `os start`) whose reader has gone now keeps running and reports its own exit status, instead of dying on its first diagnostic write. A supervisor that destroyed the read end and relied on that crash to end the child needs to end it itself.

packages/cli/bin/run.js

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,84 @@ try {
7070
// Unbuilt or half-built tree — nothing to install and nothing to say.
7171
}
7272

73+
/**
74+
* Make a FAILED stderr write non-fatal, so a caller whose read end is gone
75+
* still gets this CLI's own exit status instead of a crash. #14858, reached on
76+
* THIS entry point by the #15564 measurement.
77+
*
78+
* `process.stderr` is an `EventEmitter`, and an `error` event with nothing
79+
* listening IS an uncaught exception. `bin/run-dev.js` has carried this
80+
* listener since #14858; the published entry did not, and #15564 was filed
81+
* NOT REPRODUCED because the two probes that had been run against it — a
82+
* bad command id, and `OBJECTSTACK_DEBUG=1` over an unbuilt `@objectstack/spec`
83+
* — both answered exit 2 with no `uncaughtException`. Re-run here, they still
84+
* do (3/3 each, 57 and 35528 bytes drained). ⭐ They were not a guard; they
85+
* were the wrong lifecycle, and the difference is measurable rather than
86+
* arguable:
87+
*
88+
* leg (bin/run.js, read end destroyed) stderr writes exit
89+
* --------------------------------------- ------------- ----------------
90+
* `definitely-not-a-command` 1 @ 3231 ms 2, no crash
91+
* OBJECTSTACK_DEBUG=1 + unbuilt spec 60 @ 932-960 ms 2, no crash
92+
* `serve objectstack.config.ts` 21 @ 3180 ms on 1, `write EPIPE`
93+
* 3/3
94+
*
95+
* Two things separate the last row, and BOTH are needed:
96+
*
97+
* • an event-loop TURN between the failing write and `process.exit`. A
98+
* failing write reports through libuv's completion callback, so a write
99+
* followed by a synchronous exit is never told. Both probe legs are that
100+
* shape: everything they put on stderr is written after `run()` has already
101+
* settled, by `handle()`, which exits on top of its own report — measured
102+
* at one write 1 ms before exit, and at 59 warning blocks whose EPIPE
103+
* arrives synchronously inside the write.
104+
* • a RAW `process.stderr.write`. Node's `console.error` carries
105+
* `ignoreErrors`, which parks a temporary `error` listener across the write
106+
* — so oclif's warning blocks cannot crash this process at any size
107+
* (measured: 1 MiB through `console.error` does not, one line through
108+
* `process.stderr.write` does, 3/3 each).
109+
*
110+
* `os serve` is both: `printDiagnostic` in `src/commands/serve.ts` writes
111+
* straight to stderr (#7915) and the boot around it is asynchronous, so the
112+
* process is alive across the whole sequence. Measured on `examples/app-todo`
113+
* through this file, read end destroyed (`stdio: ['ignore','ignore','pipe']`,
114+
* then `child.stderr.destroy()`), traced with a `--import` observer that
115+
* installs NO listener here and wraps no write:
116+
*
117+
* uncaughtException code=EPIPE msg=write EPIPE
118+
* at afterWriteDispatched (node:internal/stream_base_commons:159:15)
119+
* exit code=1
120+
*
121+
* 3 of 3 runs, 3049-3433 ms in — the same frame and the same status #14858
122+
* traced on the dev shim. The same child read by a draining parent boots and
123+
* serves, exit 0 at a 20 s SIGTERM, having written 7926 bytes over 16.6 s. So
124+
* the crash costs the run at its FIRST diagnostic line and 20 of its 21 stderr
125+
* writes, on the entry point a customer's install actually runs (`files` names
126+
* only `dist`, but npm packs a `bin` target regardless — #14874).
127+
*
128+
* ⛔ Deliberately NOT narrowed to `error.code === 'EPIPE'`, for the reason
129+
* `bin/run-dev.js` records: the reason to tolerate is not WHICH error it is.
130+
* Every event here means one thing — a write to stderr failed — the only
131+
* channel it could be reported on is the stream that just failed, and there is
132+
* no other action to take.
133+
*
134+
* ⚠️ What it costs: a long-running command whose reader has gone now keeps
135+
* running instead of dying on its first diagnostic. That is the point (the
136+
* server is still serving, and its caller still gets the CLI's own status), but
137+
* it is a real behaviour change for a supervisor that destroyed the read end
138+
* and relied on the crash to end the child.
139+
*/
140+
// ⚠️ NAMED, and not for tidiness. Node parks an anonymous `once('error')` on
141+
// this stream for the duration of a `console.error` (`ignoreErrors`), so
142+
// "something is listening" is briefly true in any process and cannot tell this
143+
// listener apart from that one — a pin that polled the COUNT passed against a
144+
// tree with this whole block deleted, measured. The name is what
145+
// `published-entry-stderr-error-listener.test.ts` waits for and asserts on;
146+
// it also puts a legible frame in any listener dump.
147+
process.stderr.on('error', function objectstackStderrErrorIsNotFatal() {
148+
// Nothing to report, and nowhere left to report it.
149+
});
150+
73151
await run(process.argv.slice(2), import.meta.url)
74152
.then(async (result) => {
75153
flush();
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The #14858 crash class, manufactured INSIDE the published entry point's own
5+
* process — driven by `published-entry-stderr-error-listener.test.ts`.
6+
*
7+
* Loaded with `node --import <this> bin/run.js …` against a read end the parent
8+
* has destroyed, so everything below runs in the same process as the shipped
9+
* CLI, on the same open file description, after `bin/run.js` has had its chance
10+
* to attach the `error` listener.
11+
*
12+
* ## Why the failing write is manufactured rather than taken from a command
13+
*
14+
* The field reproduction is `os serve`: `printDiagnostic` writes straight to
15+
* stderr (#7915), the boot around it is asynchronous, and #15564 measured the
16+
* published entry dying there — `write EPIPE` at `afterWriteDispatched`, exit
17+
* 1, 3 of 3 runs, 3049-3433 ms in. Reproducing THAT needs a fixture app, a
18+
* database, a bound port and four seconds per leg, and it pins the crash to one
19+
* command that could stop writing raw tomorrow. What the entry point owes is
20+
* narrower and does not move: **a failed stderr write in this process must not
21+
* be fatal.** One raw write to a destroyed pipe is the whole of that hazard,
22+
* and it costs milliseconds.
23+
*
24+
* ⛔ The write is deliberately `process.stderr.write` and NOT `console.error`.
25+
* Node's `console.error` carries `ignoreErrors`, which parks a temporary
26+
* `error` listener across the write, so it cannot crash a process at any
27+
* payload size — measured at 1 MiB, 0 of 3, against one line through
28+
* `process.stderr.write` at 3 of 3. A probe written with `console.error` would
29+
* be green with the listener REMOVED, which is the one thing it must not be.
30+
*
31+
* ## The two arms, and why the unguarded one is not an ablation
32+
*
33+
* `OS_PUBLISHED_ENTRY_ERROR_PROBE_ARM=unguarded` makes this probe remove the
34+
* entry's listener in its own process before writing. That is the harness's
35+
* LIVE POSITIVE CONTROL: it shows, in the same run and against the same tree,
36+
* that this instrument can still see the crash — so the guarded arm's silence
37+
* is a reading rather than a zero. Nothing on disk is touched, so it costs no
38+
* restore and cannot leave a mutated tree behind.
39+
*
40+
* Markers go to a file: stderr is the thing under test and, on the arm that is
41+
* supposed to fail, the thing that is already broken.
42+
*
43+
* ## Why it waits for a NAMED listener and not for a count
44+
*
45+
* Node parks an anonymous `once('error', noop)` on this stream for the duration
46+
* of every `console.error` (`ignoreErrors`), so `listenerCount('error') > 0` is
47+
* briefly true in any process. An earlier version of this probe polled the
48+
* count, and under the ablation that deletes the entry's listener entirely it
49+
* still reported `LISTENER ATTACHED after 20 ms` — a green reading against a
50+
* tree with nothing guarding it. The name is the only thing that identifies
51+
* THIS listener, so it is what the poll waits for.
52+
*
53+
* env: `OS_PUBLISHED_ENTRY_ERROR_PROBE_MARKS` — the marker file.
54+
* env: `OS_PUBLISHED_ENTRY_ERROR_PROBE_ARM` — `guarded` (default) | `unguarded`.
55+
*/
56+
57+
import { appendFileSync } from 'node:fs';
58+
59+
const MARKS = process.env.OS_PUBLISHED_ENTRY_ERROR_PROBE_MARKS;
60+
const UNGUARDED = process.env.OS_PUBLISHED_ENTRY_ERROR_PROBE_ARM === 'unguarded';
61+
62+
const mark = (line) => appendFileSync(MARKS, `${line}\n`);
63+
64+
/**
65+
* How long to wait for `bin/run.js` to attach its listener before proceeding
66+
* anyway.
67+
*
68+
* A CONSTANT, and far above anything the attach legitimately needs: it happens
69+
* at the top of `bin/run.js`, after one dynamic `import()` of a dependency-free
70+
* module, and every `@oclif/core` byte is written later, inside `run()`. The
71+
* bound exists only so an absent listener is REPORTED rather than waited on
72+
* forever — it is not an oracle over how fast the attach is, and the harness
73+
* asserts the mark this produces rather than the number in it.
74+
*/
75+
const ATTACH_WAIT_MS = 15_000;
76+
77+
/** Comfortably finer than anything being timed. */
78+
const POLL_MS = 10;
79+
80+
/**
81+
* The listener `bin/run.js` attaches, by name. Mirrored rather than imported —
82+
* that file runs the CLI at module top, so there is nothing to import from it —
83+
* and held equal to the entry's spelling by a case in the driving suite, the
84+
* same discipline `run-dev-unbuilt-workspace.e2e.test.ts` uses for the shim's
85+
* drain bound.
86+
*/
87+
const LISTENER_NAME = 'objectstackStderrErrorIsNotFatal';
88+
89+
/** Is the entry's OWN listener on the stream right now? */
90+
const guardAttached = () => process.stderr.listeners('error').some((fn) => fn?.name === LISTENER_NAME);
91+
92+
/**
93+
* ⛔ This probe installs NO `error` listener of its own on `process.stderr`.
94+
* `uncaughtExceptionMonitor` observes the default action without preventing it,
95+
* so an unguarded run still dies exactly as it would unobserved — an
96+
* `uncaughtException` handler would have changed the very thing being read.
97+
*/
98+
process.on('uncaughtExceptionMonitor', (error) => {
99+
mark(`UNCAUGHT code=${error?.code} msg=${error?.message}`);
100+
});
101+
102+
process.on('exit', (code) => mark(`EXIT code=${code}`));
103+
104+
function writeAndOutliveIt() {
105+
// ONE raw write. The read end is already gone, so this fails; whether that
106+
// failure is fatal is the entire subject.
107+
process.stderr.write('published-entry-stderr-error-probe: one line to a read end that is gone\n');
108+
mark('WROTE');
109+
110+
// ⚠️ The turn is the point, not the delay. A failing write reports through
111+
// libuv's completion callback, so a write followed by a SYNCHRONOUS exit is
112+
// never told at all — which is exactly why the two probes on #15564's card
113+
// read clean, and why a probe that exited here would reproduce their zero
114+
// reading instead of testing anything.
115+
//
116+
// ⛔ NOT unref'd: this timer is what keeps the process alive across that
117+
// turn, and an unref'd one would let the CLI's own exit race it away.
118+
setTimeout(() => {
119+
mark('SURVIVED');
120+
// A distinctive status, so "ended on its own past the write" is evidence
121+
// about THIS probe rather than about any process that happens to exit 0.
122+
process.exit(7);
123+
}, 250);
124+
}
125+
126+
let waited = 0;
127+
const poll = setInterval(() => {
128+
const attached = guardAttached();
129+
if (!attached && waited < ATTACH_WAIT_MS) {
130+
waited += POLL_MS;
131+
return;
132+
}
133+
clearInterval(poll);
134+
mark(attached ? `LISTENER ATTACHED after ${waited} ms` : `LISTENER ABSENT after ${waited} ms`);
135+
// The raw count too, as EVIDENCE in a failure message — never as the oracle.
136+
mark(`LISTENERS count=${process.stderr.listenerCount('error')}`);
137+
if (UNGUARDED) {
138+
// The live positive control — in this process only, never on disk.
139+
//
140+
// ⛔ BY NAME, not `removeAllListeners('error')`. The two are equivalent on
141+
// today's tree, but the control has to measure "the ENTRY's listener is
142+
// absent"; clearing the stream measures "no listener at all", and the day
143+
// anything else attaches one here — a library, a future prologue, node
144+
// itself — that would silently become a different experiment from the one
145+
// the driving case claims to run.
146+
for (const fn of process.stderr.listeners('error')) {
147+
if (fn?.name === LISTENER_NAME) process.stderr.removeListener('error', fn);
148+
}
149+
mark(`ARM unguarded listeners=${process.stderr.listenerCount('error')} guard=${guardAttached()}`);
150+
} else {
151+
mark(`ARM guarded listeners=${process.stderr.listenerCount('error')}`);
152+
}
153+
writeAndOutliveIt();
154+
}, POLL_MS);

0 commit comments

Comments
 (0)