Skip to content

Commit 9d642f3

Browse files
refactor(core): improve error handling for setGlobalProxy (#12437)
1 parent 19ea68b commit 9d642f3

4 files changed

Lines changed: 85 additions & 16 deletions

File tree

packages/core/src/config/config.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,20 @@ vi.mock('../agents/subagent-tool-wrapper.js', () => ({
145145
SubagentToolWrapper: vi.fn(),
146146
}));
147147

148+
const mockCoreEvents = vi.hoisted(() => ({
149+
emitFeedback: vi.fn(),
150+
}));
151+
152+
const mockSetGlobalProxy = vi.hoisted(() => vi.fn());
153+
154+
vi.mock('../utils/events.js', () => ({
155+
coreEvents: mockCoreEvents,
156+
}));
157+
158+
vi.mock('../utils/fetch.js', () => ({
159+
setGlobalProxy: mockSetGlobalProxy,
160+
}));
161+
148162
import { BaseLlmClient } from '../core/baseLlmClient.js';
149163
import { tokenLimit } from '../core/tokenLimits.js';
150164
import { uiTelemetryService } from '../telemetry/index.js';
@@ -912,6 +926,63 @@ describe('Server Config (config.ts)', () => {
912926
expect(config.getTruncateToolOutputThreshold()).toBe(50000);
913927
});
914928
});
929+
930+
describe('Proxy Configuration Error Handling', () => {
931+
beforeEach(() => {
932+
vi.clearAllMocks();
933+
});
934+
935+
it('should call setGlobalProxy when proxy is configured', () => {
936+
const paramsWithProxy: ConfigParameters = {
937+
...baseParams,
938+
proxy: 'http://proxy.example.com:8080',
939+
};
940+
new Config(paramsWithProxy);
941+
942+
expect(mockSetGlobalProxy).toHaveBeenCalledWith(
943+
'http://proxy.example.com:8080',
944+
);
945+
});
946+
947+
it('should not call setGlobalProxy when proxy is not configured', () => {
948+
new Config(baseParams);
949+
950+
expect(mockSetGlobalProxy).not.toHaveBeenCalled();
951+
});
952+
953+
it('should emit error feedback when setGlobalProxy throws an error', () => {
954+
const proxyError = new Error('Invalid proxy URL');
955+
mockSetGlobalProxy.mockImplementation(() => {
956+
throw proxyError;
957+
});
958+
959+
const paramsWithProxy: ConfigParameters = {
960+
...baseParams,
961+
proxy: 'invalid-proxy',
962+
};
963+
new Config(paramsWithProxy);
964+
965+
expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith(
966+
'error',
967+
'Invalid proxy configuration detected. Check debug drawer for more details (F12)',
968+
proxyError,
969+
);
970+
});
971+
972+
it('should not emit error feedback when setGlobalProxy succeeds', () => {
973+
mockSetGlobalProxy.mockImplementation(() => {
974+
// Success - no error thrown
975+
});
976+
977+
const paramsWithProxy: ConfigParameters = {
978+
...baseParams,
979+
proxy: 'http://proxy.example.com:8080',
980+
};
981+
new Config(paramsWithProxy);
982+
983+
expect(mockCoreEvents.emitFeedback).not.toHaveBeenCalled();
984+
});
985+
});
915986
});
916987

917988
describe('setApprovalMode with folder trust', () => {

packages/core/src/config/config.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import type { UserTierId } from '../code_assist/types.js';
7676
import { AgentRegistry } from '../agents/registry.js';
7777
import { setGlobalProxy } from '../utils/fetch.js';
7878
import { SubagentToolWrapper } from '../agents/subagent-tool-wrapper.js';
79+
import { coreEvents } from '../utils/events.js';
7980

8081
export enum ApprovalMode {
8182
DEFAULT = 'default',
@@ -582,8 +583,17 @@ export class Config {
582583
initializeTelemetry(this);
583584
}
584585

585-
if (this.getProxy()) {
586-
setGlobalProxy(this.getProxy() as string);
586+
const proxy = this.getProxy();
587+
if (proxy) {
588+
try {
589+
setGlobalProxy(proxy);
590+
} catch (error) {
591+
coreEvents.emitFeedback(
592+
'error',
593+
'Invalid proxy configuration detected. Check debug drawer for more details (F12)',
594+
error,
595+
);
596+
}
587597
}
588598
this.geminiClient = new GeminiClient(this);
589599
this.modelRouterService = new ModelRouterService(this);

packages/core/src/tools/web-fetch.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,7 @@ import { getErrorMessage } from '../utils/errors.js';
2121
import type { Config } from '../config/config.js';
2222
import { ApprovalMode, DEFAULT_GEMINI_FLASH_MODEL } from '../config/config.js';
2323
import { getResponseText } from '../utils/partUtils.js';
24-
import {
25-
fetchWithTimeout,
26-
isPrivateIp,
27-
setGlobalProxy,
28-
} from '../utils/fetch.js';
24+
import { fetchWithTimeout, isPrivateIp } from '../utils/fetch.js';
2925
import { convert } from 'html-to-text';
3026
import {
3127
logWebFetchFallbackAttempt,
@@ -415,10 +411,6 @@ export class WebFetchTool extends BaseDeclarativeTool<
415411
false, // canUpdateOutput
416412
messageBus,
417413
);
418-
const proxy = config.getProxy();
419-
if (proxy) {
420-
setGlobalProxy(proxy);
421-
}
422414
}
423415

424416
protected override validateToolParamValues(

packages/core/src/utils/fetch.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,5 @@ export async function fetchWithTimeout(
5858
}
5959

6060
export function setGlobalProxy(proxy: string) {
61-
try {
62-
setGlobalDispatcher(new ProxyAgent(proxy));
63-
} catch (e) {
64-
console.error(`Failed to set proxy: ${getErrorMessage(e)}`);
65-
}
61+
setGlobalDispatcher(new ProxyAgent(proxy));
6662
}

0 commit comments

Comments
 (0)