HTTP Headers Support for SSE MCP Client at Start #122
Conversation
WalkthroughThis pull request introduces a new method Changes
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
client/sse.go (1)
388-406:⚠️ Potential issueMissing custom headers in the initialized notification request.
While custom headers are applied in the
Startmethod andsendRequestmethod, they're not being applied to the notification request in theInitializemethod. If authentication headers are required for all requests to the server, this omission could cause the notification to fail.Apply this diff to fix the issue:
req.Header.Set("Content-Type", "application/json") +// set custom http headers +for k, v := range c.headers { + req.Header.Set(k, v) +} resp, err := c.httpClient.Do(req)
🧹 Nitpick comments (2)
client/sse.go (2)
44-48: Function looks good but could benefit from a doc comment.The implementation of
WithHeadersfollows the functional options pattern correctly. Consider adding a doc comment describing its purpose and usage examples.+// WithHeaders sets custom HTTP headers that will be included in all requests made by the client. +// This is particularly useful for authentication (e.g., bearer tokens, API keys) and other +// custom header requirements. func WithHeaders(headers map[string]string) ClientOption { return func(sc *SSEMCPClient) { sc.headers = headers } }
97-100: Consider adding header conflict protection.Currently, users can override standard headers like "Accept" or "Content-Type" through the custom headers map. This provides flexibility but could also lead to unexpected behavior if standard headers are accidentally overridden.
Consider implementing a safeguard against overriding critical headers:
// set custom http headers for k, v := range c.headers { + // Skip standard headers that should not be overridden + switch k { + case "Accept", "Cache-Control", "Connection", "Content-Type": + continue + } req.Header.Set(k, v) }Alternatively, you could emit a warning log when a standard header is being overridden if you prefer to maintain the flexibility.
Also applies to: 310-312
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/sse.go(1 hunks)
🔇 Additional comments (3)
client/sse.go (3)
38-38: Excellent addition of headers field to support custom HTTP headers.The addition of the
headersfield to theSSEMCPClientstruct provides a clean way to store and utilize custom HTTP headers, particularly useful for authentication scenarios as mentioned in the PR objectives.
71-71: Good practice initializing the headers map.Initializing the headers map in the constructor prevents nil map panics when attempting to add headers later.
97-100: Implementation for setting custom headers looks good.The loop properly adds all custom headers to the request, which fulfills the main objective of this PR.
…der conflict protection
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
client/sse.go (2)
1-611: Add a usage example in the package documentation.The PR objectives mention the use case of bearer token authentication, but there's no example in the code comments showing how to use this feature.
Consider adding an example at the package level documentation:
// Example: // // // Create a client with authentication headers // client, err := NewSSEMCPClient( // "https://mcp.example.com", // WithHeaders(map[string]string{ // "Authorization": "Bearer your-token-here", // }), // )
74-74: Use consistent comment formatting for custom headers sections.The comments for setting custom headers use slightly different formats across the codebase. Standardizing these comments would improve readability and maintainability.
Standardize comments to this format throughout the code:
- // set custom http headers + // Set custom HTTP headersAlso applies to: 101-102, 318-319, 412-413
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/sse.go(4 hunks)
🔇 Additional comments (5)
client/sse.go (5)
38-38: Good addition of the headers field to the struct.The addition of the
headersfield to theSSEMCPClientstruct enables the client to store custom HTTP headers, which is essential for the authentication feature described in the PR objectives.
44-51: Well-documented method for setting custom headers.The
WithHeadersfunction implementation follows the existing functional options pattern and has clear documentation that explains its purpose for authentication scenarios.
74-74: Proper initialization of the headers map.Initializing the headers map in the constructor ensures it's never nil, preventing potential nil pointer dereferences when iterating over the map.
100-108: Good implementation of custom headers with proper safeguards.The implementation correctly applies custom headers while protecting standard headers that should not be overridden. This preserves the expected behavior of the SSE connection.
318-325: Consistent header handling in sendRequest method.The implementation in the
sendRequestmethod follows the same pattern as in theStartmethod, ensuring that standard headers are not overridden.
…tion to Initialize
Overview
This change implements custom HTTP headers support for the SSE MCP Client into the
Startmethod of the SSE client, particularly useful for authentication scenarios with bearer token.Changes Made
Example Usage
To create a client with authentication:
Benefits
Use Cases
This change makes the SSE MCP Client more versatile by supporting authenticated connections while maintaining a clean and simple API.
Summary by CodeRabbit