From 53f55b8edf7da419e35c6305ad4c559e95bb9283 Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Sat, 16 Nov 2024 16:18:35 +0000 Subject: [PATCH 1/6] Remove `abortAfterTimeout` It's fundamentally flawed because it doesn't support easy cancellation of the timeout. --- src/shared/protocol.ts | 2 -- src/utils.test.ts | 15 --------------- src/utils.ts | 11 ----------- 3 files changed, 28 deletions(-) delete mode 100644 src/utils.test.ts delete mode 100644 src/utils.ts diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index 8103695d1..ab8421bfb 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -48,8 +48,6 @@ export type RequestOptions = { /** * Can be used to cancel an in-flight request. This will cause an AbortError to be raised from request(). - * - * Use abortAfterTimeout() to easily implement timeouts using this signal. */ signal?: AbortSignal; }; diff --git a/src/utils.test.ts b/src/utils.test.ts deleted file mode 100644 index e4aa4e5fc..000000000 --- a/src/utils.test.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { abortAfterTimeout } from "./utils.js"; - -describe("abortAfterTimeout", () => { - it("should abort after timeout", () => { - const signal = abortAfterTimeout(0); - expect(signal.aborted).toBe(false); - - return new Promise((resolve) => { - setTimeout(() => { - expect(signal.aborted).toBe(true); - resolve(true); - }, 0); - }); - }); -}); diff --git a/src/utils.ts b/src/utils.ts deleted file mode 100644 index 3ca6af286..000000000 --- a/src/utils.ts +++ /dev/null @@ -1,11 +0,0 @@ -/** - * Returns an AbortSignal that will enter aborted state after `timeoutMs` milliseconds. - */ -export function abortAfterTimeout(timeoutMs: number): AbortSignal { - const controller = new AbortController(); - setTimeout(() => { - controller.abort("Timeout exceeded"); - }, timeoutMs); - - return controller.signal; -} From 2cc3bc8a401158c58a375c89553fb7194bd4852d Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Sat, 16 Nov 2024 16:30:20 +0000 Subject: [PATCH 2/6] Add `RequestOptions.timeout` --- src/shared/protocol.ts | 48 +++++++++++++++++++++++++++++++++++++++--- src/types.ts | 1 + 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index ab8421bfb..559d2c20f 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -37,6 +37,11 @@ export type ProtocolOptions = { enforceStrictCapabilities?: boolean; }; +/** + * The default request timeout, in miliseconds. + */ +export const DEFAULT_REQUEST_TIMEOUT_MSEC = 60000; + /** * Options that can be given per request. */ @@ -50,6 +55,13 @@ export type RequestOptions = { * Can be used to cancel an in-flight request. This will cause an AbortError to be raised from request(). */ signal?: AbortSignal; + + /** + * A timeout (in milliseconds) for this request. If exceeded, an McpError with code `RequestTimeout` will be raised from request(). + * + * If not specified, `DEFAULT_REQUEST_TIMEOUT_MSEC` will be used as the timeout. + */ + timeout?: number; }; /** @@ -379,7 +391,13 @@ export abstract class Protocol< }; } + let timeoutId: ReturnType | undefined = undefined; + this._responseHandlers.set(messageId, (response) => { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + if (options?.signal?.aborted) { return; } @@ -396,8 +414,7 @@ export abstract class Protocol< } }); - options?.signal?.addEventListener("abort", () => { - const reason = options?.signal?.reason; + const cancel = (reason: unknown) => { this._responseHandlers.delete(messageId); this._progressHandlers.delete(messageId); @@ -411,9 +428,34 @@ export abstract class Protocol< }); reject(reason); + }; + + options?.signal?.addEventListener("abort", () => { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + + cancel(options?.signal?.reason); }); - this._transport.send(jsonrpcRequest).catch(reject); + const timeout = options?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MSEC; + timeoutId = setTimeout( + () => + cancel( + new McpError(ErrorCode.RequestTimeout, "Request timed out", { + timeout, + }), + ), + timeout, + ); + + this._transport.send(jsonrpcRequest).catch((error) => { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + + reject(error); + }); }); } diff --git a/src/types.ts b/src/types.ts index 5b9fb664e..44a44d8f0 100644 --- a/src/types.ts +++ b/src/types.ts @@ -105,6 +105,7 @@ export const JSONRPCResponseSchema = z export enum ErrorCode { // SDK error codes ConnectionClosed = -1, + RequestTimeout = -2, // Standard JSON-RPC error codes ParseError = -32700, From 35396f4a8d3e64e286ea922f5249a843900d27a0 Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Sat, 16 Nov 2024 16:31:45 +0000 Subject: [PATCH 3/6] Report errors sending cancellation --- src/shared/protocol.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index 559d2c20f..03dffa778 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -418,14 +418,18 @@ export abstract class Protocol< this._responseHandlers.delete(messageId); this._progressHandlers.delete(messageId); - this._transport?.send({ - jsonrpc: "2.0", - method: "cancelled", - params: { - requestId: messageId, - reason: String(reason), - }, - }); + this._transport + ?.send({ + jsonrpc: "2.0", + method: "cancelled", + params: { + requestId: messageId, + reason: String(reason), + }, + }) + .catch((error) => + this._onerror(new Error(`Failed to send cancellation: ${error}`)), + ); reject(reason); }; From 437a8700296ec3655ca5362ecd1be5ab9bcfb62a Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Sat, 16 Nov 2024 16:37:24 +0000 Subject: [PATCH 4/6] Tests for request timeout --- src/client/index.test.ts | 56 ++++++++++++++++++++++++++++++++ src/server/index.test.ts | 70 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/src/client/index.test.ts b/src/client/index.test.ts index 0d7eb9418..55a113eaa 100644 --- a/src/client/index.test.ts +++ b/src/client/index.test.ts @@ -14,6 +14,7 @@ import { ListToolsRequestSchema, CreateMessageRequestSchema, ListRootsRequestSchema, + ErrorCode, } from "../types.js"; import { Transport } from "../shared/transport.js"; import { Server } from "../server/index.js"; @@ -491,3 +492,58 @@ test("should handle client cancelling a request", async () => { // Request should be rejected await expect(listResourcesPromise).rejects.toBe("Cancelled by test"); }); + +test("should handle request timeout", async () => { + const server = new Server( + { + name: "test server", + version: "1.0", + }, + { + capabilities: { + resources: {}, + }, + }, + ); + + // Set up server with a delayed response + server.setRequestHandler( + ListResourcesRequestSchema, + async (_request, extra) => { + const timer = new Promise((resolve) => { + const timeout = setTimeout(resolve, 100); + extra.signal.addEventListener("abort", () => clearTimeout(timeout)); + }); + + await timer; + return { + resources: [], + }; + }, + ); + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + const client = new Client( + { + name: "test client", + version: "1.0", + }, + { + capabilities: {}, + }, + ); + + await Promise.all([ + client.connect(clientTransport), + server.connect(serverTransport), + ]); + + // Request with 0 msec timeout should fail immediately + await expect( + client.listResources(undefined, { timeout: 0 }), + ).rejects.toMatchObject({ + code: ErrorCode.RequestTimeout, + }); +}); diff --git a/src/server/index.test.ts b/src/server/index.test.ts index 0697cc5cb..0a23955af 100644 --- a/src/server/index.test.ts +++ b/src/server/index.test.ts @@ -14,6 +14,7 @@ import { ListResourcesRequestSchema, ListToolsRequestSchema, SetLevelRequestSchema, + ErrorCode, } from "../types.js"; import { Transport } from "../shared/transport.js"; import { InMemoryTransport } from "../inMemory.js"; @@ -475,3 +476,72 @@ test("should handle server cancelling a request", async () => { // Request should be rejected await expect(createMessagePromise).rejects.toBe("Cancelled by test"); }); +test("should handle request timeout", async () => { + const server = new Server( + { + name: "test server", + version: "1.0", + }, + { + capabilities: { + sampling: {}, + }, + }, + ); + + // Set up client that delays responses + const client = new Client( + { + name: "test client", + version: "1.0", + }, + { + capabilities: { + sampling: {}, + }, + }, + ); + + client.setRequestHandler( + CreateMessageRequestSchema, + async (_request, extra) => { + await new Promise((resolve, reject) => { + const timeout = setTimeout(resolve, 100); + extra.signal.addEventListener("abort", () => { + clearTimeout(timeout); + reject(extra.signal.reason); + }); + }); + + return { + model: "test", + role: "assistant", + content: { + type: "text", + text: "Test response", + }, + }; + }, + ); + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + server.connect(serverTransport), + ]); + + // Request with 0 msec timeout should fail immediately + await expect( + server.createMessage( + { + messages: [], + maxTokens: 10, + }, + { timeout: 0 }, + ), + ).rejects.toMatchObject({ + code: ErrorCode.RequestTimeout, + }); +}); From 5c7434475e3f68b34631fedb745d554031d7332d Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Sat, 16 Nov 2024 16:37:32 +0000 Subject: [PATCH 5/6] Bump package version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c01a80682..e437037ab 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@modelcontextprotocol/sdk", - "version": "0.5.0", + "version": "0.6.0", "description": "Model Context Protocol implementation for TypeScript", "license": "MIT", "author": "Anthropic, PBC (https://anthropic.com)", From 9328e9b5ef8d22deb0fb044e943b0fcd1339b972 Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Sat, 16 Nov 2024 16:38:26 +0000 Subject: [PATCH 6/6] Add npm badge to README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e31bc91d2..034c18f8d 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# MCP TypeScript SDK +# MCP TypeScript SDK ![NPM Version](https://img.shields.io/npm/v/%40modelcontextprotocol%2Fsdk) TypeScript implementation of the Model Context Protocol (MCP), providing both client and server capabilities for integrating with LLM surfaces.