websocket-client: add proxy-aware connector#31622
Conversation
3b5aa07 to
c35143f
Compare
| ) -> Result<(ConnectionInner, Response), WebSocketError> { | ||
| let stream: Box<dyn AsyncIo> = match proxy_route { | ||
| OutboundProxyRoute::TransportDefault => { | ||
| let (stream, response) = connect_async_tls_with_config( |
There was a problem hiding this comment.
TransportDefault can bypass environment proxies here. Both the default ReqwestDefault policy and the system-proxy fallback can reach this branch, but Tungstenite opens a direct TCP connection and does not read HTTPS_PROXY, HTTP_PROXY, ALL_PROXY, or NO_PROXY. Could we resolve the proxy settings before this point, so the dialer always gets either Proxy or Direct? It would be good to add a test for each path too.
There was a problem hiding this comment.
Thanks for flagging this. This repository enables the proxy feature on our pinned tokio-tungstenite fork. In that revision, the default connector calls ProxyConfig::from_env before opening TCP: https://github.com/openai-oss-forks/tokio-tungstenite/blob/0e5b2d73aa18dd9f0a50ee9ff199d5aef7594186/src/connect.rs#L108-L117. from_env honors HTTP_PROXY, HTTPS_PROXY, ALL_PROXY, and NO_PROXY: https://github.com/openai-oss-forks/tungstenite-rs/blob/4fffad30fe373adbdcffab9545e9e9bf4f2fc19f/src/proxy.rs#L54-L68 and https://github.com/openai-oss-forks/tungstenite-rs/blob/4fffad30fe373adbdcffab9545e9e9bf4f2fc19f/src/proxy.rs#L95-L119. So TransportDefault here means delegate environment fallback to the transport, rather than connect directly.
I added a comment beside this branch in 2df2dfa to make that contract explicit. The crate tests cover each route through public_connector_uses_factory_and_exposes_stream_and_sink (ReqwestDefault -> TransportDefault), direct_route_connects_secure_websocket, and the HTTP/HTTPS proxy tunnel tests. The pinned transport also has its own environment-proxy integration coverage: https://github.com/openai-oss-forks/tokio-tungstenite/blob/0e5b2d73aa18dd9f0a50ee9ff199d5aef7594186/tests/proxy_integration.rs#L13-L69. I avoided duplicating process-environment mutation in this crate, consistent with the repository test guidance.
c35143f to
2df2dfa
Compare
Why
The route-aware WebSocket connection setup in #31441 is transport infrastructure rather than Responses API protocol logic. Landing it first in a dedicated crate keeps
codex-apifocused on request and response behavior and makes the transport reusable by future WebSocket clients.WebSockets must also apply the same effective outbound proxy and custom-CA policy as HTTP without disabling the lower-latency WebSocket path. Requiring an
HttpClientFactorywhen constructing the connector makes proxy-policy resolution part of the API instead of an optional call-site convention.This PR is an independent prerequisite based directly on
main. After it merges, #31441 can rebase onto it and replace its in-crate connector with this API.What changed
codex-websocket-clientworkspace crate with aWebSocketConnectorconstructed from the effectiveHttpClientFactory.WebSocketConnectionas a uniformStreamandSink, hiding route-specific transport types from protocol clients.Review guide
codex-rs/websocket-client/src/lib.rsdefines the small public API and the factory-required policy invariant.codex-rs/websocket-client/src/dialer.rscontains DNS, TCP, proxy tunneling, TLS, and WebSocket handshake setup.codex-rs/websocket-client/src/dialer_tests.rsverifies the public stream, direct and proxied WSS paths, HTTPS port preservation, and Happy Eyeballs timing.Test plan
cargo check -p codex-websocket-client --testsjust test -p codex-websocket-clientcargo shearjust bazel-lock-check