fix: Correctly handle proxy environment variables and arguments #820
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TLDR
This pull request fixes a critical issue where the NO_PROXY environment variable was ignored for LLM requests, preventing users in corporate environments from connecting to internal LLM servers. It also resolves a bug where --proxy='' failed to disable the proxy. The fix ensures that all proxy-related environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY) are respected, and the --proxy command-line argument works as expected.
Dive Deeper
Root Causes
NO_PROXY Ignored:
The
GeminiClientwas using Undici’sProxyAgentfor LLM requests, butProxyAgentdoes not support theNO_PROXYenvironment variable. As a result, requests to hosts listed inNO_PROXYwere still routed through the proxy.--proxy=''Bug:The configuration logic interpreted an empty string from the
--proxyflag as falsy, causing the system to fall back to proxy environment variables instead of disabling the proxy entirely.What This PR Changes
Proxy Agent Replacement:
Replaces
ProxyAgentwithEnvHttpProxyAgentin theGeminiClient.EnvHttpProxyAgentcorrectly reads and respects standard proxy environment variables (HTTP_PROXY,HTTPS_PROXY, andNO_PROXY), aligning LLM client behavior with the IDE client.Improved Proxy Configuration Logic:
The configuration now correctly interprets
--proxy=''as an explicit instruction to disable the proxy, rather than falling back to environment variables.Hybrid Handling of Proxy Settings:
The
setupProxyConfigurationmethod inGeminiClienthas been refactored to intelligently handle both the--proxyargument and environment variables, ensuring correct precedence (i.e., the--proxyflag overrides environment variables).Outcome
These changes result in a robust and predictable proxy implementation that:
HTTP_PROXY,HTTPS_PROXY,NO_PROXY),Reviewer Test Plan
To validate these changes, replicate the following steps:
Simulate a corporate proxy environment:
Exclude internal LLM server from proxy:
Run
qwenfor internal LLM (usingOPENAI_*variables):qwen "your prompt"your-internal-llm-host.comshould bypass the proxy.Test disabling proxy with
--proxy='':Test overriding proxy with
--proxy:http://a-different-proxy:8888instead of the environment-provided proxy.This reformatting aims to clearly break down the root causes, the implemented fixes, and the outcomes, with explicit steps for functional verification.
Testing Matrix
Linked issues / bugs
Resolves #756