diff --git a/.changeset/mcperror-double-prefix.md b/.changeset/mcperror-double-prefix.md new file mode 100644 index 0000000000..2848b8b127 --- /dev/null +++ b/.changeset/mcperror-double-prefix.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/sdk': patch +--- + +Fix v1 server sending McpError messages with a doubled prefix on the wire diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index 2637be65bc..6d42522ad4 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -822,7 +822,10 @@ export abstract class Protocol:` prefix, which is + // reconstructed by the receiving peer; send the original message instead of + // leaking the local prefixed form onto the wire (#2786). + message: error instanceof McpError ? error.originalMessage : (error.message ?? 'Internal error'), ...(error['data'] !== undefined && { data: error['data'] }) } }; diff --git a/src/types.ts b/src/types.ts index 835eac89f8..6dabc1aa63 100644 --- a/src/types.ts +++ b/src/types.ts @@ -2305,6 +2305,13 @@ export const ServerResultSchema = z.union([ ]); export class McpError extends Error { + /** + * The message exactly as passed to the constructor, without the + * `MCP error :` prefix that `.message` carries. Peers reconstruct + * the prefixed form from `code`, so this is what belongs on the wire. + */ + public readonly originalMessage: string; + constructor( public readonly code: number, message: string, @@ -2312,6 +2319,7 @@ export class McpError extends Error { ) { super(`MCP error ${code}: ${message}`); this.name = 'McpError'; + this.originalMessage = message; } /** diff --git a/test/issues/test_2786_mcp_error_wire_message.test.ts b/test/issues/test_2786_mcp_error_wire_message.test.ts new file mode 100644 index 0000000000..6f48087cee --- /dev/null +++ b/test/issues/test_2786_mcp_error_wire_message.test.ts @@ -0,0 +1,49 @@ +import { Client } from '../../src/client/index.js'; +import { InMemoryTransport } from '../../src/inMemory.js'; +import { Server } from '../../src/server/index.js'; +import { CallToolRequestSchema, ErrorCode, McpError, type JSONRPCError } from '../../src/types.js'; + +describe('Issue #2786: a handler-thrown McpError must not be double-prefixed', () => { + test('wire message carries the original message; client reconstructs a single prefix', async () => { + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const server = new Server({ name: 'test-server', version: '1.0.0' }, { capabilities: { tools: {} } }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + + server.setRequestHandler(CallToolRequestSchema, async () => { + throw new McpError(ErrorCode.MethodNotFound, 'Unknown tool: nope'); + }); + + // Capture the raw JSON-RPC error the server puts on the wire. + const wireErrors: JSONRPCError[] = []; + const originalSend = serverTransport.send.bind(serverTransport); + serverTransport.send = async message => { + if ('error' in message) { + wireErrors.push(message); + } + return originalSend(message); + }; + + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + + let caught: unknown; + try { + await client.callTool({ name: 'nope', arguments: {} }); + } catch (error) { + caught = error; + } finally { + await Promise.all([client.close(), server.close()]); + } + + // The wire carries the original message, without the local `MCP error :` prefix... + expect(wireErrors).toHaveLength(1); + expect(wireErrors[0]?.error.code).toBe(ErrorCode.MethodNotFound); + expect(wireErrors[0]?.error.message).toBe('Unknown tool: nope'); + + // ...and the client reconstructs exactly one prefix. + expect(caught).toBeInstanceOf(McpError); + const mcpError = caught as McpError; + expect(mcpError.code).toBe(ErrorCode.MethodNotFound); + expect(mcpError.message).toBe('MCP error -32601: Unknown tool: nope'); + expect(mcpError.originalMessage).toBe('Unknown tool: nope'); + }); +});