diff --git a/.changeset/protocol-error-rethrow.md b/.changeset/protocol-error-rethrow.md new file mode 100644 index 000000000..bbe3c1ade --- /dev/null +++ b/.changeset/protocol-error-rethrow.md @@ -0,0 +1,9 @@ +--- +'@modelcontextprotocol/server': minor +--- + +Re-throw all `ProtocolError` instances from `tools/call` handler as JSON-RPC errors instead of wrapping them in `isError: true` results. + +**Breaking change:** Output validation failures (missing or schema-mismatched `structuredContent`) now surface as JSON-RPC `InternalError` rejections instead of `{ isError: true }` results. Input validation failures continue to return `{ isError: true }` per the MCP spec's tool-execution-error classification. + +This also means tool handlers that deliberately `throw new ProtocolError(...)` will now propagate that as a JSON-RPC error, matching the python-sdk behavior. diff --git a/docs/migration-SKILL.md b/docs/migration-SKILL.md index 0056795c3..6ceeb557f 100644 --- a/docs/migration-SKILL.md +++ b/docs/migration-SKILL.md @@ -163,6 +163,20 @@ if (error instanceof SdkError && error.code === SdkErrorCode.ClientHttpFailedToO } ``` +### Tool error classification (McpServer tools/call) + +`McpServer` now re-throws any `ProtocolError` from the `tools/call` handler as a JSON-RPC error. Previously only `UrlElicitationRequired` was re-thrown; other protocol errors were wrapped as `{ isError: true }` results. + +Behavior changes in `callTool` results: + +- Input validation failure: `{ isError: true }` → `{ isError: true }` (unchanged) +- Output validation failure: `{ isError: true }` → throws `ProtocolError` (`InternalError`) +- Task-required without task: `{ isError: true }` → throws `ProtocolError` (`InvalidParams`) +- Handler throws `ProtocolError`: `{ isError: true }` → re-thrown as JSON-RPC error +- Handler throws plain `Error`: `{ isError: true }` → `{ isError: true }` (unchanged) + +Migration: if code checks `result.isError` to detect output-schema violations or deliberate `ProtocolError` throws, add a `try/catch` around `callTool`. If a handler throws `ProtocolError` expecting tool-level wrapping, change it to throw a plain `Error`. + ### OAuth error consolidation Individual OAuth error classes replaced with single `OAuthError` class and `OAuthErrorCode` enum: diff --git a/docs/migration.md b/docs/migration.md index 21f8b67c9..50dd0d3da 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -607,6 +607,44 @@ try { } ``` +#### Tool error classification + +The `tools/call` handler in `McpServer` now re-throws any `ProtocolError` as a JSON-RPC error instead of wrapping it in an `isError: true` result. Previously, only `UrlElicitationRequired` was re-thrown. + +This aligns error surfaces with the MCP spec's classification: + +- **Input validation failure** — unchanged, still returns `{ isError: true }` (spec classifies this as a tool-execution error) +- **Output validation failure** — now throws `ProtocolError` with `InternalError` code (was `{ isError: true }`) +- **Task-required tool called without task** — now throws `ProtocolError` with `InvalidParams` code (was `{ isError: true }`) +- **Handler throws `ProtocolError`** — now re-thrown as a JSON-RPC error (was `{ isError: true }`) +- **Handler throws plain `Error`** — unchanged, still returns `{ isError: true }` + +**Before (v1):** + +```typescript +const result = await client.callTool({ name: 'test', arguments: {} }); +if (result.isError) { + // caught output-schema mismatches, task misconfig, handler ProtocolErrors +} +``` + +**After (v2):** + +```typescript +try { + const result = await client.callTool({ name: 'test', arguments: {} }); + if (result.isError) { + // only input validation and ordinary handler exceptions land here + } +} catch (error) { + if (error instanceof ProtocolError) { + // output validation failure, task misconfig, or handler-thrown ProtocolError + } +} +``` + +If your tool handler was throwing `ProtocolError` expecting it to be wrapped as `isError: true`, throw a plain `Error` instead. + #### New `SdkErrorCode` enum The new `SdkErrorCode` enum contains string-valued codes for local SDK errors: diff --git a/packages/server/src/server/mcp.ts b/packages/server/src/server/mcp.ts index 4d9f81c50..11298a0f5 100644 --- a/packages/server/src/server/mcp.ts +++ b/packages/server/src/server/mcp.ts @@ -183,7 +183,7 @@ export class McpServer { // Handle taskSupport 'required' without task augmentation if (taskSupport === 'required' && !isTaskRequest) { throw new ProtocolError( - ProtocolErrorCode.MethodNotFound, + ProtocolErrorCode.InvalidParams, `Tool ${request.params.name} requires task augmentation (taskSupport: 'required')` ); } @@ -206,8 +206,8 @@ export class McpServer { await this.validateToolOutput(tool, result, request.params.name); return result; } catch (error) { - if (error instanceof ProtocolError && error.code === ProtocolErrorCode.UrlElicitationRequired) { - throw error; // Return the error to the caller without wrapping in CallToolResult + if (error instanceof ProtocolError) { + throw error; } return this.createToolError(error instanceof Error ? error.message : String(error)); } @@ -251,10 +251,9 @@ export class McpServer { const parseResult = await validateStandardSchema(tool.inputSchema, args ?? {}); if (!parseResult.success) { - throw new ProtocolError( - ProtocolErrorCode.InvalidParams, - `Input validation error: Invalid arguments for tool ${toolName}: ${parseResult.error}` - ); + // Per spec, input validation failures are tool-execution errors (isError: true), + // not protocol errors — throw plain Error so the catch wraps it as a tool result. + throw new Error(`Input validation error: Invalid arguments for tool ${toolName}: ${parseResult.error}`); } return parseResult.data as unknown as Args; @@ -279,7 +278,7 @@ export class McpServer { if (!result.structuredContent) { throw new ProtocolError( - ProtocolErrorCode.InvalidParams, + ProtocolErrorCode.InternalError, `Output validation error: Tool ${toolName} has an output schema but no structured content was provided` ); } @@ -288,7 +287,7 @@ export class McpServer { const parseResult = await validateStandardSchema(tool.outputSchema, result.structuredContent); if (!parseResult.success) { throw new ProtocolError( - ProtocolErrorCode.InvalidParams, + ProtocolErrorCode.InternalError, `Output validation error: Invalid structured content for tool ${toolName}: ${parseResult.error}` ); } @@ -333,7 +332,11 @@ export class McpServer { } // Return the final result - return (await ctx.task.store.getTaskResult(taskId)) as CallToolResult; + const result = (await ctx.task.store.getTaskResult(taskId)) as CallToolResult; + if (task.status === 'completed') { + await this.validateToolOutput(tool, result, request.params.name); + } + return result; } private _completionHandlerInitialized = false; diff --git a/test/integration/test/server/mcp.test.ts b/test/integration/test/server/mcp.test.ts index 967435834..d503239d5 100644 --- a/test/integration/test/server/mcp.test.ts +++ b/test/integration/test/server/mcp.test.ts @@ -1418,25 +1418,15 @@ describe('Zod v4', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); - // Call the tool and expect it to throw an error - const result = await client.callTool({ - name: 'test', - arguments: { - input: 'hello' - } - }); - - expect(result.isError).toBe(true); - expect(result.content).toEqual( - expect.arrayContaining([ - { - type: 'text', - text: expect.stringContaining( - 'Output validation error: Tool test has an output schema but no structured content was provided' - ) + // Output validation failure is a server-side bug → JSON-RPC InternalError + await expect( + client.callTool({ + name: 'test', + arguments: { + input: 'hello' } - ]) - ); + }) + ).rejects.toThrow('Output validation error: Tool test has an output schema but no structured content was provided'); }); /*** * Test: Tool with Output Schema Must Provide Structured Content @@ -1550,23 +1540,15 @@ describe('Zod v4', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); - // Call the tool and expect it to throw a server-side validation error - const result = await client.callTool({ - name: 'test', - arguments: { - input: 'hello' - } - }); - - expect(result.isError).toBe(true); - expect(result.content).toEqual( - expect.arrayContaining([ - { - type: 'text', - text: expect.stringContaining('Output validation error: Invalid structured content for tool test') + // Output validation failure is a server-side bug → JSON-RPC InternalError + await expect( + client.callTool({ + name: 'test', + arguments: { + input: 'hello' } - ]) - ); + }) + ).rejects.toThrow(/Output validation error: Invalid structured content for tool test/); }); /*** @@ -6441,16 +6423,13 @@ describe('Zod v4', () => { await Promise.all([client.connect(clientTransport), mcpServer.connect(serverTransport)]); - // Call the tool WITHOUT task augmentation - should return error - const result = await client.callTool({ - name: 'long-running-task', - arguments: { input: 'test data' } - }); - - // Should receive error result - expect(result.isError).toBe(true); - const content = result.content as TextContent[]; - expect(content[0]!.text).toContain('requires task augmentation'); + // Call the tool WITHOUT task augmentation - should throw JSON-RPC error + await expect( + client.callTool({ + name: 'long-running-task', + arguments: { input: 'test data' } + }) + ).rejects.toThrow(/requires task augmentation/); taskStore.cleanup(); });