From 950007bdc6f947b381b6dad99bc030d1a06e25d1 Mon Sep 17 00:00:00 2001 From: Thibaut Lion Date: Mon, 27 Apr 2026 09:26:26 +0200 Subject: [PATCH] fix(bridge): handle async spawn 'error' + default MAME bin to /usr/games/mame Two related fixes after debugging launches on the Pi: 1. ENOENT ("binary not found") fires asynchronously as an 'error' event on the ChildProcess, not a synchronous throw. MameProcess only listened to 'exit', so Node escalated the unhandled 'error' into a process-wide exception and the systemd service died on every missing-binary attempt. Now both events are caught, the first one that fires emits a spawn-error ExitReason, and the bridge stays alive. Two new test cases pin the contract. 2. systemd's default PATH doesn't include /usr/games, the directory Debian uses for MAME. A bare spawn("mame") found nothing even though the binary was installed. Default the bridge's MAME binary to /usr/games/mame, with SPRIXE_BRIDGE_MAME_BIN as the override for other distros. The Pi service unit now sets the env var explicitly so the resolution is unambiguous. --- .../sprixe-bridge/src/__tests__/mame.test.ts | 38 ++++++++++++++++++- packages/sprixe-bridge/src/index.ts | 8 +++- packages/sprixe-bridge/src/mame.ts | 14 +++++++ packages/sprixe-image/first-boot.sh | 1 + 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/packages/sprixe-bridge/src/__tests__/mame.test.ts b/packages/sprixe-bridge/src/__tests__/mame.test.ts index 48d39e8..6fe8e89 100644 --- a/packages/sprixe-bridge/src/__tests__/mame.test.ts +++ b/packages/sprixe-bridge/src/__tests__/mame.test.ts @@ -6,9 +6,13 @@ class FakeProcess implements SpawnedProcessLike { killed = false; killSignal: NodeJS.Signals | undefined; private exitHandler: ((code: number | null, signal: NodeJS.Signals | null) => void) | null = null; + private errorHandler: ((err: Error) => void) | null = null; - on(event: "exit", cb: (code: number | null, signal: NodeJS.Signals | null) => void): void { - if (event === "exit") this.exitHandler = cb; + on(event: "exit", cb: (code: number | null, signal: NodeJS.Signals | null) => void): void; + on(event: "error", cb: (err: Error) => void): void; + on(event: string, cb: (...args: unknown[]) => void): void { + if (event === "exit") this.exitHandler = cb as (c: number | null, s: NodeJS.Signals | null) => void; + if (event === "error") this.errorHandler = cb as (e: Error) => void; } kill(signal?: NodeJS.Signals): boolean { @@ -20,6 +24,10 @@ class FakeProcess implements SpawnedProcessLike { fireExit(code: number | null, signal: NodeJS.Signals | null = null): void { this.exitHandler?.(code, signal); } + + fireError(err: Error): void { + this.errorHandler?.(err); + } } function makeSpawner(): { spawner: Spawner; instances: FakeProcess[]; cmds: { cmd: string; args: string[] }[] } { @@ -93,6 +101,32 @@ describe("MameProcess", () => { expect(mame.isRunning()).toBe(false); }); + it("translates async ENOENT 'error' events into spawn-error (not unhandled)", () => { + const { spawner, instances } = makeSpawner(); + const mame = new MameProcess({ spawner }); + const cb = vi.fn(); + mame.onExit(cb); + mame.start({ gameId: "sf2", romPath: "/tmp/roms" }); + // Node fires 'error' asynchronously when the binary is missing. + instances[0]!.fireError(new Error("spawn mame ENOENT")); + expect(cb).toHaveBeenCalledWith({ + kind: "spawn-error", + error: expect.objectContaining({ message: "spawn mame ENOENT" }), + }); + expect(mame.isRunning()).toBe(false); + }); + + it("ignores 'exit' that fires after an 'error' (Node sometimes emits both)", () => { + const { spawner, instances } = makeSpawner(); + const mame = new MameProcess({ spawner }); + const cb = vi.fn(); + mame.onExit(cb); + mame.start({ gameId: "sf2", romPath: "/tmp/roms" }); + instances[0]!.fireError(new Error("ENOENT")); + instances[0]!.fireExit(null); + expect(cb).toHaveBeenCalledTimes(1); + }); + it("can relaunch after the previous instance exits", () => { const { spawner, instances } = makeSpawner(); const mame = new MameProcess({ spawner }); diff --git a/packages/sprixe-bridge/src/index.ts b/packages/sprixe-bridge/src/index.ts index 4600cdb..b688264 100644 --- a/packages/sprixe-bridge/src/index.ts +++ b/packages/sprixe-bridge/src/index.ts @@ -5,11 +5,17 @@ */ import { BridgeServer } from "./server.js"; +import { MameProcess } from "./mame.js"; const port = Number(process.env.SPRIXE_BRIDGE_PORT ?? "7777"); const romDir = process.env.SPRIXE_BRIDGE_ROM_DIR ?? "/tmp/sprixe-roms"; +// Debian ships MAME at /usr/games/mame which isn't in systemd's +// default PATH, so a bare "mame" spawn fails with ENOENT under +// systemd even though the binary is installed. Default to the +// canonical Debian path; override via env var on other distros. +const mameBin = process.env.SPRIXE_BRIDGE_MAME_BIN ?? "/usr/games/mame"; -const server = new BridgeServer({ port, romDir }); +const server = new BridgeServer({ port, romDir, mame: new MameProcess({ bin: mameBin }) }); await server.start(); console.log(`[sprixe-bridge] listening on http://127.0.0.1:${port} (romDir: ${romDir})`); diff --git a/packages/sprixe-bridge/src/mame.ts b/packages/sprixe-bridge/src/mame.ts index 1fbe371..0c0ca45 100644 --- a/packages/sprixe-bridge/src/mame.ts +++ b/packages/sprixe-bridge/src/mame.ts @@ -12,6 +12,7 @@ import { spawn, type ChildProcess } from "node:child_process"; export interface SpawnedProcessLike { pid?: number | undefined; on(event: "exit", cb: (code: number | null, signal: NodeJS.Signals | null) => void): void; + on(event: "error", cb: (err: Error) => void): void; kill(signal?: NodeJS.Signals): boolean; } @@ -81,7 +82,20 @@ export class MameProcess { return; } this.current = proc; + // ENOENT (binary missing) is delivered as an 'error' event, not a + // synchronous throw — without a listener, Node escalates it to an + // unhandled exception and the whole bridge dies. Treat it the same + // way as a synchronous spawn failure: clear state, fire spawn-error. + let settled = false; + proc.on("error", (err) => { + if (settled) return; + settled = true; + this.current = null; + this.notifyExit({ kind: "spawn-error", error: err }); + }); proc.on("exit", (code, signal) => { + if (settled) return; + settled = true; this.current = null; this.notifyExit({ kind: "exit", code, signal }); }); diff --git a/packages/sprixe-image/first-boot.sh b/packages/sprixe-image/first-boot.sh index 5887daa..5903728 100755 --- a/packages/sprixe-image/first-boot.sh +++ b/packages/sprixe-image/first-boot.sh @@ -193,6 +193,7 @@ WorkingDirectory=/opt/sprixe/packages/sprixe-bridge ExecStart=/usr/bin/node /opt/sprixe/packages/sprixe-bridge/dist/index.js Environment=SPRIXE_BRIDGE_PORT=7777 Environment=SPRIXE_BRIDGE_ROM_DIR=/home/sprixe/sprixe-roms +Environment=SPRIXE_BRIDGE_MAME_BIN=/usr/games/mame Restart=on-failure RestartSec=5