fix(cli): flush stdio before exiting so piped output survives #1411
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { exitAfterFlush, flushStdio, flushStream, type FlushableStream } from './flush-stdio.js'; | ||
|
|
||
| /** | ||
| * A stream that behaves the way a piped stdout does on macOS: the write is | ||
| * accepted immediately, but only completes once the OS has taken the bytes. | ||
| */ | ||
| function deferredStream(): FlushableStream & { complete: () => void; writes: string[] } { | ||
| const pending: (() => void)[] = []; | ||
| const writes: string[] = []; | ||
|
|
||
| return { | ||
| writes, | ||
| write(chunk: string, callback?: (error?: Error | null) => void): boolean { | ||
| writes.push(chunk); | ||
| if (callback) pending.push(() => callback(null)); | ||
| return true; | ||
| }, | ||
| complete(): void { | ||
| while (pending.length > 0) pending.shift()?.(); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| describe('flushStream', () => { | ||
| it('waits for the pending write to complete', async () => { | ||
| const stream = deferredStream(); | ||
| let drained = false; | ||
| const flushed = flushStream(stream).then(() => { | ||
| drained = true; | ||
| }); | ||
|
|
||
| await Promise.resolve(); | ||
| expect(drained).toBe(false); | ||
|
|
||
| stream.complete(); | ||
| await flushed; | ||
| expect(drained).toBe(true); | ||
| }); | ||
|
|
||
| it('gives up after the timeout so a stalled reader cannot wedge the exit', async () => { | ||
| vi.useFakeTimers(); | ||
| try { | ||
| const stream = deferredStream(); | ||
| const flushed = flushStream(stream, 50); | ||
| await vi.advanceTimersByTimeAsync(50); | ||
| await expect(flushed).resolves.toBeUndefined(); | ||
| } finally { | ||
| vi.useRealTimers(); | ||
| } | ||
| }); | ||
|
|
||
| it('resolves without writing when there is no usable stream', async () => { | ||
| await expect(flushStream(undefined)).resolves.toBeUndefined(); | ||
|
|
||
| const destroyed = { ...deferredStream(), destroyed: true }; | ||
| await flushStream(destroyed); | ||
| expect(destroyed.writes).toEqual([]); | ||
| }); | ||
|
|
||
| it('resolves when the write itself throws', async () => { | ||
| const stream: FlushableStream = { | ||
| write() { | ||
| throw new Error('EPIPE'); | ||
| }, | ||
| }; | ||
|
|
||
| await expect(flushStream(stream)).resolves.toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('flushStdio', () => { | ||
| it('drains every stream it is given', async () => { | ||
| const out = deferredStream(); | ||
| const err = deferredStream(); | ||
| const flushed = flushStdio({ streams: [out, err] }); | ||
|
|
||
| out.complete(); | ||
| err.complete(); | ||
|
|
||
| await expect(flushed).resolves.toBeUndefined(); | ||
| expect(out.writes).toEqual(['']); | ||
| expect(err.writes).toEqual(['']); | ||
| }); | ||
| }); | ||
|
|
||
| describe('exitAfterFlush', () => { | ||
| it('exits only once stdio has drained, with the requested code', async () => { | ||
| const out = deferredStream(); | ||
| const exit = vi.fn((code: number) => code as never); | ||
|
|
||
| const exited = exitAfterFlush(3, { streams: [out], exit }); | ||
|
|
||
| await Promise.resolve(); | ||
| expect(exit).not.toHaveBeenCalled(); | ||
|
|
||
| out.complete(); | ||
| await exited; | ||
| expect(exit).toHaveBeenCalledWith(3); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /** | ||
| * Flush `process.stdout` / `process.stderr` before a hard exit. | ||
| * | ||
| * Why this exists: Node's stdio writes are only synchronous for files and for | ||
| * POSIX TTYs. Pipes and sockets are **asynchronous on macOS**, so a | ||
| * `process.exit()` in the same tick as a `console.log()` throws away whatever | ||
| * is still buffered. The visible symptom is a command that prints fine in a | ||
| * terminal and prints *nothing* when piped or captured with `$(...)` — the | ||
| * `--json` payload disappears, and so does the error text that would have | ||
| * explained why the command failed. | ||
| * | ||
| * Every exit path that goes through a real `process.exit(code)` should await | ||
| * {@link exitAfterFlush} instead, which drains both streams first. The drain | ||
| * is bounded by a timeout so a stalled reader can never wedge the exit. | ||
| */ | ||
|
|
||
| /** Minimal writable surface we need; keeps the helper testable. */ | ||
| export interface FlushableStream { | ||
| write(chunk: string, callback?: (error?: Error | null) => void): boolean; | ||
| readonly destroyed?: boolean; | ||
| readonly writableEnded?: boolean; | ||
| } | ||
|
|
||
| /** How long to wait for a single stream to drain before giving up on it. */ | ||
| export const DEFAULT_FLUSH_TIMEOUT_MS = 2_000; | ||
|
|
||
| export interface FlushOptions { | ||
| /** Streams to drain. Defaults to `[process.stdout, process.stderr]`. */ | ||
| streams?: (FlushableStream | undefined)[]; | ||
| /** Per-stream drain budget. Defaults to {@link DEFAULT_FLUSH_TIMEOUT_MS}. */ | ||
| timeoutMs?: number; | ||
| } | ||
|
|
||
| /** | ||
| * Resolve once everything already written to `stream` has reached the OS. | ||
| * | ||
| * A zero-length write is queued behind the pending chunks, so its completion | ||
| * callback fires only after those chunks have been written — which is exactly | ||
| * the signal we lack on platforms where stdio is asynchronous. Errors (a | ||
| * closed pipe, for instance) resolve too: we are on our way out either way. | ||
| */ | ||
| export function flushStream( | ||
| stream: FlushableStream | undefined, | ||
| timeoutMs: number = DEFAULT_FLUSH_TIMEOUT_MS | ||
| ): Promise<void> { | ||
| return new Promise<void>((resolve) => { | ||
| if (!stream || typeof stream.write !== 'function' || stream.destroyed || stream.writableEnded) { | ||
| resolve(); | ||
| return; | ||
| } | ||
|
|
||
| let settled = false; | ||
| const settle = (): void => { | ||
| if (settled) return; | ||
| settled = true; | ||
| clearTimeout(timer); | ||
| resolve(); | ||
| }; | ||
|
|
||
| const timer = setTimeout(settle, timeoutMs); | ||
| // Never let the flush watchdog be the reason the process stays alive. | ||
| timer.unref?.(); | ||
|
|
||
| try { | ||
| stream.write('', () => settle()); | ||
| } catch { | ||
| settle(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** Drain stdout and stderr concurrently. Never rejects. */ | ||
| export async function flushStdio(options: FlushOptions = {}): Promise<void> { | ||
| const streams = options.streams ?? [process.stdout, process.stderr]; | ||
| const timeoutMs = options.timeoutMs ?? DEFAULT_FLUSH_TIMEOUT_MS; | ||
| await Promise.all(streams.map((stream) => flushStream(stream, timeoutMs))); | ||
| } | ||
|
|
||
| export interface ExitAfterFlushOptions extends FlushOptions { | ||
| /** Injectable for tests; defaults to the real `process.exit`. */ | ||
| exit?: (code: number) => never; | ||
| } | ||
|
|
||
| /** | ||
| * Drain stdio, then exit with `code`. | ||
| * | ||
| * Returns `Promise<never>` because the default `exit` does not return; the | ||
| * declared return type keeps call sites from needing an unreachable `return`. | ||
| */ | ||
| export async function exitAfterFlush(code: number, options: ExitAfterFlushOptions = {}): Promise<never> { | ||
| await flushStdio(options); | ||
| const exit = options.exit ?? ((value: number) => process.exit(value)); | ||
| return exit(code); | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a downstream reader closes a pipe while output is still pending (for example, a verbose command piped to
head), the pending stdout write emits an asynchronousEPIPEerrorevent. The callback and surroundingtry/catchdo not consume that event, and awaiting the flush gives Node time to treat it as unhandled, print a stack trace, and exit with code 1 instead of the command's requested code. This reproduces on Node 24 with a largeprocess.stdout.writefollowed by this zero-length flush piped tohead; install a temporary stream error handler during the flush or otherwise handleEPIPEexplicitly.Useful? React with 👍 / 👎.