Skip to content

Commit 0e914c2

Browse files
fix: send-after-close guard, onerror-before-onclose ordering, listener cleanup
- Guard send() against closed transport (write on destroyed stream returns false and never drains/errors, hanging the promise) - Fire onerror before onclose in stdout error handler - Idempotent close() via _closed flag - Symmetric error/drain listener cleanup in send() with settled guard - Add changeset and tests covering error path, idempotent close, send rejection during backpressure, and send-after-close
1 parent 546d56f commit 0e914c2

File tree

3 files changed

+111
-13
lines changed

3 files changed

+111
-13
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/server': patch
3+
---
4+
5+
Handle stdout errors (e.g. EPIPE) in `StdioServerTransport` gracefully instead of crashing. When the client disconnects abruptly, the transport now catches the stdout error, surfaces it via `onerror`, and closes.

packages/server/src/server/stdio.ts

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { process } from '@modelcontextprotocol/server/_shims';
1919
export class StdioServerTransport implements Transport {
2020
private _readBuffer: ReadBuffer = new ReadBuffer();
2121
private _started = false;
22+
private _closed = false;
2223

2324
constructor(
2425
private _stdin: Readable = process.stdin,
@@ -38,12 +39,10 @@ export class StdioServerTransport implements Transport {
3839
this.onerror?.(error);
3940
};
4041
_onstdouterror = (error: Error) => {
41-
// Handle stdout errors (e.g., EPIPE when client disconnects)
42-
// Trigger close to clean up gracefully
42+
this.onerror?.(error);
4343
this.close().catch(() => {
44-
// Ignore errors during close
44+
// Ignore errors during close — we're already in an error path
4545
});
46-
this.onerror?.(error);
4746
};
4847

4948
/**
@@ -78,6 +77,11 @@ export class StdioServerTransport implements Transport {
7877
}
7978

8079
async close(): Promise<void> {
80+
if (this._closed) {
81+
return;
82+
}
83+
this._closed = true;
84+
8185
// Remove our event listeners first
8286
this._stdin.off('data', this._ondata);
8387
this._stdin.off('error', this._onerror);
@@ -97,25 +101,37 @@ export class StdioServerTransport implements Transport {
97101
}
98102

99103
send(message: JSONRPCMessage): Promise<void> {
104+
if (this._closed) {
105+
return Promise.reject(new Error('StdioServerTransport is closed'));
106+
}
100107
return new Promise((resolve, reject) => {
101108
const json = serializeMessage(message);
102-
103-
// Handle write errors (e.g., EPIPE when client disconnects)
109+
110+
let settled = false;
104111
const onError = (error: Error) => {
112+
if (settled) return;
113+
settled = true;
105114
this._stdout.off('error', onError);
115+
this._stdout.off('drain', onDrain);
106116
reject(error);
107117
};
108-
118+
const onDrain = () => {
119+
if (settled) return;
120+
settled = true;
121+
this._stdout.off('error', onError);
122+
this._stdout.off('drain', onDrain);
123+
resolve();
124+
};
125+
109126
this._stdout.once('error', onError);
110-
127+
111128
if (this._stdout.write(json)) {
129+
if (settled) return;
130+
settled = true;
112131
this._stdout.off('error', onError);
113132
resolve();
114-
} else {
115-
this._stdout.once('drain', () => {
116-
this._stdout.off('error', onError);
117-
resolve();
118-
});
133+
} else if (!settled) {
134+
this._stdout.once('drain', onDrain);
119135
}
120136
});
121137
}

packages/server/test/server/stdio.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,80 @@ test('should read multiple messages', async () => {
102102
await finished;
103103
expect(readMessages).toEqual(messages);
104104
});
105+
106+
test('should close and fire onerror when stdout errors', async () => {
107+
const server = new StdioServerTransport(input, output);
108+
109+
let receivedError: Error | undefined;
110+
server.onerror = err => {
111+
receivedError = err;
112+
};
113+
let closeCount = 0;
114+
server.onclose = () => {
115+
closeCount++;
116+
};
117+
118+
await server.start();
119+
output.emit('error', new Error('EPIPE'));
120+
121+
expect(receivedError?.message).toBe('EPIPE');
122+
expect(closeCount).toBe(1);
123+
});
124+
125+
test('should not fire onclose twice when close() is called after stdout error', async () => {
126+
const server = new StdioServerTransport(input, output);
127+
server.onerror = () => {};
128+
129+
let closeCount = 0;
130+
server.onclose = () => {
131+
closeCount++;
132+
};
133+
134+
await server.start();
135+
output.emit('error', new Error('EPIPE'));
136+
await server.close();
137+
138+
expect(closeCount).toBe(1);
139+
});
140+
141+
test('should reject send() when stdout errors before drain', async () => {
142+
let completeWrite: ((error?: Error | null) => void) | undefined;
143+
const slowOutput = new Writable({
144+
highWaterMark: 0,
145+
write(_chunk, _encoding, callback) {
146+
completeWrite = callback;
147+
}
148+
});
149+
150+
const server = new StdioServerTransport(input, slowOutput);
151+
server.onerror = () => {};
152+
await server.start();
153+
154+
const sendPromise = server.send({ jsonrpc: '2.0', id: 1, method: 'ping' });
155+
completeWrite!(new Error('write EPIPE'));
156+
157+
await expect(sendPromise).rejects.toThrow('write EPIPE');
158+
expect(slowOutput.listenerCount('drain')).toBe(0);
159+
expect(slowOutput.listenerCount('error')).toBe(0);
160+
});
161+
162+
test('should reject send() after transport is closed', async () => {
163+
const server = new StdioServerTransport(input, output);
164+
await server.start();
165+
await server.close();
166+
167+
await expect(server.send({ jsonrpc: '2.0', id: 1, method: 'ping' })).rejects.toThrow('closed');
168+
});
169+
170+
test('should fire onerror before onclose on stdout error', async () => {
171+
const server = new StdioServerTransport(input, output);
172+
173+
const events: string[] = [];
174+
server.onerror = () => events.push('error');
175+
server.onclose = () => events.push('close');
176+
177+
await server.start();
178+
output.emit('error', new Error('EPIPE'));
179+
180+
expect(events).toEqual(['error', 'close']);
181+
});

0 commit comments

Comments
 (0)