Skip to content

fix(server): return JSON-RPC errors for missing tools #1674

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
fahe1em1 wants to merge 2 commits into
base: main
Choose a base branch
from
+79 −4
Open

fix(server): return JSON-RPC errors for missing tools #1674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b5bd67
server: surface parse failures through transport onerror
fahe1em1 Mar 12, 2026
d8f0744
fix(server): return JSON-RPC errors for missing tools
fahe1em1 Mar 12, 2026
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
4 changes: 2 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 @@ -213,8 +213,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) {
Copy link

@Bortlesboat Bortlesboat Mar 15, 2026

Choose a reason for hiding this comment

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

This broadens the catch from only UrlElicitationRequired to all ProtocolError instances. Before this change, a ProtocolError with any other code (e.g. MethodNotFound thrown by user code inside a tool handler) would have been caught by the outer branch and returned as a CallToolResult with isError: true. Now it becomes a JSON-RPC error response instead.

Is this intentional for all protocol error codes? For example, if a tool implementation internally calls another protocol method that throws a ProtocolError, that would now surface as a JSON-RPC error rather than a tool-level error — which changes what the client sees. The test only covers the tool not found case (-32602), but this catch clause now affects every ProtocolError subtype.

Worth documenting which ProtocolError codes are expected to reach this path, or alternatively using an allowlist (e.g. UrlElicitationRequired, InvalidParams) rather than a blanket catch.

throw error; // Keep protocol-level failures as JSON-RPC errors.
}
return this.createToolError(error instanceof Error ? error.message : String(error));
}
Expand Down
6 changes: 4 additions & 2 deletions packages/server/src/server/streamableHttp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
if (options?.parsedBody === undefined) {
try {
rawMessage = await req.json();
} catch {
} catch (error) {
this.onerror?.(error instanceof Error ? error : new Error(String(error)));
return this.createJsonErrorResponse(400, -32_700, 'Parse error: Invalid JSON');
}
} else {
Expand All @@ -649,7 +650,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
messages = Array.isArray(rawMessage)
? rawMessage.map(msg => JSONRPCMessageSchema.parse(msg))
: [JSONRPCMessageSchema.parse(rawMessage)];
} catch {
} catch (error) {
this.onerror?.(error instanceof Error ? error : new Error(String(error)));
return this.createJsonErrorResponse(400, -32_700, 'Parse error: Invalid JSON-RPC message');
}

Expand Down
73 changes: 73 additions & 0 deletions packages/server/test/server/streamableHttp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,54 @@ describe('Zod v4', () => {
});

describe('POST Requests', () => {
it('should call onerror when request JSON is invalid', async () => {
const errorSpy = vi.fn();
transport.onerror = errorSpy;

const request = new Request('http://localhost/mcp', {
method: 'POST',
headers: {
Accept: 'application/json, text/event-stream',
'Content-Type': 'application/json'
},
body: '{invalid-json'
});

const response = await transport.handleRequest(request);

expect(response.status).toBe(400);
const errorData = await response.json();
expectErrorResponse(errorData, -32_700, /Invalid JSON/);
expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy.mock.calls[0]?.[0]).toBeInstanceOf(Error);
});

it('should call onerror when request JSON-RPC message is invalid', async () => {
const errorSpy = vi.fn();
transport.onerror = errorSpy;

const request = new Request('http://localhost/mcp', {
method: 'POST',
headers: {
Accept: 'application/json, text/event-stream',
'Content-Type': 'application/json'
},
body: JSON.stringify({
jsonrpc: '2.0',
method: 123,
params: {}
})
});

const response = await transport.handleRequest(request);

expect(response.status).toBe(400);
const errorData = await response.json();
expectErrorResponse(errorData, -32_700, /Invalid JSON-RPC message/);
expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy.mock.calls[0]?.[0]).toBeInstanceOf(Error);
});

it('should handle post requests via SSE response correctly', async () => {
sessionId = await initializeServer();

Expand Down Expand Up @@ -271,6 +319,31 @@ describe('Zod v4', () => {
});
});

it('should return a JSON-RPC error when calling an unknown tool', async () => {
sessionId = await initializeServer();

const toolCallMessage: JSONRPCMessage = {
jsonrpc: '2.0',
method: 'tools/call',
params: {
name: 'missing-tool',
arguments: {}
},
id: 'call-missing'
};

const request = createRequest('POST', toolCallMessage, { sessionId });
const response = await transport.handleRequest(request);

expect(response.status).toBe(200);

const text = await readSSEEvent(response);
const eventData = parseSSEData(text);

expectErrorResponse(eventData, -32_602, /Tool missing-tool not found/);
expect((eventData as JSONRPCErrorResponse).id).toBe('call-missing');
});

it('should reject requests without a valid session ID', async () => {
const request = createRequest('POST', TEST_MESSAGES.toolsList);
const response = await transport.handleRequest(request);
Expand Down
Loading