fix(cli): surface the real spawn error in process supervisor (#8091) (#8158)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-22 11:28:20 -03:00
committed by GitHub
parent 9d0bdb871d
commit 40bd70c9cd
3 changed files with 125 additions and 1 deletions

View File

@@ -73,12 +73,32 @@ export class ServerSupervisor {
return this.child;
}
handleExit(code) {
handleExit(code, err) {
// Node.js v24+ requires process.exit() to receive a number. Spawn-error events
// deliver err.code (a string like 'ENOENT') via the 'error' listener; normalise here.
const exitCode = typeof code === "number" ? code : null;
cleanupPidFile("server");
// #8091: the child's spawn 'error' listener passes `err` through as a second
// argument, but it used to be silently dropped — the user only ever saw the
// hardcoded "code=-1" with a permanently empty crash log, with no way to
// diagnose why the child never started (ENOENT/EACCES/bad path/etc.). Surface
// the real reason immediately, both on the console and in the crash-log buffer
// so `dumpCrashLog()` shows it too.
if (err) {
const detail = [
err.code && `code=${err.code}`,
err.syscall && `syscall=${err.syscall}`,
err.path && `path=${err.path}`,
err.message,
]
.filter(Boolean)
.join(" ");
const line = `⚠ Spawn error: ${detail || String(err)}`;
console.error(line);
this.crashLog.push(line);
}
// #4425: only exit on an intentional shutdown. A spontaneous code-0 exit (e.g. a
// systemd MemoryMax cgroup kill, which reports the process exited cleanly) is anomalous
// and must be restarted, not treated as a graceful stop that leaves the gateway dead.

View File

@@ -0,0 +1 @@
- fix(cli): surface the real spawn error (err.code/path/syscall/message) in the process supervisor instead of silently swallowing it on a child spawn failure (#8091)

View File

@@ -0,0 +1,103 @@
import test from "node:test";
import assert from "node:assert/strict";
// #8091: when the child process itself fails to spawn (ENOENT/EACCES/etc.), the
// 'error' listener passes `err` into handleExit(-1, err) but handleExit only ever
// declared a single `code` parameter — the real reason (err.code/err.path/err.message)
// was silently dropped. The user only ever saw "Server exited (code=-1)" plus a
// permanently empty crash log, with no way to diagnose the underlying spawn failure.
//
// This test spawns a server that does not exist on disk, which forces a deterministic,
// cross-platform ENOENT 'error' event (no Windows/Bun needed to reproduce the bug class),
// and asserts the real error surfaces both via console.error and via crashLog/dumpCrashLog().
process.env.PORT = "0";
test("ServerSupervisor surfaces the real spawn error (ENOENT) instead of swallowing it (#8091)", async () => {
const { ServerSupervisor } = await import("../../bin/cli/runtime/processSupervisor.mjs");
const logs: string[] = [];
const origErr = console.error.bind(console);
console.error = (...args: unknown[]) => {
logs.push(args.map((a) => String(a)).join(" "));
};
const origExit = process.exit.bind(process);
// @ts-ignore
process.exit = () => {};
const supervisor = new ServerSupervisor({
serverPath: "/definitely/does/not/exist/server.js",
env: {},
maxRestarts: 0,
});
// Real spawn+'error' path the supervisor uses in production; ENOENT fires async.
supervisor.start();
await new Promise<void>((resolve) => {
const check = setInterval(() => {
if (logs.length || supervisor.crashLog.length) {
clearInterval(check);
resolve();
}
}, 10);
setTimeout(() => {
clearInterval(check);
resolve();
}, 2000);
});
// @ts-ignore
process.exit = origExit;
console.error = origErr;
const printed = logs.join("\n");
const inCrashLog = supervisor.crashLog.join("\n");
assert.ok(
printed.includes("ENOENT") || inCrashLog.includes("ENOENT"),
`expected the real spawn error (ENOENT) to be surfaced via console.error or crashLog, got:\nconsole.error: ${printed}\ncrashLog: ${inCrashLog}`
);
});
test("ServerSupervisor.handleExit(code, err) logs err.code/err.path/err.message and pushes into crashLog", async () => {
const { ServerSupervisor } = await import("../../bin/cli/runtime/processSupervisor.mjs");
const logs: string[] = [];
const origErr = console.error.bind(console);
console.error = (...args: unknown[]) => {
logs.push(args.map((a) => String(a)).join(" "));
};
const supervisor = new ServerSupervisor({
serverPath: "/fake/server.js",
env: {},
maxRestarts: 5,
});
// @ts-ignore — stub start() so the scheduled restart never spawns a real process
supervisor.start = () => null;
supervisor.startedAt = Date.now() - 100;
const fakeErr = Object.assign(new Error("spawn /fake/server.js ENOENT"), {
code: "ENOENT",
path: "/fake/server.js",
syscall: "spawn",
});
supervisor.handleExit(-1, fakeErr);
console.error = origErr;
const printed = logs.join("\n");
assert.ok(printed.includes("ENOENT"), `expected err.code (ENOENT) to be printed, got: ${printed}`);
assert.ok(
printed.includes("/fake/server.js"),
`expected err.path to be printed, got: ${printed}`
);
assert.ok(
supervisor.crashLog.some((l: string) => l.includes("ENOENT")),
`expected the error to be pushed into crashLog, got: ${JSON.stringify(supervisor.crashLog)}`
);
await new Promise((r) => setTimeout(r, 1100)); // drain the scheduled restart timer
});