From 93787f0b24c4dd7e6229ebabb58867a2e1c30724 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 22 Oct 2026 22:24:28 +0700 Subject: [PATCH] Answer an HTTP request whose reply cannot be generated with Internal error ## Motivation and Context A response can hold a string that is not valid UTF-8 when the request carried one and the SDK or a handler repeated it, for example a tool name in its `Tool not found` error, since the JSON parser passes such bytes in a string value through. Generating that response raised `JSON::GeneratorError`. On the Streamable HTTP legacy path the error escaped while the SSE body was being written, cutting the response off; in JSON response mode and on the 2026-07-28 path the transport's rescue answered 500 with the error reported, for a failure that belongs to the one request. `JsonRpcHandler.handle_json` now answers such a request with `-32603` Internal error carrying the request id, and the 2026-07-28 path does the same before it writes the response. The error carries the request id when the response has one. In a batch each item that cannot be generated is replaced on its own, so the other items keep their results. The failure is passed to the exception reporter, since the Internal error sent to the client no longer shows what went wrong. Over stdio a batch is now answered by `handle_json` as well, with the request id, ahead of the transport's own fallback, which answered it without one. ## How Has This Been Tested? `test/json_rpc_handler_test.rb` and `test/mcp/server/transports/streamable_http_transport_test.rb` (the legacy SSE path, JSON response mode, and the 2026-07-28 path, each with a tool whose reply cannot be generated). Against the previous library the handler test raises `JSON::GeneratorError`, the SSE body raises it while it is written, and the other two answer 500. The handler test for a batch in which one item cannot be generated fails against the previous library, which has no `on_generate_error:`. The transport tests check that the failure reaches the exception reporter. The stdio batch test expects the item's id accordingly. ## Breaking Changes None. --- lib/json_rpc_handler.rb | 34 +++++++- lib/mcp/server.rb | 3 +- .../transports/streamable_http_transport.rb | 16 +++- test/json_rpc_handler_test.rb | 32 +++++++ .../server/transports/stdio_transport_test.rb | 4 +- .../streamable_http_transport_test.rb | 83 +++++++++++++++++++ 6 files changed, 164 insertions(+), 8 deletions(-) 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", {})))