fix: correct error handling for unknown tools and resources (#1389)
Co-authored-by: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com>
This commit is contained in:
parent
ad4b2f9cac
commit
108f2f3ab6
5 changed files with 123 additions and 23 deletions
15
.changeset/fix-unknown-tool-protocol-error.md
Normal file
15
.changeset/fix-unknown-tool-protocol-error.md
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
---
|
||||
"@modelcontextprotocol/core": minor
|
||||
"@modelcontextprotocol/server": major
|
||||
---
|
||||
|
||||
Fix error handling for unknown tools and resources per MCP spec.
|
||||
|
||||
**Tools:** Unknown or disabled tool calls now return JSON-RPC protocol errors with
|
||||
code `-32602` (InvalidParams) instead of `CallToolResult` with `isError: true`.
|
||||
Callers who checked `result.isError` for unknown tools should catch rejected promises instead.
|
||||
|
||||
**Resources:** Unknown resource reads now return error code `-32002` (ResourceNotFound)
|
||||
instead of `-32602` (InvalidParams).
|
||||
|
||||
Added `ProtocolErrorCode.ResourceNotFound`.
|
||||
|
|
@ -305,7 +305,7 @@ describe('createPrivateKeyJwtAuth', () => {
|
|||
|
||||
const params = new URLSearchParams();
|
||||
await expect(addClientAuth(new Headers(), params, 'https://auth.example.com/token', undefined)).rejects.toThrow(
|
||||
/Invalid character/
|
||||
/cannot be part of a valid base64|Invalid character/
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -233,6 +233,7 @@ export enum ProtocolErrorCode {
|
|||
InternalError = -32_603,
|
||||
|
||||
// MCP-specific error codes
|
||||
ResourceNotFound = -32_002,
|
||||
UrlElicitationRequired = -32_042
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -166,15 +166,15 @@ export class McpServer {
|
|||
);
|
||||
|
||||
this.server.setRequestHandler('tools/call', async (request, ctx): Promise<CallToolResult | CreateTaskResult> => {
|
||||
try {
|
||||
const tool = this._registeredTools[request.params.name];
|
||||
if (!tool) {
|
||||
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Tool ${request.params.name} not found`);
|
||||
}
|
||||
if (!tool.enabled) {
|
||||
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Tool ${request.params.name} disabled`);
|
||||
}
|
||||
const tool = this._registeredTools[request.params.name];
|
||||
if (!tool) {
|
||||
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Tool ${request.params.name} not found`);
|
||||
}
|
||||
if (!tool.enabled) {
|
||||
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Tool ${request.params.name} disabled`);
|
||||
}
|
||||
|
||||
try {
|
||||
const isTaskRequest = !!request.params.task;
|
||||
const taskSupport = tool.execution?.taskSupport;
|
||||
const isTaskHandler = 'createTask' in (tool.handler as AnyToolHandler<AnySchema>);
|
||||
|
|
@ -506,7 +506,7 @@ export class McpServer {
|
|||
}
|
||||
}
|
||||
|
||||
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Resource ${uri} not found`);
|
||||
throw new ProtocolError(ProtocolErrorCode.ResourceNotFound, `Resource ${uri} not found`);
|
||||
});
|
||||
|
||||
this._resourceHandlersInitialized = true;
|
||||
|
|
|
|||
|
|
@ -1777,22 +1777,59 @@ describe('Zod v4', () => {
|
|||
|
||||
await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);
|
||||
|
||||
const result = await client.request({
|
||||
method: 'tools/call',
|
||||
params: {
|
||||
name: 'nonexistent-tool'
|
||||
}
|
||||
await expect(
|
||||
client.request({
|
||||
method: 'tools/call',
|
||||
params: {
|
||||
name: 'nonexistent-tool'
|
||||
}
|
||||
})
|
||||
).rejects.toMatchObject({
|
||||
code: ProtocolErrorCode.InvalidParams,
|
||||
message: expect.stringContaining('nonexistent-tool')
|
||||
});
|
||||
});
|
||||
|
||||
/***
|
||||
* Test: ProtocolError for Disabled Tool
|
||||
*/
|
||||
test('should throw ProtocolError for disabled tool', async () => {
|
||||
const mcpServer = new McpServer({
|
||||
name: 'test server',
|
||||
version: '1.0'
|
||||
});
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect(result.content).toEqual(
|
||||
expect.arrayContaining([
|
||||
const client = new Client({
|
||||
name: 'test client',
|
||||
version: '1.0'
|
||||
});
|
||||
|
||||
const tool = mcpServer.registerTool('test-tool', {}, async () => ({
|
||||
content: [
|
||||
{
|
||||
type: 'text',
|
||||
text: expect.stringContaining('Tool nonexistent-tool not found')
|
||||
text: 'Test response'
|
||||
}
|
||||
])
|
||||
);
|
||||
]
|
||||
}));
|
||||
|
||||
tool.disable();
|
||||
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
|
||||
|
||||
await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);
|
||||
|
||||
await expect(
|
||||
client.request({
|
||||
method: 'tools/call',
|
||||
params: {
|
||||
name: 'test-tool'
|
||||
}
|
||||
})
|
||||
).rejects.toMatchObject({
|
||||
code: ProtocolErrorCode.InvalidParams,
|
||||
message: expect.stringContaining('disabled')
|
||||
});
|
||||
});
|
||||
|
||||
/***
|
||||
|
|
@ -2797,7 +2834,51 @@ describe('Zod v4', () => {
|
|||
uri: 'test://nonexistent'
|
||||
}
|
||||
})
|
||||
).rejects.toThrow(/Resource test:\/\/nonexistent not found/);
|
||||
).rejects.toMatchObject({
|
||||
code: ProtocolErrorCode.ResourceNotFound,
|
||||
message: expect.stringContaining('not found')
|
||||
});
|
||||
});
|
||||
|
||||
/***
|
||||
* Test: ProtocolError for Disabled Resource
|
||||
*/
|
||||
test('should throw ProtocolError for disabled resource', async () => {
|
||||
const mcpServer = new McpServer({
|
||||
name: 'test server',
|
||||
version: '1.0'
|
||||
});
|
||||
const client = new Client({
|
||||
name: 'test client',
|
||||
version: '1.0'
|
||||
});
|
||||
|
||||
const resource = mcpServer.registerResource('test', 'test://resource', {}, async () => ({
|
||||
contents: [
|
||||
{
|
||||
uri: 'test://resource',
|
||||
text: 'Test content'
|
||||
}
|
||||
]
|
||||
}));
|
||||
|
||||
resource.disable();
|
||||
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
|
||||
|
||||
await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);
|
||||
|
||||
await expect(
|
||||
client.request({
|
||||
method: 'resources/read',
|
||||
params: {
|
||||
uri: 'test://resource'
|
||||
}
|
||||
})
|
||||
).rejects.toMatchObject({
|
||||
code: ProtocolErrorCode.InvalidParams,
|
||||
message: expect.stringContaining('disabled')
|
||||
});
|
||||
});
|
||||
|
||||
/***
|
||||
|
|
@ -3693,7 +3774,10 @@ describe('Zod v4', () => {
|
|||
name: 'nonexistent-prompt'
|
||||
}
|
||||
})
|
||||
).rejects.toThrow(/Prompt nonexistent-prompt not found/);
|
||||
).rejects.toMatchObject({
|
||||
code: ProtocolErrorCode.InvalidParams,
|
||||
message: expect.stringContaining('not found')
|
||||
});
|
||||
});
|
||||
|
||||
/***
|
||||
|
|
|
|||
Loading…
Reference in a new issue