Merge pull request #630 from koic/drop_a_buffered_notification_that_cannot_be_generated

Drop a buffered notification that cannot be generated

Author: koic

lib/mcp/server/transports/streamable_http_transport.rb

@@ -521,12 +521,19 @@ def send_notification(method, params = nil, session_id: nil, related_request_id:
           # a per-request sink for its ephemeral session and flushes it as SSE frames ahead of the final response.
           # Checked before the stateless guard, since modern requests are served in stateless deployments too.
           # The sink is bounded; past `MAX_MODERN_REQUEST_NOTIFICATIONS` the notification is dropped as
-          # non-delivery (`false`), never handed to the legacy paths below.
+          # non-delivery (`false`), never handed to the legacy paths below. It holds each notification as JSON,
+          # generated here so that one that cannot be generated is dropped on its own instead of failing
+          # the whole response when the sink is flushed.
           sink = @mutex.synchronize { session_id && @modern_request_sinks[session_id] }
           if sink
             return false if sink.size >= MAX_MODERN_REQUEST_NOTIFICATIONS
 
-            sink << notification
+            begin
+              sink << notification.to_json
+            rescue JSON::GeneratorError => e
+              MCP.configuration.exception_reporter.call(e, { session_id: session_id, error: "Failed to generate notification" })
+              return false
+            end
             return true
           end
 
@@ -949,7 +956,7 @@ def handle_modern(request, header_version, body_string: nil)
           if notifications.empty?
             [modern_http_status(response), { "content-type" => "application/json" }, [response_json]]
           else
-            frames = notifications.map(&:to_json) + [response_json]
+            frames = notifications + [response_json]
             sse_body = frames.map { |frame| "event: message\ndata: #{frame}\n\n" }.join
             [200, SSE_HEADERS.dup, [sse_body]]
           end

test/mcp/server/transports/streamable_http_transport_test.rb

@@ -6561,6 +6561,48 @@ def string
           assert_equal overflow, delivery_results.count { |result| result == false }
         end
 
+        test "modern POST drops a handler notification that cannot be generated and keeps the response" do
+          reported = []
+          original_reporter = MCP.configuration.exception_reporter
+          MCP.configuration.exception_reporter = ->(e, context) { reported << [e.class, context[:error]] }
+          delivery_results = []
+          @server.define_tool(name: "logging_tool") do |server_context:|
+            delivery_results << server_context.notify_log_message(data: "\xFF".dup.force_encoding(Encoding::UTF_8), level: "info")
+            delivery_results << server_context.notify_log_message(data: "hi", level: "info")
+            Tool::Response.new([{ type: "text", text: "done" }])
+          end
+
+          body = {
+            jsonrpc: "2.0",
+            method: "tools/call",
+            id: 1,
+            params: {
+              name: "logging_tool",
+              arguments: {},
+              _meta: {
+                "io.modelcontextprotocol/protocolVersion": "2026-07-28",
+                "io.modelcontextprotocol/clientInfo": { name: "modern_client", version: "2.0" },
+                "io.modelcontextprotocol/clientCapabilities": {},
+                "io.modelcontextprotocol/logLevel": "info",
+              },
+            },
+          }.to_json
+
+          response = @transport.handle_request(modern_rack_request(body))
+
+          assert_equal 200, response[0]
+          frames = response[2][0].split("\n\n").reject(&:empty?).map do |chunk|
+            JSON.parse(chunk.sub(/\Aevent: message\ndata: /, ""))
+          end
+          assert_equal 2, frames.size
+          assert_equal "hi", frames.first.dig("params", "data")
+          assert_equal [{ "type" => "text", "text" => "done" }], frames.last.dig("result", "content")
+          assert_equal [false, true], delivery_results
+          assert_equal [[JSON::GeneratorError, "Failed to generate notification"]], reported
+        ensure
+          MCP.configuration.exception_reporter = original_reporter
+        end
+
         test "modern POST without handler notifications keeps the single JSON exchange" do
           response = @transport.handle_request(modern_rack_request(modern_body("tools/list", {})))