Skip to content

[SseClientSessionTransport] SseClientSessionTransport Fails to Handle Empty String Responses from Rust MCP Server #247

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

Closed
bmrxntfj opened this issue Apr 9, 2025 · 4 comments
Labels
bug Something isn't working

Comments

@bmrxntfj
Copy link

bmrxntfj commented Apr 9, 2025

Describe the bug
The SseClientSessionTransport class determines whether a response is an SSE message by checking if responseContent.Equals("accepted", StringComparison.OrdinalIgnoreCase). This logic is flawed because it assumes that the server will always return the string "accepted" for valid SSE messages. When using a C# MCP client to connect to an MCP server written in Rust, the server returns an empty string, causing the client to fail. However, the same Rust server works correctly when connected to via the VSCode MCP extension plugin.

To Reproduce
Steps to reproduce the behavior:

  1. Use the C# MCP client to connect to an MCP server implemented in Rust.
  2. Observe that the connection fails due to the server returning an empty string instead of "accepted".

Expected behavior
The client should correctly identify valid SSE messages even if the server returns an empty string or other valid responses, as long as the server adheres to the SSE protocol.

Logs
If applicable, add logs to help explain your problem:
• Example log: "Failed to send message: response content was empty."

Additional context
The VSCode MCP extension plugin successfully connects to the same Rust MCP server, indicating that its logic for identifying valid SSE messages is more robust. The SseClientSessionTransport class should be updated to handle a broader range of valid server responses, including empty strings, to ensure compatibility with different server implementations.

@bmrxntfj bmrxntfj added the bug Something isn't working label Apr 9, 2025
@bmrxntfj bmrxntfj changed the title [SseClientSessionTransport]responseContent.Equals("accepted", StringComparison.OrdinalIgnoreCase) [SseClientSessionTransport]SseClientSessionTransport Fails to Handle Empty String Responses from Rust MCP Server Apr 9, 2025
@bmrxntfj bmrxntfj changed the title [SseClientSessionTransport]SseClientSessionTransport Fails to Handle Empty String Responses from Rust MCP Server [SseClientSessionTransport] SseClientSessionTransport Fails to Handle Empty String Responses from Rust MCP Server Apr 9, 2025
@eiriktsarpalis
Copy link
Contributor

What would be a valid way of distinguishing between an SSE and a regular response? I tried looking at what the spec has to say but it seems to be sparse on detail. Presumably checking if the response content is JSON? Something more than that?

cc @halter73

@PederHP
Copy link
Collaborator

PederHP commented Apr 9, 2025

I believe the official SDKs also check for "accepted", at least they did at one point. I have a recollection of replicating that behavior when I noticed those SDK clients failed on other responses.

I agree that this should be more robust, and really just verify the right HTTP status. Contents shouldn't matter (as the protocol response isn't delivered here anyway).

@mikekistler
Copy link
Contributor

mikekistler commented Apr 9, 2025

The normal HTTP way to know this is from the content-type header. But maybe there is no content type header in an SSE response?

Update: SSE responses do send response headers, and the content-type is set correctly, so can we use that?

< HTTP/1.1 200 OK
< Content-Type: text/event-stream

@halter73
Copy link
Contributor

halter73 commented Apr 9, 2025

I believe the official SDKs also check for "accepted", at least they did at one point

I'm glad I'm not the only one who remembers this. However, I see nothing about it in the 11-05 spec here https://modelcontextprotocol.io/specification/2024-11-05/basic/transports and it's not in the 03-26 spec either. I think it was in what became the 03-26 spec in the really late specs though.

As of #251, the SSE client now allow empty strings from the "message" endpoint we expect to get a 202 response from (so not the one we expect the 200 text/event-stream from), so I think this issue can be closed. Thanks @239573049!

Of course, there's more work to do for full compliance with the 03-26 Streamable HTTP transport, but I'm on it.

@halter73 halter73 closed this as completed Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants