fix(sse-client): Skip SSE in StreamableHttpClientTransport when data is empty #433
Conversation
Refactor StreamableHttpClientTransport to skip SSE with an empty data field.
- Added a new test case to verify handling of empty Server-Sent Events (SSE). - Replace `OldSchemaMockMcp` with `MockMcp`
| // reset | ||
| id = null | ||
| eventName = null | ||
| sb.clear() |
There was a problem hiding this comment.
less side-effects inside dispath
...commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/StreamableHttpClientTransport.kt
Outdated
Show resolved
Hide resolved
| "id: ${Uuid.random()}\n", | ||
| "data: \n", | ||
| "\n", | ||
| "id: ${Uuid.random()}\n", | ||
| "event: message\n", | ||
| @Suppress("MaxLineLength") | ||
| "data: {\"result\":{\"protocolVersion\":\"2025-06-18\",\"capabilities\":{},\"serverInfo\":{\"name\":\"simple-streamable-http-server\",\"version\":\"1.0.0\"}},\"jsonrpc\":\"2.0\",\"id\":\"7ce065b0678f49e5b04ce5a0fcc7d518\"}\n", | ||
| "\n", |
There was a problem hiding this comment.
raw SSE stream data
kpavlov
changed the title
kpavlov
changed the title
| lastEventId = it | ||
| onResumptionToken?.invoke(it) | ||
| } | ||
| if (data.isBlank()) { |
There was a problem hiding this comment.
Should it be isEmpty() instead?
There was a problem hiding this comment.
Agree
isBlank will also skip a string if it contains only whitespace. But such a value is valid
There was a problem hiding this comment.
There’s no need to worry about spaces when parsing, since it’s not JSON.
| internal class StreamableHttpClientTest : AbstractStreamableHttpClientTest() { | ||
|
|
||
| @Test | ||
| fun `Should skip empty SSE`(): Unit = runBlocking { |
There was a problem hiding this comment.
plus the following cases?
should handle whitespace-only SSE data fieldsshould handle multi-line SSE data fields
flowOf(
"event: message\n",
"data: {\"result\":{\"protocolVersion\":\"2025-06-18\",\n",
"data: \"capabilities\":{},\n",
"data: \"serverInfo\":{\"name\":\"simple-streamable-http-server\",\"version\":\"1.0.0\"}},\n",
"data: \"jsonrpc\":\"2.0\",\"id\":\"7ce065b0678f49e5b04ce5a0fcc7d518\"}\n",
"\n",
)
There was a problem hiding this comment.
Good point. I will update the integration test for multiline
| lastEventId = it | ||
| onResumptionToken?.invoke(it) | ||
| } | ||
| if (data.isBlank()) { |
There was a problem hiding this comment.
Agree
isBlank will also skip a string if it contains only whitespace. But such a value is valid
| val line = channel.readUTF8Line() ?: break | ||
| if (line.isEmpty()) { | ||
| dispatch(sb.toString()) | ||
| dispatch(id = id, eventName = eventName, data = sb.toString()) |
There was a problem hiding this comment.
In that case, you can move dispatch fun out of handleInlineSse
There was a problem hiding this comment.
Not quite yet. The "lastEventId" is updated inside. I wanted to avoid a major refactoring for now.
… scenarios - Refactored SSE test cases to include handling of empty data, tabs, spaces, and multiline data payloads.
Refactor StreamableHttpClientTransport to skip SSE when the data field is empty.
Motivation and Context
TypeScript SSE Server started returning Server-Sent Events with only id and empty data field (Heartbeat/Checkpoint), followed by an event with data, e.g.:
Such empty events might be used by the server as Heartbeat or Checkpoint events. The client should retain the updated Last-Event-ID and disregard the payload. It is essential to handle such responses to ensure compatibility with updated TypeScript SDK.
How Has This Been Tested?
Breaking Changes
No
Types of changes
Checklist
Additional context