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`.
This commit is contained in:
parent
03a78f9344
commit
590feb13a5
2 changed files with 133 additions and 48 deletions
|
|
@ -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<Hash>] 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<Hash>] 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<Hash>] 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<Hash>] 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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue