Skip to content

Commit f05ac43

Browse files
author
kai-agent-free
committed
fix: address review feedback on EPIPE handling
- Make close() idempotent with _closed guard - Track pending drain reject to prevent hanging promises on EPIPE - Add test for drain + EPIPE error interaction - Replace em dash with double dash in comment
1 parent 9a96b23 commit f05ac43

2 files changed

Lines changed: 61 additions & 2 deletions

File tree

packages/server/src/server/stdio.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ 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;
23+
private _pendingDrainReject?: (reason: Error) => void;
2224

2325
constructor(
2426
private _stdin: Readable = process.stdin,
@@ -39,7 +41,7 @@ export class StdioServerTransport implements Transport {
3941
};
4042
_onstdouterror = (error: NodeJS.ErrnoException) => {
4143
if (error.code === 'EPIPE' || error.code === 'ERR_STREAM_DESTROYED') {
42-
// Client disconnected close gracefully instead of crashing.
44+
// Client disconnected -- close gracefully instead of crashing.
4345
void this.close();
4446
} else {
4547
this.onerror?.(error);
@@ -78,6 +80,15 @@ export class StdioServerTransport implements Transport {
7880
}
7981

8082
async close(): Promise<void> {
83+
if (this._closed) return;
84+
this._closed = true;
85+
86+
// Reject any pending drain promise so send() does not hang forever
87+
if (this._pendingDrainReject) {
88+
this._pendingDrainReject(new Error('Transport closed before drain'));
89+
this._pendingDrainReject = undefined;
90+
}
91+
8192
// Remove our event listeners first
8293
this._stdin.off('data', this._ondata);
8394
this._stdin.off('error', this._onerror);
@@ -107,7 +118,13 @@ export class StdioServerTransport implements Transport {
107118
if (this._stdout.write(json)) {
108119
resolve();
109120
} else {
110-
this._stdout.once('drain', resolve);
121+
// Track the reject so close() can settle this promise
122+
// if an error (e.g. EPIPE) fires before drain.
123+
this._pendingDrainReject = reject;
124+
this._stdout.once('drain', () => {
125+
this._pendingDrainReject = undefined;
126+
resolve();
127+
});
111128
}
112129
} catch (error: unknown) {
113130
const errno = error as NodeJS.ErrnoException;

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,45 @@ test('should remove stdout error listener on close', async () => {
186186

187187
expect(listenersAfter).toBeLessThan(listenersBefore);
188188
});
189+
190+
test('should reject send and close when EPIPE fires while waiting for drain', async () => {
191+
// Create a stream where write() returns false to trigger drain waiting
192+
const slowOutput = new Writable({
193+
highWaterMark: 1,
194+
write(_chunk, _encoding, callback) {
195+
// Delay callback to keep backpressure
196+
setTimeout(callback, 100);
197+
}
198+
});
199+
200+
const server = new StdioServerTransport(input, slowOutput);
201+
202+
let didClose = false;
203+
server.onclose = () => {
204+
didClose = true;
205+
};
206+
207+
await server.start();
208+
209+
// Fill the buffer so write() returns false
210+
const message: JSONRPCMessage = {
211+
jsonrpc: '2.0',
212+
id: 1,
213+
method: 'ping'
214+
};
215+
216+
// Start a send that will wait for drain
217+
const sendPromise = server.send(message);
218+
219+
// Give the event loop a tick so the write() call executes
220+
await new Promise(resolve => setTimeout(resolve, 10));
221+
222+
// Emit EPIPE before drain fires
223+
const epipeError = new Error('write EPIPE') as NodeJS.ErrnoException;
224+
epipeError.code = 'EPIPE';
225+
slowOutput.emit('error', epipeError);
226+
227+
// The send promise should reject (not hang forever)
228+
await expect(sendPromise).rejects.toThrow('Transport closed before drain');
229+
expect(didClose).toBeTruthy();
230+
});

0 commit comments

Comments
 (0)