Skip to content

Commit a988d37

Browse files
committed
fix: Return JSON-RPC protocol errors for unknown tools
1 parent 95b44fb commit a988d37

File tree

2 files changed

+137
-21
lines changed

2 files changed

+137
-21
lines changed

src/mcp/server/lowlevel/server.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,14 @@ async def handler(req: types.CallToolRequest):
452452
arguments = req.params.arguments or {}
453453
tool = await self._get_cached_tool_definition(tool_name)
454454

455+
# Check if tool exists - return protocol error if not found
456+
if tool is None:
457+
raise McpError(
458+
types.ErrorData(
459+
code=types.METHOD_NOT_FOUND,
460+
message=f"Unknown tool: {tool_name}",
461+
)
462+
)
455463
# input validation
456464
if validate_input and tool:
457465
try:
@@ -499,6 +507,8 @@ async def handler(req: types.CallToolRequest):
499507
isError=False,
500508
)
501509
)
510+
except McpError:
511+
raise
502512
except Exception as e:
503513
return self._make_error_result(str(e))
504514

tests/server/test_lowlevel_input_validation.py

Lines changed: 127 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,26 @@
1212
from mcp.server.lowlevel import NotificationOptions
1313
from mcp.server.models import InitializationOptions
1414
from mcp.server.session import ServerSession
15+
from mcp.shared.exceptions import McpError
1516
from mcp.shared.message import SessionMessage
1617
from mcp.shared.session import RequestResponder
17-
from mcp.types import CallToolResult, ClientResult, ServerNotification, ServerRequest, TextContent, Tool
18+
from mcp.types import (
19+
METHOD_NOT_FOUND,
20+
CallToolResult,
21+
ClientResult,
22+
ErrorData,
23+
ServerNotification,
24+
ServerRequest,
25+
TextContent,
26+
Tool,
27+
)
1828

1929

2030
async def run_tool_test(
2131
tools: list[Tool],
2232
call_tool_handler: Callable[[str, dict[str, Any]], Awaitable[list[TextContent]]],
23-
test_callback: Callable[[ClientSession], Awaitable[CallToolResult]],
24-
) -> CallToolResult:
33+
test_callback: Callable[[ClientSession], Awaitable[Any]],
34+
) -> Any:
2535
"""Helper to run a tool test with minimal boilerplate.
2636
2737
Args:
@@ -263,8 +273,9 @@ async def test_callback(client_session: ClientSession) -> CallToolResult:
263273

264274

265275
@pytest.mark.anyio
266-
async def test_tool_not_in_list_logs_warning(caplog):
267-
"""Test that calling a tool not in list_tools logs a warning and skips validation."""
276+
async def test_tool_not_in_list_logs_warning_before_protocol_error(caplog):
277+
"""Test that calling a tool not in list_tools logs a warning before returning protocol error."""
278+
268279
tools = [
269280
Tool(
270281
name="add",
@@ -281,30 +292,125 @@ async def test_tool_not_in_list_logs_warning(caplog):
281292
]
282293

283294
async def call_tool_handler(name: str, arguments: dict[str, Any]) -> list[TextContent]:
284-
# This should be reached since validation is skipped for unknown tools
285-
if name == "unknown_tool":
286-
# Even with invalid arguments, this should execute since validation is skipped
287-
return [TextContent(type="text", text="Unknown tool executed without validation")]
295+
# This should not be reached due to protocol error for unknown tools
296+
if name == "add":
297+
result = arguments["a"] + arguments["b"]
298+
return [TextContent(type="text", text=f"Result: {result}")]
288299
else:
289300
raise ValueError(f"Unknown tool: {name}")
290301

291-
async def test_callback(client_session: ClientSession) -> CallToolResult:
292-
# Call a tool that's not in the list with invalid arguments
293-
# This should trigger the warning about validation not being performed
294-
return await client_session.call_tool("unknown_tool", {"invalid": "args"})
302+
async def test_callback(client_session: ClientSession):
303+
# Call a tool that's not in the list - should now raise McpError
304+
try:
305+
return await client_session.call_tool("unknown_tool", {"invalid": "args"})
306+
except McpError as e:
307+
return e
295308

296309
with caplog.at_level(logging.WARNING):
297310
result = await run_tool_test(tools, call_tool_handler, test_callback)
298311

299-
# Verify results - should succeed because validation is skipped for unknown tools
300-
assert result is not None
301-
assert not result.isError
302-
assert len(result.content) == 1
303-
assert result.content[0].type == "text"
304-
assert isinstance(result.content[0], TextContent)
305-
assert result.content[0].text == "Unknown tool executed without validation"
312+
# Verify it's the correct protocol error
313+
assert isinstance(result, McpError), f"Expected McpError but got {type(result)}"
314+
assert isinstance(result.error, ErrorData)
315+
assert result.error.code == METHOD_NOT_FOUND
316+
assert "Unknown tool: unknown_tool" in result.error.message
306317

307-
# Verify warning was logged
318+
# Verify warning was still logged during the tool lookup process
308319
assert any(
309320
"Tool 'unknown_tool' not listed, no validation will be performed" in record.message for record in caplog.records
310321
)
322+
323+
324+
@pytest.mark.anyio
325+
async def test_unknown_tool_returns_protocol_error():
326+
"""Test that calling an unknown tool returns a proper JSON-RPC protocol error."""
327+
328+
tools = [
329+
Tool(
330+
name="add",
331+
description="Add two numbers",
332+
inputSchema={
333+
"type": "object",
334+
"properties": {
335+
"a": {"type": "number"},
336+
"b": {"type": "number"},
337+
},
338+
"required": ["a", "b"],
339+
},
340+
)
341+
]
342+
343+
async def call_tool_handler(name: str, arguments: dict[str, Any]) -> list[TextContent]:
344+
# This should not be reached for unknown tools due to protocol error
345+
if name == "add":
346+
result = arguments["a"] + arguments["b"]
347+
return [TextContent(type="text", text=f"Result: {result}")]
348+
else:
349+
raise ValueError(f"Unknown tool: {name}")
350+
351+
async def test_callback(client_session: ClientSession):
352+
# Try to call a tool that doesn't exist - should raise McpError
353+
try:
354+
return await client_session.call_tool("unknown_tool", {"invalid": "args"})
355+
except McpError as e:
356+
return e
357+
358+
result = await run_tool_test(tools, call_tool_handler, test_callback)
359+
360+
# Verify it's the correct protocol error
361+
assert isinstance(result, McpError), f"Expected McpError but got {type(result)}"
362+
assert isinstance(result.error, ErrorData)
363+
assert result.error.code == METHOD_NOT_FOUND
364+
assert "Unknown tool: unknown_tool" in result.error.message
365+
366+
367+
@pytest.mark.anyio
368+
async def test_tool_execution_error_vs_protocol_error():
369+
"""Test the difference between tool execution errors and protocol errors."""
370+
371+
tools = [
372+
Tool(
373+
name="failing_tool",
374+
description="A tool that always fails during execution",
375+
inputSchema={
376+
"type": "object",
377+
"properties": {
378+
"input": {"type": "string"},
379+
},
380+
},
381+
)
382+
]
383+
384+
async def call_tool_handler(name: str, arguments: dict[str, Any]) -> list[TextContent]:
385+
if name == "failing_tool":
386+
# This should cause a tool execution error (not a protocol error)
387+
raise RuntimeError("Tool execution failed")
388+
else:
389+
raise ValueError(f"Unknown tool: {name}")
390+
391+
# Test 1: Tool execution error (valid tool that fails)
392+
async def test_execution_error(client_session: ClientSession):
393+
return await client_session.call_tool("failing_tool", {"input": "test"})
394+
395+
execution_result = await run_tool_test(tools, call_tool_handler, test_execution_error)
396+
397+
# Should return CallToolResult with isError=True (tool execution error)
398+
assert isinstance(execution_result, CallToolResult)
399+
assert execution_result.isError
400+
assert isinstance(execution_result.content[0], TextContent)
401+
assert "Tool execution failed" in execution_result.content[0].text
402+
403+
# Test 2: Protocol error (unknown tool)
404+
async def test_protocol_error(client_session: ClientSession):
405+
try:
406+
return await client_session.call_tool("nonexistent_tool", {"input": "test"})
407+
except McpError as e:
408+
return e
409+
410+
protocol_result = await run_tool_test(tools, call_tool_handler, test_protocol_error)
411+
412+
# Should return McpError (protocol error)
413+
assert isinstance(protocol_result, McpError), f"Expected McpError but got {type(protocol_result)}"
414+
assert isinstance(protocol_result.error, ErrorData)
415+
assert protocol_result.error.code == METHOD_NOT_FOUND
416+
assert "Unknown tool: nonexistent_tool" in protocol_result.error.message

0 commit comments

Comments
 (0)