Skip to content

Improve SSE response handling and URI construction in SseClientSessio… #251

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

Merged
merged 2 commits into from
Apr 9, 2025

Conversation

239573049
Copy link
Contributor

This pull request includes several changes to the SseClientSessionTransport class to improve the handling of SSE messages and URI composition. The most important changes include enhancing the conditions for checking response content and refactoring the URI composition logic.

Enhancements to response content checks:

  • Updated the condition to check if responseContent is either empty or equals "accepted" in two locations within the SendMessageAsync method. [1] [2]

Refactoring URI composition:

  • Refactored the HandleEndpointEvent method to use UriBuilder for more robust URI composition, including removing the manual string concatenation and handling the "/sse" suffix more effectively.

@239573049
Copy link
Contributor Author

#250 in order to solve this problem

Copy link
Contributor

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

Normally, I'd ask for tests. But I'm working to support quickly support the 2025-03-26 Streamable HTTP spec right now, so I'll add some tests in a follow up.

https://modelcontextprotocol.io/specification/2025-03-26/basic/transports#streamable-http

@halter73 halter73 merged commit cae5dff into modelcontextprotocol:main Apr 9, 2025
8 checks passed
@halter73
Copy link
Contributor

halter73 commented Apr 9, 2025

Thanks!

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.

2 participants