From a5f5282816ac38e67b785736d759b185340ba3fe Mon Sep 17 00:00:00 2001 From: qingminlong Date: Thu, 9 Jul 2026 16:43:40 +0800 Subject: [PATCH] fix(scripts): report startup help stream errors when pipes fail (#102436) * fix(scripts): fail startup help rendering on stream errors * fix(scripts): preserve startup render failure provenance --------- Co-authored-by: Peter Steinberger --- scripts/write-cli-startup-metadata.ts | 29 +++++++++- .../write-cli-startup-metadata.test.ts | 58 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/scripts/write-cli-startup-metadata.ts b/scripts/write-cli-startup-metadata.ts index 856fe758ad9..3319120c9fc 100644 --- a/scripts/write-cli-startup-metadata.ts +++ b/scripts/write-cli-startup-metadata.ts @@ -382,14 +382,16 @@ async function spawnText( failureMessage: string; killGraceMs?: number; maxOutputBytes?: number; + spawnProcess?: typeof spawn; timeoutMs: number; }, ): Promise { const maxOutputBytes = options.maxOutputBytes ?? COMMAND_HELP_RENDER_MAX_OUTPUT_BYTES; const killGraceMs = options.killGraceMs ?? COMMAND_HELP_RENDER_KILL_GRACE_MS; + const spawnProcess = options.spawnProcess ?? spawn; const useProcessGroup = process.platform !== "win32"; return await new Promise((resolve, reject) => { - const child = spawn(process.execPath, args, { + const child = spawnProcess(process.execPath, args, { cwd: options.cwd, detached: useProcessGroup, env: options.env, @@ -399,6 +401,7 @@ async function spawnText( let stderr = ""; let outputBytes = 0; let outputExceeded = false; + let outputStreamError: { streamName: "stdout" | "stderr"; error: Error } | undefined; let settled = false; let timedOut = false; let waitingForKillGrace = false; @@ -483,6 +486,15 @@ async function spawnText( }; const finishClose = (result: { code: number | null; signal: NodeJS.Signals | null }) => { settle(() => { + if (outputStreamError) { + reject( + new Error( + `${options.failureMessage}: ${outputStreamError.streamName} read error: ${outputStreamError.error.message}`, + { cause: outputStreamError.error }, + ), + ); + return; + } if (result.code === 0 && !timedOut && !outputExceeded) { resolve(stdout); return; @@ -522,6 +534,15 @@ async function spawnText( signalChild("SIGTERM"); scheduleKill(); }; + const failOutputStream = (streamName: "stdout" | "stderr", error: Error) => { + // Keep the first stop cause: killing for a timeout or output cap can make + // the stdio pipes fail secondarily while the child is shutting down. + if (outputStreamError || timedOut || outputExceeded) { + return; + } + outputStreamError = { streamName, error }; + requestStop(); + }; const timeout = setTimeout(() => { timedOut = true; requestStop(); @@ -553,6 +574,12 @@ async function spawnText( } stderr += chunk; }); + child.stdout.once("error", (error: Error) => { + failOutputStream("stdout", error); + }); + child.stderr.once("error", (error: Error) => { + failOutputStream("stderr", error); + }); child.once("error", (error) => { settle(() => { reject(error); diff --git a/test/scripts/write-cli-startup-metadata.test.ts b/test/scripts/write-cli-startup-metadata.test.ts index 9db1329f25f..c9606492af9 100644 --- a/test/scripts/write-cli-startup-metadata.test.ts +++ b/test/scripts/write-cli-startup-metadata.test.ts @@ -1,7 +1,9 @@ // Write Cli Startup Metadata tests cover write cli startup metadata script behavior. import { spawn } from "node:child_process"; +import { EventEmitter } from "node:events"; import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; +import { PassThrough } from "node:stream"; import { pathToFileURL } from "node:url"; import { describe, expect, it, vi } from "vitest"; import { resolveWindowsTaskkillPath } from "../../scripts/lib/windows-taskkill.mjs"; @@ -70,6 +72,14 @@ function expectedTaskkillPath(): string { return resolveWindowsTaskkillPath(); } +function createSpawnTextChild() { + return Object.assign(new EventEmitter(), { + kill: vi.fn(() => true), + stderr: new PassThrough(), + stdout: new PassThrough(), + }); +} + async function waitForProcessExit( pid: number, timeoutMs = LOAD_SENSITIVE_PROCESS_TIMEOUT_MS, @@ -135,6 +145,54 @@ describe("write-cli-startup-metadata", () => { ).rejects.toThrow("render failed: output exceeded 1024 bytes"); }); + it.each(["stdout", "stderr"] as const)( + "fails command help rendering when %s emits a stream error", + async (streamName) => { + const child = createSpawnTextChild(); + const spawnProcess = vi.fn(() => child as unknown as ReturnType); + const streamError = new Error(`${streamName} pipe failed`); + + const render = __testing.spawnText(["--help"], { + cwd: process.cwd(), + env: process.env, + failureMessage: "render failed", + killGraceMs: 25, + maxOutputBytes: 1024, + spawnProcess, + timeoutMs: 5_000, + }); + + child[streamName].emit("error", streamError); + child.emit("close", null, "SIGTERM"); + + await expect(render).rejects.toMatchObject({ + message: `render failed: ${streamName} read error: ${streamName} pipe failed`, + cause: streamError, + }); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + }, + ); + + it("preserves an output-limit failure when shutdown also errors a stream", async () => { + const child = createSpawnTextChild(); + const spawnProcess = vi.fn(() => child as unknown as ReturnType); + const render = __testing.spawnText(["--help"], { + cwd: process.cwd(), + env: process.env, + failureMessage: "render failed", + killGraceMs: 25, + maxOutputBytes: 5, + spawnProcess, + timeoutMs: 5_000, + }); + + child.stdout.emit("data", "123456"); + child.stdout.emit("error", new Error("pipe closed during shutdown")); + child.emit("close", null, "SIGTERM"); + + await expect(render).rejects.toThrow("render failed: output exceeded 5 bytes"); + }); + it("signals Windows command help render process trees with taskkill", () => { const childKill = vi.fn(() => true); const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 }));