Skip to content

Commit 3c5b5db

Browse files
authored
feat(core): use experiment flags for default fetch timeouts (#24261)
1 parent 986293b commit 3c5b5db

File tree

10 files changed

+164
-20
lines changed

10 files changed

+164
-20
lines changed

packages/cli/src/test-utils/mockConfig.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ export const createMockConfig = (overrides: Partial<Config> = {}): Config =>
136136
getRetryFetchErrors: vi.fn().mockReturnValue(true),
137137
getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
138138
getShellToolInactivityTimeout: vi.fn().mockReturnValue(300000),
139+
getRequestTimeoutMs: vi.fn().mockReturnValue(undefined),
139140
getShellExecutionConfig: vi.fn().mockReturnValue({
140141
sandboxManager: new NoopSandboxManager(),
141142
sanitizationConfig: {

packages/core/src/code_assist/experiments/flagNames.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export const ExperimentFlags = {
1919
GEMINI_3_1_PRO_LAUNCHED: 45760185,
2020
PRO_MODEL_NO_ACCESS: 45768879,
2121
GEMINI_3_1_FLASH_LITE_LAUNCHED: 45771641,
22+
DEFAULT_REQUEST_TIMEOUT: 45773134,
2223
} as const;
2324

2425
export type ExperimentFlagName =

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,58 @@ describe('Server Config (config.ts)', () => {
644644
},
645645
);
646646
});
647+
648+
describe('getRequestTimeoutMs', () => {
649+
it('should return undefined if the flag is not set', () => {
650+
const config = new Config(baseParams);
651+
expect(config.getRequestTimeoutMs()).toBeUndefined();
652+
});
653+
654+
it('should return timeout in milliseconds if flag is set', () => {
655+
const config = new Config({
656+
...baseParams,
657+
experiments: {
658+
flags: {
659+
[ExperimentFlags.DEFAULT_REQUEST_TIMEOUT]: {
660+
intValue: '30',
661+
},
662+
},
663+
experimentIds: [],
664+
},
665+
} as unknown as ConfigParameters);
666+
expect(config.getRequestTimeoutMs()).toBe(30000);
667+
});
668+
669+
it('should return undefined if intValue is not a valid integer', () => {
670+
const config = new Config({
671+
...baseParams,
672+
experiments: {
673+
flags: {
674+
[ExperimentFlags.DEFAULT_REQUEST_TIMEOUT]: {
675+
intValue: 'abc',
676+
},
677+
},
678+
experimentIds: [],
679+
},
680+
} as unknown as ConfigParameters);
681+
expect(config.getRequestTimeoutMs()).toBeUndefined();
682+
});
683+
684+
it('should return undefined if intValue is negative', () => {
685+
const config = new Config({
686+
...baseParams,
687+
experiments: {
688+
flags: {
689+
[ExperimentFlags.DEFAULT_REQUEST_TIMEOUT]: {
690+
intValue: '-10',
691+
},
692+
},
693+
experimentIds: [],
694+
},
695+
} as unknown as ConfigParameters);
696+
expect(config.getRequestTimeoutMs()).toBeUndefined();
697+
});
698+
});
647699
});
648700

649701
describe('refreshAuth', () => {
@@ -2078,8 +2130,18 @@ describe('BaseLlmClient Lifecycle', () => {
20782130
usageStatisticsEnabled: false,
20792131
};
20802132

2133+
it('should throw an error if getBaseLlmClient is called before experiments have been fetched', () => {
2134+
const config = new Config(baseParams);
2135+
// By default on a new Config instance, experiments are undefined
2136+
expect(() => config.getBaseLlmClient()).toThrow(
2137+
'BaseLlmClient not initialized. Ensure experiments have been fetched and configuration is ready.',
2138+
);
2139+
});
2140+
20812141
it('should throw an error if getBaseLlmClient is called before refreshAuth', () => {
20822142
const config = new Config(baseParams);
2143+
// Explicitly set experiments to avoid triggering the new missing-experiments error
2144+
config.setExperiments({ flags: {}, experimentIds: [] });
20832145
expect(() => config.getBaseLlmClient()).toThrow(
20842146
'BaseLlmClient not initialized. Ensure authentication has occurred and ContentGenerator is ready.',
20852147
);

packages/core/src/config/config.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ import {
160160
} from '../code_assist/experiments/experiments.js';
161161
import { AgentRegistry } from '../agents/registry.js';
162162
import { AcknowledgedAgentsService } from '../agents/acknowledgedAgents.js';
163-
import { setGlobalProxy } from '../utils/fetch.js';
163+
import { setGlobalProxy, updateGlobalFetchTimeouts } from '../utils/fetch.js';
164164
import { SubagentTool } from '../agents/subagent-tool.js';
165165
import { ExperimentFlags } from '../code_assist/experiments/flagNames.js';
166166
import { debugLogger } from '../utils/debugLogger.js';
@@ -1548,9 +1548,6 @@ export class Config implements McpContext, AgentLoopContext {
15481548
// Only assign to instance properties after successful initialization
15491549
this.contentGeneratorConfig = newContentGeneratorConfig;
15501550

1551-
// Initialize BaseLlmClient now that the ContentGenerator is available
1552-
this.baseLlmClient = new BaseLlmClient(this.contentGenerator, this);
1553-
15541551
const codeAssistServer = getCodeAssistServer(this);
15551552
const quotaPromise = codeAssistServer?.projectId
15561553
? this.refreshUserQuota()
@@ -1566,6 +1563,17 @@ export class Config implements McpContext, AgentLoopContext {
15661563
return undefined;
15671564
});
15681565

1566+
// Fetch experiments and update timeouts before continuing initialization
1567+
const experiments = await this.experimentsPromise;
1568+
1569+
const requestTimeoutMs = this.getRequestTimeoutMs();
1570+
if (requestTimeoutMs !== undefined) {
1571+
updateGlobalFetchTimeouts(requestTimeoutMs);
1572+
}
1573+
1574+
// Initialize BaseLlmClient now that the ContentGenerator and experiments are available
1575+
this.baseLlmClient = new BaseLlmClient(this.contentGenerator, this);
1576+
15691577
await quotaPromise;
15701578

15711579
const authType = this.contentGeneratorConfig.authType;
@@ -1585,9 +1593,6 @@ export class Config implements McpContext, AgentLoopContext {
15851593
this.setModel(DEFAULT_GEMINI_MODEL_AUTO);
15861594
}
15871595

1588-
// Fetch admin controls
1589-
const experiments = await this.experimentsPromise;
1590-
15911596
const adminControlsEnabled =
15921597
experiments?.flags[ExperimentFlags.ENABLE_ADMIN_CONTROLS]?.boolValue ??
15931598
false;
@@ -1633,6 +1638,11 @@ export class Config implements McpContext, AgentLoopContext {
16331638
getBaseLlmClient(): BaseLlmClient {
16341639
if (!this.baseLlmClient) {
16351640
// Handle cases where initialization might be deferred or authentication failed
1641+
if (!this.experiments) {
1642+
throw new Error(
1643+
'BaseLlmClient not initialized. Ensure experiments have been fetched and configuration is ready.',
1644+
);
1645+
}
16361646
if (this.contentGenerator) {
16371647
this.baseLlmClient = new BaseLlmClient(
16381648
this.getContentGenerator(),
@@ -3153,6 +3163,21 @@ export class Config implements McpContext, AgentLoopContext {
31533163
);
31543164
}
31553165

3166+
/**
3167+
* Returns the configured default request timeout in milliseconds.
3168+
*/
3169+
getRequestTimeoutMs(): number | undefined {
3170+
const flag =
3171+
this.experiments?.flags?.[ExperimentFlags.DEFAULT_REQUEST_TIMEOUT];
3172+
if (flag?.intValue !== undefined) {
3173+
const seconds = parseInt(flag.intValue, 10);
3174+
if (Number.isInteger(seconds) && seconds >= 0) {
3175+
return seconds * 1000; // Convert seconds to milliseconds
3176+
}
3177+
}
3178+
return undefined;
3179+
}
3180+
31563181
/**
31573182
* Returns whether Gemini 3.1 Flash Lite has been launched.
31583183
*

packages/core/src/core/baseLlmClient.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ describe('BaseLlmClient', () => {
102102
);
103103

104104
mockConfig = {
105+
getRequestTimeoutMs: vi.fn().mockReturnValue(undefined),
105106
getSessionId: vi.fn().mockReturnValue('test-session-id'),
106107
getContentGeneratorConfig: vi
107108
.fn()

packages/core/src/core/client.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ describe('Gemini Client (client.ts)', () => {
203203
authType: AuthType.USE_GEMINI,
204204
};
205205
mockConfig = {
206+
getRequestTimeoutMs: vi.fn().mockReturnValue(undefined),
206207
getContentGeneratorConfig: vi
207208
.fn()
208209
.mockReturnValue(contentGeneratorConfig),

packages/core/src/core/geminiChat.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ describe('GeminiChat', () => {
142142
let currentActiveModel = 'gemini-pro';
143143

144144
mockConfig = {
145+
getRequestTimeoutMs: vi.fn().mockReturnValue(undefined),
145146
get config() {
146147
return this;
147148
},

packages/core/src/core/geminiChat_network_retry.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ describe('GeminiChat Network Retries', () => {
8383
const testMessageBus = { publish: vi.fn(), subscribe: vi.fn() };
8484

8585
mockConfig = {
86+
getRequestTimeoutMs: vi.fn().mockReturnValue(undefined),
8687
get config() {
8788
return this;
8889
},

packages/core/src/utils/fetch.test.ts

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,37 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7+
import { updateGlobalFetchTimeouts } from './fetch.js';
78
import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest';
8-
import {
9-
isPrivateIp,
10-
isPrivateIpAsync,
11-
isAddressPrivate,
12-
fetchWithTimeout,
13-
} from './fetch.js';
149
import * as dnsPromises from 'node:dns/promises';
1510
import type { LookupAddress, LookupAllOptions } from 'node:dns';
1611
import ipaddr from 'ipaddr.js';
1712

13+
const { setGlobalDispatcher, Agent, ProxyAgent } = vi.hoisted(() => ({
14+
setGlobalDispatcher: vi.fn(),
15+
Agent: vi.fn(),
16+
ProxyAgent: vi.fn(),
17+
}));
18+
19+
vi.mock('undici', () => ({
20+
setGlobalDispatcher,
21+
Agent,
22+
ProxyAgent,
23+
}));
24+
1825
vi.mock('node:dns/promises', () => ({
1926
lookup: vi.fn(),
2027
}));
2128

29+
// Import after mocks are established
30+
const {
31+
isPrivateIp,
32+
isPrivateIpAsync,
33+
isAddressPrivate,
34+
fetchWithTimeout,
35+
setGlobalProxy,
36+
} = await import('./fetch.js');
37+
2238
// Mock global fetch
2339
const originalFetch = global.fetch;
2440
global.fetch = vi.fn();
@@ -183,4 +199,19 @@ describe('fetch utils', () => {
183199
);
184200
});
185201
});
202+
203+
describe('setGlobalProxy', () => {
204+
it('should configure ProxyAgent with experiment flag timeout', () => {
205+
const proxyUrl = 'http://proxy.example.com';
206+
updateGlobalFetchTimeouts(45773134);
207+
setGlobalProxy(proxyUrl);
208+
209+
expect(ProxyAgent).toHaveBeenCalledWith({
210+
uri: proxyUrl,
211+
headersTimeout: 45773134,
212+
bodyTimeout: 45773134,
213+
});
214+
expect(setGlobalDispatcher).toHaveBeenCalled();
215+
});
216+
});
186217
});

packages/core/src/utils/fetch.ts

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ import { Agent, ProxyAgent, setGlobalDispatcher } from 'undici';
1010
import ipaddr from 'ipaddr.js';
1111
import { lookup } from 'node:dns/promises';
1212

13-
const DEFAULT_HEADERS_TIMEOUT = 300000; // 5 minutes
14-
const DEFAULT_BODY_TIMEOUT = 300000; // 5 minutes
15-
1613
export class FetchError extends Error {
1714
constructor(
1815
message: string,
@@ -31,14 +28,36 @@ export class PrivateIpError extends Error {
3128
}
3229
}
3330

31+
let defaultTimeout = 300000; // 5 minutes
32+
let currentProxy: string | undefined = undefined;
33+
3434
// Configure default global dispatcher with higher timeouts
3535
setGlobalDispatcher(
3636
new Agent({
37-
headersTimeout: DEFAULT_HEADERS_TIMEOUT,
38-
bodyTimeout: DEFAULT_BODY_TIMEOUT,
37+
headersTimeout: defaultTimeout,
38+
bodyTimeout: defaultTimeout,
3939
}),
4040
);
4141

42+
export function updateGlobalFetchTimeouts(timeoutMs: number) {
43+
if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) {
44+
throw new RangeError(
45+
`Invalid timeout value: ${timeoutMs}. Must be a positive finite number.`,
46+
);
47+
}
48+
defaultTimeout = timeoutMs;
49+
if (currentProxy) {
50+
setGlobalProxy(currentProxy);
51+
} else {
52+
setGlobalDispatcher(
53+
new Agent({
54+
headersTimeout: defaultTimeout,
55+
bodyTimeout: defaultTimeout,
56+
}),
57+
);
58+
}
59+
}
60+
4261
/**
4362
* Sanitizes a hostname by stripping IPv6 brackets if present.
4463
*/
@@ -191,11 +210,12 @@ export async function fetchWithTimeout(
191210
}
192211

193212
export function setGlobalProxy(proxy: string) {
213+
currentProxy = proxy;
194214
setGlobalDispatcher(
195215
new ProxyAgent({
196216
uri: proxy,
197-
headersTimeout: DEFAULT_HEADERS_TIMEOUT,
198-
bodyTimeout: DEFAULT_BODY_TIMEOUT,
217+
headersTimeout: defaultTimeout,
218+
bodyTimeout: defaultTimeout,
199219
}),
200220
);
201221
}

0 commit comments

Comments
 (0)