Skip to content

Fix tool error handling to follow MCP spec #165

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

Fix tool error handling to follow MCP spec #165

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
12eb2ba
Fix tool error handling to follow MCP spec
Jflick58 Oct 11, 2025
f383442
handle tool errors and protocol errors distinctly.
Jflick58 Oct 13, 2025
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
43 changes: 33 additions & 10 deletions lib/mcp/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,37 +258,60 @@ def list_tools(request)

def call_tool(request)
tool_name = request[:name]
arguments = request[:arguments] || {}
Copy link
Member

Choose a reason for hiding this comment

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

It seems better to keep it at its original position on line 267, since there's no need to process it until it becomes necessary.


tool = tools[tool_name]
unless tool
add_instrumentation_data(error: :tool_not_found)
raise RequestHandlerError.new("Tool not found #{tool_name}", request, error_type: :tool_not_found)
add_instrumentation_data(tool_name:, error: :tool_not_found)
return Tool::Response.new(
[{
type: "text",
text: "Tool not found: #{tool_name}",
}],
error: true,
).to_h
end

arguments = request[:arguments] || {}
add_instrumentation_data(tool_name:)

if tool.input_schema&.missing_required_arguments?(arguments)
add_instrumentation_data(error: :missing_required_arguments)
raise RequestHandlerError.new(
"Missing required arguments: #{tool.input_schema.missing_required_arguments(arguments).join(", ")}",
request,
error_type: :missing_required_arguments,
)
missing = tool.input_schema.missing_required_arguments(arguments).join(", ")
return Tool::Response.new(
[{
type: "text",
text: "Missing required arguments: #{missing}",
}],
error: true,
).to_h
end

if configuration.validate_tool_call_arguments && tool.input_schema
begin
tool.input_schema.validate_arguments(arguments)
rescue Tool::InputSchema::ValidationError => e
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

begin
call_tool_with_args(tool, arguments)
rescue => e
Copy link
Contributor

@atesgoral atesgoral Oct 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a step in the right direction, but I think we can take this a few more steps further:

This rescue should be at the call_tool level: Any mistake made by the caller that falls out of valid tool contracts (e.g. invalid tool name, invalid args) should be non-exception tool results with isError: true. Only JSON-RPC-level protocol errors should be allowed to be returned as JSON-RPC errors with error codes.

Copy link
Author

Choose a reason for hiding this comment

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

@atesgoral makes sense. This is how it should work now:

Original (Everything was a Protocol Error)

  {
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
      "code": -32603,
      "message": "Internal error",
      "data": "Tool not found unknown_tool"
    }
  }

Now - Protocol Error

  {
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
      "code": -32601,
      "message": "Method not found",
      "data": "unknown_method"
    }
  }

Now - Tool error:

  {
    "jsonrpc": "2.0",
    "id": 1,
    "result": {
      "content": [{
        "type": "text",
        "text": "Tool not found: unknown_tool"
      }],
      "isError": true
    }
  }

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}",
}],
error: true,
).to_h
end
end

Expand Down
7 changes: 4 additions & 3 deletions test/mcp/server_context_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,10 @@ def call(message:, server_context:)

response = server_no_context.handle(request)

assert response[:error]
# The error is wrapped as "Internal error calling tool..."
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 "Internal error calling tool tool_with_required_context", response[:result][:content][0][:text]
end

test "call_tool_with_args correctly detects server_context parameter presence" do
Expand Down
62 changes: 35 additions & 27 deletions test/mcp/server_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ class ServerTest < ActiveSupport::TestCase
assert_instrumentation_data({ method: "tools/call", tool_name: })
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(
name: "test_tool",
title: "Test tool",
Expand All @@ -279,8 +279,10 @@ class ServerTest < ActiveSupport::TestCase

response = server.handle(request)

assert_equal "Missing required arguments: message", response[:error][:data]
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 "Missing required arguments: message", response[:result][:content][0][:text]
end

test "#handle_json tools/call executes tool and returns result" do
Expand Down Expand Up @@ -340,17 +342,12 @@ def call(message:, server_context: nil)
assert_equal({ content: [{ type: "text", content: "OK" }], isError: false }, response[:result])
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|
assert_not_nil exception
assert_equal(
{
request: {
jsonrpc: "2.0",
method: "tools/call",
params: { name: "tool_that_raises", arguments: { message: "test" } },
id: 1,
},
request: { name: "tool_that_raises", arguments: { message: "test" } },
},
server_context,
)
Expand All @@ -368,12 +365,14 @@ def call(message:, server_context: nil)

response = @server.handle(request)

assert_equal "Internal error", response[:error][:message]
assert_equal "Internal error calling tool tool_that_raises", response[:error][:data]
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", error: :internal_error })
assert_nil response[:error], "Expected no JSON-RPC error"
assert response[:result][:isError]
assert_equal "text", response[:result][:content][0][:type]
assert_equal "Internal error calling tool tool_that_raises", response[:result][:content][0][:text]
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises" })
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({
jsonrpc: "2.0",
method: "tools/call",
Expand All @@ -385,12 +384,14 @@ def call(message:, server_context: nil)
})

response = JSON.parse(@server.handle_json(request), symbolize_names: true)
assert_equal "Internal error", response[:error][:message]
assert_equal "Internal error calling tool tool_that_raises", response[:error][:data]
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", error: :internal_error })
assert_nil response[:error], "Expected no JSON-RPC error"
assert response[:result][:isError]
assert_equal "text", response[:result][:content][0][:type]
assert_equal "Internal error calling tool tool_that_raises", response[:result][:content][0][:text]
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises" })
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 = {
jsonrpc: "2.0",
method: "tools/call",
Expand All @@ -402,12 +403,14 @@ def call(message:, server_context: nil)
}

response = @server.handle(request)
assert_equal "Internal error", response[:error][:message]
assert_equal "Tool not found unknown_tool", response[:error][:data]
assert_instrumentation_data({ method: "tools/call", error: :tool_not_found })
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]
assert_instrumentation_data({ method: "tools/call", tool_name: "unknown_tool", error: :tool_not_found })
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({
jsonrpc: "2.0",
method: "tools/call",
Expand All @@ -419,7 +422,10 @@ def call(message:, server_context: nil)
})

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

test "#tools_call_handler sets the tools/call handler" do
Expand Down Expand Up @@ -945,8 +951,9 @@ def call(message:, server_context: nil)

assert_equal "2.0", response[:jsonrpc]
assert_equal 1, response[:id]
assert_equal(-32603, response[:error][:code])
assert_includes response[:error][:data], "Missing required arguments"
assert_nil response[:error], "Expected no JSON-RPC error"
assert response[:result][:isError]
assert_includes response[:result][:content][0][:text], "Missing required arguments"
end

test "tools/call validates arguments against input schema when validate_tool_call_arguments is true" do
Expand All @@ -969,8 +976,9 @@ def call(message:, server_context: nil)

assert_equal "2.0", response[:jsonrpc]
assert_equal 1, response[:id]
assert_equal(-32603, response[:error][:code])
assert_includes response[:error][:data], "Invalid arguments"
assert_nil response[:error], "Expected no JSON-RPC error"
assert response[:result][:isError]
assert_includes response[:result][:content][0][:text], "Invalid arguments"
end

test "tools/call skips argument validation when validate_tool_call_arguments is false" do
Expand Down