add custom headers on initial _startOrAuth call #318
+34
−7
Conversation
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
|
This fix for |
|
@ihrpr Could you please help with the merge? Thanks! |
Pizzaface
pushed a commit
to RewstApp/mcp-inspector
that referenced
this pull request
May 2, 2025
Update README requirement node js version
ochafik
requested changes
Jun 30, 2025
There was a problem hiding this comment.
I think we should update the commonHeaders method & align it w/ the streamableHttp version. Something like:
private async _commonHeaders(): Promise<Headers> {
const headers: HeadersInit = {};
if (this._authProvider) {
const tokens = await this._authProvider.tokens();
if (tokens) {
headers["Authorization"] = `Bearer ${tokens.access_token}`;
}
}
return new Headers(
{ ...headers, ...this._requestInit?.headers }
);
}Could you please also update the header usage in the send method?
const headers = await this._commonHeaders();
headers.set("content-type", "application/json");
Comment on lines
+117
to
+124
| const commonHeaders = await this._commonHeaders(); | ||
| const allHeaders = { ...commonHeaders, ...this._requestInit?.headers}; | ||
| return fetch(url, { | ||
| ...init, | ||
| headers: { | ||
| ...allHeaders, | ||
| Accept: "text/event-stream" | ||
| } |
There was a problem hiding this comment.
Suggested change
| const commonHeaders = await this._commonHeaders(); | |
| const allHeaders = { ...commonHeaders, ...this._requestInit?.headers}; | |
| return fetch(url, { | |
| ...init, | |
| headers: { | |
| ...allHeaders, | |
| Accept: "text/event-stream" | |
| } | |
| const headers = await this._commonHeaders(); | |
| headers.set("Accept", "text/event-stream"); | |
| return fetch(url, { | |
| ...init, | |
| headers, | |
| }) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Ensure that custom headers provided in
this._requestInit?.headersare included in the initial call to the/sseendpoint.Motivation and Context
Fixes #317, where custom headers (e.g.
Host,X-*) were not sent in the initial/sserequest, but were correctly sent in/messages. This is needed for MCP servers behind proxies that rely on these headers.This PR aligns the behavior of the
/sserequest withsend()by ensuring that all custom headers (when provided) are forwarded appropriately.How Has This Been Tested?
/sserequest.Breaking Changes
No
Types of changes
Checklist
Additional context
N/a