Skip to content

fix(server): GET SSE stream crash on Netty in Streamable HTTP#681

Merged
devcrocod merged 1 commit intomainfrom
devcrocod/streamable-http-get-sse
Apr 10, 2026
Merged

fix(server): GET SSE stream crash on Netty in Streamable HTTP#681
devcrocod merged 1 commit intomainfrom
devcrocod/streamable-http-get-sse

Conversation

@devcrocod
Copy link
Copy Markdown
Contributor

appendSseHeaders() was called inside Ktor's sse {} block where response headers are already committed. On Netty this throws UnsupportedOperationException, killing the SSE stream and causing the client to retry in a loop

How Has This Been Tested?

unit, simple-streamable-server sample

Breaking Changes

none

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • [x ] I have added appropriate error handling
  • [ x] I have added or updated documentation as needed

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Netty crash that occurred when setting SSE response headers after Ktor’s sse {} handler had already committed headers, which could terminate the SSE stream and cause client retry loops.

Changes:

  • Stop calling appendSseHeaders() inside Ktor’s sse {} blocks for GET SSE streams / replay paths.
  • Set Mcp-Session-Id for GET SSE responses earlier in the Ktor pipeline (before headers are committed).
  • Add a regression test ensuring the GET SSE stream returns Mcp-Session-Id and remains open.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransportTest.kt Adds a test covering GET SSE header presence + connection staying open.
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransport.kt Removes header mutation inside sse {} and relies on early header setting + flush.
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/KtorServer.kt Adds a route interceptor to attach Mcp-Session-Id on GET before SSE commits headers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// Set Mcp-Session-Id on GET responses before Ktor's sse {} commits headers.
intercept(ApplicationCallPipeline.Plugins) {
if (context.request.httpMethod == HttpMethod.Get) {
val sessionId = context.request.header(MCP_SESSION_ID_HEADER)
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GET interceptor reads the session ID via context.request.header(...) (single value), but the transport validation later rejects requests that contain multiple Mcp-Session-Id header values. In that case this interceptor can still echo a session ID onto a response that will be rejected. Consider using context.request.headers.getAll(MCP_SESSION_ID_HEADER) and only setting the response header when there is exactly one non-blank value (matching StreamableHttpServerTransport.validateSession).

Suggested change
val sessionId = context.request.header(MCP_SESSION_ID_HEADER)
val sessionIds = context.request.headers.getAll(MCP_SESSION_ID_HEADER)
val sessionId =
sessionIds
?.singleOrNull()
?.takeIf { it.isNotBlank() }

Copilot uses AI. Check for mistakes.
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...delcontextprotocol/kotlin/sdk/server/KtorServer.kt 80.00% 0 Missing and 1 partial ⚠️
...kotlin/sdk/server/StreamableHttpServerTransport.kt 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@e5l e5l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@devcrocod devcrocod merged commit 8a7f9f8 into main Apr 10, 2026
24 checks passed
@devcrocod devcrocod deleted the devcrocod/streamable-http-get-sse branch April 10, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants