-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: add proxy support for Gemini API client #9466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Created proxy-aware fetch utility that respects HTTP_PROXY/HTTPS_PROXY env vars - Falls back to axios when native fetch fails with proxy settings - Updated GeminiHandler constructor to setup proxy-aware fetch - This fixes the 'TypeError: fetch failed sending request' error when behind a proxy Fixes #9465
Review completed. Found 5 issues that need attention:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| // Setup proxy-aware fetch for GoogleGenAI client | ||
| // This ensures that the Gemini API client respects system proxy settings | ||
| try { | ||
| setupGlobalProxyFetch() | ||
| } catch (error) { | ||
| console.warn("Failed to setup proxy-aware fetch for Gemini:", error) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This modifies global fetch for the entire application every time a GeminiHandler is constructed, not just for Gemini API calls. If multiple GeminiHandler instances are created (which happens in testing, multi-workspace scenarios, or when users switch models), this will repeatedly overwrite global fetch. Additionally, the original fetch is never stored before the first modification, so restoreOriginalFetch() cannot actually restore it. This approach affects all fetch calls across the entire VSCode extension, potentially breaking other API providers or extension features that rely on native fetch behavior.
Fix it with Roo Code or mention @roomote and request a fix.
| // Configure HTTP agent with proxy if needed | ||
| if (httpProxy) { | ||
| try { | ||
| const HttpsProxyAgent = require("https-proxy-agent").HttpsProxyAgent | ||
| agents.httpAgent = new HttpsProxyAgent(httpProxy) | ||
| } catch (error) { | ||
| // If https-proxy-agent is not available, we'll try another approach | ||
| console.warn("https-proxy-agent not available, proxy support may be limited") | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code uses require('https-proxy-agent') but this package is not declared in dependencies or devDependencies in package.json. When users install the extension, the https-proxy-agent package will not be available, causing proxy support to silently fail with console warnings. The try-catch blocks will swallow these errors, leaving users without proxy functionality. Either add https-proxy-agent to package.json dependencies or use a different approach that doesn't require external packages.
Fix it with Roo Code or mention @roomote and request a fix.
|
|
||
| import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata } from "../index" | ||
| import { BaseProvider } from "./base-provider" | ||
| import { setupGlobalProxyFetch, restoreOriginalFetch } from "../../utils/proxy-fetch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The restoreOriginalFetch function is imported but never used in this file. Since the global fetch is being modified in the constructor and there's no cleanup mechanism, this creates a permanent modification that persists throughout the application lifecycle. Either the function should be called in a destructor/cleanup method, or it should be removed from the imports if it's not needed.
Fix it with Roo Code or mention @roomote and request a fix.
| export function getProxyAgent(): { httpAgent?: http.Agent; httpsAgent?: https.Agent } { | ||
| const httpProxy = process.env.HTTP_PROXY || process.env.http_proxy | ||
| const httpsProxy = process.env.HTTPS_PROXY || process.env.https_proxy | ||
| const noProxy = process.env.NO_PROXY || process.env.no_proxy | ||
|
|
||
| const agents: { httpAgent?: http.Agent; httpsAgent?: https.Agent } = {} | ||
|
|
||
| // Configure HTTP agent with proxy if needed | ||
| if (httpProxy) { | ||
| try { | ||
| const HttpsProxyAgent = require("https-proxy-agent").HttpsProxyAgent | ||
| agents.httpAgent = new HttpsProxyAgent(httpProxy) | ||
| } catch (error) { | ||
| // If https-proxy-agent is not available, we'll try another approach | ||
| console.warn("https-proxy-agent not available, proxy support may be limited") | ||
| } | ||
| } | ||
|
|
||
| // Configure HTTPS agent with proxy if needed | ||
| if (httpsProxy) { | ||
| try { | ||
| const HttpsProxyAgent = require("https-proxy-agent").HttpsProxyAgent | ||
| agents.httpsAgent = new HttpsProxyAgent(httpsProxy) | ||
| } catch (error) { | ||
| // If https-proxy-agent is not available, we'll try another approach | ||
| console.warn("https-proxy-agent not available, proxy support may be limited") | ||
| } | ||
| } | ||
|
|
||
| return agents | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getProxyAgent function is defined but never called anywhere in the codebase. This 30-line function creates HttpsProxyAgent instances that are never utilized. Either this function should be integrated into the proxy logic or removed to avoid dead code.
Fix it with Roo Code or mention @roomote and request a fix.
| export function restoreOriginalFetch(): void { | ||
| // Store original fetch if not already stored | ||
| if (!(globalThis as any).__originalFetch && globalThis.fetch) { | ||
| ;(globalThis as any).__originalFetch = globalThis.fetch | ||
| } | ||
|
|
||
| // Restore original fetch | ||
| if ((globalThis as any).__originalFetch) { | ||
| ;(globalThis as any).fetch = (globalThis as any).__originalFetch | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The restoreOriginalFetch function attempts to store the original fetch after it may have already been overwritten by setupGlobalProxyFetch. This creates a circular reference where the "original" fetch is actually the proxy fetch. The original fetch should be stored in a module-level variable before any modifications occur, not when restoreOriginalFetch is called. This breaks any attempt to restore the true native fetch implementation.
Fix it with Roo Code or mention @roomote and request a fix.
Description
This PR attempts to address Issue #9465. Feedback and guidance are welcome.
Problem
Users behind corporate proxies were experiencing "TypeError: fetch failed sending request" errors when using the Gemini API with Roo Code. The root cause is that the @google/genai library uses native fetch which doesn't automatically respect system proxy settings (HTTP_PROXY/HTTPS_PROXY environment variables).
Solution
Testing
Impact
This fix enables users behind corporate proxies to use Gemini models with Roo Code, resolving the fetch failure errors they were experiencing.
Fixes #9465