feat: support HTTP_PROXY environment variable for default HTTP client - #2547
Conversation
- Add Proxy: http.ProxyFromEnvironment to default transport - Allow users to set global proxy via Docker environment variables - Per-channel proxy settings still override global proxy - Fully backward compatible
WalkthroughA new Proxy field is added to the HTTP transport in InitHttpClient, leveraging Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
service/http_client.go (1)
137-145: Consider explicitly settingProxy: nilfor clarity.The SOCKS5 transport correctly uses a custom
DialContextto route traffic through the SOCKS5 proxy. For clarity and to prevent future confusion about proxy behavior, consider explicitly settingProxy: nilto document that no additional HTTP/HTTPS proxy layer should be applied.🔎 Optional clarification
client := &http.Client{ Transport: &http.Transport{ MaxIdleConns: common.RelayMaxIdleConns, MaxIdleConnsPerHost: common.RelayMaxIdleConnsPerHost, ForceAttemptHTTP2: true, + Proxy: nil, // SOCKS5 handles proxying via custom dialer DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { return dialer.Dial(network, addr) }, }, CheckRedirect: checkRedirect, }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
service/http_client.go
🔇 Additional comments (1)
service/http_client.go (1)
41-41: LGTM! Clean implementation of environment proxy support.The addition of
Proxy: http.ProxyFromEnvironmentcorrectly enables the default HTTP client to respect standard proxy environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY). This is the idiomatic Go approach and fully backward compatible.
Description
Enable the default HTTP client to respect
HTTP_PROXY,HTTPS_PROXY, andNO_PROXYenvironment variables by addingProxy: http.ProxyFromEnvironmentto the transport configuration.Motivation
Currently, setting proxy environment variables in Docker Compose has no effect on API requests because the custom
http.TransportinInitHttpClient()doesn't configure proxy support. Users have to manually set proxy URLs for each channel, which is tedious when managing hundreds of channels.Changes
service/http_client.go: AddedProxy: http.ProxyFromEnvironmentto the default transportSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.