fix: add Transfer-Encoding header to SSE responses for HTTP/2 compati…#1620
fix: add Transfer-Encoding header to SSE responses for HTTP/2 compati…#1620tinynakji wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
|
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
felixweinberger
left a comment
There was a problem hiding this comment.
Thanks - did you run into this with an actual server or is this a speculative fix?
A regression test clearly demonstrating the issue would be required to be able to merge something like this.
|
Thanks for this. |
…bility
Adds
Transfer-Encoding: chunkedheader to all SSE response headers inWebStandardStreamableHTTPServerTransportto prevent HTTP/2 protocol errors.Motivation and Context
Some HTTP adapters (e.g.,
@hono/node-server) buffer small SSE responses and automatically add aContent-Lengthheader. When the SSE stream closes, this causes an HTTP/2PROTOCOL_ERRORbecause HTTP/2 doesn't allowContent-Lengthwith streaming responses.Adding
Transfer-Encoding: chunkedprevents this buffering behavior and ensures SSE streams work correctly over HTTP/2.Fixes #1619
How Has This Been Tested?
@modelcontextprotocol/server: 28 tests passed@modelcontextprotocol/client: 38 tests passed@modelcontextprotocol/node: 73 tests passedBreaking Changes
None. This is an additive change to response headers.
Types of changes
Checklist
Additional context
Affected locations in
packages/server/src/server/streamableHttp.ts:handleGetRequestmethod (~line 450)replayEventsmethod (~line 507)handlePostRequestmethod (~line 757)Reference: https://github.com/honojs/node-server/blob/main/src/listener.ts#L463-L491