mcp: write SSE comment on standalone stream so HTTP/2 reverse proxies flush HEADERS frame#938
Open
jchangx wants to merge 1 commit intomodelcontextprotocol:mainfrom
Open
mcp: write SSE comment on standalone stream so HTTP/2 reverse proxies flush HEADERS frame#938jchangx wants to merge 1 commit intomodelcontextprotocol:mainfrom
jchangx wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
… flush HEADERS frame Fixes modelcontextprotocol#937. The standalone SSE GET stream (s.id == "") writes response headers, calls Flush(), and then waits indefinitely for server-pushed events. Issue modelcontextprotocol#410 / PR modelcontextprotocol#413 (refactored in PR modelcontextprotocol#870 to use http.NewResponseController) ensure Flush() is called, which is correct for HTTP/1.1. On HTTP/2, headers and body travel as separate frames (HEADERS and DATA). Reverse proxies (Envoy, Caddy, net/http/httputil, etc.) commonly buffer the HEADERS frame until they have a DATA frame to coalesce it with — there is no HTTP/2 equivalent of HTTP/1.1's Transfer-Encoding: chunked signal that says "this is streaming, send headers now". Calling Flush() pushes the in-process buffer to the proxy, but the proxy still holds the HEADERS frame waiting for body data. Real-world impact: an MCP server behind Envoy hangs ~31s on session startup (matching the configured request timeout) because the client never sees the response headers until the proxy tears down the stream on timeout. Behavior reproduces with net/http/httputil in HTTP/2 mode as well; it is not Envoy-specific. This change writes an SSE comment (": ok\n\n") immediately after WriteHeader, before Flush. SSE comments are explicitly defined by the spec as lines starting with ":" that clients ignore, so the change is behavior-preserving for SSE clients. The point is the DATA frame this produces, which forces HTTP/2 reverse proxies to forward both frames together. Also adds a regression test that exercises the standalone SSE GET stream and asserts the first body bytes start with ":". The test times out at 2s without the fix and completes in <1ms with it. Refs: golang/go#31125 caddyserver/caddy#4247
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #937.
What this changes
Adds a single line — an SSE comment (
: ok\n\n) — afterWriteHeaderand beforeFlushon the standalone SSE GET stream. SSE comments are explicitly ignored by clients (any line starting with:per the SSE spec), but they produce an HTTP/2 DATA frame, which is what HTTP/2 reverse proxies need before they will forward the HEADERS frame.Why the existing fix isn't sufficient
Issue #410 was closed by #413, which added
WriteHeader(StatusOK)+Flush()to the same site. #870 refactored theFlush()to usehttp.NewResponseController. Both correctly address HTTP/1.1:Flush()writes the headers as text on the TCP stream and any HTTP/1.1-aware proxy forwards them.In HTTP/2, headers and body are separate frame types (HEADERS, DATA). Reverse proxies batch them for efficiency and won't forward the HEADERS frame on its own.
Flush()on the response writer only pushes the in-process buffer to the HTTP/2 stack; the proxy still holds the HEADERS frame waiting for DATA. The standalone SSE stream never sends body data on connect (the whole point — it's the long-poll listener for server-pushed notifications), so without a deliberate "kick" the headers never reach the client until the proxy tears down the stream on timeout.The SSE-comment trick is the established mitigation for SSE servers behind HTTP/2 proxies. It's used in Caddy (caddyserver/caddy#4247), referenced in the Go issue tracker (golang/go#31125), and is independent of the proxy implementation.
The diff
fmtis already imported in this file; no other dependencies change.Test
Added
TestStandaloneSSEEmitsCommentForHTTP2Flush. It initializes a streamable session via raw POST, then opens the standalone SSE GET and asserts the first body byte starts with:(the SSE comment marker, which is what produces the DATA frame HTTP/2 proxies need).The test catches the bug behaviorally without requiring a full HTTP/2 reverse-proxy setup: it verifies the SDK's contract (emit body bytes after headers on the standalone SSE stream). HTTP/2-specific behavior is then a property of any conforming proxy.
I confirmed the test fails on
mainwithout the patch — it times out at 2s waiting for the first body byte — and passes in <1ms with the patch. Full streamable test suite still passes.Reproduction matrix (from #937)
Risk
:line is a valid SSE comment and is ignored by every conforming SSE client).