fix: return protocol errors for invalid arguments and server errors
Per the MCP spec, invalid arguments and server errors are protocol errors (JSON-RPC error responses), not tool execution errors (isError: true). PR #165 moved all error cases to isError: true, and PR #231 only restored protocol errors for unknown tools. This fixes the remaining two cases: invalid arguments (-32602) and server errors (-32603). Ref: https://modelcontextprotocol.io/specification/2024-11-05/server/tools#error-handling
This commit is contained in:
parent
56319902dd
commit
8dd0379f16
3 changed files with 59 additions and 59 deletions
|
|
@ -316,13 +316,12 @@ module MCP
|
||||||
add_instrumentation_data(client: client) if client
|
add_instrumentation_data(client: client) if client
|
||||||
|
|
||||||
result
|
result
|
||||||
|
rescue RequestHandlerError => e
|
||||||
|
report_exception(e.original_error || e, { request: request })
|
||||||
|
add_instrumentation_data(error: e.error_type)
|
||||||
|
raise e
|
||||||
rescue => e
|
rescue => e
|
||||||
report_exception(e, { request: request })
|
report_exception(e, { request: request })
|
||||||
if e.is_a?(RequestHandlerError)
|
|
||||||
add_instrumentation_data(error: e.error_type)
|
|
||||||
raise e
|
|
||||||
end
|
|
||||||
|
|
||||||
add_instrumentation_data(error: :internal_error)
|
add_instrumentation_data(error: :internal_error)
|
||||||
raise RequestHandlerError.new("Internal error handling #{method} request", request, original_error: e)
|
raise RequestHandlerError.new("Internal error handling #{method} request", request, original_error: e)
|
||||||
end
|
end
|
||||||
|
|
@ -421,7 +420,7 @@ module MCP
|
||||||
add_instrumentation_data(error: :missing_required_arguments)
|
add_instrumentation_data(error: :missing_required_arguments)
|
||||||
|
|
||||||
missing = tool.input_schema.missing_required_arguments(arguments).join(", ")
|
missing = tool.input_schema.missing_required_arguments(arguments).join(", ")
|
||||||
return error_tool_response("Missing required arguments: #{missing}")
|
raise RequestHandlerError.new("Missing required arguments: #{missing}", request, error_type: :invalid_params)
|
||||||
end
|
end
|
||||||
|
|
||||||
if configuration.validate_tool_call_arguments && tool.input_schema
|
if configuration.validate_tool_call_arguments && tool.input_schema
|
||||||
|
|
@ -430,7 +429,7 @@ module MCP
|
||||||
rescue Tool::InputSchema::ValidationError => e
|
rescue Tool::InputSchema::ValidationError => e
|
||||||
add_instrumentation_data(error: :invalid_schema)
|
add_instrumentation_data(error: :invalid_schema)
|
||||||
|
|
||||||
return error_tool_response(e.message)
|
raise RequestHandlerError.new(e.message, request, error_type: :invalid_params)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -440,9 +439,12 @@ module MCP
|
||||||
rescue RequestHandlerError
|
rescue RequestHandlerError
|
||||||
raise
|
raise
|
||||||
rescue => e
|
rescue => e
|
||||||
report_exception(e, request: request)
|
raise RequestHandlerError.new(
|
||||||
|
"Internal error calling tool #{tool_name}: #{e.message}",
|
||||||
error_tool_response("Internal error calling tool #{tool_name}: #{e.message}")
|
request,
|
||||||
|
error_type: :internal_error,
|
||||||
|
original_error: e,
|
||||||
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
def list_prompts(request)
|
def list_prompts(request)
|
||||||
|
|
|
||||||
|
|
@ -199,7 +199,7 @@ module MCP
|
||||||
response[:result][:content][0][:content]
|
response[:result][:content][0][:content]
|
||||||
end
|
end
|
||||||
|
|
||||||
test "tool with required server_context fails when server has no context" do
|
test "tool with required server_context returns protocol error in JSON-RPC format when server has no context" do
|
||||||
server_no_context = Server.new(
|
server_no_context = Server.new(
|
||||||
name: "test_server_no_context",
|
name: "test_server_no_context",
|
||||||
tools: [ToolWithRequiredContext],
|
tools: [ToolWithRequiredContext],
|
||||||
|
|
@ -217,10 +217,10 @@ module MCP
|
||||||
|
|
||||||
response = server_no_context.handle(request)
|
response = server_no_context.handle(request)
|
||||||
|
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32603, response[:error][:code])
|
||||||
assert_equal "text", response[:result][:content][0][:type]
|
assert_equal "Internal error", response[:error][:message]
|
||||||
assert_match(/Internal error calling tool tool_with_required_context: /, response[:result][:content][0][:text])
|
assert_match(/Internal error calling tool tool_with_required_context: /, response[:error][:data])
|
||||||
end
|
end
|
||||||
|
|
||||||
test "call_tool_with_args correctly detects server_context parameter presence" do
|
test "call_tool_with_args correctly detects server_context parameter presence" do
|
||||||
|
|
|
||||||
|
|
@ -312,7 +312,7 @@ module MCP
|
||||||
assert_instrumentation_data({ method: "tools/call", tool_name: tool_name, tool_arguments: tool_args })
|
assert_instrumentation_data({ method: "tools/call", tool_name: tool_name, tool_arguments: tool_args })
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle tools/call returns error response with isError true if required tool arguments are missing" do
|
test "#handle tools/call returns protocol error in JSON-RPC format if required tool arguments are missing" do
|
||||||
tool_with_required_argument = Tool.define(
|
tool_with_required_argument = Tool.define(
|
||||||
name: "test_tool",
|
name: "test_tool",
|
||||||
title: "Test tool",
|
title: "Test tool",
|
||||||
|
|
@ -336,10 +336,10 @@ module MCP
|
||||||
|
|
||||||
response = server.handle(request)
|
response = server.handle(request)
|
||||||
|
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32602, response[:error][:code])
|
||||||
assert_equal "text", response[:result][:content][0][:type]
|
assert_equal "Invalid params", response[:error][:message]
|
||||||
assert_equal "Missing required arguments: message", response[:result][:content][0][:text]
|
assert_includes response[:error][:data], "Missing required arguments: message"
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle_json tools/call executes tool and returns result" do
|
test "#handle_json tools/call executes tool and returns result" do
|
||||||
|
|
@ -407,17 +407,7 @@ module MCP
|
||||||
assert_equal({ content: [{ type: "text", content: "OK" }], isError: false }, response[:result])
|
assert_equal({ content: [{ type: "text", content: "OK" }], isError: false }, response[:result])
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle tools/call returns error response with isError true if the tool raises an error" do
|
test "#handle tools/call returns protocol error in JSON-RPC format if the tool raises an uncaught exception" do
|
||||||
@server.configuration.exception_reporter.expects(:call).with do |exception, server_context|
|
|
||||||
assert_not_nil exception
|
|
||||||
assert_equal(
|
|
||||||
{
|
|
||||||
request: { name: "tool_that_raises", arguments: { message: "test" } },
|
|
||||||
},
|
|
||||||
server_context,
|
|
||||||
)
|
|
||||||
end
|
|
||||||
|
|
||||||
request = {
|
request = {
|
||||||
jsonrpc: "2.0",
|
jsonrpc: "2.0",
|
||||||
method: "tools/call",
|
method: "tools/call",
|
||||||
|
|
@ -428,13 +418,18 @@ module MCP
|
||||||
id: 1,
|
id: 1,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@server.configuration.exception_reporter.expects(:call).with do |exception, server_context|
|
||||||
|
refute_kind_of MCP::Server::RequestHandlerError, exception
|
||||||
|
assert_equal({ request: request }, server_context)
|
||||||
|
end
|
||||||
|
|
||||||
response = @server.handle(request)
|
response = @server.handle(request)
|
||||||
|
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32603, response[:error][:code])
|
||||||
assert_equal "text", response[:result][:content][0][:type]
|
assert_equal "Internal error", response[:error][:message]
|
||||||
assert_match(/Internal error calling tool tool_that_raises: /, response[:result][:content][0][:text])
|
assert_match(/Internal error calling tool tool_that_raises: /, response[:error][:data])
|
||||||
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", tool_arguments: { message: "test" } })
|
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", tool_arguments: { message: "test" }, error: :internal_error })
|
||||||
end
|
end
|
||||||
|
|
||||||
test "registers tools with the same class name in different namespaces" do
|
test "registers tools with the same class name in different namespaces" do
|
||||||
|
|
@ -475,7 +470,7 @@ module MCP
|
||||||
MESSAGE
|
MESSAGE
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle_json returns error response with isError true if the tool raises an error" do
|
test "#handle_json returns protocol error in JSON-RPC format if the tool raises an uncaught exception" do
|
||||||
request = JSON.generate({
|
request = JSON.generate({
|
||||||
jsonrpc: "2.0",
|
jsonrpc: "2.0",
|
||||||
method: "tools/call",
|
method: "tools/call",
|
||||||
|
|
@ -487,14 +482,14 @@ module MCP
|
||||||
})
|
})
|
||||||
|
|
||||||
response = JSON.parse(@server.handle_json(request), symbolize_names: true)
|
response = JSON.parse(@server.handle_json(request), symbolize_names: true)
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32603, response[:error][:code])
|
||||||
assert_equal "text", response[:result][:content][0][:type]
|
assert_equal "Internal error", response[:error][:message]
|
||||||
assert_match(/Internal error calling tool tool_that_raises: /, response[:result][:content][0][:text])
|
assert_match(/Internal error calling tool tool_that_raises: /, response[:error][:data])
|
||||||
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", tool_arguments: { message: "test" } })
|
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises", tool_arguments: { message: "test" }, error: :internal_error })
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle tools/call returns error response with isError true if input_schema raises an error during validation" do
|
test "#handle tools/call returns protocol error in JSON-RPC format if input_schema raises an error during validation" do
|
||||||
tool = Tool.define(
|
tool = Tool.define(
|
||||||
name: "tool_with_faulty_schema",
|
name: "tool_with_faulty_schema",
|
||||||
title: "Tool with faulty schema",
|
title: "Tool with faulty schema",
|
||||||
|
|
@ -518,10 +513,10 @@ module MCP
|
||||||
|
|
||||||
response = server.handle(request)
|
response = server.handle(request)
|
||||||
|
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32603, response[:error][:code])
|
||||||
assert_equal "text", response[:result][:content][0][:type]
|
assert_equal "Internal error", response[:error][:message]
|
||||||
assert_match(/Internal error calling tool tool_with_faulty_schema: Unexpected schema error/, response[:result][:content][0][:text])
|
assert_match(/Internal error calling tool tool_with_faulty_schema: Unexpected schema error/, response[:error][:data])
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle tools/call returns JSON-RPC error for unknown tool" do
|
test "#handle tools/call returns JSON-RPC error for unknown tool" do
|
||||||
|
|
@ -1310,7 +1305,7 @@ module MCP
|
||||||
refute response[:result].key?(:instructions)
|
refute response[:result].key?(:instructions)
|
||||||
end
|
end
|
||||||
|
|
||||||
test "tools/call handles missing arguments field" do
|
test "tools/call returns protocol error in JSON-RPC format for missing arguments" do
|
||||||
server = Server.new(
|
server = Server.new(
|
||||||
tools: [TestTool],
|
tools: [TestTool],
|
||||||
configuration: Configuration.new(validate_tool_call_arguments: true),
|
configuration: Configuration.new(validate_tool_call_arguments: true),
|
||||||
|
|
@ -1329,12 +1324,13 @@ module MCP
|
||||||
|
|
||||||
assert_equal "2.0", response[:jsonrpc]
|
assert_equal "2.0", response[:jsonrpc]
|
||||||
assert_equal 1, response[:id]
|
assert_equal 1, response[:id]
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32602, response[:error][:code])
|
||||||
assert_includes response[:result][:content][0][:text], "Missing required arguments"
|
assert_equal "Invalid params", response[:error][:message]
|
||||||
|
assert_includes response[:error][:data], "Missing required arguments"
|
||||||
end
|
end
|
||||||
|
|
||||||
test "tools/call validates arguments against input schema when validate_tool_call_arguments is true" do
|
test "tools/call returns protocol error in JSON-RPC format for invalid arguments when validate_tool_call_arguments is true" do
|
||||||
server = Server.new(
|
server = Server.new(
|
||||||
tools: [TestTool],
|
tools: [TestTool],
|
||||||
configuration: Configuration.new(validate_tool_call_arguments: true),
|
configuration: Configuration.new(validate_tool_call_arguments: true),
|
||||||
|
|
@ -1354,9 +1350,10 @@ module MCP
|
||||||
|
|
||||||
assert_equal "2.0", response[:jsonrpc]
|
assert_equal "2.0", response[:jsonrpc]
|
||||||
assert_equal 1, response[:id]
|
assert_equal 1, response[:id]
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32602, response[:error][:code])
|
||||||
assert_includes response[:result][:content][0][:text], "Invalid arguments"
|
assert_equal "Invalid params", response[:error][:message]
|
||||||
|
assert_includes response[:error][:data], "Invalid arguments"
|
||||||
end
|
end
|
||||||
|
|
||||||
test "tools/call skips argument validation when validate_tool_call_arguments is false" do
|
test "tools/call skips argument validation when validate_tool_call_arguments is false" do
|
||||||
|
|
@ -1441,7 +1438,7 @@ module MCP
|
||||||
assert_equal "OK", response[:result][:content][0][:content]
|
assert_equal "OK", response[:result][:content][0][:content]
|
||||||
end
|
end
|
||||||
|
|
||||||
test "tools/call disallows additional properties when additionalProperties set to false" do
|
test "tools/call returns protocol error in JSON-RPC format when additionalProperties set to false" do
|
||||||
server = Server.new(
|
server = Server.new(
|
||||||
tools: [TestToolWithAdditionalPropertiesSetToFalse],
|
tools: [TestToolWithAdditionalPropertiesSetToFalse],
|
||||||
configuration: Configuration.new(validate_tool_call_arguments: true),
|
configuration: Configuration.new(validate_tool_call_arguments: true),
|
||||||
|
|
@ -1464,9 +1461,10 @@ module MCP
|
||||||
|
|
||||||
assert_equal "2.0", response[:jsonrpc]
|
assert_equal "2.0", response[:jsonrpc]
|
||||||
assert_equal 1, response[:id]
|
assert_equal 1, response[:id]
|
||||||
assert_nil response[:error], "Expected no JSON-RPC error"
|
assert_nil response[:result]
|
||||||
assert response[:result][:isError]
|
assert_equal(-32602, response[:error][:code])
|
||||||
assert_includes response[:result][:content][0][:text], "Invalid arguments"
|
assert_equal "Invalid params", response[:error][:message]
|
||||||
|
assert_includes response[:error][:data], "Invalid arguments"
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle completion/complete returns default completion result" do
|
test "#handle completion/complete returns default completion result" do
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue