-
Notifications
You must be signed in to change notification settings - Fork 664
[SSEClient] Add ability to override the http.Client #109
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
Conversation
WalkthroughThe pull request introduces a new function Changes
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 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 (
|
Could you add a simple test just so its documented how this is used? |
91ba3c1
to
b568a36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
client/transport/sse.go (2)
48-52
: Add documentation comment for the WithHTTPClient functionThe function is well-implemented, but it's missing a documentation comment that explains its purpose and usage. Consider adding a comment similar to other option functions in the file.
+// WithHTTPClient sets a custom HTTP client for the SSE transport. +// This allows users to configure custom timeouts, transport settings, or middleware. func WithHTTPClient(httpClient *http.Client) ClientOption { return func(sc *SSE) { sc.httpClient = httpClient } }
48-52
: Add safety check for nil HTTP clientThe function doesn't check if the provided HTTP client is nil. If a nil client is passed and used later, it will cause a panic when HTTP requests are made.
func WithHTTPClient(httpClient *http.Client) ClientOption { return func(sc *SSE) { + if httpClient == nil { + return + } sc.httpClient = httpClient } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/sse.go
(1 hunks)client/transport/sse.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/sse.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
client/transport/sse.go (2)
client/sse.go (1)
WithHTTPClient
(15-17)client/client.go (1)
Client
(16-24)
🔇 Additional comments (1)
client/transport/sse.go (1)
48-52
: The implementation follows the existing pattern correctlyThe implementation of
WithHTTPClient
follows the same functional options pattern used elsewhere in the codebase (likeWithHeaders
). This is a good approach for providing configurable options to users while maintaining backward compatibility.
b568a36
to
c04f42a
Compare
Done |
Summary by CodeRabbit