Merge pull request #165 from Jflick58/fix/make-tool-error-spec-compliant

Fix tool error handling to follow MCP spec

Author: koic

lib/mcp/server.rb

@@ -258,37 +258,60 @@ def list_tools(request)
 
     def call_tool(request)
       tool_name = request[:name]
+
       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
-        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
 

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_match(/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

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
@@ -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,
         )
@@ -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_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
 
-    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,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_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
 
-    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",
@@ -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",
@@ -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
@@ -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
@@ -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