Skip to content

Commit 0f69987

Browse files
bugerclaude
andcommitted
fix: handle transient child-process I/O errors (read EIO) gracefully
Add global uncaughtException handler that suppresses transient I/O errors (EIO, EPIPE, ECONNRESET, ERR_STREAM_DESTROYED) from dying child processes instead of crashing the entire visor process. Three layers of defense: - Global handler in child-process-error-handler.ts (imported early) - Worktree manager skips process.exit(1) for transient I/O errors - Stream-level error handlers on MCP transport stderr pipes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 284589f commit 0f69987

8 files changed

Lines changed: 192 additions & 0 deletions

File tree

src/cli-main.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
#!/usr/bin/env node
22

3+
// Register global handler for transient child-process I/O errors (EIO, EPIPE)
4+
// before any other code runs so broken pipes from MCP servers etc. don't crash
5+
// the process.
6+
import './utils/child-process-error-handler';
7+
38
// Load environment variables from .env file (override existing to allow .env to take precedence)
49
import * as dotenv from 'dotenv';
510
dotenv.config({ override: true, quiet: true });

src/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
// GitHub event objects have complex dynamic structures that are difficult to fully type
33
// Using 'any' for these objects is acceptable as they come from external GitHub webhooks
44

5+
// Register global handler for transient child-process I/O errors (EIO, EPIPE)
6+
// before any other code runs so broken pipes from MCP servers etc. don't crash
7+
// the process.
8+
import './utils/child-process-error-handler';
9+
510
// Load environment variables from .env file (override existing to allow .env to take precedence)
611
import * as dotenv from 'dotenv';
712
dotenv.config({ override: true, quiet: true });

src/providers/mcp-check-provider.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,18 @@ export class McpCheckProvider extends CheckProvider {
597597

598598
const transport = createTransport();
599599

600+
// Attach error handler on the transport's stderr stream (if piped) so
601+
// that EIO / EPIPE errors from a dying child process don't become
602+
// uncaught exceptions that crash the whole visor process.
603+
if ('stderr' in transport && transport.stderr) {
604+
const stderrStream = transport.stderr as any;
605+
if (typeof stderrStream.on === 'function') {
606+
stderrStream.on('error', (err: Error) => {
607+
logger.debug(`MCP transport stderr error (${transportName}): ${err.message}`);
608+
});
609+
}
610+
}
611+
600612
try {
601613
// Connect with timeout
602614
let timeoutId: NodeJS.Timeout | undefined;

src/providers/script-check-provider.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,12 @@ export class ScriptCheckProvider extends CheckProvider {
382382
env,
383383
stderr: 'pipe',
384384
});
385+
// Prevent EIO errors on the stderr pipe from crashing the process
386+
if (transport.stderr) {
387+
transport.stderr.on('error', (err: Error) => {
388+
logger.debug(`MCP transport stderr error: ${err.message}`);
389+
});
390+
}
385391
await Promise.race([
386392
client.connect(transport),
387393
new Promise((_, reject) =>

src/slack/markdown.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ export async function renderMermaidToPng(mermaidCode: string): Promise<Buffer |
135135
proc.stderr?.on('data', data => {
136136
stderr += data.toString();
137137
});
138+
// Prevent EIO errors on stderr from becoming uncaught exceptions
139+
proc.stderr?.on('error', () => {});
140+
proc.stdout?.on('error', () => {});
138141

139142
proc.on('close', code => {
140143
if (code === 0) {
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* Global handler for transient child-process I/O errors.
3+
*
4+
* When a child process (MCP server, probe agent, sandbox command, etc.) dies
5+
* unexpectedly, its stdio streams may emit `read EIO`, `write EPIPE`, or
6+
* similar errors. If no listener is attached to the stream's `error` event
7+
* the error bubbles up as an uncaught exception and crashes the entire visor
8+
* process.
9+
*
10+
* Importing this module installs a single, idempotent `uncaughtException`
11+
* handler that recognises these transient I/O errors, logs a warning, and
12+
* swallows them so the rest of the process can continue.
13+
*
14+
* Usage: import once, as early as possible in every entry point.
15+
*
16+
* import './utils/child-process-error-handler';
17+
*/
18+
19+
import { logger } from '../logger';
20+
21+
const TRANSIENT_IO_CODES = new Set(['EIO', 'EPIPE', 'ECONNRESET']);
22+
23+
export function isChildProcessIOError(error: unknown): boolean {
24+
if (!(error instanceof Error)) return false;
25+
const errno = (error as NodeJS.ErrnoException).code;
26+
if (errno && TRANSIENT_IO_CODES.has(errno)) return true;
27+
const msg = error.message;
28+
if (msg.includes('read EIO') || msg.includes('write EPIPE')) return true;
29+
if (msg.includes('ERR_STREAM_DESTROYED')) return true;
30+
return false;
31+
}
32+
33+
// Guard: register at most once per process, even if the module is loaded
34+
// multiple times (ESM + CJS dual-instance, or re-imported).
35+
const GUARD = Symbol.for('visor.childProcessErrorHandler');
36+
if (!(globalThis as any)[GUARD]) {
37+
(globalThis as any)[GUARD] = true;
38+
39+
process.on('uncaughtException', (error: Error, origin: string) => {
40+
if (isChildProcessIOError(error)) {
41+
// Log but do NOT crash. The broken pipe is from a child process that
42+
// already exited; crashing here would be collateral damage.
43+
logger.warn(
44+
`[child-process-error-handler] Suppressed transient I/O error (${origin}): ${error.message}`
45+
);
46+
return;
47+
}
48+
// Not our error — let other handlers (or the default behaviour) deal with it.
49+
// Note: Node.js invokes all registered handlers; if none of them call
50+
// process.exit() the process will continue. The worktree-manager handler
51+
// already calls process.exit(1) for non-IO errors.
52+
});
53+
}

src/utils/worktree-manager.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import * as fsp from 'fs/promises';
1010
import * as path from 'path';
1111
import * as crypto from 'crypto';
1212
import { commandExecutor } from './command-executor';
13+
import { isChildProcessIOError } from './child-process-error-handler';
1314
import { logger } from '../logger';
1415
import type {
1516
WorktreeMetadata,
@@ -1041,6 +1042,12 @@ export class WorktreeManager {
10411042

10421043
// Cleanup on uncaught exception
10431044
process.on('uncaughtException', async error => {
1045+
// EIO errors from child process stdio streams (e.g. MCP servers dying)
1046+
// are transient and should not crash the entire process.
1047+
if (isChildProcessIOError(error)) {
1048+
logger.warn(`Ignoring transient child-process I/O error: ${error.message}`);
1049+
return;
1050+
}
10441051
logger.error(`Uncaught exception, cleaning up worktrees: ${error}`);
10451052
await this.cleanupProcessWorktrees();
10461053
process.exit(1);
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* Tests for the global child-process I/O error handler.
3+
*
4+
* The handler suppresses transient EIO / EPIPE errors that originate from
5+
* broken child-process stdio pipes so the visor process doesn't crash.
6+
*/
7+
8+
// Helper to emit uncaughtException without TS overload complaints
9+
function emitUncaughtException(err: Error): void {
10+
(process as any).emit('uncaughtException', err, 'uncaughtException');
11+
}
12+
13+
describe('child-process-error-handler', () => {
14+
let originalListeners: NodeJS.UncaughtExceptionListener[];
15+
16+
beforeAll(() => {
17+
// Capture pre-existing uncaughtException listeners so we can restore later
18+
originalListeners = process.listeners(
19+
'uncaughtException'
20+
) as NodeJS.UncaughtExceptionListener[];
21+
});
22+
23+
afterAll(() => {
24+
// Remove any listeners our import added and restore originals
25+
process.removeAllListeners('uncaughtException');
26+
for (const fn of originalListeners) {
27+
process.on('uncaughtException', fn);
28+
}
29+
});
30+
31+
beforeEach(() => {
32+
// Clear the guard so the module re-registers on each test
33+
delete (globalThis as any)[Symbol.for('visor.childProcessErrorHandler')];
34+
});
35+
36+
it('should register an uncaughtException listener', () => {
37+
const before = process.listenerCount('uncaughtException');
38+
jest.isolateModules(() => {
39+
require('../../src/utils/child-process-error-handler');
40+
});
41+
const after = process.listenerCount('uncaughtException');
42+
expect(after).toBeGreaterThan(before);
43+
});
44+
45+
it('should suppress EIO errors without crashing', () => {
46+
jest.isolateModules(() => {
47+
require('../../src/utils/child-process-error-handler');
48+
});
49+
50+
const err = Object.assign(new Error('read EIO'), { code: 'EIO' });
51+
expect(() => emitUncaughtException(err)).not.toThrow();
52+
});
53+
54+
it('should suppress EPIPE errors without crashing', () => {
55+
jest.isolateModules(() => {
56+
require('../../src/utils/child-process-error-handler');
57+
});
58+
59+
const err = Object.assign(new Error('write EPIPE'), { code: 'EPIPE' });
60+
expect(() => emitUncaughtException(err)).not.toThrow();
61+
});
62+
63+
it('should suppress ECONNRESET errors without crashing', () => {
64+
jest.isolateModules(() => {
65+
require('../../src/utils/child-process-error-handler');
66+
});
67+
68+
const err = Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' });
69+
expect(() => emitUncaughtException(err)).not.toThrow();
70+
});
71+
72+
it('should suppress ERR_STREAM_DESTROYED errors without crashing', () => {
73+
jest.isolateModules(() => {
74+
require('../../src/utils/child-process-error-handler');
75+
});
76+
77+
const err = new Error('ERR_STREAM_DESTROYED');
78+
expect(() => emitUncaughtException(err)).not.toThrow();
79+
});
80+
81+
it('should NOT suppress unrelated errors', () => {
82+
jest.isolateModules(() => {
83+
require('../../src/utils/child-process-error-handler');
84+
});
85+
86+
const err = new Error('Something completely different');
87+
expect(() => emitUncaughtException(err)).not.toThrow();
88+
});
89+
90+
it('should only register once even when imported twice', () => {
91+
jest.isolateModules(() => {
92+
require('../../src/utils/child-process-error-handler');
93+
});
94+
const afterFirst = process.listenerCount('uncaughtException');
95+
jest.isolateModules(() => {
96+
require('../../src/utils/child-process-error-handler');
97+
});
98+
const afterSecond = process.listenerCount('uncaughtException');
99+
expect(afterSecond).toBe(afterFirst);
100+
});
101+
});

0 commit comments

Comments
 (0)