Skip to content

Commit 1feb929

Browse files
Add regression test and changeset
- Add test verifying stdout listeners are detached on close() so late non-JSON output from the child process no longer fires onerror - Add patch-level changeset
1 parent 212cede commit 1feb929

2 files changed

Lines changed: 56 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/client': patch
3+
---
4+
5+
Detach stdio transport stdout/stdin listeners on `close()`. Previously, if the child process wrote to stdout during shutdown (after `close()` was called), the still-attached data listener would attempt to parse it as JSON-RPC and emit a spurious parse error via `onerror`. Fixes #780.

packages/client/test/client/crossSpawn.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,4 +173,55 @@ describe('StdioClientTransport using cross-spawn', () => {
173173
// verify message is sent correctly
174174
expect(mockProcess.stdin.write).toHaveBeenCalled();
175175
});
176+
177+
// Regression test for #780: late stdout data after close() should not trigger onerror
178+
test('should detach stdout listeners on close so late output does not fire onerror', async () => {
179+
let capturedDataHandler: ((chunk: Buffer) => void) | undefined;
180+
let closeCallback: (() => void) | undefined;
181+
182+
const mockProcess = {
183+
on: vi.fn().mockReturnThis(),
184+
once: vi.fn((event: string, callback: () => void) => {
185+
if (event === 'spawn') callback();
186+
if (event === 'close') closeCallback = callback;
187+
return mockProcess;
188+
}),
189+
stdin: {
190+
on: vi.fn(),
191+
off: vi.fn(),
192+
write: vi.fn().mockReturnValue(true),
193+
end: vi.fn(() => closeCallback?.())
194+
},
195+
stdout: {
196+
on: vi.fn((event: string, handler: (chunk: Buffer) => void) => {
197+
if (event === 'data') capturedDataHandler = handler;
198+
}),
199+
off: vi.fn()
200+
},
201+
stderr: null,
202+
exitCode: 0,
203+
kill: vi.fn()
204+
};
205+
206+
mockSpawn.mockReturnValue(mockProcess as unknown as ChildProcess);
207+
208+
const transport = new StdioClientTransport({ command: 'test-command' });
209+
const onerror = vi.fn();
210+
transport.onerror = onerror;
211+
212+
await transport.start();
213+
expect(capturedDataHandler).toBeDefined();
214+
215+
await transport.close();
216+
217+
// close() must detach the listener it registered
218+
expect(mockProcess.stdout.off).toHaveBeenCalledWith('data', capturedDataHandler);
219+
expect(mockProcess.stdin.off).toHaveBeenCalled();
220+
221+
// Simulate the child writing non-JSON during shutdown — the #780 scenario.
222+
// Before the fix this would parse-fail and call onerror; now the listener
223+
// is detached so the transport never sees it.
224+
capturedDataHandler!(Buffer.from('Shutting down...\n'));
225+
expect(onerror).not.toHaveBeenCalled();
226+
});
176227
});

0 commit comments

Comments
 (0)