From 4cdc5c4e76e8db93a211f1bd325c76f0c6c78f6c Mon Sep 17 00:00:00 2001 From: Aliou Diallo Date: Wed, 5 Aug 2026 09:16:27 +0200 Subject: [PATCH] fix(manager): capture async spawn errors on null-pid starts A spawn that fails during initialization (missing cwd, non-executable shell) returns a child with no pid and emits its error event asynchronously. The null-pid path returned before any error listener was attached, so the error crashed the host via uncaughtException. The null-pid path now attaches an error listener that records the real failure reason (ENOENT/EACCES) on the already-exited record instead of crashing. Fixes #62 --- .changeset/handle-null-pid-spawn-errors.md | 5 +++ src/manager/index.test.ts | 35 +++++++++++++++ src/manager/process-runtime-controller.ts | 50 +++++++++++++++------- tests/e2e/spawn-error.e2e.ts | 39 +++++++++++++++++ 4 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 .changeset/handle-null-pid-spawn-errors.md create mode 100644 tests/e2e/spawn-error.e2e.ts diff --git a/.changeset/handle-null-pid-spawn-errors.md b/.changeset/handle-null-pid-spawn-errors.md new file mode 100644 index 0000000..01426ce --- /dev/null +++ b/.changeset/handle-null-pid-spawn-errors.md @@ -0,0 +1,5 @@ +--- +"@aliou/pi-processes": patch +--- + +Handle async spawn errors on failed starts. When a spawn fails during initialization (e.g. a non-existent cwd), the child has no pid and emits its `error` event asynchronously; the null-pid path now attaches an error listener so the failure is captured on the record (real `ENOENT`/`EACCES` reason) instead of crashing the host via `uncaughtException`. diff --git a/src/manager/index.test.ts b/src/manager/index.test.ts index 68f425b..6559379 100644 --- a/src/manager/index.test.ts +++ b/src/manager/index.test.ts @@ -125,6 +125,17 @@ vi.mock("../utils/command-executor", () => ({ spawnCommand: vi.fn((command: string) => { const child = new FakeChildProcess(); if (command === "missing pid") child.pid = undefined; + if (command === "missing pid error") { + child.pid = undefined; + process.nextTick(() => { + child.emit( + "error", + Object.assign(new Error("spawn /bin/bash ENOENT"), { + code: "ENOENT", + }), + ); + }); + } if (child.pid !== undefined) fakeProcesses.set(child.pid, child); queueMicrotask(() => { if (command === "spawn error") { @@ -363,6 +374,30 @@ describe("lifecycle events", () => { ); }); + it("handles async spawn errors on null-pid starts without crashing", async () => { + using manager = new ProcessManager(); + const events = collectEvents(manager); + // The async `error` event must be handled: attaching no listener would + // crash the host via uncaughtException and fail the whole test run. + const info = manager.start("test", "missing pid error", "/tmp"); + await new Promise((r) => setImmediate(r)); + + const ended = events.filter((e) => e.type === "process_ended"); + expect(ended).toHaveLength(1); + // The record is transitioned to exited synchronously; the async error + // handler then enriches it in place with the real reason. + const updated = manager.get(info.id); + expect(updated).toEqual( + expect.objectContaining({ + status: "exited", + success: false, + exitCode: -1, + endReason: "spawn_error", + errorMessage: "spawn /bin/bash ENOENT", + }), + ); + }); + it("records lost processes found by liveness watcher", async () => { vi.useFakeTimers(); using manager = new ProcessManager(); diff --git a/src/manager/process-runtime-controller.ts b/src/manager/process-runtime-controller.ts index 629aa22..1289848 100644 --- a/src/manager/process-runtime-controller.ts +++ b/src/manager/process-runtime-controller.ts @@ -83,6 +83,11 @@ export class ProcessRuntimeController { this.registry.add(managed); if (!child.pid) { + // No pid means the process never started. The async spawn `error` + // event may not have been delivered yet (Node emits it on a later + // turn of the event loop, and it cannot fire while start() runs), + // so attach a handler that finalizes the record with the real + // reason when it arrives. this.logs.appendErrorLine(managed.stderrFile, "Spawn error: missing pid"); managed.exitCode = -1; managed.success = false; @@ -91,6 +96,14 @@ export class ProcessRuntimeController { managed.endTime = Date.now(); this.releaseRuntimeHandles(managed); this.transition(managed, "exited"); + child.on("error", (err) => { + this.logs.appendErrorLine( + managed.stderrFile, + `Process error: ${err.message}`, + ); + managed.endReason = "spawn_error"; + managed.errorMessage = err.message; + }); return managed; } @@ -361,24 +374,31 @@ export class ProcessRuntimeController { }); child.on("error", (err) => { - this.logs.appendErrorLine( - managed.stderrFile, - `Process error: ${err.message}`, - ); - - if (!managed.endTime) { - this.releaseRuntimeHandles(managed); - managed.exitCode = -1; - managed.success = false; - managed.endReason = "spawn_error"; - managed.errorMessage = err.message; - managed.endTime = Date.now(); - this.output.flush(managed); - this.transition(managed, "exited"); - } + this.handleSpawnError(managed, err); }); } + private handleSpawnError( + managed: ManagedProcessRecord, + err: NodeJS.ErrnoException, + ): void { + this.logs.appendErrorLine( + managed.stderrFile, + `Process error: ${err.message}`, + ); + + if (managed.endTime) return; + + this.releaseRuntimeHandles(managed); + managed.exitCode = -1; + managed.success = false; + managed.endReason = "spawn_error"; + managed.errorMessage = err.message; + managed.endTime = Date.now(); + this.output.flush(managed); + this.transition(managed, "exited"); + } + private ensureWatcherRunning(): void { if (this.watcher) return; if (!this.registry.hasAliveishProcesses()) return; diff --git a/tests/e2e/spawn-error.e2e.ts b/tests/e2e/spawn-error.e2e.ts new file mode 100644 index 0000000..0e45918 --- /dev/null +++ b/tests/e2e/spawn-error.e2e.ts @@ -0,0 +1,39 @@ +import { join } from "node:path"; + +import { expect } from "vitest"; +import { getManager } from "../../src/get-manager"; +import { test } from "./fixtures"; +import { collectEvents } from "./utils"; + +test("captures async spawn error for a non-existent cwd without crashing", async ({ + cwd, +}) => { + using manager = getManager(); + const events = collectEvents(manager); + + // The cwd does not exist, so the spawn fails during initialization: + // child.pid is undefined and an async ENOENT `error` event fires on the + // next tick. Without an error listener attached before start() returns, + // this crashes the process via uncaughtException (which would fail the + // whole test run). + const missingCwd = join(cwd, "does-not-exist"); + const info = manager.start("missing-cwd", "true", missingCwd); + + // The record is transitioned to exited synchronously; give the async + // spawn error a turn to arrive and enrich the record with the real reason. + await new Promise((r) => setImmediate(r)); + + const ended = manager.get(info.id); + expect(ended).toEqual( + expect.objectContaining({ + status: "exited", + success: false, + exitCode: -1, + endReason: "spawn_error", + }), + ); + expect(ended?.errorMessage).toMatch(/ENOENT/); + + const endedEvents = events.filter((e) => e.type === "process_ended"); + expect(endedEvents).toHaveLength(1); +}); -- 2.51.2