-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(server): exit when MCP client closes stdin pipe #2003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
9378f8b
a3a4e04
7e70ca3
27f0874
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,11 @@ export class StdioServerTransport implements Transport { | |
| // Ignore errors during close — we're already in an error path | ||
| }); | ||
| }; | ||
| _onstdinclose = () => { | ||
| this.close().catch(() => { | ||
| // Ignore errors during close — stdin pipe ended | ||
| }); | ||
| }; | ||
|
|
||
| /** | ||
| * Starts listening for messages on `stdin`. | ||
|
|
@@ -58,6 +63,7 @@ export class StdioServerTransport implements Transport { | |
| this._started = true; | ||
| this._stdin.on('data', this._ondata); | ||
| this._stdin.on('error', this._onerror); | ||
| this._stdin.on('close', this._onstdinclose); | ||
| this._stdout.on('error', this._onstdouterror); | ||
| } | ||
|
|
||
|
|
@@ -85,6 +91,7 @@ export class StdioServerTransport implements Transport { | |
| // Remove our event listeners first | ||
| this._stdin.off('data', this._ondata); | ||
| this._stdin.off('error', this._onerror); | ||
| this._stdin.off('close', this._onstdinclose); | ||
| this._stdout.off('error', this._onstdouterror); | ||
|
|
||
|
Comment on lines
97
to
103
|
||
| // Check if we were the only data listener | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -179,3 +179,46 @@ test('should fire onerror before onclose on stdout error', async () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(events).toEqual(['error', 'close']); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test('should fire onclose when stdin emits close', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const server = new StdioServerTransport(input, output); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.onerror = error => { throw error; }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let closeCount = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.onclose = () => { closeCount++; }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await server.start(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| input.emit('close'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(closeCount).toBe(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test('should fire onclose when stdin emits end', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const server = new StdioServerTransport(input, output); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.onerror = error => { throw error; }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let closeCount = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server.onclose = () => { closeCount++; }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await server.start(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| input.push(null); // signals end-of-stream | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Allow microtasks to flush | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const server = new StdioServerTransport(input, output); | |
| server.onerror = error => { throw error; }; | |
| let closeCount = 0; | |
| server.onclose = () => { closeCount++; }; | |
| await server.start(); | |
| input.push(null); // signals end-of-stream | |
| // Allow microtasks to flush | |
| await new Promise(resolve => setTimeout(resolve, 0)); | |
| const endOnlyInput = new Readable({ | |
| autoDestroy: false, | |
| emitClose: false, | |
| // We'll use endOnlyInput.push() instead. | |
| read: () => {} | |
| }); | |
| const server = new StdioServerTransport(endOnlyInput, output); | |
| server.onerror = error => { throw error; }; | |
| let closeCount = 0; | |
| let inputCloseCount = 0; | |
| server.onclose = () => { closeCount++; }; | |
| endOnlyInput.on('close', () => { inputCloseCount++; }); | |
| await server.start(); | |
| endOnlyInput.push(null); // signals end-of-stream without emitting close | |
| // Allow microtasks to flush | |
| await new Promise(resolve => setTimeout(resolve, 0)); | |
| expect(inputCloseCount).toBe(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start()only listens forstdin'scloseevent, but a disconnected pipe commonly emitsend(andcloseis not guaranteed for all Readable streams). To reliably shut down on client disconnect, also listen forstdin'sendevent and remove that listener inclose()alongside the existingclosecleanup.