Skip to content

Comments

Fix: Skip non-message SSE events in processStream#637

Merged
findleyr merged 3 commits intomodelcontextprotocol:mainfrom
raphaelmansuy:fix/issue-636-filter-ping-events
Nov 14, 2025
Merged

Fix: Skip non-message SSE events in processStream#637
findleyr merged 3 commits intomodelcontextprotocol:mainfrom
raphaelmansuy:fix/issue-636-filter-ping-events

Conversation

@raphaelmansuy
Copy link
Contributor

@raphaelmansuy raphaelmansuy commented Nov 13, 2025

Fixes #636

Problem

The StreamableClientTransport tries to JSON-decode ALL SSE events, including ping events sent by servers for keep-alive purposes. This causes errors like 'invalid character 'p' looking for beginning of value' when encountering non-JSON event data.

Solution

Modified processStream to check evt.Name and skip events that are not 'message' events (or unnamed events, which default to 'message' per SSE spec).

Changes

  • Added event name check in processStream before attempting JSON decode
  • Added test case for ping event filtering
  • Added test for scanning multiple events including ping events

Testing

  • All existing tests pass
  • New tests verify ping events are properly filtered
  • Tested with DeepWiki MCP server which sends ping events

@raphaelmansuy
Fix: Skip non-message SSE events in processStream
7e5b2c2 
Fixes modelcontextprotocol#636 - SDK now ignores ping and other non-message events
@raphaelmansuy raphaelmansuy marked this pull request as draft November 13, 2025 15:41
@raphaelmansuy raphaelmansuy marked this pull request as ready for review November 13, 2025 15:41
findleyr
findleyr previously approved these changes Nov 13, 2025
View reviewed changes
Copy link
Contributor

@findleyr findleyr left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix, and thorough tests!

@findleyr
Copy link
Contributor

findleyr commented Nov 13, 2025

Looks like mcp/streamable_test.go is not formatted. Could you go fmt it? Thanks.

@raphaelmansuy
Copy link
Contributor Author

raphaelmansuy commented Nov 14, 2025

Added formatting commit (SHA: 1172815). I ran ok github.com/modelcontextprotocol/go-sdk (cached)
ok github.com/modelcontextprotocol/go-sdk/auth (cached)
? github.com/modelcontextprotocol/go-sdk/examples/client/listfeatures [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/client/loadtest [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/client/middleware [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/http [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/server/basic [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/server/completion [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/server/custom-transport [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/server/distributed [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/server/elicitation [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/server/everything [no test files]
? github.com/modelcontextprotocol/go-sdk/examples/server/hello [no test files]
ok github.com/modelcontextprotocol/go-sdk/examples/server/memory (cached)
? github.com/modelcontextprotocol/go-sdk/examples/server/middleware [no test files]
ok github.com/modelcontextprotocol/go-sdk/examples/server/sequentialthinking (cached)
? github.com/modelcontextprotocol/go-sdk/examples/server/sse [no test files]
ok github.com/modelcontextprotocol/go-sdk/examples/server/toolschemas (cached)
? github.com/modelcontextprotocol/go-sdk/internal/docs [no test files]
ok github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2 (cached)
? github.com/modelcontextprotocol/go-sdk/internal/readme [no test files]
? github.com/modelcontextprotocol/go-sdk/internal/readme/client [no test files]
? github.com/modelcontextprotocol/go-sdk/internal/readme/server [no test files]
? github.com/modelcontextprotocol/go-sdk/internal/util [no test files]
? github.com/modelcontextprotocol/go-sdk/internal/xcontext [no test files]
? github.com/modelcontextprotocol/go-sdk/jsonrpc [no test files]
ok github.com/modelcontextprotocol/go-sdk/mcp (cached)
? github.com/modelcontextprotocol/go-sdk/oauthex [no test files] and all packages passed. Pushed to origin/fix/issue-636-filter-ping-events.

@raphaelmansuy
Copy link
Contributor Author

raphaelmansuy commented Nov 14, 2025

Thanks — I formatted mcp/streamable_test.go and pushed the change (commit 1172815). I ran go test ./... and all packages passed. Please re-run CI as needed.

@findleyr findleyr merged commit 9ef2b27 into modelcontextprotocol:main Nov 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@findleyr findleyr findleyr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

StreamableClient tries to JSON-decode ping message

2 participants

@raphaelmansuy @findleyr