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 | 94 ++++++++++++++++++++--------------------- test/mcp/client_test.rb | 87 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 48 deletions(-) diff --git a/lib/mcp/client.rb b/lib/mcp/client.rb index 6437dd68..7c89bcb1 100644 --- a/lib/mcp/client.rb +++ b/lib/mcp/client.rb @@ -6,6 +6,27 @@ 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 @@ def initialize(transport:) # 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 @@ def tools # # @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 @@ def resources # # @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 @@ def resource_templates # # @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 @@ def call_tool(name: nil, tool: nil, arguments: nil, progress_token: nil) 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 @@ def call_tool(name: nil, tool: nil, arguments: nil, progress_token: nil) # @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 @@ def read_resource(uri:) # @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_id - SecureRandom.uuid - end + def request(method:, params: nil) + request_body = { + jsonrpc: JsonRpcHandler::Version::V2_0, + id: request_id, + method: method, + } + request_body[:params] = params if params - class RequestHandlerError < StandardError - attr_reader :error_type, :original_error, :request + response = transport.send_request(request: request_body) - def initialize(message, request, error_type: :internal_error, original_error: nil) - super(message) - @request = request - @error_type = error_type - @original_error = original_error + # 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 end end diff --git a/test/mcp/client_test.rb b/test/mcp/client_test.rb index bf83542e..5495b15f 100644 --- a/test/mcp/client_test.rb +++ b/test/mcp/client_test.rb @@ -369,5 +369,92 @@ def test_call_tool_omits_meta_when_no_progress_token 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