From 16a82a04484f76947f755f5efb7ae11e54e0104e Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 19 Oct 2026 17:26:07 +0900 Subject: [PATCH] Keep the stdio server running when a reply cannot be encoded ## Motivation and Context `StdioTransport#open` generates each reply with `JSON.generate` and nothing rescued what that raised, so a reply holding a string that is not valid UTF-8 ended the read loop and the server process with `JSON::GeneratorError`. A request can carry such a string without being malformed: the json gem's 2.x releases, which Ruby 3.3, 3.4 and 4.0 ship as their default, decode an unpaired low surrogate escape such as `\udfff` into bytes that are not valid UTF-8, and a JavaScript client writes one whenever a string was cut in the middle of a surrogate pair. Any reply repeating it then fails: the `Tool not found` error data for such a tool name, a resource read echoing its URI, or a tool echoing an argument. The json gem's 3.x releases refuse the escape as a parse error instead. The Python server refuses it at parse time as well, and the TypeScript server writes it back as an escape, so neither stops on it. The frame handling of `#open` now rescues a failure past the server's own error handling, reports it to the exception reporter, and answers the frame with `-32603` Internal error, carrying the request id when it can be written back, as for any other request the server cannot complete; the loop goes on with the next frame. A notification still gets no response. The parsed frame is reset for each frame, so a frame whose parse fails this way is not answered with the id of the frame before it. ## How Has This Been Tested? `test/mcp/server/transports/stdio_transport_test.rb`: a tool whose reply cannot be encoded, as a single request and in a batch, is answered with Internal error and the next request is served. Against the previous library both raise `JSON::GeneratorError` out of `#open`. A frame whose parse raises past the parser's own error, after a request and after a notification, is answered without an id; against the previous library it raises out of `#open`. ## Breaking Changes None. --- lib/mcp/server/transports/stdio_transport.rb | 32 +++++++- .../server/transports/stdio_transport_test.rb | 82 +++++++++++++++++++ 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/lib/mcp/server/transports/stdio_transport.rb b/lib/mcp/server/transports/stdio_transport.rb index 37d33fb3..3ad5dce5 100644 --- a/lib/mcp/server/transports/stdio_transport.rb +++ b/lib/mcp/server/transports/stdio_transport.rb @@ -50,9 +50,25 @@ def open break if line.nil? line = line.strip - parsed = parse_line(line) - response = parsed ? dispatch_with_era(parsed) : @session.handle_json(line) - send_response(response) if response + + # Reset for each frame: the loop body shares one scope, so a frame whose parse raises would otherwise + # be answered with the previous frame's id. + parsed = nil + + begin + parsed = parse_line(line) + response = parsed ? dispatch_with_era(parsed) : @session.handle_json(line) + + send_response(response) if response + rescue StandardError => e + # One frame must not end the connection: a reply that cannot be generated (a request string + # the JSON parser passed through without being valid UTF-8, echoed by the SDK or a handler) or + # any other failure past the server's own error handling answers that frame with Internal error, + # unless the frame is a notification, which gets no response. + MCP.configuration.exception_reporter.call(e, { error: "Failed to handle stdio frame" }) + + send_response(internal_error_response(parsed)) unless parsed.is_a?(Hash) && !parsed.key?(:id) + end end rescue Interrupt warn("\nExiting...") @@ -70,6 +86,16 @@ def send_response(message) $stdout.flush end + # The error answering a frame whose handling failed. It carries the request id when the frame was + # a single request whose id can be written back, and no `id` otherwise, as for an id that could not be read. + def internal_error_response(parsed) + response = { jsonrpc: JsonRpcHandler::Version::V2_0 } + id = parsed[:id] if parsed.is_a?(Hash) + response[:id] = id if id.is_a?(Integer) || (id.is_a?(String) && id.valid_encoding?) + response[:error] = { code: JsonRpcHandler::ErrorCode::INTERNAL_ERROR, message: "Internal error" } + response + end + def send_notification(method, params = nil) notification = { jsonrpc: "2.0", diff --git a/test/mcp/server/transports/stdio_transport_test.rb b/test/mcp/server/transports/stdio_transport_test.rb index 86fbb2c0..f2419668 100644 --- a/test/mcp/server/transports/stdio_transport_test.rb +++ b/test/mcp/server/transports/stdio_transport_test.rb @@ -547,6 +547,88 @@ class StdioTransportTest < ActiveSupport::TestCase end end + test "open answers a request whose reply cannot be encoded with Internal error and keeps serving" do + reported = [] + original_reporter = MCP.configuration.exception_reporter + MCP.configuration.exception_reporter = ->(e, _context) { reported << e.class } + 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) }]) + end + @transport = StdioTransport.new(server) + + responses = run_transport_session([ + initialize_request(id: 1), + { jsonrpc: "2.0", method: "notifications/initialized" }, + { jsonrpc: "2.0", id: 2, method: "tools/call", params: { name: "invalid_text", arguments: {} } }, + { jsonrpc: "2.0", id: 3, method: "ping" }, + ]) + + assert_equal( + { 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]) + assert_equal([JSON::GeneratorError], reported) + ensure + MCP.configuration.exception_reporter = original_reporter + end + + test "open answers a frame whose parse fails unexpectedly without the id of the previous frame" do + # The parser raises `EncodingError` rather than `JSON::ParserError` for a key that is not valid UTF-8. + # The frames are read as bytes, so the key reaches the parser whatever `String#strip` does. + bad = "{\"jsonrpc\":\"2.0\",\"id\":9,\"method\":\"ping\",\"params\":{\"k\xFF\":1}}" + frames = [ + JSON.generate(initialize_request(id: 1)), + JSON.generate({ jsonrpc: "2.0", method: "notifications/initialized" }), + JSON.generate({ jsonrpc: "2.0", id: 3, method: "ping" }), + bad, + JSON.generate({ jsonrpc: "2.0", method: "notifications/initialized" }), + bad, + JSON.generate({ jsonrpc: "2.0", id: 4, method: "ping" }), + ] + input = StringIO.new((frames.join("\n") + "\n").b) + output = StringIO.new + original_stdin = $stdin + original_stdout = $stdout + + begin + $stdin = input + $stdout = output + @transport.open + ensure + $stdin = original_stdin + $stdout = original_stdout + end + + # Each bad frame gets one error without an id, whichever code it carries: Internal error here, or the parse + # error once the parser's `EncodingError` is answered as one. + responses = output.string.each_line.map { |line| JSON.parse(line, symbolize_names: true) }.drop(1) + assert_equal([3, nil, nil, 4], responses.map { |response| response[:id] }) + 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 + 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) }]) + end + @transport = StdioTransport.new(server) + + responses = run_transport_session([ + initialize_request(id: 1), + { jsonrpc: "2.0", method: "notifications/initialized" }, + [{ jsonrpc: "2.0", id: 2, method: "tools/call", params: { name: "invalid_text", arguments: {} } }], + { jsonrpc: "2.0", id: 3, method: "ping" }, + ]) + + assert_equal( + { jsonrpc: "2.0", error: { code: JsonRpcHandler::ErrorCode::INTERNAL_ERROR, message: "Internal error" } }, + responses[1], + ) + assert_equal({ jsonrpc: "2.0", id: 3, result: {} }, responses[2]) + end + test "send_request raises when a response frame exceeds max_line_bytes without a newline" do transport = StdioTransport.new(@server, max_line_bytes: 64) input = StringIO.new("A" * 200) # No newline: an unbounded frame.