add functions for proxy on adjust headers #275
Conversation
WalkthroughThe pull request introduces enhancements to the WebSocket client configuration by adding two new features to the Changes
Sequence DiagramsequenceDiagram
participant Client
participant ClientOptions
participant WebSocketDialer
participant ProxyConfig
participant HeaderProcessor
Client->>ClientOptions: Configure Proxy and Header Processor
Client->>WebSocketDialer: Establish Connection
WebSocketDialer->>ProxyConfig: Apply Proxy Settings
WebSocketDialer->>HeaderProcessor: Process WebSocket Headers
WebSocketDialer->>Client: Return WebSocket Connection
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🧹 Nitpick comments (2)
pkg/client/interfaces/v1/types-client.go (1)
19-20: LGTM! Consider enhancing the documentation.The new fields provide flexible solutions for proxy configuration and header processing. The implementation aligns well with Go best practices by using function types.
Consider adding more detailed documentation for the new fields:
- Proxy func(*http.Request) (*url.URL, error) // provide function for proxy -- e.g. http.ProxyFromEnvironment - WSHeaderProcessor func(http.Header) // process headers before dialing for websocket connection + // Proxy specifies the function to determine the proxy for a given request. + // For example, http.ProxyFromEnvironment can be used to respect standard proxy environment variables. + Proxy func(*http.Request) (*url.URL, error) + + // WSHeaderProcessor allows custom processing of WebSocket headers before the connection is established. + // This can be used to modify or remove headers that might interfere with the WebSocket handshake. + WSHeaderProcessor func(http.Header)pkg/client/common/v1/websocket.go (1)
202-202: LGTM! Consider reducing code duplication in dialer configuration.The proxy configuration is correctly set for both secure and non-secure connections.
Consider reducing code duplication by extracting common dialer options:
+ // Common dialer options + dialerOpts := websocket.Dialer{ + HandshakeTimeout: 15 * time.Second, + RedirectService: c.cOptions.RedirectService, + Proxy: c.cOptions.Proxy, + } + // if host starts with "ws://", then disable TLS - var dialer websocket.Dialer if url[:5] == "ws://" { - dialer = websocket.Dialer{ - HandshakeTimeout: 15 * time.Second, - RedirectService: c.cOptions.RedirectService, - Proxy: c.cOptions.Proxy, - } + dialer = dialerOpts } else { - dialer = websocket.Dialer{ - HandshakeTimeout: 15 * time.Second, - /* #nosec G402 */ - TLSClientConfig: &tls.Config{InsecureSkipVerify: c.cOptions.SkipServerAuth}, - RedirectService: c.cOptions.RedirectService, - SkipServerAuth: c.cOptions.SkipServerAuth, - Proxy: c.cOptions.Proxy, - } + dialer = dialerOpts + dialer.TLSClientConfig = &tls.Config{InsecureSkipVerify: c.cOptions.SkipServerAuth} + dialer.SkipServerAuth = c.cOptions.SkipServerAuth }Also applies to: 211-211
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/client/common/v1/websocket.go(3 hunks)pkg/client/interfaces/v1/types-client.go(1 hunks)
🔇 Additional comments (1)
pkg/client/common/v1/websocket.go (1)
162-164: LGTM! Header processing is implemented correctly.The header processor is called at the right time, after all standard headers are set but before the connection is established.
|
Testing this now, thanks @tomboulossf! |
There was a problem hiding this comment.
This looks good to me, and similar to something we do on the Node SDK. Pending testing, this is a great addition @jpvajda
Proposed changes
When operating Deepgram on-premises in a k8s environment using istio service-mesh, the websocket handshake fails to complete with the standard setup. In our environment the proxy configuration options are provided via the standard environment variables, so resolving the proxy via the golang net/http standard function performs the proper proxy configuration. The presence of the "Host" header still prevents a successful websocket setup to be established, but rather than simply remove that header, providing the ability of the caller to modify headers prior to websocket setup appeared a preferable solution .
Types of changes
What types of changes does your code introduce to the community Go SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
Summary by CodeRabbit
New Features
Improvements