[streamable-http] Add ability to override the http.Client #359
Conversation
WalkthroughA new functional option, WithHTTPBasicClient, has been introduced to the StreamableHTTPCOption type. This addition enables the injection of a custom *http.Client instance into a StreamableHTTP object by assigning it to the httpClient field. No other logic or behavior has been modified. Changes
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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}
client/transport/streamable_http.go
Outdated
| func WithHTTPClient(client *http.Client) StreamableHTTPCOption { | ||
| return func(sc *StreamableHTTP) { | ||
| sc.httpClient = client | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add input validation and documentation.
The implementation correctly follows the existing functional option pattern, but needs input validation and documentation.
+// WithHTTPClient allows setting a custom HTTP client for the StreamableHTTP transport.
+// If nil is passed, the function will panic when the client is used.
func WithHTTPClient(client *http.Client) StreamableHTTPCOption {
return func(sc *StreamableHTTP) {
+ if client == nil {
+ panic("http.Client cannot be nil")
+ }
sc.httpClient = client
}
}Note: Be aware that if you use both WithHTTPClient and WithHTTPTimeout, the order matters. WithHTTPClient will override any timeout previously set by WithHTTPTimeout.
Let me verify how the httpClient field is used throughout the codebase to confirm the nil validation is necessary:
🏁 Script executed:
#!/bin/bash
# Description: Check all usages of httpClient field to verify nil safety requirements
# Expected: Find usages that would panic if httpClient is nil
rg -A 3 -B 1 "\.httpClient\." --type goLength of output: 3364
Add nil check and documentation to WithHTTPClient
- Validate that the provided
*http.Clientis non-nil to avoid runtime panics when callingDo. - Add a doc comment explaining its purpose and that passing
nilwill panic. - Note that using
WithHTTPClientafterWithHTTPTimeoutwill discard any previously set timeout, whereas callingWithHTTPTimeoutlast applies the timeout to your custom client.
+// WithHTTPClient sets a custom HTTP client on the StreamableHTTP transport.
// Client must be non-nil; passing nil will cause a panic when the client is used.
func WithHTTPClient(client *http.Client) StreamableHTTPCOption {
- return func(sc *StreamableHTTP) {
+ return func(sc *StreamableHTTP) {
+ if client == nil {
+ panic("http.Client cannot be nil")
+ }
sc.httpClient = client
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func WithHTTPClient(client *http.Client) StreamableHTTPCOption { | |
| return func(sc *StreamableHTTP) { | |
| sc.httpClient = client | |
| } | |
| } | |
| // WithHTTPClient sets a custom HTTP client on the StreamableHTTP transport. | |
| // Client must be non-nil; passing nil will cause a panic when the client is used. | |
| func WithHTTPClient(client *http.Client) StreamableHTTPCOption { | |
| return func(sc *StreamableHTTP) { | |
| if client == nil { | |
| panic("http.Client cannot be nil") | |
| } | |
| sc.httpClient = client | |
| } | |
| } |
🤖 Prompt for AI Agents
In client/transport/streamable_http.go around lines 24 to 28, add a nil check in
the WithHTTPClient function to validate that the provided *http.Client is not
nil to prevent runtime panics. Also, add a doc comment above the function
explaining its purpose, that passing nil will cause a panic, and note that using
WithHTTPClient after WithHTTPTimeout will override any previously set timeout.
This ensures safer usage and clearer documentation.
Description
Fixes #<issue_number> (if applicable)
Type of Change
Checklist
MCP Spec Compliance
Additional Information
Summary by CodeRabbit