feat(client/sse): allow custom fetch implementation#296
Closed
NitroRCr wants to merge 3 commits intomodelcontextprotocol:mainfrom
Closed
feat(client/sse): allow custom fetch implementation#296NitroRCr wants to merge 3 commits intomodelcontextprotocol:mainfrom
NitroRCr wants to merge 3 commits intomodelcontextprotocol:mainfrom
Conversation
bhosmer-ant
added a commit
that referenced
this pull request
May 8, 2025
When an SSE client connects with a custom path (e.g., /api/v1/custom/sse), ensure the endpoint URL maintains the same base path structure but with /messages instead of /sse. This fixes issues where custom endpoints were getting collapsed to the root path. Should fix issues reported in modelcontextprotocol/inspector#313 and #296 Tests demonstrating the issue were added in PR #439 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
bhosmer-ant
added a commit
that referenced
this pull request
May 12, 2025
When an SSE client connects with a custom path (e.g., /api/v1/custom/sse), ensure the endpoint URL maintains the same base path structure but with /messages instead of /sse. This fixes issues where custom endpoints were getting collapsed to the root path. Should fix issues reported in modelcontextprotocol/inspector#313 and #296 Tests demonstrating the issue were added in PR #439 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ihrpr
requested changes
Jun 26, 2025
Contributor
ihrpr
left a comment
ihrpr
left a comment
There was a problem hiding this comment.
Thank you for your contribution. The PR addresses a valid asymmetry issue but there are a few things that need to be considered:
- the auth flow needs to use custom fetch, tests are required for this functionality change, and the new
fetchoption needs documentation. - Consider also implementing the same feature for StreamableHTTPClientTransport to address issue #476.
- tests are missing
Member
|
Definitely need this in the Inspector, where I'm trying to bridge the WWW-Authenticate header across the two transports of the proxy. |
9 tasks
Member
|
Since this PR appears to be abandoned yet needed, I created #721 to carry on the work. We can close this PR when that one is merged. |
Contributor
|
Closing as #721 was merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add an optional
fetchparam to SSEClientTransportOptions to allow custom fetch implementation.Motivation and Context
For SSE transport, previously the
eventSourceInitparam allowed to customize thefetchfunction used by EventSource. However, thesendmethod still use the nativefetch, which was not customizable. In some use cases, we might want both to use a customfetchimplementation. For example, in Tauri/Capacitor applications , the frontend runs in a webview, and the nativefetchcannot perform cross-origin requests directly. Therefore, a customfetchfunction is needed to bypass CORS.How Has This Been Tested?
npm testpassed. Additionally, I have used the modified transport in AIaW with a customfetchfunction to test and verify cross-origin functionality.Breaking Changes
No
Types of changes
Checklist
Additional context