Skip to content

Commit f07368b

Browse files
committed
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
1 parent 729cce1 commit f07368b

3 files changed

Lines changed: 62 additions & 6 deletions

File tree

packages/cli/bin/run.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,14 @@ try {
137137
* it is a real behaviour change for a supervisor that destroyed the read end
138138
* and relied on the crash to end the child.
139139
*/
140-
process.stderr.on('error', () => {
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.e2e.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() {
141148
// Nothing to report, and nowhere left to report it.
142149
});
143150

packages/cli/test/fixtures/published-entry-stderr-error-probe.mjs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@
4040
* Markers go to a file: stderr is the thing under test and, on the arm that is
4141
* supposed to fail, the thing that is already broken.
4242
*
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+
*
4353
* env: `OS_PUBLISHED_ENTRY_ERROR_PROBE_MARKS` — the marker file.
4454
* env: `OS_PUBLISHED_ENTRY_ERROR_PROBE_ARM` — `guarded` (default) | `unguarded`.
4555
*/
@@ -67,6 +77,18 @@ const ATTACH_WAIT_MS = 15_000;
6777
/** Comfortably finer than anything being timed. */
6878
const POLL_MS = 10;
6979

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+
7092
/**
7193
* ⛔ This probe installs NO `error` listener of its own on `process.stderr`.
7294
* `uncaughtExceptionMonitor` observes the default action without preventing it,
@@ -103,13 +125,15 @@ function writeAndOutliveIt() {
103125

104126
let waited = 0;
105127
const poll = setInterval(() => {
106-
const attached = process.stderr.listenerCount('error') > 0;
128+
const attached = guardAttached();
107129
if (!attached && waited < ATTACH_WAIT_MS) {
108130
waited += POLL_MS;
109131
return;
110132
}
111133
clearInterval(poll);
112134
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')}`);
113137
if (UNGUARDED) {
114138
// The live positive control — in this process only, never on disk.
115139
process.stderr.removeAllListeners('error');

packages/cli/test/published-entry-stderr-error-listener.e2e.test.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@
6060
* between the two entries is what #15564 refused to accept as evidence, and a
6161
* case asserting it would smuggle that reading back in.
6262
*
63-
* The reachability half is not left unheld either — case 3 keeps the premise
64-
* the measurement rests on: `serve` still writes to stderr RAW.
63+
* The reachability half is not left unheld either — the last case below keeps
64+
* the premise the measurement rests on: `serve` still writes to stderr RAW.
6565
*/
6666

6767
import { spawn } from 'node:child_process';
@@ -166,21 +166,46 @@ afterAll(() => {
166166
});
167167

168168
describe('the published entry point survives a failed stderr write', () => {
169-
it('attaches the listener before anything of its own can write', () => {
169+
it('attaches its OWN listener before anything of its own can write', () => {
170170
// The probe reports what it SAW rather than being assumed to have found it:
171171
// `LISTENER ABSENT` is the reading when `bin/run.js` stops attaching one,
172172
// and it is a different sentence from "the probe never ran".
173+
//
174+
// ⚠️ It looks for the listener BY NAME, and that is a correction rather
175+
// than a flourish: node parks an anonymous `once('error', noop)` across
176+
// every `console.error`, so a count-based version of this case reported
177+
// `LISTENER ATTACHED after 20 ms` against a tree with the whole block
178+
// deleted — measured, under the ablation below. The count is carried in
179+
// the markers as evidence and decides nothing.
173180
expect(
174181
guarded.marks,
175182
`the probe never reached the listener check, so it measured NOTHING — a zero reading, not a pass. Markers:\n${guarded.marks}`,
176183
).toMatch(/LISTENER (ATTACHED|ABSENT)/);
177184
expect(
178185
guarded.marks,
179-
`bin/run.js no longer attaches an \`error\` listener to process.stderr — a failed stderr write is an ` +
186+
`bin/run.js no longer attaches its \`error\` listener to process.stderr — a failed stderr write is an ` +
180187
`uncaught exception again on the entry point a customer's install runs (#14858, #15564). Markers:\n${guarded.marks}`,
181188
).toContain('LISTENER ATTACHED');
182189
});
183190

191+
it("keeps the probe's mirror of the listener name equal to the entry's own", () => {
192+
// The probe cannot import the name — `bin/run.js` runs the CLI at module
193+
// top — so it mirrors it, and a mirror with nothing holding it is how a
194+
// synchronisation point ends up waiting for a spelling that moved. Same
195+
// discipline as `run-dev-unbuilt-workspace.e2e.test.ts` keeps over the
196+
// shim's drain bound. ⛔ A renamed listener would not red the cases above:
197+
// the probe would simply time out and write early, which on a fast box
198+
// still survives.
199+
const mirrored = /const LISTENER_NAME = '([A-Za-z0-9_$]+)';/.exec(readFileSync(PROBE, 'utf8'))?.[1];
200+
expect(mirrored, `no LISTENER_NAME declaration found in ${PROBE}`).toBeDefined();
201+
const entry = maskComments(readFileSync(RUN_JS, 'utf8'));
202+
expect(
203+
entry,
204+
`${RUN_JS} does not attach a listener named ${mirrored}, which is the name the probe waits for — ` +
205+
`either the entry renamed it or it stopped attaching one at all.`,
206+
).toContain(`function ${mirrored}(`);
207+
});
208+
184209
it('outlives a failed write and ends with its own status, not a crash', () => {
185210
const evidence = `ceiling ${HARD_CAP_MS} ms (constant, load-independent by design); this child ran ${guarded.elapsedMs} ms. Markers:\n${guarded.marks}`;
186211
expect(guarded.marks, `the probe never made its write. ${evidence}`).toContain('WROTE');

0 commit comments

Comments
 (0)