Skip to content
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

feat: WebSocket server transport implementation #232

Closed
wants to merge 3 commits into from

Conversation

bitnom
Copy link

@bitnom bitnom commented Mar 28, 2025

Resolve disparity between typescript-sdk and https://github.com/modelcontextprotocol/python-sdk/blob/2ea14958f0d78ddbab0c0c3bba05ec38ccc47b56/src/mcp/server/websocket.py

Motivation and Context

websockets is necessary

How Has This Been Tested?

no testing yet. draft

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

Additional context

none

@jspahrsummers
Copy link
Member

WebSocket is not a standardized transport in the MCP spec, so we shouldn't offer this in the SDK.

@netroy
Copy link

netroy commented Apr 7, 2025

we shouldn't offer this in the SDK

But, it's already offered as a transport for clients in this SDK, and as transport for servers and clients in the python SDK.

@jspahrsummers
Copy link
Member

Those should be removed.

@netroy
Copy link

netroy commented Apr 7, 2025

Some additional thoughts on why WebSockets should be a supported transport:
Sooner or later people are going to run into issues in their browser based clients, because there is a limit on max number of connections that browsers allow for SSE, and when that connection limit is reached, all network calls to that host get blocked until some of the SSE connections close.
This is supposed to be by-design on Chrome, as well as on Firefox.
Even though SSE is still marked a HTML Living Standard, it is very unlikely that this connection limit issue will ever be fixed.

WebSockets offer some additional benefits - better bidirectional communication, no need for separate POST requests, improved binary data handling, and built-in heartbeat mechanisms.
That said, WebSockets aren't perfect. There are quite a few issues with WebSockets support on reverse-proxies. They don't support CORS (making them vulnerable to cross-origin attacks), and inherently a more complicated protocol to support than SSE is.

That's (unfortunately) the current state of Push mechanisms over HTTP. There is no ideal solution, and it's generally (still) recommended to support both SSE and WebSockets.

However, I completely respect your decision on this PR, and am not necessarily arguing to reopen it; That's your call to make.
I'm just leaving this argument here for the future, if/when someone hits this connection limit issue, to hopefully help them understand the issue better.

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