From e7d08c738e75eedf49fb385b99d14e1b2fa6a2b9 Mon Sep 17 00:00:00 2001 From: Sumitesh Naithani Date: Sun, 22 Dec 2024 23:12:22 +0530 Subject: [PATCH 1/4] fix(protocol): restrict SDK error codes to JSON-RPC server error range The SDK's custom error codes were previously using arbitrary negative numbers (-1, -2) which could conflict with application-specific error codes. This change: - Updates ConnectionClosed from -1 to -32000 - Updates RequestTimeout from -2 to -32001 - Keeps standard JSON-RPC error codes unchanged - Adds documentation explaining the server error range usage This modification ensures compliance with the JSON-RPC 2.0 specification, which reserves -32000 to -32099 for implementation-defined server errors. This change allows applications to freely use other error code ranges without potential conflicts with SDK-generated errors. Issue: #85 --- src/types.ts | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/types.ts b/src/types.ts index 44a44d8f..a0199ecf 100644 --- a/src/types.ts +++ b/src/types.ts @@ -29,7 +29,7 @@ const BaseRequestParamsSchema = z */ progressToken: z.optional(ProgressTokenSchema), }) - .passthrough(), + .passthrough() ), }) .passthrough(); @@ -100,12 +100,15 @@ export const JSONRPCResponseSchema = z .strict(); /** - * An incomplete set of error codes that may appear in JSON-RPC responses. + * @author : Sumitesh Naithani + * @link : https://docs.trafficserver.apache.org/en/latest/developer-guide/jsonrpc/jsonrpc-node-errors.en.html#standard-errors + * @description : An incomplete set of error codes that may appear in JSON-RPC responses. + * @note : SDK-specific errors should use the server error range (-32000 to -32099), as per JSON-RPC 2.0 specification. */ export enum ErrorCode { - // SDK error codes - ConnectionClosed = -1, - RequestTimeout = -2, + // SDK error codes (using server error range) + ConnectionClosed = -32000, + RequestTimeout = -32001, // Standard JSON-RPC error codes ParseError = -32700, @@ -214,7 +217,7 @@ export const ClientCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough(), + .passthrough() ), }) .passthrough(); @@ -258,7 +261,7 @@ export const ServerCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough(), + .passthrough() ), /** * Present if the server offers any resources to read. @@ -276,7 +279,7 @@ export const ServerCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough(), + .passthrough() ), /** * Present if the server offers any tools to call. @@ -289,7 +292,7 @@ export const ServerCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough(), + .passthrough() ), }) .passthrough(); @@ -480,7 +483,7 @@ export const ListResourcesResultSchema = PaginatedResultSchema.extend({ export const ListResourceTemplatesRequestSchema = PaginatedRequestSchema.extend( { method: z.literal("resources/templates/list"), - }, + } ); /** @@ -508,7 +511,7 @@ export const ReadResourceRequestSchema = RequestSchema.extend({ */ export const ReadResourceResultSchema = ResultSchema.extend({ contents: z.array( - z.union([TextResourceContentsSchema, BlobResourceContentsSchema]), + z.union([TextResourceContentsSchema, BlobResourceContentsSchema]) ), }); @@ -747,7 +750,7 @@ export const ListToolsResultSchema = PaginatedResultSchema.extend({ */ export const CallToolResultSchema = ResultSchema.extend({ content: z.array( - z.union([TextContentSchema, ImageContentSchema, EmbeddedResourceSchema]), + z.union([TextContentSchema, ImageContentSchema, EmbeddedResourceSchema]) ), isError: z.boolean().default(false).optional(), }); @@ -758,7 +761,7 @@ export const CallToolResultSchema = ResultSchema.extend({ export const CompatibilityCallToolResultSchema = CallToolResultSchema.or( ResultSchema.extend({ toolResult: z.unknown(), - }), + }) ); /** @@ -919,7 +922,7 @@ export const CreateMessageResultSchema = ResultSchema.extend({ * The reason why sampling stopped. */ stopReason: z.optional( - z.enum(["endTurn", "stopSequence", "maxTokens"]).or(z.string()), + z.enum(["endTurn", "stopSequence", "maxTokens"]).or(z.string()) ), role: z.enum(["user", "assistant"]), content: z.discriminatedUnion("type", [ @@ -1104,7 +1107,7 @@ export class McpError extends Error { constructor( public readonly code: number, message: string, - public readonly data?: unknown, + public readonly data?: unknown ) { super(`MCP error ${code}: ${message}`); } From d77cd0deb34c29ec4af1f9b6eeb11afb61592bf8 Mon Sep 17 00:00:00 2001 From: Sumitesh Naithani Date: Sun, 22 Dec 2024 23:20:35 +0530 Subject: [PATCH 2/4] style: fixed linting issues --- src/types.ts | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/types.ts b/src/types.ts index a0199ecf..8ebe8bf5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -29,7 +29,7 @@ const BaseRequestParamsSchema = z */ progressToken: z.optional(ProgressTokenSchema), }) - .passthrough() + .passthrough(), ), }) .passthrough(); @@ -100,16 +100,16 @@ export const JSONRPCResponseSchema = z .strict(); /** - * @author : Sumitesh Naithani - * @link : https://docs.trafficserver.apache.org/en/latest/developer-guide/jsonrpc/jsonrpc-node-errors.en.html#standard-errors - * @description : An incomplete set of error codes that may appear in JSON-RPC responses. - * @note : SDK-specific errors should use the server error range (-32000 to -32099), as per JSON-RPC 2.0 specification. - */ +* @author : Sumitesh Naithani +* @link : https://docs.trafficserver.apache.org/en/latest/developer-guide/jsonrpc/jsonrpc-node-errors.en.html#standard-errors +* @description : An incomplete set of error codes that may appear in JSON-RPC responses. +* @note : SDK-specific errors should use the server error range (-32000 to -32099), as per JSON-RPC 2.0 specification. +*/ export enum ErrorCode { // SDK error codes (using server error range) ConnectionClosed = -32000, RequestTimeout = -32001, - + // Standard JSON-RPC error codes ParseError = -32700, InvalidRequest = -32600, @@ -217,7 +217,7 @@ export const ClientCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough() + .passthrough(), ), }) .passthrough(); @@ -261,7 +261,7 @@ export const ServerCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough() + .passthrough(), ), /** * Present if the server offers any resources to read. @@ -279,7 +279,7 @@ export const ServerCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough() + .passthrough(), ), /** * Present if the server offers any tools to call. @@ -292,7 +292,7 @@ export const ServerCapabilitiesSchema = z */ listChanged: z.optional(z.boolean()), }) - .passthrough() + .passthrough(), ), }) .passthrough(); @@ -483,7 +483,7 @@ export const ListResourcesResultSchema = PaginatedResultSchema.extend({ export const ListResourceTemplatesRequestSchema = PaginatedRequestSchema.extend( { method: z.literal("resources/templates/list"), - } + }, ); /** @@ -511,7 +511,7 @@ export const ReadResourceRequestSchema = RequestSchema.extend({ */ export const ReadResourceResultSchema = ResultSchema.extend({ contents: z.array( - z.union([TextResourceContentsSchema, BlobResourceContentsSchema]) + z.union([TextResourceContentsSchema, BlobResourceContentsSchema]), ), }); @@ -750,7 +750,7 @@ export const ListToolsResultSchema = PaginatedResultSchema.extend({ */ export const CallToolResultSchema = ResultSchema.extend({ content: z.array( - z.union([TextContentSchema, ImageContentSchema, EmbeddedResourceSchema]) + z.union([TextContentSchema, ImageContentSchema, EmbeddedResourceSchema]), ), isError: z.boolean().default(false).optional(), }); @@ -761,7 +761,7 @@ export const CallToolResultSchema = ResultSchema.extend({ export const CompatibilityCallToolResultSchema = CallToolResultSchema.or( ResultSchema.extend({ toolResult: z.unknown(), - }) + }), ); /** @@ -922,7 +922,7 @@ export const CreateMessageResultSchema = ResultSchema.extend({ * The reason why sampling stopped. */ stopReason: z.optional( - z.enum(["endTurn", "stopSequence", "maxTokens"]).or(z.string()) + z.enum(["endTurn", "stopSequence", "maxTokens"]).or(z.string()), ), role: z.enum(["user", "assistant"]), content: z.discriminatedUnion("type", [ @@ -1107,7 +1107,7 @@ export class McpError extends Error { constructor( public readonly code: number, message: string, - public readonly data?: unknown + public readonly data?: unknown, ) { super(`MCP error ${code}: ${message}`); } @@ -1240,4 +1240,4 @@ export type ClientResult = z.infer; /* Server messages */ export type ServerRequest = z.infer; export type ServerNotification = z.infer; -export type ServerResult = z.infer; +export type ServerResult = z.infer; \ No newline at end of file From 8fa0a5c88b8042fd8902e4c3a131cfb0b4aee181 Mon Sep 17 00:00:00 2001 From: Sumitesh Naithani Date: Mon, 23 Dec 2024 00:10:14 +0530 Subject: [PATCH 3/4] test: added tests for checking correct error codes are thrown on request timeout and connection closed scenarios --- src/shared/protocol.test.ts | 63 +++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 src/shared/protocol.test.ts diff --git a/src/shared/protocol.test.ts b/src/shared/protocol.test.ts new file mode 100644 index 00000000..a56ced0d --- /dev/null +++ b/src/shared/protocol.test.ts @@ -0,0 +1,63 @@ +import { Protocol } from "./protocol.js"; +import { Transport } from "./transport.js"; +import { + McpError, + ErrorCode, + Request, + Result, + Notification, +} from "../types.js"; +import { ZodType, z } from "zod"; + +// Mock Transport class +class MockTransport implements Transport { + onclose?: () => void; + onerror?: (error: Error) => void; + onmessage?: (message: unknown) => void; + + async start(): Promise {} + async close(): Promise { + this.onclose?.(); + } + async send(_message: unknown): Promise {} +} + +describe("protocol tests", () => { + let protocol: Protocol; + let transport: MockTransport; + + beforeEach(() => { + transport = new MockTransport(); + protocol = new (class extends Protocol { + protected assertCapabilityForMethod(): void {} + protected assertNotificationCapability(): void {} + protected assertRequestHandlerCapability(): void {} + })(); + }); + + test("should throw a timeout error if the request exceeds the timeout", async () => { + await protocol.connect(transport); + const request = { method: "example", params: {} }; + try { + const mockSchema: ZodType<{ result: string }> = z.object({ + result: z.string(), + }); + await protocol.request(request, mockSchema, { + timeout: 100, + }); // Short timeout for test + } catch (error) { + expect(error).toBeInstanceOf(McpError); + if (error instanceof McpError) { + expect(error.code).toBe(ErrorCode.RequestTimeout); + } + } + }); + + test("should invoke onclose when the connection is closed", async () => { + const oncloseMock = jest.fn(); + protocol.onclose = oncloseMock; + await protocol.connect(transport); + await transport.close(); + expect(oncloseMock).toHaveBeenCalled(); + }); +}); From 21e3d465ca1e782932020d4ef5c5df2010467586 Mon Sep 17 00:00:00 2001 From: Sumitesh Naithani Date: Fri, 3 Jan 2025 06:59:01 +0530 Subject: [PATCH 4/4] chore: updated documentation comment in ErrorCode enum and set protocol test timeout to 0 --- src/shared/protocol.test.ts | 4 ++-- src/types.ts | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/shared/protocol.test.ts b/src/shared/protocol.test.ts index a56ced0d..9423a2a7 100644 --- a/src/shared/protocol.test.ts +++ b/src/shared/protocol.test.ts @@ -43,8 +43,8 @@ describe("protocol tests", () => { result: z.string(), }); await protocol.request(request, mockSchema, { - timeout: 100, - }); // Short timeout for test + timeout: 0, + }); } catch (error) { expect(error).toBeInstanceOf(McpError); if (error instanceof McpError) { diff --git a/src/types.ts b/src/types.ts index 8ebe8bf5..59b055a8 100644 --- a/src/types.ts +++ b/src/types.ts @@ -100,13 +100,10 @@ export const JSONRPCResponseSchema = z .strict(); /** -* @author : Sumitesh Naithani -* @link : https://docs.trafficserver.apache.org/en/latest/developer-guide/jsonrpc/jsonrpc-node-errors.en.html#standard-errors -* @description : An incomplete set of error codes that may appear in JSON-RPC responses. -* @note : SDK-specific errors should use the server error range (-32000 to -32099), as per JSON-RPC 2.0 specification. -*/ + * Error codes defined by the JSON-RPC specification. + */ export enum ErrorCode { - // SDK error codes (using server error range) + // SDK error codes ConnectionClosed = -32000, RequestTimeout = -32001,