Fix tool error handling to follow MCP spec

Tool execution errors now return successful responses with isError: true
instead of JSON-RPC protocol errors, per the MCP specification.

This allows clients to distinguish between protocol-level errors and
tool execution errors, providing better error context.

Fixes #159
This commit is contained in:
Justin Flick 2025-10-21 18:33:42 -07:00
commit 389cc2e8e6
3 changed files with 71 additions and 39 deletions

View file

@ -258,10 +258,17 @@ module MCP
def call_tool(request) def call_tool(request)
tool_name = request[:name] tool_name = request[:name]
tool = tools[tool_name] tool = tools[tool_name]
unless tool unless tool
add_instrumentation_data(error: :tool_not_found) add_instrumentation_data(tool_name:, error: :tool_not_found)
raise RequestHandlerError.new("Tool not found #{tool_name}", request, error_type: :tool_not_found) return Tool::Response.new(
[{
type: "text",
text: "Tool not found: #{tool_name}",
}],
error: true,
).to_h
end end
arguments = request[:arguments] || {} arguments = request[:arguments] || {}
@ -269,11 +276,14 @@ module MCP
if tool.input_schema&.missing_required_arguments?(arguments) if tool.input_schema&.missing_required_arguments?(arguments)
add_instrumentation_data(error: :missing_required_arguments) add_instrumentation_data(error: :missing_required_arguments)
raise RequestHandlerError.new( missing = tool.input_schema.missing_required_arguments(arguments).join(", ")
"Missing required arguments: #{tool.input_schema.missing_required_arguments(arguments).join(", ")}", return Tool::Response.new(
request, [{
error_type: :missing_required_arguments, type: "text",
) text: "Missing required arguments: #{missing}",
}],
error: true,
).to_h
end end
if configuration.validate_tool_call_arguments && tool.input_schema if configuration.validate_tool_call_arguments && tool.input_schema
@ -281,14 +291,27 @@ module MCP
tool.input_schema.validate_arguments(arguments) tool.input_schema.validate_arguments(arguments)
rescue Tool::InputSchema::ValidationError => e rescue Tool::InputSchema::ValidationError => e
add_instrumentation_data(error: :invalid_schema) add_instrumentation_data(error: :invalid_schema)
raise RequestHandlerError.new(e.message, request, error_type: :invalid_schema) return Tool::Response.new(
[{
type: "text",
text: e.message,
}],
error: true,
).to_h
end end
end end
begin begin
call_tool_with_args(tool, arguments) call_tool_with_args(tool, arguments)
rescue => e rescue => e
raise RequestHandlerError.new("Internal error calling tool #{tool_name}", request, original_error: e) report_exception(e, { request: request })
Tool::Response.new(
[{
type: "text",
text: "Internal error calling tool #{tool_name}: #{e.message}",
}],
error: true,
).to_h
end end
end end

View file

@ -152,9 +152,10 @@ module MCP
response = server_no_context.handle(request) response = server_no_context.handle(request)
assert response[:error] assert_nil response[:error], "Expected no JSON-RPC error"
# The error is wrapped as "Internal error calling tool..." assert response[:result][:isError]
assert_equal "Internal error", response[:error][:message] assert_equal "text", response[:result][:content][0][:type]
assert_match(/Internal error calling tool tool_with_required_context: /, response[:result][:content][0][:text])
end end
test "call_tool_with_args correctly detects server_context parameter presence" do test "call_tool_with_args correctly detects server_context parameter presence" do

View file

@ -255,7 +255,7 @@ module MCP
assert_instrumentation_data({ method: "tools/call", tool_name: }) assert_instrumentation_data({ method: "tools/call", tool_name: })
end end
test "#handle tools/call returns error if required tool arguments are missing" do test "#handle tools/call returns error response with isError true if required tool arguments are missing" do
tool_with_required_argument = Tool.define( tool_with_required_argument = Tool.define(
name: "test_tool", name: "test_tool",
title: "Test tool", title: "Test tool",
@ -279,8 +279,10 @@ module MCP
response = server.handle(request) response = server.handle(request)
assert_equal "Missing required arguments: message", response[:error][:data] assert_nil response[:error], "Expected no JSON-RPC error"
assert_equal "Internal error", response[:error][:message] assert response[:result][:isError]
assert_equal "text", response[:result][:content][0][:type]
assert_equal "Missing required arguments: message", response[:result][:content][0][:text]
end end
test "#handle_json tools/call executes tool and returns result" do test "#handle_json tools/call executes tool and returns result" do
@ -340,17 +342,12 @@ module MCP
assert_equal({ content: [{ type: "text", content: "OK" }], isError: false }, response[:result]) assert_equal({ content: [{ type: "text", content: "OK" }], isError: false }, response[:result])
end end
test "#handle tools/call returns internal error and reports exception if the tool raises an error" do test "#handle tools/call returns error response with isError true if the tool raises an error" do
@server.configuration.exception_reporter.expects(:call).with do |exception, server_context| @server.configuration.exception_reporter.expects(:call).with do |exception, server_context|
assert_not_nil exception assert_not_nil exception
assert_equal( assert_equal(
{ {
request: { request: { name: "tool_that_raises", arguments: { message: "test" } },
jsonrpc: "2.0",
method: "tools/call",
params: { name: "tool_that_raises", arguments: { message: "test" } },
id: 1,
},
}, },
server_context, server_context,
) )
@ -368,12 +365,14 @@ module MCP
response = @server.handle(request) response = @server.handle(request)
assert_equal "Internal error", response[:error][:message] assert_nil response[:error], "Expected no JSON-RPC error"
assert_equal "Internal error calling tool tool_that_raises", response[:error][:data] assert response[:result][:isError]
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", error: :internal_error }) assert_equal "text", response[:result][:content][0][:type]
assert_match(/Internal error calling tool tool_that_raises: /, response[:result][:content][0][:text])
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises" })
end end
test "#handle_json returns internal error and reports exception if the tool raises an error" do test "#handle_json returns error response with isError true if the tool raises an error" do
request = JSON.generate({ request = JSON.generate({
jsonrpc: "2.0", jsonrpc: "2.0",
method: "tools/call", method: "tools/call",
@ -385,12 +384,14 @@ module MCP
}) })
response = JSON.parse(@server.handle_json(request), symbolize_names: true) response = JSON.parse(@server.handle_json(request), symbolize_names: true)
assert_equal "Internal error", response[:error][:message] assert_nil response[:error], "Expected no JSON-RPC error"
assert_equal "Internal error calling tool tool_that_raises", response[:error][:data] assert response[:result][:isError]
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", error: :internal_error }) assert_equal "text", response[:result][:content][0][:type]
assert_match(/Internal error calling tool tool_that_raises: /, response[:result][:content][0][:text])
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises" })
end end
test "#handle tools/call returns error for unknown tool" do test "#handle tools/call returns error response with isError true for unknown tool" do
request = { request = {
jsonrpc: "2.0", jsonrpc: "2.0",
method: "tools/call", method: "tools/call",
@ -402,12 +403,14 @@ module MCP
} }
response = @server.handle(request) response = @server.handle(request)
assert_equal "Internal error", response[:error][:message] assert_nil response[:error], "Expected no JSON-RPC error"
assert_equal "Tool not found unknown_tool", response[:error][:data] assert response[:result][:isError]
assert_instrumentation_data({ method: "tools/call", error: :tool_not_found }) assert_equal "text", response[:result][:content][0][:type]
assert_equal "Tool not found: unknown_tool", response[:result][:content][0][:text]
assert_instrumentation_data({ method: "tools/call", tool_name: "unknown_tool", error: :tool_not_found })
end end
test "#handle_json returns error for unknown tool" do test "#handle_json returns error response with isError true for unknown tool" do
request = JSON.generate({ request = JSON.generate({
jsonrpc: "2.0", jsonrpc: "2.0",
method: "tools/call", method: "tools/call",
@ -419,7 +422,10 @@ module MCP
}) })
response = JSON.parse(@server.handle_json(request), symbolize_names: true) response = JSON.parse(@server.handle_json(request), symbolize_names: true)
assert_equal "Internal error", response[:error][:message] assert_nil response[:error], "Expected no JSON-RPC error"
assert response[:result][:isError]
assert_equal "text", response[:result][:content][0][:type]
assert_equal "Tool not found: unknown_tool", response[:result][:content][0][:text]
end end
test "#tools_call_handler sets the tools/call handler" do test "#tools_call_handler sets the tools/call handler" do
@ -945,8 +951,9 @@ module MCP
assert_equal "2.0", response[:jsonrpc] assert_equal "2.0", response[:jsonrpc]
assert_equal 1, response[:id] assert_equal 1, response[:id]
assert_equal(-32603, response[:error][:code]) assert_nil response[:error], "Expected no JSON-RPC error"
assert_includes response[:error][:data], "Missing required arguments" assert response[:result][:isError]
assert_includes response[:result][:content][0][:text], "Missing required arguments"
end end
test "tools/call validates arguments against input schema when validate_tool_call_arguments is true" do test "tools/call validates arguments against input schema when validate_tool_call_arguments is true" do
@ -969,8 +976,9 @@ module MCP
assert_equal "2.0", response[:jsonrpc] assert_equal "2.0", response[:jsonrpc]
assert_equal 1, response[:id] assert_equal 1, response[:id]
assert_equal(-32603, response[:error][:code]) assert_nil response[:error], "Expected no JSON-RPC error"
assert_includes response[:error][:data], "Invalid arguments" assert response[:result][:isError]
assert_includes response[:result][:content][0][:text], "Invalid arguments"
end end
test "tools/call skips argument validation when validate_tool_call_arguments is false" do test "tools/call skips argument validation when validate_tool_call_arguments is false" do