Skip to content

Commit 89772bf

Browse files
committed
Drop a buffered notification that cannot be generated
## Motivation and Context On the 2026-07-28 path the Streamable HTTP transport buffers the notifications a handler emits during a request and writes them as SSE frames ahead of the response once the handler returns. A notification holding a string that is not valid UTF-8, such as log data a tool passed to `notify_log_message`, raised `JSON::GeneratorError` while the frames were written, and the whole exchange was answered with 500, losing the response the handler had produced. The transport now generates each notification when it is buffered. One that cannot be generated is reported to the exception reporter and dropped as non-delivery (`false`), as the buffer already does past its cap, and the other notifications and the response are written as before. ## How Has This Been Tested? `test/mcp/server/transports/streamable_http_transport_test.rb`. Against the previous library the new test answers 500. ## Breaking Changes None.
1 parent 37270d6 commit 89772bf

2 files changed

Lines changed: 52 additions & 3 deletions

File tree

‎lib/mcp/server/transports/streamable_http_transport.rb‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,12 +521,19 @@ def send_notification(method, params = nil, session_id: nil, related_request_id:
521521
# a per-request sink for its ephemeral session and flushes it as SSE frames ahead of the final response.
522522
# Checked before the stateless guard, since modern requests are served in stateless deployments too.
523523
# The sink is bounded; past `MAX_MODERN_REQUEST_NOTIFICATIONS` the notification is dropped as
524-
# non-delivery (`false`), never handed to the legacy paths below.
524+
# non-delivery (`false`), never handed to the legacy paths below. It holds each notification as JSON,
525+
# generated here so that one that cannot be generated is dropped on its own instead of failing
526+
# the whole response when the sink is flushed.
525527
sink = @mutex.synchronize { session_id && @modern_request_sinks[session_id] }
526528
if sink
527529
return false if sink.size >= MAX_MODERN_REQUEST_NOTIFICATIONS
528530

529-
sink << notification
531+
begin
532+
sink << notification.to_json
533+
rescue JSON::GeneratorError => e
534+
MCP.configuration.exception_reporter.call(e, { session_id: session_id, error: "Failed to generate notification" })
535+
return false
536+
end
530537
return true
531538
end
532539

@@ -949,7 +956,7 @@ def handle_modern(request, header_version, body_string: nil)
949956
if notifications.empty?
950957
[modern_http_status(response), { "content-type" => "application/json" }, [response_json]]
951958
else
952-
frames = notifications.map(&:to_json) + [response_json]
959+
frames = notifications + [response_json]
953960
sse_body = frames.map { |frame| "event: message\ndata: #{frame}\n\n" }.join
954961
[200, SSE_HEADERS.dup, [sse_body]]
955962
end

‎test/mcp/server/transports/streamable_http_transport_test.rb‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6561,6 +6561,48 @@ def string
65616561
assert_equal overflow, delivery_results.count { |result| result == false }
65626562
end
65636563

6564+
test "modern POST drops a handler notification that cannot be generated and keeps the response" do
6565+
reported = []
6566+
original_reporter = MCP.configuration.exception_reporter
6567+
MCP.configuration.exception_reporter = ->(e, context) { reported << [e.class, context[:error]] }
6568+
delivery_results = []
6569+
@server.define_tool(name: "logging_tool") do |server_context:|
6570+
delivery_results << server_context.notify_log_message(data: "\xFF".dup.force_encoding(Encoding::UTF_8), level: "info")
6571+
delivery_results << server_context.notify_log_message(data: "hi", level: "info")
6572+
Tool::Response.new([{ type: "text", text: "done" }])
6573+
end
6574+
6575+
body = {
6576+
jsonrpc: "2.0",
6577+
method: "tools/call",
6578+
id: 1,
6579+
params: {
6580+
name: "logging_tool",
6581+
arguments: {},
6582+
_meta: {
6583+
"io.modelcontextprotocol/protocolVersion": "2026-07-28",
6584+
"io.modelcontextprotocol/clientInfo": { name: "modern_client", version: "2.0" },
6585+
"io.modelcontextprotocol/clientCapabilities": {},
6586+
"io.modelcontextprotocol/logLevel": "info",
6587+
},
6588+
},
6589+
}.to_json
6590+
6591+
response = @transport.handle_request(modern_rack_request(body))
6592+
6593+
assert_equal 200, response[0]
6594+
frames = response[2][0].split("\n\n").reject(&:empty?).map do |chunk|
6595+
JSON.parse(chunk.sub(/\Aevent: message\ndata: /, ""))
6596+
end
6597+
assert_equal 2, frames.size
6598+
assert_equal "hi", frames.first.dig("params", "data")
6599+
assert_equal [{ "type" => "text", "text" => "done" }], frames.last.dig("result", "content")
6600+
assert_equal [false, true], delivery_results
6601+
assert_equal [[JSON::GeneratorError, "Failed to generate notification"]], reported
6602+
ensure
6603+
MCP.configuration.exception_reporter = original_reporter
6604+
end
6605+
65646606
test "modern POST without handler notifications keeps the single JSON exchange" do
65656607
response = @transport.handle_request(modern_rack_request(modern_body("tools/list", {})))
65666608

0 commit comments

Comments
 (0)