fix(providers): disable proxy for local endpoints, respect env proxy for cloud - #4367
Conversation
…for cloud When the host has HTTP_PROXY / HTTPS_PROXY / ALL_PROXY set, httpx routes all traffic through the proxy — including requests to localhost or LAN addresses that the proxy typically cannot reach. This breaks local model servers (Ollama, llama.cpp, vLLM) silently. - Local endpoints: pass transport=httpx.AsyncHTTPTransport(proxy=None) so proxy env vars are ignored for local traffic. - Cloud endpoints: pass trust_env=True so corporate/VPN proxies work without explicit configuration. Fixes HKUDS#4366
chengyongru
left a comment
There was a problem hiding this comment.
Thanks for this PR! The local endpoint proxy bypass is well done — using an explicit transport=httpx.AsyncHTTPTransport(proxy=None) is the correct way to skip proxy env vars, since httpx sets allow_env_proxies = trust_env and transport is None, so any non-None transport effectively disables env proxy reading.
However, the cloud branch introduces a regression.
Cloud endpoint client loses SDK defaults
Before this PR, cloud endpoints passed http_client=None to AsyncOpenAI, so the SDK created its own DefaultAsyncHttpxClient with sensible defaults:
follow_redirects=Truelimits=Limits(max_connections=1000, max_keepalive_connections=100, keepalive_expiry=5.0)trust_env=True(readsHTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY)
The new else branch creates a bare httpx.AsyncClient(timeout=timeout_s, trust_env=True), which replaces that with:
follow_redirects=False(httpx default) — breaks any cloud provider that returns 3xx redirects- No connection pool limits — unbounded connections under concurrency
trust_env=Trueis a no-op — it's already httpx's default
Suggestion: The simplest fix is to remove the else branch entirely and let http_client stay None for cloud endpoints. The SDK's default client already reads proxy env vars through its own trust_env=True, so the stated goal (respect proxy env for cloud) is already met without any code change on that path.
if self._is_local:
_local_limits = httpx.Limits(keepalive_expiry=0)
http_client = httpx.AsyncClient(
limits=_local_limits,
timeout=timeout_s,
transport=httpx.AsyncHTTPTransport(proxy=None, limits=_local_limits),
)
# else: http_client stays None → SDK creates DefaultAsyncHttpxClient
# which already reads proxy env vars via trust_env=True
If there's a specific reason a custom client is needed for cloud, please carry over the SDK defaults explicitly (follow_redirects=True, proper limits).
This review was generated by nanobot — an AI agent for code review.
| http_client = httpx.AsyncClient( | ||
| timeout=timeout_s, | ||
| trust_env=True, | ||
| ) |
There was a problem hiding this comment.
This custom client for cloud endpoints silently drops two SDK defaults that the pre-PR code path had:
-
follow_redirects: httpx defaults toFalse, but the SDK'sDefaultAsyncHttpxClientsets it toTrue. Some cloud providers return 3xx redirects, which would now break. -
Connection pool limits: the SDK sets
Limits(max_connections=1000, max_keepalive_connections=100, keepalive_expiry=5.0). This bare client has no limits at all.
Also, trust_env=True here is already httpx's default, so it's redundant — the real effect of the pre-PR http_client=None path was letting the SDK create its own client with trust_env=True baked in, which already respects proxy env vars.
The simplest fix: remove this else branch and leave http_client as None for cloud endpoints.
…y defaults chengyongru reviewed HKUDS#4367 and identified that the cloud branch created a bare httpx.AsyncClient that lacked the SDK's default settings (follow_redirects, connection pool limits). Since the SDK's DefaultAsyncHttpxClient already has trust_env=True and proper defaults, the simplest fix is to let http_client stay None for cloud endpoints. Also updated the test to match the new behavior (http_client is None).
|
Good catch, thanks for the detailed review. Removed the Updated the test to match (asserts |
chengyongru
left a comment
There was a problem hiding this comment.
Thanks for the fix! The local-endpoint proxy bypass is correct and well-tested.
However, the cloud-endpoint branch introduces a regression: by creating a custom httpx.AsyncClient(timeout=timeout_s, trust_env=True) instead of leaving http_client=None (which lets the OpenAI SDK use DefaultAsyncHttpxClient), the cloud branch loses:
follow_redirects=True—DefaultAsyncHttpxClientsets this,httpx.AsyncClientdefaults toFalse. Cloud providers occasionally return redirects.- Connection pool limits —
DefaultAsyncHttpxClientsetslimits=Limits(max_connections=1000, max_keepalive_connections=100), custom client has unbounded pool. trust_env=Trueis redundant — httpx already defaults toTrue.
The fix: keep http_client=None (the original behavior) for the cloud path. The SDK's default client already respects proxy env vars via trust_env=True default. If you must create a custom client, add follow_redirects=True and connection limits to match.
Reviewed by nanobot
chengyongru
left a comment
There was a problem hiding this comment.
The author already addressed the cloud-endpoint regression from the previous review. The else branch with the custom httpx.AsyncClient is gone — cloud endpoints now correctly keep http_client=None, letting the SDK's DefaultAsyncHttpxClient handle proxy env vars, connection pool limits, and redirects.
The local-endpoint fix is solid: transport=httpx.AsyncHTTPTransport(proxy=None) cleanly bypasses proxy env vars for localhost/LAN addresses. Tests cover both local and cloud paths.
Looks good to merge.
Reviewed by nanobot
Fixes #4366
Problem
When the host has HTTP_PROXY / HTTPS_PROXY / ALL_PROXY set,
httpx.AsyncClientroutes all traffic through the proxy — including requests to localhost or LAN addresses that the proxy typically cannot reach. This silently breaks local model servers (Ollama, llama.cpp, vLLM) behind a proxy-configured host.Fix
Two changes in
openai_compat_provider.py:Local endpoints: Create the httpx client with
transport=httpx.AsyncHTTPTransport(proxy=None)to explicitly bypass proxy for local traffic. Thelimitsparameter is also forwarded to the transport (httpx does not propagateclient.limitsto custom transports).Cloud endpoints: Create the httpx client with
trust_env=Trueso corporate/VPN proxies work without explicit nanobot configuration.How to Test
export HTTPS_PROXY=http://127.0.0.1:9999Automated tests:
Checklist
pytest tests/providers/ -v)