Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/protocol-error-rethrow.md
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 14 additions & 0 deletions docs/migration-SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` (`MethodNotFound`)
- 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:
Expand Down
38 changes: 38 additions & 0 deletions docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `MethodNotFound` 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) {
Comment thread
claude[bot] marked this conversation as resolved.
// 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:
Expand Down
15 changes: 7 additions & 8 deletions packages/server/src/server/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ export class McpServer {
await this.validateToolOutput(tool, result, request.params.name);
return result;
} catch (error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broadened catch now re-throws ProtocolError(InvalidParams) from RequestTaskStore.getTask (taskManager.ts:666-668) when a task vanishes mid-polling at L327. Before this PR, that was wrapped as {isError: true}; now it surfaces as a JSON-RPC InvalidParams error — but a task going missing during automatic polling isn't the client's fault.

Options:

  • Catch-and-convert in handleAutomaticTaskPolling before L327's getTask result is consumed
  • Or fix the underlying RequestTaskStore.getTask to throw InternalError instead of InvalidParams for task-not-found

Also: the null-check at L328-330 (if (!task) throw new ProtocolError(InternalError, ...)) is dead code — getTask throws before it can return null.

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;
}
Comment thread
claude[bot] marked this conversation as resolved.
return this.createToolError(error instanceof Error ? error.message : String(error));
}
Comment thread
claude[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -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;
Expand All @@ -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`
);
}
Expand All @@ -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}`
);
}
Expand Down
67 changes: 23 additions & 44 deletions test/integration/test/server/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/);
});

/***
Expand Down Expand Up @@ -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();
});
Expand Down
Loading