diff --git a/lib/json_rpc_handler.rb b/lib/json_rpc_handler.rb index 4d59f877..3ce87a96 100644 --- a/lib/json_rpc_handler.rb +++ b/lib/json_rpc_handler.rb @@ -53,7 +53,9 @@ def handle(request, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &metho end end - def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &method_finder) + # `on_generate_error`, when given, is called with the `JSON::GeneratorError` of a response that cannot be generated, + # before that response is replaced with Internal error. + def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, on_generate_error: nil, &method_finder) begin begin request = JSON.parse(request_json, symbolize_names: true) @@ -77,7 +79,35 @@ def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTE }) end - response&.to_json + begin + response&.to_json + rescue JSON::GeneratorError => e + on_generate_error&.call(e) + encodable_response(response, id_validation_pattern: id_validation_pattern).to_json + end + end + + # `response` with each part JSON cannot generate replaced with Internal error. A batch is replaced item by item, + # so the other items keep their results. + def encodable_response(response, id_validation_pattern: nil) + return response.map { |item| encodable_response(item, id_validation_pattern: id_validation_pattern) } if response.is_a?(Array) + + response.to_json + response + rescue JSON::GeneratorError + internal_error_for(response, id_validation_pattern: id_validation_pattern) + end + + # The error answering a request whose response cannot be generated: a result or an error holding a string that + # is not valid UTF-8, typically one the request carried and the SDK or a handler repeated, such as a tool name + # in its Tool not found error. It carries the request id when the response has one, which was validated before + # the method ran. + def internal_error_for(response, id_validation_pattern: nil) + id = response[:id] if response.is_a?(Hash) + error_response(id: id.nil? ? :unknown_id : id, id_validation_pattern: id_validation_pattern, error: { + code: ErrorCode::INTERNAL_ERROR, + message: "Internal error", + }) end def process_request(request, id_validation_pattern:, &method_finder) diff --git a/lib/mcp/server.rb b/lib/mcp/server.rb index dbb58035..53f99d8a 100644 --- a/lib/mcp/server.rb +++ b/lib/mcp/server.rb @@ -303,7 +303,8 @@ def handle(request, session: nil, auth_info: nil) # for the current request, as in `handle`. # @return [String, nil] The JSON-RPC response as JSON, or `nil` for notifications. def handle_json(request, session: nil, auth_info: nil) - JsonRpcHandler.handle_json(request, id_validation_pattern: nil) do |method, request_id| + on_generate_error = ->(error) { report_exception(error, { error: "Failed to generate response" }) } + JsonRpcHandler.handle_json(request, id_validation_pattern: nil, on_generate_error: on_generate_error) do |method, request_id| handle_request(request, method, session: session, related_request_id: request_id, auth_info: auth_info) end end diff --git a/lib/mcp/server/transports/streamable_http_transport.rb b/lib/mcp/server/transports/streamable_http_transport.rb index cc91f585..37b71760 100644 --- a/lib/mcp/server/transports/streamable_http_transport.rb +++ b/lib/mcp/server/transports/streamable_http_transport.rb @@ -931,16 +931,26 @@ def handle_modern(request, header_version, body_string: nil) # `nil` covers notifications and cancellation-suppressed responses; ack with 202 like the legacy notification path. return handle_accepted if response.nil? + # A response that cannot be generated answers its request with Internal error, as `JsonRpcHandler.handle_json` + # does on the legacy paths, instead of failing the whole exchange with a 500. + response_json = begin + response.to_json + rescue JSON::GeneratorError => e + MCP.configuration.exception_reporter.call(e, { error: "Failed to generate response" }) + response = JsonRpcHandler.encodable_response(response) + response.to_json + end + # Notifications a handler emitted during the request ride the request's own response stream as SSE frames ahead of # the final response (SEP-2575); a request that emitted none keeps the single JSON exchange and its HTTP status ladder. # Delivery is buffered: frames flush after the handler returns, preserving order but not real-time interleaving # (the TypeScript and Python SDKs stream live), and the sink grows with the handler's notification count. # Live streaming can follow without changing the wire shape. if notifications.empty? - [modern_http_status(response), { "content-type" => "application/json" }, [response.to_json]] + [modern_http_status(response), { "content-type" => "application/json" }, [response_json]] else - frames = notifications + [response] - sse_body = frames.map { |frame| "event: message\ndata: #{frame.to_json}\n\n" }.join + frames = notifications.map(&:to_json) + [response_json] + sse_body = frames.map { |frame| "event: message\ndata: #{frame}\n\n" }.join [200, SSE_HEADERS.dup, [sse_body]] end rescue StandardError => e diff --git a/test/json_rpc_handler_test.rb b/test/json_rpc_handler_test.rb index 7bbc5136..994bb72c 100644 --- a/test/json_rpc_handler_test.rb +++ b/test/json_rpc_handler_test.rb @@ -488,6 +488,38 @@ } end + it "returns an error with the code set to -32603 and the request id when the result cannot be generated" do + register("echo") { |_params| { text: "\xFF".dup.force_encoding(Encoding::UTF_8) } } + + handle_json({ jsonrpc: "2.0", id: 1, method: "echo" }.to_json) + + assert_equal(1, @response[:id]) + assert_rpc_error expected_error: { + code: -32603, + message: "Internal error", + } + end + + it "replaces only the batch item whose result cannot be generated and reports the error" do + register("echo") { |_params| { text: "\xFF".dup.force_encoding(Encoding::UTF_8) } } + register("ping") { |_params| {} } + errors = [] + + response_json = JsonRpcHandler.handle_json( + [{ jsonrpc: "2.0", id: 1, method: "echo" }, { jsonrpc: "2.0", id: 2, method: "ping" }].to_json, + on_generate_error: ->(error) { errors << error.class }, + ) { |method_name, _request_id| @registry[method_name] } + + assert_equal( + [ + { jsonrpc: "2.0", id: 1, error: { code: -32603, message: "Internal error" } }, + { jsonrpc: "2.0", id: 2, result: {} }, + ], + JSON.parse(response_json, symbolize_names: true), + ) + assert_equal([JSON::GeneratorError], errors) + end + it "returns nil when the method does not exist and the id is nil" do handle jsonrpc: "2.0", method: "add", params: { a: 1, b: 2 } diff --git a/test/mcp/server/transports/stdio_transport_test.rb b/test/mcp/server/transports/stdio_transport_test.rb index 3c7ab151..1aa01bdf 100644 --- a/test/mcp/server/transports/stdio_transport_test.rb +++ b/test/mcp/server/transports/stdio_transport_test.rb @@ -735,7 +735,7 @@ class StdioTransportTest < ActiveSupport::TestCase assert_equal([false, true, true, false], responses.map { |response| response.key?(:error) }) end - test "open answers a batch whose reply cannot be encoded with Internal error without an id" do + test "open answers a batch whose reply cannot be encoded with Internal error and the item's id" do server = Server.new(name: "test_server") server.define_tool(name: "invalid_text") do Tool::Response.new([{ type: "text", text: "\xFF".dup.force_encoding(Encoding::UTF_8) }]) @@ -750,7 +750,7 @@ class StdioTransportTest < ActiveSupport::TestCase ]) assert_equal( - { jsonrpc: "2.0", error: { code: JsonRpcHandler::ErrorCode::INTERNAL_ERROR, message: "Internal error" } }, + { jsonrpc: "2.0", id: 2, error: { code: JsonRpcHandler::ErrorCode::INTERNAL_ERROR, message: "Internal error" } }, responses[1], ) assert_equal({ jsonrpc: "2.0", id: 3, result: {} }, responses[2]) diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index 6807ebcd..e1c78348 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -2792,6 +2792,68 @@ def string assert_equal "456", body["id"] end + test "a reply that cannot be generated is answered with Internal error on the legacy SSE path" do + reported = [] + configuration = MCP::Configuration.new + configuration.exception_reporter = ->(e, context) { reported << [e.class, context] } + @server = Server.new(name: "test_server", configuration: configuration) + @server.define_tool(name: "invalid_text") do + Tool::Response.new([{ type: "text", text: "\xFF".dup.force_encoding(Encoding::UTF_8) }]) + end + @transport.close + @transport = StreamableHTTPTransport.new(@server, listen_keepalive_interval: nil) + session_id = initialize_test_session + + response = @transport.handle_request(create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + { jsonrpc: "2.0", id: 2, method: "tools/call", params: { name: "invalid_text", arguments: {} } }.to_json, + )) + io = StringIO.new + response[2].call(io) + + assert_equal 200, response[0] + assert_equal( + { "jsonrpc" => "2.0", "id" => 2, "error" => { "code" => -32603, "message" => "Internal error" } }, + JSON.parse(io.string[/^data: (.+)$/, 1]), + ) + assert_equal([[JSON::GeneratorError, { error: "Failed to generate response" }]], reported) + end + + test "a reply that cannot be generated is answered with Internal error in JSON response mode" do + reported = [] + configuration = MCP::Configuration.new + configuration.exception_reporter = ->(e, context) { reported << [e.class, context] } + server = Server.new(name: "test", configuration: configuration) + server.define_tool(name: "invalid_text") do + Tool::Response.new([{ type: "text", text: "\xFF".dup.force_encoding(Encoding::UTF_8) }]) + end + transport = StreamableHTTPTransport.new(server, enable_json_response: true) + session_id = transport.handle_request(create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json" }, + { jsonrpc: "2.0", method: "initialize", id: "init", params: initialize_params }.to_json, + ))[1]["mcp-session-id"] + + response = transport.handle_request(create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + { jsonrpc: "2.0", id: 2, method: "tools/call", params: { name: "invalid_text", arguments: {} } }.to_json, + )) + + assert_equal 200, response[0] + assert_equal( + { "jsonrpc" => "2.0", "id" => 2, "error" => { "code" => -32603, "message" => "Internal error" } }, + JSON.parse(response[2][0]), + ) + assert_equal([[JSON::GeneratorError, { error: "Failed to generate response" }]], reported) + ensure + transport.close + end + test "JSON response mode returns application/json for POST requests" do server = Server.new(name: "test", tools: [], prompts: [], resources: []) transport = StreamableHTTPTransport.new(server, enable_json_response: true) @@ -6280,6 +6342,27 @@ def string assert_equal 405, @transport.handle_request(delete_request)[0] end + test "modern POST answers a reply that cannot be generated with Internal error" do + reported = [] + original_reporter = MCP.configuration.exception_reporter + MCP.configuration.exception_reporter = ->(e, context) { reported << [e.class, context] } + @server.define_tool(name: "invalid_text") do + Tool::Response.new([{ type: "text", text: "\xFF".dup.force_encoding(Encoding::UTF_8) }]) + end + + response = @transport.handle_request(modern_rack_request( + modern_body("tools/call", { name: "invalid_text", arguments: {} }), + )) + + assert_equal 200, response[0] + body = JSON.parse(response[2][0]) + assert_equal 1, body["id"] + assert_equal({ "code" => -32603, "message" => "Internal error" }, body["error"]) + assert_equal([[JSON::GeneratorError, { error: "Failed to generate response" }]], reported) + ensure + MCP.configuration.exception_reporter = original_reporter + end + test "modern POST maps an unknown method to 404 with -32601" do response = @transport.handle_request(modern_rack_request(modern_body("no/such_method", {})))