Skip to content

Commit b5934e0

Browse files
authored
Set delays to 0 (#2971)
1 parent ebd7212 commit b5934e0

File tree

2 files changed

+19
-37
lines changed

2 files changed

+19
-37
lines changed

src/extension/prompt/node/chatMLFetcher.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ export abstract class AbstractChatMLFetcher implements IChatMLFetcher {
8787

8888
export class ChatMLFetcherImpl extends AbstractChatMLFetcher {
8989

90+
/**
91+
* Delays (in ms) between connectivity check attempts before retrying a failed request.
92+
* Configurable for testing purposes.
93+
*/
94+
public connectivityCheckDelays = [1000, 10000, 10000];
95+
9096
constructor(
9197
@IFetcherService private readonly _fetcherService: IFetcherService,
9298
@ITelemetryService private readonly _telemetryService: ITelemetryService,
@@ -397,7 +403,7 @@ export class ChatMLFetcherImpl extends AbstractChatMLFetcher {
397403

398404
private async _checkNetworkConnectivity(useFetcher?: FetcherId): Promise<{ retryRequest: boolean; connectivityTestError?: string; connectivityTestErrorGitHubRequestId?: string }> {
399405
// Ping CAPI to check network connectivity before retrying
400-
const delays = [1000, 10000, 10000];
406+
const delays = this.connectivityCheckDelays;
401407
let connectivityTestError: string | undefined = undefined;
402408
let connectivityTestErrorGitHubRequestId: string | undefined = undefined;
403409
for (const delay of delays) {

src/extension/prompt/node/test/chatMLFetcherRetry.spec.ts

Lines changed: 12 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Raw } from '@vscode/prompt-tsx';
7-
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
7+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
88
import { IAuthenticationService } from '../../../../platform/authentication/common/authentication';
99
import { CopilotToken } from '../../../../platform/authentication/common/copilotToken';
1010
import { IFetchMLOptions } from '../../../../platform/chat/common/chatMLFetcher';
@@ -41,7 +41,6 @@ describe('ChatMLFetcherImpl retry logic', () => {
4141
let endpoint: IChatEndpoint;
4242

4343
beforeEach(() => {
44-
vi.useFakeTimers();
4544
disposables = new DisposableStore();
4645
cancellationTokenSource = disposables.add(new CancellationTokenSource());
4746

@@ -69,24 +68,15 @@ describe('ChatMLFetcherImpl retry logic', () => {
6968
configurationService,
7069
experimentationService,
7170
);
71+
72+
// Skip delays in tests for faster execution
73+
fetcher.connectivityCheckDelays = [0, 0, 0];
7274
});
7375

7476
afterEach(() => {
7577
disposables.dispose();
76-
vi.useRealTimers();
77-
vi.restoreAllMocks();
7878
});
7979

80-
/**
81-
* Advances fake timers in small increments, allowing microtasks to settle between advances.
82-
* This is more reliable than a single large advanceTimersByTimeAsync call for complex async chains.
83-
*/
84-
async function advanceTimersWithMicrotasks(totalMs: number, stepMs = 100): Promise<void> {
85-
for (let elapsed = 0; elapsed < totalMs; elapsed += stepMs) {
86-
await vi.advanceTimersByTimeAsync(Math.min(stepMs, totalMs - elapsed));
87-
}
88-
}
89-
9080
function createBaseOpts(): IFetchMLOptions {
9181
return {
9282
debugName: 'test',
@@ -106,10 +96,7 @@ describe('ChatMLFetcherImpl retry logic', () => {
10696
mockFetcherService.queueResponse(createSuccessResponse('{}')); // connectivity check
10797
mockFetcherService.queueResponse(createSuccessResponse('Hello!')); // retry
10898

109-
const resultPromise = fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
110-
// Advance timers to skip the 1000ms delay before connectivity check
111-
await advanceTimersWithMicrotasks(1000);
112-
const result = await resultPromise;
99+
const result = await fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
113100

114101
expect(result.type).toBe(ChatFetchResponseType.Success);
115102
expect(mockFetcherService.fetchCallCount).toBeGreaterThanOrEqual(2);
@@ -121,9 +108,7 @@ describe('ChatMLFetcherImpl retry logic', () => {
121108
mockFetcherService.queueResponse(createSuccessResponse('{}')); // connectivity check
122109
mockFetcherService.queueResponse(createSuccessResponse('Success!')); // retry
123110

124-
const resultPromise = fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
125-
await advanceTimersWithMicrotasks(1000);
126-
const result = await resultPromise;
111+
const result = await fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
127112

128113
expect(result.type).toBe(ChatFetchResponseType.Success);
129114
});
@@ -172,9 +157,7 @@ describe('ChatMLFetcherImpl retry logic', () => {
172157
mockFetcherService.queueResponse(createSuccessResponse('{}')); // connectivity check
173158
mockFetcherService.queueResponse(createSuccessResponse('Success!')); // retry
174159

175-
const resultPromise = fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
176-
await advanceTimersWithMicrotasks(1000);
177-
const result = await resultPromise;
160+
const result = await fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
178161

179162
expect(result.type).toBe(ChatFetchResponseType.Success);
180163
});
@@ -200,9 +183,7 @@ describe('ChatMLFetcherImpl retry logic', () => {
200183
mockFetcherService.queueResponse(createSuccessResponse('{}')); // connectivity check
201184
mockFetcherService.queueResponse(createSuccessResponse('Success!')); // retry
202185

203-
const resultPromise = fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
204-
await advanceTimersWithMicrotasks(1000);
205-
const result = await resultPromise;
186+
const result = await fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
206187

207188
expect(result.type).toBe(ChatFetchResponseType.Success);
208189
});
@@ -215,9 +196,7 @@ describe('ChatMLFetcherImpl retry logic', () => {
215196
mockFetcherService.queueResponse(createSuccessResponse('Success!')); // retry
216197

217198
// Should still retry on 500 even with invalid entry in config
218-
const resultPromise = fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
219-
await advanceTimersWithMicrotasks(1000);
220-
const result = await resultPromise;
199+
const result = await fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
221200

222201
expect(result.type).toBe(ChatFetchResponseType.Success);
223202
});
@@ -251,17 +230,14 @@ describe('ChatMLFetcherImpl retry logic', () => {
251230
it('does not retry server error when connectivity check fails', async () => {
252231
configurationService.setConfig(ConfigKey.TeamInternal.RetryServerErrorStatusCodes, '500,502');
253232

254-
// Order: 1) initial fetch → 500, 2) connectivity checks fail (3 attempts with delays)
233+
// Order: 1) initial fetch → 500, 2) connectivity checks fail (3 attempts)
255234
mockFetcherService.queueResponse(createErrorResponse(500, 'Internal Server Error'));
256-
// Connectivity check retries 3 times with delays [1000, 10000, 10000]
235+
// Connectivity check retries 3 times (with 0ms delays in tests)
257236
mockFetcherService.queueError(createNetworkError('ENOTFOUND')); // 1st connectivity check
258237
mockFetcherService.queueError(createNetworkError('ENOTFOUND')); // 2nd connectivity check
259238
mockFetcherService.queueError(createNetworkError('ENOTFOUND')); // 3rd connectivity check
260239

261-
const resultPromise = fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
262-
// Advance through all connectivity check delays: 1000 + 10000 + 10000 = 21000ms
263-
await advanceTimersWithMicrotasks(21000);
264-
const result = await resultPromise;
240+
const result = await fetcher.fetchMany(createBaseOpts(), cancellationTokenSource.token);
265241

266242
// Should fail because connectivity check never succeeded
267243
expect(result.type).toBe(ChatFetchResponseType.Failed);

0 commit comments

Comments
 (0)