From 590feb13a57919d8e0a019932cd4631af525031f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 31 Mar 2026 12:01:18 +0900 Subject: [PATCH] Fix client methods silently swallowing JSON-RPC error responses ## Motivation and Context Client methods (`tools`, `resources`, `resource_templates`, `prompts`, `read_resource`, `get_prompt`, `call_tool`) silently swallowed JSON-RPC error responses by falling back to empty defaults. This did not conform to JSON-RPC 2.0, which clearly distinguishes error responses from success responses. Both the Python SDK (`MCPError`) and TypeScript SDK (`ProtocolError`) raise exceptions on error responses. The Ruby SDK was the only one silently discarding them. Introduces a common `request` method that handles JSON-RPC request construction and error checking in one place, replacing the duplicated `transport.send_request` calls across all client methods. ## How Has This Been Tested? Added tests for each affected method verifying that `ServerError` is raised with the correct `code`, `message`, and `data` attributes. ## Breaking Changes The previous behavior of silently returning empty defaults on server errors did not conform to JSON-RPC 2.0. Error responses should be surfaced to callers, not swallowed. Callers that relied on the silent fallback behavior will need to rescue `MCP::Client::ServerError`. --- lib/mcp/client.rb | 96 ++++++++++++++++++++--------------------- test/mcp/client_test.rb | 87 +++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 49 deletions(-) diff --git a/lib/mcp/client.rb b/lib/mcp/client.rb index 6437dd6..7c89bcb 100644 --- a/lib/mcp/client.rb +++ b/lib/mcp/client.rb @@ -6,6 +6,27 @@ require_relative "client/tool" module MCP class Client + class ServerError < StandardError + attr_reader :code, :data + + def initialize(message, code:, data: nil) + super(message) + @code = code + @data = data + end + end + + class RequestHandlerError < StandardError + attr_reader :error_type, :original_error, :request + + def initialize(message, request, error_type: :internal_error, original_error: nil) + super(message) + @request = request + @error_type = error_type + @original_error = original_error + end + end + # Initializes a new MCP::Client instance. # # @param transport [Object] The transport object to use for communication with the server. @@ -33,11 +54,7 @@ module MCP # puts tool.name # end def tools - response = transport.send_request(request: { - jsonrpc: JsonRpcHandler::Version::V2_0, - id: request_id, - method: "tools/list", - }) + response = request(method: "tools/list") response.dig("result", "tools")&.map do |tool| Tool.new( @@ -53,11 +70,7 @@ module MCP # # @return [Array] An array of available resources. def resources - response = transport.send_request(request: { - jsonrpc: JsonRpcHandler::Version::V2_0, - id: request_id, - method: "resources/list", - }) + response = request(method: "resources/list") response.dig("result", "resources") || [] end @@ -67,11 +80,7 @@ module MCP # # @return [Array] An array of available resource templates. def resource_templates - response = transport.send_request(request: { - jsonrpc: JsonRpcHandler::Version::V2_0, - id: request_id, - method: "resources/templates/list", - }) + response = request(method: "resources/templates/list") response.dig("result", "resourceTemplates") || [] end @@ -81,11 +90,7 @@ module MCP # # @return [Array] An array of available prompts. def prompts - response = transport.send_request(request: { - jsonrpc: JsonRpcHandler::Version::V2_0, - id: request_id, - method: "prompts/list", - }) + response = request(method: "prompts/list") response.dig("result", "prompts") || [] end @@ -119,12 +124,7 @@ module MCP params[:_meta] = { progressToken: progress_token } end - transport.send_request(request: { - jsonrpc: JsonRpcHandler::Version::V2_0, - id: request_id, - method: "tools/call", - params: params, - }) + request(method: "tools/call", params: params) end # Reads a resource from the server by URI and returns the contents. @@ -132,12 +132,7 @@ module MCP # @param uri [String] The URI of the resource to read. # @return [Array] An array of resource contents (text or blob). def read_resource(uri:) - response = transport.send_request(request: { - jsonrpc: JsonRpcHandler::Version::V2_0, - id: request_id, - method: "resources/read", - params: { uri: uri }, - }) + response = request(method: "resources/read", params: { uri: uri }) response.dig("result", "contents") || [] end @@ -147,31 +142,34 @@ module MCP # @param name [String] The name of the prompt to get. # @return [Hash] A hash containing the prompt details. def get_prompt(name:) - response = transport.send_request(request: { - jsonrpc: JsonRpcHandler::Version::V2_0, - id: request_id, - method: "prompts/get", - params: { name: name }, - }) + response = request(method: "prompts/get", params: { name: name }) response.fetch("result", {}) end private + def request(method:, params: nil) + request_body = { + jsonrpc: JsonRpcHandler::Version::V2_0, + id: request_id, + method: method, + } + request_body[:params] = params if params + + response = transport.send_request(request: request_body) + + # Guard with `is_a?(Hash)` because custom transports may return non-Hash values. + if response.is_a?(Hash) && response.key?("error") + error = response["error"] + raise ServerError.new(error["message"], code: error["code"], data: error["data"]) + end + + response + end + def request_id SecureRandom.uuid end - - class RequestHandlerError < StandardError - attr_reader :error_type, :original_error, :request - - def initialize(message, request, error_type: :internal_error, original_error: nil) - super(message) - @request = request - @error_type = error_type - @original_error = original_error - end - end end end diff --git a/test/mcp/client_test.rb b/test/mcp/client_test.rb index bf83542..5495b15 100644 --- a/test/mcp/client_test.rb +++ b/test/mcp/client_test.rb @@ -369,5 +369,92 @@ module MCP client = Client.new(transport: transport) client.call_tool(tool: tool, arguments: arguments) end + + def test_tools_raises_server_error_on_error_response + transport = mock + mock_response = { "error" => { "code" => -32_601, "message" => "Method not found" } } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + error = assert_raises(Client::ServerError) { client.tools } + assert_equal(-32_601, error.code) + assert_equal("Method not found", error.message) + end + + def test_resources_raises_server_error_on_error_response + transport = mock + mock_response = { "error" => { "code" => -32_602, "message" => "Invalid params" } } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + error = assert_raises(Client::ServerError) { client.resources } + assert_equal(-32_602, error.code) + end + + def test_read_resource_raises_server_error_on_error_response + transport = mock + mock_response = { "error" => { "code" => -32_602, "message" => "Resource not found" } } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + assert_raises(Client::ServerError) { client.read_resource(uri: "file:///missing") } + end + + def test_get_prompt_raises_server_error_on_error_response + transport = mock + mock_response = { "error" => { "code" => -32_602, "message" => "Prompt not found" } } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + assert_raises(Client::ServerError) { client.get_prompt(name: "missing") } + end + + def test_prompts_raises_server_error_on_error_response + transport = mock + mock_response = { "error" => { "code" => -32_601, "message" => "Method not found" } } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + assert_raises(Client::ServerError) { client.prompts } + end + + def test_resource_templates_raises_server_error_on_error_response + transport = mock + mock_response = { "error" => { "code" => -32_601, "message" => "Method not found" } } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + assert_raises(Client::ServerError) { client.resource_templates } + end + + def test_call_tool_raises_server_error_on_error_response + transport = mock + mock_response = { "error" => { "code" => -32_602, "message" => "Tool not found" } } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + error = assert_raises(Client::ServerError) { client.call_tool(name: "missing") } + assert_equal(-32_602, error.code) + end + + def test_server_error_includes_data_field + transport = mock + mock_response = { + "error" => { "code" => -32_603, "message" => "Internal error", "data" => "extra details" }, + } + + transport.expects(:send_request).returns(mock_response).once + + client = Client.new(transport: transport) + error = assert_raises(Client::ServerError) { client.tools } + assert_equal("extra details", error.data) + end end end