From 9d642f3bb1dcf8380822b025adabb06262364ef2 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Mon, 3 Nov 2025 10:13:52 -0800 Subject: [PATCH] refactor(core): improve error handling for setGlobalProxy (#12437) --- packages/core/src/config/config.test.ts | 71 +++++++++++++++++++++++++ packages/core/src/config/config.ts | 14 ++++- packages/core/src/tools/web-fetch.ts | 10 +--- packages/core/src/utils/fetch.ts | 6 +-- 4 files changed, 85 insertions(+), 16 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index ae5ddaa573..e50b31829d 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -145,6 +145,20 @@ vi.mock('../agents/subagent-tool-wrapper.js', () => ({ SubagentToolWrapper: vi.fn(), })); +const mockCoreEvents = vi.hoisted(() => ({ + emitFeedback: vi.fn(), +})); + +const mockSetGlobalProxy = vi.hoisted(() => vi.fn()); + +vi.mock('../utils/events.js', () => ({ + coreEvents: mockCoreEvents, +})); + +vi.mock('../utils/fetch.js', () => ({ + setGlobalProxy: mockSetGlobalProxy, +})); + import { BaseLlmClient } from '../core/baseLlmClient.js'; import { tokenLimit } from '../core/tokenLimits.js'; import { uiTelemetryService } from '../telemetry/index.js'; @@ -912,6 +926,63 @@ describe('Server Config (config.ts)', () => { expect(config.getTruncateToolOutputThreshold()).toBe(50000); }); }); + + describe('Proxy Configuration Error Handling', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should call setGlobalProxy when proxy is configured', () => { + const paramsWithProxy: ConfigParameters = { + ...baseParams, + proxy: 'http://proxy.example.com:8080', + }; + new Config(paramsWithProxy); + + expect(mockSetGlobalProxy).toHaveBeenCalledWith( + 'http://proxy.example.com:8080', + ); + }); + + it('should not call setGlobalProxy when proxy is not configured', () => { + new Config(baseParams); + + expect(mockSetGlobalProxy).not.toHaveBeenCalled(); + }); + + it('should emit error feedback when setGlobalProxy throws an error', () => { + const proxyError = new Error('Invalid proxy URL'); + mockSetGlobalProxy.mockImplementation(() => { + throw proxyError; + }); + + const paramsWithProxy: ConfigParameters = { + ...baseParams, + proxy: 'invalid-proxy', + }; + new Config(paramsWithProxy); + + expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith( + 'error', + 'Invalid proxy configuration detected. Check debug drawer for more details (F12)', + proxyError, + ); + }); + + it('should not emit error feedback when setGlobalProxy succeeds', () => { + mockSetGlobalProxy.mockImplementation(() => { + // Success - no error thrown + }); + + const paramsWithProxy: ConfigParameters = { + ...baseParams, + proxy: 'http://proxy.example.com:8080', + }; + new Config(paramsWithProxy); + + expect(mockCoreEvents.emitFeedback).not.toHaveBeenCalled(); + }); + }); }); describe('setApprovalMode with folder trust', () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index c610202bf8..8dab6cbaa4 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -76,6 +76,7 @@ import type { UserTierId } from '../code_assist/types.js'; import { AgentRegistry } from '../agents/registry.js'; import { setGlobalProxy } from '../utils/fetch.js'; import { SubagentToolWrapper } from '../agents/subagent-tool-wrapper.js'; +import { coreEvents } from '../utils/events.js'; export enum ApprovalMode { DEFAULT = 'default', @@ -582,8 +583,17 @@ export class Config { initializeTelemetry(this); } - if (this.getProxy()) { - setGlobalProxy(this.getProxy() as string); + const proxy = this.getProxy(); + if (proxy) { + try { + setGlobalProxy(proxy); + } catch (error) { + coreEvents.emitFeedback( + 'error', + 'Invalid proxy configuration detected. Check debug drawer for more details (F12)', + error, + ); + } } this.geminiClient = new GeminiClient(this); this.modelRouterService = new ModelRouterService(this); diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index c914885af9..c3ffe373a4 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -21,11 +21,7 @@ import { getErrorMessage } from '../utils/errors.js'; import type { Config } from '../config/config.js'; import { ApprovalMode, DEFAULT_GEMINI_FLASH_MODEL } from '../config/config.js'; import { getResponseText } from '../utils/partUtils.js'; -import { - fetchWithTimeout, - isPrivateIp, - setGlobalProxy, -} from '../utils/fetch.js'; +import { fetchWithTimeout, isPrivateIp } from '../utils/fetch.js'; import { convert } from 'html-to-text'; import { logWebFetchFallbackAttempt, @@ -415,10 +411,6 @@ export class WebFetchTool extends BaseDeclarativeTool< false, // canUpdateOutput messageBus, ); - const proxy = config.getProxy(); - if (proxy) { - setGlobalProxy(proxy); - } } protected override validateToolParamValues( diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index ffe6165507..ba9bb83c80 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -58,9 +58,5 @@ export async function fetchWithTimeout( } export function setGlobalProxy(proxy: string) { - try { - setGlobalDispatcher(new ProxyAgent(proxy)); - } catch (e) { - console.error(`Failed to set proxy: ${getErrorMessage(e)}`); - } + setGlobalDispatcher(new ProxyAgent(proxy)); }