Skip to content

Commit f316cc2

Browse files
committed
fix(server): handle stdout errors in StdioServerTransport
A stdio client that disconnects makes the server's next write fail with EPIPE, and nothing listened for 'error' on stdout, so that became an unhandled 'error' event and took the whole Node process down. Listen for it, report it through onerror and close the transport instead. send() also waited for a 'drain' event that a destroyed stream never emits. It now settles from an error listener armed before the write, removes both listeners on every exit path, and rejects after close() instead of writing to a stream the transport has released. close() is idempotent, so an error-triggered close followed by an explicit one fires onclose once. Backport of #1568 to the v1.x line.
1 parent 2d889f2 commit f316cc2

3 files changed

Lines changed: 130 additions & 3 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@modelcontextprotocol/sdk': patch
3+
---
4+
5+
Handle `stdout` errors in `StdioServerTransport` instead of crashing the process. When a stdio client disconnects, the server's next write fails with `EPIPE`, and because nothing listened for `'error'` on `stdout` that became an unhandled `'error'` event, which takes the whole
6+
Node process down. The transport now listens for it, reports it through `onerror`, and closes itself, so a client that goes away ends the session rather than killing the server.
7+
8+
`send()` no longer waits for a `'drain'` event that a destroyed stream can never emit. It settles from an error listener armed before the write and removes both listeners on every exit path, so a write that fails rejects with the underlying error instead of leaving the promise
9+
pending, and `send()` after `close()` rejects rather than writing to a stream the transport has let go of. `close()` is idempotent too, so an error-triggered close followed by an explicit one fires `onclose` once.
10+
11+
This backports the fix that shipped for the v2 line, so both lines now behave the same way on a disconnecting client.

src/server/stdio.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Transport } from '../shared/transport.js';
1212
export class StdioServerTransport implements Transport {
1313
private _readBuffer: ReadBuffer;
1414
private _started = false;
15+
private _closed = false;
1516

1617
constructor(
1718
private _stdin: Readable = process.stdin,
@@ -46,6 +47,12 @@ export class StdioServerTransport implements Transport {
4647
_onerror = (error: Error) => {
4748
this.onerror?.(error);
4849
};
50+
_onstdouterror = (error: Error) => {
51+
this.onerror?.(error);
52+
this.close().catch(() => {
53+
// Ignore errors during close — we're already in an error path
54+
});
55+
};
4956

5057
/**
5158
* Starts listening for messages on stdin.
@@ -60,6 +67,7 @@ export class StdioServerTransport implements Transport {
6067
this._started = true;
6168
this._stdin.on('data', this._ondata);
6269
this._stdin.on('error', this._onerror);
70+
this._stdout.on('error', this._onstdouterror);
6371
}
6472

6573
private processReadBuffer() {
@@ -78,9 +86,15 @@ export class StdioServerTransport implements Transport {
7886
}
7987

8088
async close(): Promise<void> {
89+
if (this._closed) {
90+
return;
91+
}
92+
this._closed = true;
93+
8194
// Remove our event listeners first
8295
this._stdin.off('data', this._ondata);
8396
this._stdin.off('error', this._onerror);
97+
this._stdout.off('error', this._onstdouterror);
8498

8599
// Check if we were the only data listener
86100
const remainingDataListeners = this._stdin.listenerCount('data');
@@ -96,12 +110,37 @@ export class StdioServerTransport implements Transport {
96110
}
97111

98112
send(message: JSONRPCMessage): Promise<void> {
99-
return new Promise(resolve => {
113+
if (this._closed) {
114+
return Promise.reject(new Error('StdioServerTransport is closed'));
115+
}
116+
return new Promise((resolve, reject) => {
100117
const json = serializeMessage(message);
118+
119+
let settled = false;
120+
const onError = (error: Error) => {
121+
if (settled) return;
122+
settled = true;
123+
this._stdout.off('error', onError);
124+
this._stdout.off('drain', onDrain);
125+
reject(error);
126+
};
127+
const onDrain = () => {
128+
if (settled) return;
129+
settled = true;
130+
this._stdout.off('error', onError);
131+
this._stdout.off('drain', onDrain);
132+
resolve();
133+
};
134+
135+
this._stdout.once('error', onError);
136+
101137
if (this._stdout.write(json)) {
138+
if (settled) return;
139+
settled = true;
140+
this._stdout.off('error', onError);
102141
resolve();
103-
} else {
104-
this._stdout.once('drain', resolve);
142+
} else if (!settled) {
143+
this._stdout.once('drain', onDrain);
105144
}
106145
});
107146
}

test/server/stdio.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,80 @@ test('should fire onerror and close when ReadBuffer overflows', async () => {
148148
expect(receivedError?.message).toMatch(/ReadBuffer exceeded maximum size/);
149149
expect(closeCount).toBe(1);
150150
});
151+
152+
test('should close and fire onerror when stdout errors', async () => {
153+
const server = new StdioServerTransport(input, output);
154+
155+
let receivedError: Error | undefined;
156+
server.onerror = err => {
157+
receivedError = err;
158+
};
159+
let closeCount = 0;
160+
server.onclose = () => {
161+
closeCount++;
162+
};
163+
164+
await server.start();
165+
output.emit('error', new Error('EPIPE'));
166+
167+
expect(receivedError?.message).toBe('EPIPE');
168+
expect(closeCount).toBe(1);
169+
});
170+
171+
test('should fire onerror before onclose on stdout error', async () => {
172+
const server = new StdioServerTransport(input, output);
173+
174+
const events: string[] = [];
175+
server.onerror = () => events.push('error');
176+
server.onclose = () => events.push('close');
177+
178+
await server.start();
179+
output.emit('error', new Error('EPIPE'));
180+
181+
expect(events).toEqual(['error', 'close']);
182+
});
183+
184+
test('should not fire onclose twice when close() is called after stdout error', async () => {
185+
const server = new StdioServerTransport(input, output);
186+
server.onerror = () => {};
187+
188+
let closeCount = 0;
189+
server.onclose = () => {
190+
closeCount++;
191+
};
192+
193+
await server.start();
194+
output.emit('error', new Error('EPIPE'));
195+
await server.close();
196+
197+
expect(closeCount).toBe(1);
198+
});
199+
200+
test('should reject send() when stdout errors before drain', async () => {
201+
let completeWrite: ((error?: Error | null) => void) | undefined;
202+
const slowOutput = new Writable({
203+
highWaterMark: 0,
204+
write(_chunk, _encoding, callback) {
205+
completeWrite = callback;
206+
}
207+
});
208+
209+
const server = new StdioServerTransport(input, slowOutput);
210+
server.onerror = () => {};
211+
await server.start();
212+
213+
const sendPromise = server.send({ jsonrpc: '2.0', id: 1, method: 'ping' });
214+
completeWrite!(new Error('write EPIPE'));
215+
216+
await expect(sendPromise).rejects.toThrow('write EPIPE');
217+
expect(slowOutput.listenerCount('drain')).toBe(0);
218+
expect(slowOutput.listenerCount('error')).toBe(0);
219+
});
220+
221+
test('should reject send() after transport is closed', async () => {
222+
const server = new StdioServerTransport(input, output);
223+
await server.start();
224+
await server.close();
225+
226+
await expect(server.send({ jsonrpc: '2.0', id: 1, method: 'ping' })).rejects.toThrow('closed');
227+
});

0 commit comments

Comments
 (0)