Skip to content

fix: return protocol errors for invalid tool args #2163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
he-yufeng wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: return protocol errors for invalid tool args #2163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions packages/server/src/server/mcp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,11 @@ 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 &&
(error.code === ProtocolErrorCode.UrlElicitationRequired || error.message.startsWith('Input validation error:'))
) {
throw error;
}
return this.createToolError(error instanceof Error ? error.message : String(error));
}
Expand Down
44 changes: 43 additions & 1 deletion packages/server/test/server/mcp.compat.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { JSONRPCMessage } from '@modelcontextprotocol/core';
import { InMemoryTransport, isStandardSchema, LATEST_PROTOCOL_VERSION } from '@modelcontextprotocol/core';
import { InMemoryTransport, isStandardSchema, LATEST_PROTOCOL_VERSION, ProtocolErrorCode } from '@modelcontextprotocol/core';
import { describe, expect, expectTypeOf, it, vi } from 'vitest';
import * as z from 'zod/v4';
import { McpServer } from '../../src/index.js';
Expand Down Expand Up @@ -119,6 +119,48 @@ describe('registerTool/registerPrompt accept raw Zod shape (auto-wrapped)', () =

await server.close();
});

it('returns a protocol error for invalid tool arguments', async () => {
const server = new McpServer({ name: 't', version: '1.0.0' });

server.registerTool('echo', { inputSchema: { x: z.number() } }, async ({ x }) => ({
content: [{ type: 'text' as const, text: String(x) }]
}));

const [client, srv] = InMemoryTransport.createLinkedPair();
await server.connect(srv);
await client.start();

const responses: JSONRPCMessage[] = [];
client.onmessage = m => responses.push(m);

await client.send({
jsonrpc: '2.0',
id: 1,
method: 'initialize',
params: {
protocolVersion: LATEST_PROTOCOL_VERSION,
capabilities: {},
clientInfo: { name: 'c', version: '1.0.0' }
}
} as JSONRPCMessage);
await client.send({ jsonrpc: '2.0', method: 'notifications/initialized' } as JSONRPCMessage);
await client.send({
jsonrpc: '2.0',
id: 2,
method: 'tools/call',
params: { name: 'echo', arguments: { x: 'nope' } }
} as JSONRPCMessage);

await vi.waitFor(() => expect(responses.some(r => 'id' in r && r.id === 2)).toBe(true));

const response = responses.find(r => 'id' in r && r.id === 2) as { error?: { code: number; message: string }; result?: unknown };
expect(response.result).toBeUndefined();
expect(response.error?.code).toBe(ProtocolErrorCode.InvalidParams);
expect(response.error?.message).toContain('Invalid arguments for tool echo');

await server.close();
});
});

describe('InferRawShape', () => {
Expand Down
123 changes: 58 additions & 65 deletions test/integration/test/server/mcp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
getDisplayName,
InMemoryTaskStore,
InMemoryTransport,
ProtocolError,
ProtocolErrorCode,
UriTemplate,
UrlElicitationRequiredError
Expand All @@ -28,6 +29,26 @@ function createLatch() {
};
}

async function expectInvalidToolArguments(request: Promise<unknown>, expectedMessages: Array<string | RegExp>) {
let error: unknown;
try {
await request;
} catch (error_) {
error = error_;
}

expect(error).toBeInstanceOf(ProtocolError);
const protocolError = error as ProtocolError;
expect(protocolError.code).toBe(ProtocolErrorCode.InvalidParams);
for (const expected of expectedMessages) {
if (typeof expected === 'string') {
expect(protocolError.message).toContain(expected);
} else {
expect(protocolError.message).toMatch(expected);
}
}
}

describe('Zod v4', () => {
describe('McpServer', () => {
/***
Expand Down Expand Up @@ -1212,25 +1233,18 @@ describe('Zod v4', () => {

await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);

const result = await client.request({
method: 'tools/call',
params: {
name: 'test',
arguments: {
await expectInvalidToolArguments(
client.request({
method: 'tools/call',
params: {
name: 'test',
value: 'not a number'
}
}
});

expect(result.isError).toBe(true);
expect(result.content).toEqual(
expect.arrayContaining([
{
type: 'text',
text: expect.stringContaining('Input validation error: Invalid arguments for tool test')
arguments: {
name: 'test',
value: 'not a number'
}
}
])
}),
['Input validation error: Invalid arguments for tool test']
);
});

Expand Down Expand Up @@ -5149,22 +5163,15 @@ describe('Zod v4', () => {
await server.connect(serverTransport);
await client.connect(clientTransport);

const invalidTypeResult = await client.callTool({
name: 'union-test',
arguments: {
type: 'a',
value: 123
}
});

expect(invalidTypeResult.isError).toBe(true);
expect(invalidTypeResult.content).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: 'text',
text: expect.stringContaining('Input validation error')
})
])
await expectInvalidToolArguments(
client.callTool({
name: 'union-test',
arguments: {
type: 'a',
value: 123
}
}),
['Input validation error']
);
});
});
Expand Down Expand Up @@ -6407,40 +6414,26 @@ describe('Zod v4', () => {
await server.connect(serverTransport);
await client.connect(clientTransport);

const invalidTypeResult = await client.callTool({
name: 'union-test',
arguments: {
type: 'a',
value: 123
}
});

expect(invalidTypeResult.isError).toBe(true);
expect(invalidTypeResult.content).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: 'text',
text: expect.stringContaining('Input validation error')
})
])
await expectInvalidToolArguments(
client.callTool({
name: 'union-test',
arguments: {
type: 'a',
value: 123
}
}),
['Input validation error']
);

const invalidDiscriminatorResult = await client.callTool({
name: 'union-test',
arguments: {
type: 'c',
value: 'test'
}
});

expect(invalidDiscriminatorResult.isError).toBe(true);
expect(invalidDiscriminatorResult.content).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: 'text',
text: expect.stringContaining('Input validation error')
})
])
await expectInvalidToolArguments(
client.callTool({
name: 'union-test',
arguments: {
type: 'c',
value: 'test'
}
}),
['Input validation error']
);
});
});
Expand Down
Loading
Loading