From 12eb2bae5dac756c4f3bca8d3e375c4a8bda6e80 Mon Sep 17 00:00:00 2001 From: Justin Flick Date: Sat, 11 Oct 2025 16:47:36 -0700 Subject: [PATCH 1/2] 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 --- lib/mcp/server.rb | 9 ++++++++- test/mcp/server_context_test.rb | 7 ++++--- test/mcp/server_test.rb | 27 +++++++++++++-------------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/lib/mcp/server.rb b/lib/mcp/server.rb index fe8c9bd5..cc23404b 100644 --- a/lib/mcp/server.rb +++ b/lib/mcp/server.rb @@ -288,7 +288,14 @@ def call_tool(request) begin call_tool_with_args(tool, arguments) 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}", + }], + error: true, + ).to_h end end diff --git a/test/mcp/server_context_test.rb b/test/mcp/server_context_test.rb index 03ad3e6c..5fda0218 100644 --- a/test/mcp/server_context_test.rb +++ b/test/mcp/server_context_test.rb @@ -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 diff --git a/test/mcp/server_test.rb b/test/mcp/server_test.rb index f714e17d..b731f598 100644 --- a/test/mcp/server_test.rb +++ b/test/mcp/server_test.rb @@ -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, ) @@ -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", @@ -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 From f38344217e0eb59eb9e4c23b465e9bf7ba94d2e7 Mon Sep 17 00:00:00 2001 From: Justin Flick Date: Sun, 12 Oct 2025 23:35:26 -0700 Subject: [PATCH 2/2] handle tool errors and protocol errors distinctly. --- lib/mcp/server.rb | 34 +++++++++++++++++++++++++--------- test/mcp/server_test.rb | 35 ++++++++++++++++++++++------------- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/lib/mcp/server.rb b/lib/mcp/server.rb index cc23404b..98e11733 100644 --- a/lib/mcp/server.rb +++ b/lib/mcp/server.rb @@ -258,22 +258,32 @@ def list_tools(request) def call_tool(request) tool_name = request[:name] + arguments = request[:arguments] || {} + 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 @@ -281,7 +291,13 @@ def call_tool(request) 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 diff --git a/test/mcp/server_test.rb b/test/mcp/server_test.rb index b731f598..5505ca68 100644 --- a/test/mcp/server_test.rb +++ b/test/mcp/server_test.rb @@ -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", @@ -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 @@ -389,7 +391,7 @@ def call(message:, server_context: nil) 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", @@ -401,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", @@ -418,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 @@ -944,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 @@ -968,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