From dabc40d42c604d10efbc2105487750155d087843 Mon Sep 17 00:00:00 2001 From: lsr911 Date: Tue, 7 Jul 2026 01:42:55 +0800 Subject: [PATCH] fix(crestodian): suppress unhandled stdout/stderr stream errors in probeLocalCommand (#100741) * fix(crestodian): suppress unhandled stdout/stderr stream errors in probeLocalCommand Add no-op error listeners on child stdout/stderr in probeLocalCommand to prevent unhandled EventEmitter errors from crashing the process. Replace standalone proof scripts with Vitest regression test that verifies error listeners are registered on the production code path. Co-Authored-By: Claude * test(crestodian): exercise probe stream failures --------- Co-authored-by: Claude Co-authored-by: Peter Steinberger --- src/crestodian/probes.stream-errors.test.ts | 59 +++++++++++++++++++++ src/crestodian/probes.ts | 5 ++ 2 files changed, 64 insertions(+) create mode 100644 src/crestodian/probes.stream-errors.test.ts diff --git a/src/crestodian/probes.stream-errors.test.ts b/src/crestodian/probes.stream-errors.test.ts new file mode 100644 index 000000000000..8a5e97442a5d --- /dev/null +++ b/src/crestodian/probes.stream-errors.test.ts @@ -0,0 +1,59 @@ +// Crestodian probe stream-error handling tests. +import type { ChildProcess, SpawnOptions } from "node:child_process"; +import { EventEmitter } from "node:events"; +import { PassThrough } from "node:stream"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const spawnMock = vi.hoisted(() => vi.fn()); + +type MockChildProcess = EventEmitter & { + stdin: PassThrough; + stdout: PassThrough; + stderr: PassThrough; + kill: ReturnType; +}; + +function createMockChildProcess(): MockChildProcess { + const child = new EventEmitter() as MockChildProcess; + child.stdin = new PassThrough(); + child.stdout = new PassThrough(); + child.stderr = new PassThrough(); + child.kill = vi.fn(); + return child; +} + +vi.mock("node:child_process", async () => { + const actual = await vi.importActual("node:child_process"); + return { ...actual, spawn: spawnMock }; +}); + +describe("probeLocalCommand stream error handling", () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + }); + + it("keeps child close authoritative when stdout and stderr emit errors", async () => { + const child = createMockChildProcess(); + + spawnMock.mockImplementationOnce( + (_cmd: string, _args: readonly string[], _opts: SpawnOptions): ChildProcess => { + process.nextTick(() => { + child.stdout.emit("error", new Error("stdout closed")); + child.stderr.emit("error", new Error("stderr closed")); + child.emit("close", 0); + }); + return child as unknown as ChildProcess; + }, + ); + + const { probeLocalCommand } = await import("./probes.js"); + + await expect(probeLocalCommand("echo", ["test"], { timeoutMs: 1000 })).resolves.toEqual({ + command: "echo", + found: true, + version: undefined, + error: undefined, + }); + }); +}); diff --git a/src/crestodian/probes.ts b/src/crestodian/probes.ts index 401e3f98c966..bb8cc739c841 100644 --- a/src/crestodian/probes.ts +++ b/src/crestodian/probes.ts @@ -19,6 +19,9 @@ export type LocalCommandProbe = { const LOCAL_COMMAND_PROBE_OUTPUT_MAX_CHARS = 16 * 1024; const LOCAL_COMMAND_PROBE_KILL_GRACE_MS = 500; +// The child close/error events own the probe result; pipe errors must not escape and crash setup. +const ignoreOutputStreamError = () => {}; + function appendBounded(previous: string, chunk: string, limit: number): string { const next = previous + chunk; return next.length > limit ? next.slice(-limit) : next; @@ -79,9 +82,11 @@ export async function probeLocalCommand( child.stdout.on("data", (chunk) => { stdout = appendBounded(stdout, String(chunk), outputLimit); }); + child.stdout.on("error", ignoreOutputStreamError); child.stderr.on("data", (chunk) => { stderr = appendBounded(stderr, String(chunk), outputLimit); }); + child.stderr.on("error", ignoreOutputStreamError); child.on("error", (err: NodeJS.ErrnoException) => { finish({ command,