Skip to content

fix: improve SSE event handling to gracefully ignore unrecognized events #423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

tzolov
Copy link
Contributor

@tzolov tzolov commented Jul 24, 2025

This change improves client resilience by gracefully handling unknown events instead of failing, allowing clients to continue processing valid events even when encountering unexpected ones.

  • Replace McpError exceptions with debug/warning logs for unrecognized SSE event types
  • Continue processing instead of failing when encountering unknown SSE events
  • Update transport implementations:
    • WebClientStreamableHttpTransport: return empty tuple instead of throwing
    • WebFluxSseClientTransport: complete stream instead of erroring
    • HttpClientSseClientTransport: call sink.success() instead of sink.error()
    • HttpClientStreamableHttpTransport: return empty Flux for unknown events

This improves client resilience when servers send non-standard or future SSE event types.

Resolves #272 , #223 , #93, #421

Improve SSE event handling to gracefully ignore unrecognized events instead of throwing errors

Motivation and Context

Previously, when MCP clients received unrecognized Server-Sent Events (SSE) from servers, they would throw McpError exceptions and terminate the connection. This created fragility when:

  • Servers send non-standard SSE event types
  • Future MCP versions introduce new SSE event types that older clients don't recognize
  • SSE comment lines (starting with :) are sent, which should be ignored per the SSE specification

How Has This Been Tested?

  • Added unit test testCommentSseMessage() to verify SSE comment lines are properly ignored
  • Existing transport tests continue to pass, ensuring backward compatibility
  • Tested scenarios include:
    • SSE comment lines (: prefixed lines)
    • Unknown SSE event types
    • Mixed streams with both recognized and unrecognized events

Breaking Changes

No breaking changes. This is a backward-compatible improvement that makes clients more resilient. Existing functionality remains unchanged.

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
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Implementation Details:

  • WebClientStreamableHttpTransport: Returns empty tuple instead of throwing
  • WebFluxSseClientTransport: Completes stream with warning log instead of erroring
  • HttpClientSseClientTransport: Calls sink.success() with debug log instead of sink.error()
  • HttpClientStreamableHttpTransport: Returns empty Flux for unknown events

tzolov added 5 commits July 24, 2025 16:17
- Replace McpError exceptions with debug/warning logs for unrecognized SSE event types
- Continue processing instead of failing when encountering unknown SSE events
- Update transport implementations:
  - WebClientStreamableHttpTransport: return empty tuple instead of throwing
  - WebFluxSseClientTransport: complete stream instead of erroring
  - HttpClientSseClientTransport: call sink.success() instead of sink.error()
  - HttpClientStreamableHttpTransport: return empty Flux for unknown events

This improves client resilience when servers send non-standard or future SSE event types.

Resolves modelcontextprotocol#272 , modelcontextprotocol#223 , modelcontextprotocol#93

Signed-off-by: Christian Tzolov <[email protected]>
Signed-off-by: Christian Tzolov <[email protected]>
Copy link
Contributor

@sunyuhan1998 sunyuhan1998 left a comment

Choose a reason for hiding this comment

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

That's great! I think this PR can solve many issues.

tzolov added a commit that referenced this pull request Jul 25, 2025
…nts (#423)

- Replace McpError exceptions with debug/warning logs for unrecognized SSE event types
- Continue processing instead of failing when encountering unknown SSE events
- Update transport implementations:
  - WebClientStreamableHttpTransport: return empty tuple instead of throwing
  - WebFluxSseClientTransport: complete stream instead of erroring
  - HttpClientSseClientTransport: call sink.success() instead of sink.error()
  - HttpClientStreamableHttpTransport: return empty Flux for unknown events

This improves client resilience when servers send non-standard or future SSE event types.

Resolves #272 , #223 , #93, #421

Signed-off-by: Christian Tzolov <[email protected]>
@tzolov
Copy link
Contributor Author

tzolov commented Jul 25, 2025

Rebased, squashed and merged at c3a0b18

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.

3 participants