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 1 commit
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
9 changes: 8 additions & 1 deletion lib/mcp/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,14 @@ def call_tool(request)
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
27 changes: 13 additions & 14 deletions test/mcp/server_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,17 +340,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 +363,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,9 +382,11 @@ 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
Expand Down