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 <steipete@gmail.com>
This commit is contained in:
qingminlong 2026-07-09 16:43:40 +08:00 committed by GitHub
parent ecc2cffad8
commit a5f5282816
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 86 additions and 1 deletions

View file

@ -382,14 +382,16 @@ async function spawnText(
failureMessage: string;
killGraceMs?: number;
maxOutputBytes?: number;
spawnProcess?: typeof spawn;
timeoutMs: number;
},
): Promise<string> {
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);

View file

@ -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<typeof spawn>);
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<typeof spawn>);
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 }));