Skip to content

Commit 511c1a1

Browse files
committed
fix: ensure transport is closed when connection initialization fails during client connect
1 parent 7ba58da commit 511c1a1

2 files changed

Lines changed: 37 additions & 10 deletions

File tree

packages/client/src/client/client.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -490,17 +490,17 @@ export class Client extends Protocol<ClientContext> {
490490
* ```
491491
*/
492492
override async connect(transport: Transport, options?: RequestOptions): Promise<void> {
493-
await super.connect(transport);
494-
// When transport sessionId is already set this means we are trying to reconnect.
495-
// Restore the protocol version negotiated during the original initialize handshake
496-
// so HTTP transports include the required mcp-protocol-version header, but skip re-init.
497-
if (transport.sessionId !== undefined) {
498-
if (this._negotiatedProtocolVersion !== undefined && transport.setProtocolVersion) {
499-
transport.setProtocolVersion(this._negotiatedProtocolVersion);
500-
}
501-
return;
502-
}
503493
try {
494+
await super.connect(transport);
495+
// When transport sessionId is already set this means we are trying to reconnect.
496+
// Restore the protocol version negotiated during the original initialize handshake
497+
// so HTTP transports include the required mcp-protocol-version header, but skip re-init.
498+
if (transport.sessionId !== undefined) {
499+
if (this._negotiatedProtocolVersion !== undefined && transport.setProtocolVersion) {
500+
transport.setProtocolVersion(this._negotiatedProtocolVersion);
501+
}
502+
return;
503+
}
504504
const result = await this._requestWithSchema(
505505
{
506506
method: 'initialize',

test/integration/test/client/client.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,33 @@ test('should reject unsupported protocol version', async () => {
217217
expect(clientTransport.close).toHaveBeenCalled();
218218
});
219219

220+
/***
221+
* Test: Connection Initialization Failure Cleanup
222+
*/
223+
test('should close transport and clean up if connection initialization fails', async () => {
224+
const clientTransport: Transport = {
225+
start: vi.fn().mockRejectedValue(new Error('Transport start failed')),
226+
close: vi.fn().mockResolvedValue(undefined),
227+
send: vi.fn().mockResolvedValue(undefined)
228+
};
229+
230+
const client = new Client(
231+
{
232+
name: 'test client',
233+
version: '1.0'
234+
},
235+
{
236+
capabilities: {}
237+
}
238+
);
239+
240+
await expect(client.connect(clientTransport)).rejects.toThrow('Transport start failed');
241+
242+
// The transport.close() method should be called to clean up resources
243+
// because the client.connect() method caught the error and called this.close()
244+
expect(clientTransport.close).toHaveBeenCalled();
245+
});
246+
220247
/***
221248
* Test: Connect New Client to Old Supported Server Version
222249
*/

0 commit comments

Comments
 (0)