From 422512d2b2c9b63d3e9d48bdcd7c350e1aea7f80 Mon Sep 17 00:00:00 2001 From: dtzp555 Date: Sun, 31 May 2026 17:18:15 +1000 Subject: [PATCH] fix(server): add process-level crash handlers unhandledRejection/uncaughtException/clientError (P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registers three crash-prevention handlers at module level so they are installed before the first request arrives. These are no-ops on the happy-path /v1/chat/completions request cycle. process.on('unhandledRejection', ...) — logs and stays alive process.on('uncaughtException', ...) — logs and stays alive server.on('clientError', ...) — destroys the broken socket process.exit is intentionally NOT called in unhandledRejection / uncaughtException — the daemon logs the event and continues serving. The clientError handler destroys the socket to release the file descriptor; if left open, broken sockets from mid-upload client aborts or malformed HTTP headers accumulate and exhaust fd limits. server.on('clientError') is attached inside createOlpServer() (the factory function) so it is present on every server instance, including test servers created via the factory — enabling the 45f listener-count assertion. Authority (ALIGNMENT.md Rule 1 + Rule 5): Entry-surface scope — OpenAI Chat Completions API contract unchanged. https://platform.openai.com/docs/api-reference/chat happy-path /v1/chat/completions is unaffected; these handlers only fire on low-level TCP/parse errors or unhandled promises outside the request cycle. Port source: OCP server.mjs:2134-2146. Tests (Suite 45): 45a–45c: Task A — stdin EPIPE guard fires and logs stdin_write_error in anthropic, codex, and mistral providers (mock spawn with EventEmitter stdin that emits EPIPE). 45d: process.listenerCount('unhandledRejection') >= 1 45e: process.listenerCount('uncaughtException') >= 1 45f: server created by createOlpServer() has a clientError listener >= 1 Total test count: 819 pass / 0 fail (813 pre-existing + 6 new). CHANGELOG.md: adds Unreleased entry for both P1 crash-hardening fixes per release_kit phase_rolling_mode policy (Phase 7 closed at v0.7.0; D-day commits land under Unreleased until Phase 8 close). Co-Authored-By: Claude --- CHANGELOG.md | 15 ++- server.mjs | 27 ++++- test-features.mjs | 250 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1289827..a4f5a4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,20 @@ All notable changes to OLP land here. Per `CLAUDE.md` release_kit overlay, this ## Unreleased -(no in-flight changes) +### P1 daemon-crash hardening (fix/olp-p1-crash-hardening) + +- **fix(providers): guard stdin EPIPE on fast-failing CLI spawn** — added + `proc.stdin.on('error', ...)` guard in `lib/providers/anthropic.mjs`, + `lib/providers/codex.mjs`, and `lib/providers/mistral.mjs` before any + `stdin.write()` / `stdin.end()` call. Prevents a fast-failing CLI (auth + error, bad model, oversized prompt) from crashing the daemon via an + unhandled EPIPE on the Node stdin Writable. Port of OCP server.mjs:780. + +- **fix(server): process-level crash handlers unhandledRejection / uncaughtException / clientError** — + registered `process.on('unhandledRejection', ...)`, + `process.on('uncaughtException', ...)`, and `server.on('clientError', ...)` + at module level. The daemon logs the event and stays alive; `process.exit` + is intentionally not called. Port of OCP server.mjs:2134-2146. ## v0.7.0 — 2026-05-29 — Phase 7 close: Solution 1 isolation + opus 4.8 diff --git a/server.mjs b/server.mjs index 9d55071..7b0d741 100644 --- a/server.mjs +++ b/server.mjs @@ -2383,6 +2383,23 @@ async function router(req, res) { } } +// ── Process-level crash handlers ───────────────────────────────────────── +// Prevent unhandled async rejections and synchronous exceptions from crashing +// the daemon. Registered once at module level so they are installed before +// the first request arrives. These are no-ops on the happy path. +// Port of OCP server.mjs:2134-2146. +// Authority: OpenAI Chat Completions entry-surface contract unchanged +// (https://platform.openai.com/docs/api-reference/chat); happy-path +// /v1/chat/completions is unaffected — these handlers only fire on +// low-level TCP/parse errors or unhandled promises outside the request cycle. +// Note: process.exit is intentionally NOT called — log only, daemon stays alive. +process.on('unhandledRejection', (reason) => + logEvent('error', 'unhandled_rejection', { reason: String(reason) }) +); +process.on('uncaughtException', (err) => + logEvent('error', 'uncaught_exception', { error: err && err.message, stack: err && err.stack }) +); + // ── Server factory + main guard ─────────────────────────────────────────── // // Factory pattern: `createOlpServer()` returns an http.Server bound to the @@ -2392,7 +2409,15 @@ async function router(req, res) { // import-time side effects when tests pull in server.mjs. export function createOlpServer() { - return createServer(router); + const srv = createServer(router); + // Destroy the socket on low-level HTTP parse errors (client abort mid-upload, + // malformed HTTP header, etc.) so broken connections don't crash the daemon. + // Port of OCP server.mjs:2144-2146. + // Authority: OpenAI Chat Completions entry-surface contract unchanged + // (https://platform.openai.com/docs/api-reference/chat); happy-path /v1/chat/completions + // is unaffected — clientError only fires on pre-request TCP-level failures. + srv.on('clientError', (err, socket) => { try { socket.destroy(); } catch {} }); + return srv; } export { router, loadedProviders, VERSION }; diff --git a/test-features.mjs b/test-features.mjs index c269f1e..d613417 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -18318,3 +18318,253 @@ describe('Suite 44 — sandbox Layer 3 E2E test (PI231 only) [SKIP: awaiting Tas assert.ok(true, 'placeholder — real test lands in Task #9'); }); }); + +// ── Suite 45 — P1 daemon-crash hardening (fix/olp-p1-crash-hardening) ──── +// +// Two groups of tests: +// 45a–45c Task A: stdin EPIPE guard in provider plugins +// 45d–45f Task B: process-level crash handlers in server.mjs +// +// Authority citations (Task A): +// claude CLI v2.1.104 observed behaviour — fast-failing spawn closes stdin +// before OLP finishes writing; Node stdin Writable surfaces EPIPE. +// Port of OCP server.mjs:780. +// ALIGNMENT.md Rule 2: no CLI operation changed; guard is a Node stream +// safety net only. +// +// Authority citations (Task B): +// OpenAI Chat Completions entry-surface contract unchanged +// (https://platform.openai.com/docs/api-reference/chat); happy-path +// /v1/chat/completions is unaffected. +// Port of OCP server.mjs:2134-2146. + +describe('Suite 45 — P1 daemon-crash hardening: stdin EPIPE guard + process crash handlers', () => { + + // ── 45a: anthropic.mjs stdin EPIPE guard ────────────────────────────── + // Injects a mock spawn where proc.stdin is an EventEmitter that emits an + // EPIPE error immediately after the spawn call. Asserts the guard catches + // the error (console.error called with 'stdin_write_error'), the generator + // exits cleanly, and no unhandled exception is thrown. + it('45a: anthropic stdin-error guard catches EPIPE and logs stdin_write_error without throwing', async () => { + const errors = []; + const origConsoleError = console.error; + console.error = (...args) => errors.push(args.join(' ')); + + // Build a mock spawn where stdin is an EventEmitter that fires EPIPE + // synchronously after the outer function returns (via setImmediate). + // The generator will call proc.stdin.on('error', handler) before write/end, + // so the handler is installed when the error event fires. + const stdinEE = new EventEmitter(); + stdinEE.write = () => {}; + stdinEE.end = () => { + // Simulate: child closes stdin immediately (auth error, bad model, etc.) + setImmediate(() => stdinEE.emit('error', Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }))); + }; + + let spawnCalled = false; + const mockSpawn = (_bin, _args, _opts) => { + spawnCalled = true; + const proc = new EventEmitter(); + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.stdin = stdinEE; + proc.killed = false; + proc.kill = () => {}; + // Close the process cleanly so the generator can finish + stdinEE.end = () => { + setImmediate(() => { + stdinEE.emit('error', Object.assign(new Error('write EPIPE'), { code: 'EPIPE' })); + proc.stdout.emit('end'); + proc.stderr.emit('end'); + proc.emit('close', 1, null); + }); + }; + return proc; + }; + + __setSpawnImpl(mockSpawn); + try { + const ir = { + irVersion: 1, + model: 'claude-sonnet-4-6', + stream: false, + messages: [{ role: 'user', content: 'hi' }], + }; + // Consume the generator; it will hit EPIPE + throw ProviderError on exit 1. + // The EPIPE error must NOT propagate as unhandled — the guard absorbs it. + const gen = anthropic.spawn(ir, { accessToken: 'tok' }, null); + try { + // eslint-disable-next-line no-unused-vars + for await (const _chunk of gen) { /* drain */ } + } catch { + // Provider may throw ProviderError on exit code 1 — that is expected and OK. + // What must NOT happen is an unhandled EPIPE propagation. + } + } finally { + __resetSpawnImpl(); + console.error = origConsoleError; + } + + assert.ok(spawnCalled, 'mock spawn must have been called'); + // At least one console.error call must contain 'stdin_write_error' + const stdinErrorLogs = errors.filter(e => e.includes('stdin_write_error')); + assert.ok( + stdinErrorLogs.length >= 1, + `Expected at least one log containing "stdin_write_error"; got: ${JSON.stringify(errors)}`, + ); + // Verify the log is structured JSON with the expected fields + const parsed = JSON.parse(stdinErrorLogs[0]); + assert.equal(parsed.level, 'warn', 'log level must be warn'); + assert.equal(parsed.event, 'stdin_write_error', 'log event must be stdin_write_error'); + assert.ok(typeof parsed.error === 'string', 'log must include error string'); + }); + + // ── 45b: codex.mjs stdin EPIPE guard ────────────────────────────────── + it('45b: codex stdin-error guard catches EPIPE and logs stdin_write_error without throwing', async () => { + const errors = []; + const origConsoleError = console.error; + console.error = (...args) => errors.push(args.join(' ')); + + const stdinEE = new EventEmitter(); + stdinEE.write = () => {}; + + const mockSpawn = (_bin, _args, _opts) => { + const proc = new EventEmitter(); + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.stdin = stdinEE; + proc.killed = false; + proc.kill = () => {}; + stdinEE.end = () => { + setImmediate(() => { + stdinEE.emit('error', Object.assign(new Error('write EPIPE'), { code: 'EPIPE' })); + proc.stdout.emit('end'); + proc.stderr.emit('end'); + proc.emit('close', 1, null); + }); + }; + return proc; + }; + + codexSetSpawnImpl(mockSpawn); + try { + const ir = { + irVersion: 1, + model: 'codex-mini-latest', + stream: false, + // Multi-line prompt triggers useStdin=true path in codex.mjs + messages: [{ role: 'user', content: 'line1\nline2' }], + }; + const gen = codex.spawn(ir, { accessToken: 'test-key' }, null); + try { + for await (const _chunk of gen) { /* drain */ } + } catch { + // ProviderError on exit 1 — expected; EPIPE must be absorbed by guard. + } + } finally { + codexResetSpawnImpl(); + console.error = origConsoleError; + } + + const stdinErrorLogs = errors.filter(e => e.includes('stdin_write_error')); + assert.ok( + stdinErrorLogs.length >= 1, + `Expected at least one log containing "stdin_write_error"; got: ${JSON.stringify(errors)}`, + ); + const parsed = JSON.parse(stdinErrorLogs[0]); + assert.equal(parsed.level, 'warn'); + assert.equal(parsed.event, 'stdin_write_error'); + assert.ok(typeof parsed.error === 'string'); + }); + + // ── 45c: mistral.mjs stdin EPIPE guard ──────────────────────────────── + it('45c: mistral stdin-error guard catches EPIPE and logs stdin_write_error without throwing', async () => { + const errors = []; + const origConsoleError = console.error; + console.error = (...args) => errors.push(args.join(' ')); + + const stdinEE = new EventEmitter(); + stdinEE.write = () => {}; + + const mockSpawn = (_bin, _args, _opts) => { + const proc = new EventEmitter(); + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.stdin = stdinEE; + proc.killed = false; + proc.kill = () => {}; + // mistral only calls proc.stdin.end() — trigger EPIPE on end + stdinEE.end = () => { + setImmediate(() => { + stdinEE.emit('error', Object.assign(new Error('write EPIPE'), { code: 'EPIPE' })); + proc.stdout.emit('end'); + proc.stderr.emit('end'); + proc.emit('close', 1, null); + }); + }; + return proc; + }; + + mistralSetSpawnImpl(mockSpawn); + try { + const ir = { + irVersion: 1, + model: 'mistral-medium-latest', + stream: false, + messages: [{ role: 'user', content: 'hi' }], + }; + const gen = mistral.spawn(ir, { apiKey: 'test-key' }, null); + try { + for await (const _chunk of gen) { /* drain */ } + } catch { + // ProviderError on exit 1 — expected; EPIPE must be absorbed by guard. + } + } finally { + mistralResetSpawnImpl(); + console.error = origConsoleError; + } + + const stdinErrorLogs = errors.filter(e => e.includes('stdin_write_error')); + assert.ok( + stdinErrorLogs.length >= 1, + `Expected at least one log containing "stdin_write_error"; got: ${JSON.stringify(errors)}`, + ); + const parsed = JSON.parse(stdinErrorLogs[0]); + assert.equal(parsed.level, 'warn'); + assert.equal(parsed.event, 'stdin_write_error'); + assert.ok(typeof parsed.error === 'string'); + }); + + // ── 45d: process-level unhandledRejection handler is registered ──────── + it('45d: process.on(unhandledRejection) handler is registered (at least 1 listener)', () => { + // The handler is registered at server.mjs module load time (import-time side + // effect). Since server.mjs is already imported at the top of this test file + // (via createOlpServer), the listener must already be present. + const count = process.listenerCount('unhandledRejection'); + assert.ok( + count >= 1, + `Expected at least 1 unhandledRejection listener; got ${count}`, + ); + }); + + // ── 45e: process-level uncaughtException handler is registered ───────── + it('45e: process.on(uncaughtException) handler is registered (at least 1 listener)', () => { + const count = process.listenerCount('uncaughtException'); + assert.ok( + count >= 1, + `Expected at least 1 uncaughtException listener; got ${count}`, + ); + }); + + // ── 45f: server created by createOlpServer() has a clientError listener ── + it('45f: server returned by createOlpServer() has a clientError listener', async () => { + const { createOlpServer: createSrv45 } = await import('./server.mjs'); + const srv = createSrv45(); + const count = srv.listenerCount('clientError'); + srv.close(); + assert.ok( + count >= 1, + `Expected at least 1 clientError listener on server; got ${count}`, + ); + }); +});