From 6e6f3c408fc19115d6cbe097e2291fcb8bb753c7 Mon Sep 17 00:00:00 2001 From: joaovictor91123 Date: Sat, 25 Jul 2026 18:12:51 -0700 Subject: [PATCH] fix(mcp): exit cleanly on stdout/stderr EPIPE instead of crashing Closes #8691 --- packages/loopover-mcp/bin/loopover-mcp.ts | 25 +++++ test/unit/mcp-cli-epipe.test.ts | 111 ++++++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 test/unit/mcp-cli-epipe.test.ts diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index 4baef27098..a6063cc630 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -1673,6 +1673,31 @@ function stdioToolDescription(name: any) { return tool.description; } +/** + * Attach error listeners so a broken pipe (EPIPE) or destroyed stream exits cleanly instead of + * crashing with an uncaught `Error: write EPIPE` (#8691). Injected streams/exit keep this unit-testable + * for codecov/patch; the real CLI wires process.stdout/stderr at entrypoint startup. + */ +export function installStdioErrorHandlers( + stdout: NodeJS.EventEmitter = process.stdout, + stderr: NodeJS.EventEmitter = process.stderr, + exitProcess: (code: number) => void = (code) => { + process.exit(code); + }, +): void { + const onWriteError = (_error: NodeJS.ErrnoException) => { + // Any stdout/stderr write failure (EPIPE when `| head` closes early, ERR_STREAM_DESTROYED, etc.) + // must end as a normal exit — never an uncaughtException stack dump. + exitProcess(1); + }; + stdout.on("error", onWriteError); + stderr.on("error", onWriteError); +} + +if (runAsCliEntrypoint) { + installStdioErrorHandlers(); +} + /* v8 ignore next 5 -- draining stdout/stderr before exit only matters for the real launched process writing to an OS pipe (large output can exceed the pipe's buffer, and a POSIX pipe write is asynchronous); the in-process test harness below never spawns a subprocess, so this never runs there. */ diff --git a/test/unit/mcp-cli-epipe.test.ts b/test/unit/mcp-cli-epipe.test.ts new file mode 100644 index 0000000000..1f172adf5e --- /dev/null +++ b/test/unit/mcp-cli-epipe.test.ts @@ -0,0 +1,111 @@ +import { EventEmitter } from "node:events"; +import { spawn } from "node:child_process"; +import { describe, expect, it, vi } from "vitest"; +import { bin } from "./support/mcp-cli-harness"; + +// TS5097: keep the .ts specifier out of a literal import() position. +const BIN_MODULE = "../../packages/loopover-mcp/bin/loopover-mcp.ts"; + +describe("stdio EPIPE handlers (#8691)", () => { + it("installStdioErrorHandlers exits cleanly when stdout or stderr emits an error", async () => { + const { installStdioErrorHandlers } = (await import(BIN_MODULE)) as { + installStdioErrorHandlers: ( + stdout?: EventEmitter, + stderr?: EventEmitter, + exitProcess?: (code: number) => void, + ) => void; + }; + + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + const exitProcess = vi.fn(); + installStdioErrorHandlers(stdout, stderr, exitProcess); + + stdout.emit("error", Object.assign(new Error("write EPIPE"), { code: "EPIPE" })); + expect(exitProcess).toHaveBeenCalledWith(1); + + exitProcess.mockClear(); + stderr.emit("error", Object.assign(new Error("write EPIPE"), { code: "EPIPE" })); + expect(exitProcess).toHaveBeenCalledWith(1); + + exitProcess.mockClear(); + stdout.emit("error", Object.assign(new Error("destroyed"), { code: "ERR_STREAM_DESTROYED" })); + expect(exitProcess).toHaveBeenCalledWith(1); + }); + + it("keeps normal version --json output unaffected", () => { + const stdout = spawn(process.execPath, [bin, "version", "--json"], { + env: { + ...process.env, + LOOPOVER_SKIP_NPM_VERSION_CHECK: "1", + LOOPOVER_API_TIMEOUT_MS: "1000", + }, + stdio: ["ignore", "pipe", "pipe"], + }); + return new Promise((resolve, reject) => { + const chunks: Buffer[] = []; + const errChunks: Buffer[] = []; + stdout.stdout?.on("data", (c) => chunks.push(c)); + stdout.stderr?.on("data", (c) => errChunks.push(c)); + stdout.on("error", reject); + stdout.on("close", (code) => { + try { + expect(code).toBe(0); + const payload = JSON.parse(Buffer.concat(chunks).toString("utf8")) as { name: string; version: string }; + expect(payload.name).toBeTruthy(); + expect(payload.version).toBeTruthy(); + expect(Buffer.concat(errChunks).toString("utf8")).not.toMatch(/EPIPE|uncaught/i); + resolve(); + } catch (error) { + reject(error); + } + }); + }); + }); + + it("exits cleanly when a large tools --json payload is piped into an early-closing reader", () => { + // Real broken-pipe scenario: CLI writes a large JSON list; consumer reads one chunk then exits, + // closing the pipe. Without the error listener this crashes with uncaught EPIPE. + const cli = spawn(process.execPath, [bin, "tools", "--json"], { + env: { + ...process.env, + LOOPOVER_SKIP_NPM_VERSION_CHECK: "1", + LOOPOVER_API_TIMEOUT_MS: "1000", + }, + stdio: ["ignore", "pipe", "pipe"], + }); + const consumer = spawn( + process.execPath, + ["-e", "process.stdin.once('data', () => process.exit(0)); process.stdin.resume();"], + { stdio: ["pipe", "ignore", "ignore"] }, + ); + cli.stdout?.pipe(consumer.stdin!); + + return new Promise((resolve, reject) => { + const errChunks: Buffer[] = []; + cli.stderr?.on("data", (c) => errChunks.push(c)); + const timer = setTimeout(() => { + cli.kill("SIGKILL"); + consumer.kill("SIGKILL"); + reject(new Error("CLI did not exit within timeout after broken pipe")); + }, 15_000); + cli.on("error", (error) => { + clearTimeout(timer); + reject(error); + }); + cli.on("close", (code, signal) => { + clearTimeout(timer); + try { + // Clean exit (code set, no signal kill) — not an uncaught-exception abort. + expect(signal).toBeNull(); + expect(code === 0 || code === 1).toBe(true); + const errText = Buffer.concat(errChunks).toString("utf8"); + expect(errText).not.toMatch(/uncaughtException|Unhandled|write EPIPE/i); + resolve(); + } catch (error) { + reject(error); + } + }); + }); + }); +});