-
Notifications
You must be signed in to change notification settings - Fork 359
Fix support for streamable-http connections #339
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
Fix support for streamable-http connections #339
Conversation
* In server/index.js - add get/post handlers for /mcp - amend console log on SSE connect, with deprecation message - add /stdio GET handler and refactored /sse GET handler to not also do stdio. Each transport has its own handler now - add appropriate headers to streamable-http request * In /client/src/lib/hooks/useConnection.ts - in connect function - create server url properly based on new transport type.
@cliffhall Small bug I found when testing with a different server that supports streamable HTTP:
In my case, since streamable HTTP was added in |
Should be possible to fix by changing the inspector's protocol version that it sends to the server - server returns what it receives from the client |
I pulled this version of the inspector. I get the "SSE connection not established" (also referenced here #295) I mentioned in the original Streamable HTTP PR (#294 (comment)) very frequently. I was also getting this error with another server (https://github.com/ferrants/mcp-streamable-http-typescript-server), but was unsure if my server was to blame, so the official This is what I see In the inspector logs
It doesn't seem like others get this error in testing the Streamable HTTP transport, so my testing is probably invalid in some way, but I wanted to report my experience and see if i can help at all. Thanks for your efforts on this. |
FWIW, I tested this PR against modelcontextprotocol/servers#1496. It works fine for me. Steps to test:
I do this because I don't want any stale connections on the inspector backend to interact with the everything MCP Server. @ferrants are there a different set of steps you are following to test this. If you are, would you be able to share steps to reproduce the error you are seeing? |
It is the case that one instance of the inspector using this PR can connect to that server. Problem is that the proxy server connection to the actual MCP server is still using an The problem that we're not seeing is in handling SSE events, which this new protocol fetches via a GET request. I believe that server will handle them properly, but the proxy server will not. |
NOTE: I'm working on updates to this PR, just resuming this morning.
|
@shivdeepak , that is the set of steps I was using. I was setting up a recording and did a fresh Working with the everything server for me! Also worked with the template one I'm building off of the repo examples: https://github.com/ferrants/mcp-streamable-http-typescript-server Thanks!
|
I think this is owing to the fact that the Inspector's
It has the illusion of working, but it really isn't properly proxying yet. I've been pushing air around in a balloon dog all day trying to get that to work properly, but it's just not having any of it. |
I forked the inspector a few weeks ago and ran into this - for me, removing the backend connection and instead connecting directly from browser helped with this but ti's been a while and I'm fuzzy about the coding. I have a version of it here: https://github.com/QuantGeekDev/mcp-debug you can try it with I'm at an offsite for the this week but if it's still an issue next week I'm happy to jump in and try my hand at it |
… works fine with STDIO and SSE servers. * In index.ts, - refactor transport webAppTransports to be a map with the session id as key and transport as value. * Implement /mcp GET and POST endpoints using StreamableHTTPServerTransport and doing the new session in the POST (opposite from SSE) handler. * In package.json - update the SDK to 1.10.2 * In useConnection.ts - import StreamableHTTPClientTransport - NOTE: while we NEED to do this, it causes useConnection.test.ts to fail with " ReferenceError: TransformStream is not defined" - in connect method - instantiate the appropriate transport
@cliffhall I have spend close 5 hours trying to figure out why MCPProxy with Here are my findings: I tried following scenarios: Scenario 1 (Current Impl on main):
This works fine. Scenario 2 (Using StreamableHTTPClient/Server as a Proxy):
This doesn't work because the initialize request's response payload seems empty on the browser dev console.. It works fine when run from Node.js. Scenario 3 (Independent Vite App that uses StreamableHTTPClient): I am running into the same issue as Scenario 2. On the browser, I don't see the response payload for the initialize request. ![]() ![]() This leads me to believe that there is some issue with I also ran mcp-sniffer between the client and server, to see if the server is not sending the data properly. But looks like it does, the response is present in the payload. ![]() ![]() So, finally, I believe it's the browser (chrome and safari) that is not able to deserialize the SSE Message and Possibly the HTTP Response is not up to standard. i.e. Node.js can understand it, but Google Chrome cannot. it's also possible that the StreamableHTTPClient behaves as expected on Node.js, but not the browser. But Chrome not showing the response payload is not normal However, I do see Postman, and Safari show the response payload. But Safari has the same behavior as Chrome. The StreamableHTTPClient is not sending the following initialize notification with mcp-session-id header. ![]() ![]() ![]() Note: I also have cors enabled on the server side, so I am pretty sure CORS is not the issue. |
Here is the PR with the fix -- cliffhall#1 Tested on local. Seems to be working fine. |
fix cors issue with accessing mcp-session-id header
After @shivdeepak's fix for the session id header problem, we are cooking with gas now. Full streamable-http connection from client to proxy to server and back. The only outstanding problem is that the React unit tests are failing now with I'm currently running Node 20 and TransformStream exists. EDIT: This is problem is now fixed with jest-fixed-jsdom. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this with the sample Streamable HTTP server at https://github.com/modelcontextprotocol/csharp-sdk/tree/v0.1.0-preview.11/samples/AspNetCoreSseServer (it has yet to be renamed from "Sse"), and things seem to work pretty well. I tested sampling using the "sampleLLM" tool, and that worked nicely.
Unfortunately, the Streamable HTTP transport also does not immediately report a connection error and ask "Connection Error, is your MCP server running?" when the connection is refused because there's no server listening like the SSE transport does. It just stays in the "Disconnected" state not indicating that it's connecting or that it even registered you clicking the connect button until about a minute later when it shows an error.
SSE ECONREFUSED logs
[1] New SSE connection. NOTE: The sse transport is deprecated and has been replaced by streamable-http
[1] Query parameters: [Object: null prototype] {
[1] url: 'http://localhost:3001/',
[1] transportType: 'sse'
[1] }
[1] SSE transport: url=http://localhost:3001/, headers=Accept
[1] Error in /sse route: SseError: SSE error: TypeError: fetch failed: connect ECONNREFUSED ::1:3001, connect ECONNREFUSED 127.0.0.1:3001
[1] at EventSource._eventSource.onerror (H:\modelcontextprotocol\inspector\node_modules\@modelcontextprotocol\sdk\src\client\sse.ts:133:23)
[1] at EventSource.scheduleReconnect_fn (H:\modelcontextprotocol\inspector\node_modules\eventsource\src\EventSource.ts:557:5)
[1] at <anonymous> (H:\modelcontextprotocol\inspector\node_modules\eventsource\src\EventSource.ts:441:5)
[1] at process.processTicksAndRejections (node:internal/process/task_queues:105:5) {
[1] code: undefined,
[1] event: {
[1] type: 'error',
[1] message: 'TypeError: fetch failed: connect ECONNREFUSED ::1:3001, connect ECONNREFUSED 127.0.0.1:3001',
[1] code: undefined,
[1] defaultPrevented: false,
[1] cancelable: false,
[1] timeStamp: 2422561.2383
[1] }
[1] }
Streamable HTTP ECONREFUSED logs
[1] New streamable-http connection
[1] Query parameters: [Object: null prototype] {
[1] url: 'http://localhost:3001/',
[1] transportType: 'streamable-http'
[1] }
[1] Connected to Streamable HTTP transport
[1] Connected MCP client to backing server transport
[1] Created streamable web app transport e020a675-58df-4165-bc34-a20b4ba4763c
[1] Error from MCP server: TypeError: fetch failed
[1] at node:internal/deps/undici/undici:13502:13
[1] at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
[1] at async StreamableHTTPClientTransport.send (H:\modelcontextprotocol\inspector\node_modules\@modelcontextprotocol\sdk\src\client\streamableHttp.ts:394:24) {
[1] [cause]: AggregateError [ECONNREFUSED]:
[1] at internalConnectMultiple (node:net:1139:18)
[1] at afterConnectMultiple (node:net:1712:7) {
[1] code: 'ECONNREFUSED',
[1] [errors]: [ [Error], [Error] ]
[1] }
[1] }
The connection error reporting issue could probably be fixed in a follow up. It's great to have the mcp-session-id header now forwarded and see the inspector working end-to-end with the Streamable HTTP transport. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add anything to the Readme related to this update?
@QuantGeekDev As I was working on this PR I questioned why we even needed this proxy. I came to the conclusion that it is only truly necessary for STDIO servers, which you could not access directly from a web browser. It would be possible to make the client connect directly to SSE/StreamableHTTP servers, and there's an argument to be made that it would be more illustrative to devs than the complicated proxy path we are using. But since the proxy is required for STDIO, it's just easier to use the proxy for all connections, and I suspect this is how it will continue to work for the near future. |
- Add last-event-id to STREAMABLE_HTTP_HEADERS_PASSTHROUGH.
@olaservo As we were discussing in Discord, I think we need to expand the README into a more full-blown set of docs that would go deeper on the different transport types and features, etc. But currently, there's no section that talks about SSE vs STDIO where we would, so I don't think any of these changes necessitate doc updates. A new transport type is available, the old ones are still there. IMO, there's nothing operating differently to callout now. |
Are we missing specifying the |
@cliffhall We're building an authenticated Remote MCP Server based on the Streamable HTTP Transport and we tested it with the changes on your branch, but it seems the authentication flow isn't being triggered when the server returns a 401 Unauthorized as opposed to the SSE transport implementation. Do you know why is that? |
Hi @sbosio! That's a great and timely catch.
@ihrpr can you add a unit test to streamableHttp.test.ts that is similar to the "attempts auth flow on 401 during SSE connection" or "attempts auth flow on 401 during POST request" so we can rule out the SDK? |
Is the authentication flow supposed to be working already? I figured that would be follow-up work after this gets in. I'm also looking to leverage that. Very timely. |
I didn't follow. Were you referring to the SDK or the Inspector? We're putting up a POC and we were able to make everything work using the latest version of the Inspector using the Then I tried this branch and selected the new Transport type from the dropdown, but when I put the MCP endpoint in the URL and click Connect I only get the 401 error and the OAuth flow isn't triggered. OTOH, if I use the current Inspector and the SSE transport type (which the server really doesn't implement because it's a stateless server with the Streamable HTTP transport) and set the URL to the base domain of the server, the OAuth flow works perfectly, but it obviously fails when getting the root path, because that endpoint doesn't exists. |
@sbosio I was referring to the inspector. I didn't know the oauth flow was set up to work in the inspector yet. Thanks. |
@cliffhall, the test "attempts auth flow on 401 during SSE connection" asserts As for the second test, I opened up a PR - assertion checks out, like for SSE -> modelcontextprotocol/typescript-sdk#411 |
There is support for OAuth in the inspector. See this previously merged PR which has a demo video showing operation. That said the new authorization spec recently landed and there is more work to do to ensure compliance. |
Given that this fixes streamable-http and that we are preparing to do a bunch of work to get the inspector into compliance with the new auth spec, I feel like holding this one up for another week of reviews is counterproductive. We will get a reference server implementing streamable-http that we can test against and work that issue separately. |
Description
In server/index.js
mcp-session-id
andlast-event-id
toSTREAMABLE_HTTP_HEADERS_PASSTHROUGH
In /client/src/lib/hooks/useConnection.ts
Motivation and Context
The PR that added support for streamable-http servers was incomplete, but it actually seemed to work because the proxy server had
/sse
and/message
endpoints.This adds
/mcp
GET/POST endpoint support to the MCP proxy server, and slightly refactors the creation of the transport in the UI.How Has This Been Tested?
Locally against stdio, sse, and streamble-http servers
Streamable HTTP (/mcp)
Stdio (/stdio + /message)
SSE (/sse + /message)
Breaking Changes
Types of changes
Checklist
Additional context
Testing streamable-http with this PR on the
everything
server.