mcp: write SSE comment on standalone stream so HTTP/2 reverse proxies flush HEADERS frame (#938)
Fixes #937. ### What this changes Adds a single line — an SSE comment (`: ok\n\n`) — after `WriteHeader` and before `Flush` on 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 the `Flush()` to use `http.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 (https://github.com/caddyserver/caddy/issues/4247), referenced in the Go issue tracker (https://github.com/golang/go/issues/31125), and is independent of the proxy implementation. ### The diff ```go if s.id == "" { // Issue #410: the standalone SSE stream is likely not to receive messages // for a long time. Ensure that headers are flushed. + // + // On HTTP/2, headers and body travel as separate frames (HEADERS and + // DATA). Reverse proxies (e.g. Envoy, Caddy, net/http/httputil) + // 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() alone is not sufficient: it pushes + // the kernel buffer to the proxy, but the proxy still holds the + // HEADERS frame. + // + // Write an SSE comment (lines starting with ":" are ignored by + // clients per RFC) so a DATA frame is produced, which forces the + // proxy to forward both frames. See: + // https://github.com/golang/go/issues/31125 + // https://github.com/caddyserver/caddy/issues/4247 w.WriteHeader(http.StatusOK) + fmt.Fprint(w, ": ok\n\n") rc := http.NewResponseController(w) // Ignore returned error as flushing is best-effort. _ = rc.Flush() } ``` `fmt` is 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 `main` without 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) | Configuration | TTFB | Result | |---|---|---| | HTTP/2 + 30s request timeout | ~31s | Headers only on stream tear-down | | HTTP/2 + no request timeout | never | Headers never arrive | | HTTP/1.1 + 30s request timeout | ~300ms | Headers immediate | | HTTP/1.1 + no request timeout | ~270ms | Headers immediate | | HTTP/2 POST (response has body) | ~350ms | Works (DATA frame coalesces) | | **HTTP/2 + this PR** | ~1ms | Headers immediate | ### Risk - Behavior-preserving for HTTP/1.1 clients (a leading `:` line is a valid SSE comment and is ignored by every conforming SSE client). - No protocol-level concern: SSE explicitly defines comment lines as a no-op for the consumer. - The DATA frame adds 7 bytes per standalone SSE connection, sent once on connect. Co-authored-by: Maciej Kisiel <mkisiel@google.com>
This commit is contained in:
parent
438cd4384c
commit
d10c315008
1 changed files with 112 additions and 0 deletions
|
|
@ -1061,7 +1061,23 @@ func (c *streamableServerConn) acquireStream(ctx context.Context, w http.Respons
|
|||
if s.id == "" {
|
||||
// Issue #410: the standalone SSE stream is likely not to receive messages
|
||||
// for a long time. Ensure that headers are flushed.
|
||||
//
|
||||
// On HTTP/2, headers and body travel as separate frames (HEADERS and
|
||||
// DATA). Reverse proxies (e.g. Envoy, Caddy, net/http/httputil)
|
||||
// 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() alone is not sufficient: it pushes
|
||||
// the kernel buffer to the proxy, but the proxy still holds the
|
||||
// HEADERS frame.
|
||||
//
|
||||
// Write an SSE comment (lines starting with ":" are ignored by
|
||||
// clients per RFC) so a DATA frame is produced, which forces the
|
||||
// proxy to forward both frames. See:
|
||||
// https://github.com/golang/go/issues/31125
|
||||
// https://github.com/caddyserver/caddy/issues/4247
|
||||
w.WriteHeader(http.StatusOK)
|
||||
fmt.Fprint(w, ": ok\n\n")
|
||||
rc := http.NewResponseController(w)
|
||||
// Ignore returned error as flushing is best-effort.
|
||||
_ = rc.Flush()
|
||||
|
|
|
|||
Loading…
Reference in a new issue