Skip to content

Commit 0776627

Browse files
authored
Merge pull request #2420 from huww98/fix/ctrl-y-retry-rate-limit
feat: allow Ctrl+Y to skip rate-limit retry delay immediately
2 parents 4c594e2 + 8712670 commit 0776627

5 files changed

Lines changed: 234 additions & 15 deletions

File tree

packages/cli/src/ui/hooks/useGeminiStream.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type {
99
Config,
1010
EditorType,
1111
GeminiClient,
12+
RetryInfo,
1213
ServerGeminiChatCompressedEvent,
1314
ServerGeminiContentEvent as ContentEvent,
1415
ServerGeminiFinishedEvent,
@@ -272,6 +273,7 @@ export const useGeminiStream = (
272273
*/
273274
const clearRetryCountdown = useCallback(() => {
274275
stopRetryCountdownTimer();
276+
skipRetryDelayRef.current = null;
275277
setPendingRetryErrorItem(null);
276278
setPendingRetryCountdownItem(null);
277279
}, [
@@ -280,14 +282,14 @@ export const useGeminiStream = (
280282
stopRetryCountdownTimer,
281283
]);
282284

285+
// Holds the skipDelay callback from the current rate-limit RetryInfo.
286+
// Managed symmetrically: set in startRetryCountdown, cleared in clearRetryCountdown.
287+
const skipRetryDelayRef = useRef<(() => void) | null>(null);
288+
283289
const startRetryCountdown = useCallback(
284-
(retryInfo: {
285-
message?: string;
286-
attempt: number;
287-
maxRetries: number;
288-
delayMs: number;
289-
}) => {
290+
(retryInfo: RetryInfo) => {
290291
stopRetryCountdownTimer();
292+
skipRetryDelayRef.current = retryInfo.skipDelay;
291293
const startTime = Date.now();
292294
const { message, attempt, maxRetries, delayMs } = retryInfo;
293295
const retryReasonText =
@@ -1391,6 +1393,15 @@ export const useGeminiStream = (
13911393
* when the user presses Ctrl+Y (bound to Command.RETRY_LAST in keyBindings.ts).
13921394
*/
13931395
const retryLastPrompt = useCallback(async () => {
1396+
// During a rate-limit retry countdown, skip the delay so the generator
1397+
// retries immediately — no abort/re-submit needed.
1398+
if (skipRetryDelayRef.current) {
1399+
skipRetryDelayRef.current();
1400+
skipRetryDelayRef.current = null;
1401+
clearRetryCountdown();
1402+
return;
1403+
}
1404+
13941405
if (
13951406
streamingState === StreamingState.Responding ||
13961407
streamingState === StreamingState.WaitingForConfirmation

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

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,6 +1146,162 @@ describe('GeminiChat', async () => {
11461146
}
11471147
});
11481148

1149+
it('should retry immediately when skipDelay is called during rate-limit wait', async () => {
1150+
vi.useFakeTimers();
1151+
1152+
try {
1153+
const tpmError = new StreamContentError(
1154+
'{"error":{"code":"429","message":"Throttling: TPM(1/1)"}}',
1155+
);
1156+
const successStream = (async function* () {
1157+
yield {
1158+
candidates: [
1159+
{
1160+
content: { parts: [{ text: 'Success after skip' }] },
1161+
finishReason: 'STOP',
1162+
},
1163+
],
1164+
} as unknown as GenerateContentResponse;
1165+
})();
1166+
1167+
vi.mocked(mockContentGenerator.generateContentStream)
1168+
.mockResolvedValueOnce(
1169+
(async function* () {
1170+
throw tpmError;
1171+
1172+
yield {} as GenerateContentResponse;
1173+
})(),
1174+
)
1175+
.mockResolvedValueOnce(successStream);
1176+
1177+
const stream = await chat.sendMessageStream(
1178+
'test-model',
1179+
{ message: 'test' },
1180+
'prompt-id-skip-delay',
1181+
);
1182+
1183+
const iterator = stream[Symbol.asyncIterator]();
1184+
// First event: RETRY with retryInfo containing skipDelay
1185+
const first = await iterator.next();
1186+
expect(first.value.type).toBe(StreamEventType.RETRY);
1187+
const skipDelay = first.value.retryInfo!.skipDelay!;
1188+
1189+
// Resume generator — it's now awaiting the 60s delay.
1190+
// Call skipDelay() to resolve it immediately instead of advancing timers.
1191+
const secondPromise = iterator.next();
1192+
skipDelay();
1193+
const second = await secondPromise;
1194+
1195+
// The generator should have continued to the next attempt immediately
1196+
expect(second.done).toBe(false);
1197+
expect(second.value.type).toBe(StreamEventType.RETRY); // retry-start marker
1198+
1199+
// Consume remaining events
1200+
const events: StreamEvent[] = [first.value, second.value];
1201+
for (;;) {
1202+
const next = await iterator.next();
1203+
if (next.done) break;
1204+
events.push(next.value);
1205+
}
1206+
1207+
expect(
1208+
mockContentGenerator.generateContentStream,
1209+
).toHaveBeenCalledTimes(2);
1210+
expect(
1211+
events.some(
1212+
(e) =>
1213+
e.type === StreamEventType.CHUNK &&
1214+
e.value.candidates?.[0]?.content?.parts?.[0]?.text ===
1215+
'Success after skip',
1216+
),
1217+
).toBe(true);
1218+
} finally {
1219+
vi.useRealTimers();
1220+
}
1221+
});
1222+
1223+
it('should exit retry loop when aborted during rate-limit delay', async () => {
1224+
vi.useFakeTimers();
1225+
1226+
try {
1227+
const tpmError = new StreamContentError(
1228+
'{"error":{"code":"429","message":"Throttling: TPM(1/1)"}}',
1229+
);
1230+
async function* failingStreamGenerator() {
1231+
throw tpmError;
1232+
1233+
yield {} as GenerateContentResponse;
1234+
}
1235+
1236+
const abortController = new AbortController();
1237+
1238+
vi.mocked(mockContentGenerator.generateContentStream)
1239+
.mockResolvedValueOnce(failingStreamGenerator())
1240+
// Should never be called — abort should prevent the second attempt
1241+
.mockResolvedValueOnce(failingStreamGenerator());
1242+
1243+
const stream = await chat.sendMessageStream(
1244+
'test-model',
1245+
{ message: 'test', config: { abortSignal: abortController.signal } },
1246+
'prompt-id-abort-delay',
1247+
);
1248+
1249+
const iterator = stream[Symbol.asyncIterator]();
1250+
// First event: RETRY with retryInfo
1251+
const first = await iterator.next();
1252+
expect(first.value.type).toBe(StreamEventType.RETRY);
1253+
1254+
// Abort while the generator is awaiting the 60s delay
1255+
const nextPromise = iterator.next();
1256+
abortController.abort();
1257+
1258+
// The generator should throw the abort error
1259+
await expect(nextPromise).rejects.toThrow();
1260+
1261+
// Only one API call should have been made (no retry after abort)
1262+
expect(
1263+
mockContentGenerator.generateContentStream,
1264+
).toHaveBeenCalledTimes(1);
1265+
1266+
// Verify the next sendMessageStream is not blocked by the old delay.
1267+
// If sendPromise were still pending, this would hang until the 60s
1268+
// timer fires — which never happens under fake timers, causing a timeout.
1269+
const nextStream = (async function* () {
1270+
yield {
1271+
candidates: [
1272+
{
1273+
content: { parts: [{ text: 'Next request OK' }] },
1274+
finishReason: 'STOP',
1275+
},
1276+
],
1277+
} as unknown as GenerateContentResponse;
1278+
})();
1279+
vi.mocked(mockContentGenerator.generateContentStream)
1280+
.mockReset()
1281+
.mockResolvedValueOnce(nextStream);
1282+
1283+
const stream2 = await chat.sendMessageStream(
1284+
'test-model',
1285+
{ message: 'follow-up' },
1286+
'prompt-id-after-abort',
1287+
);
1288+
const events: StreamEvent[] = [];
1289+
for await (const e of stream2) {
1290+
events.push(e);
1291+
}
1292+
expect(
1293+
events.some(
1294+
(e) =>
1295+
e.type === StreamEventType.CHUNK &&
1296+
e.value.candidates?.[0]?.content?.parts?.[0]?.text ===
1297+
'Next request OK',
1298+
),
1299+
).toBe(true);
1300+
} finally {
1301+
vi.useRealTimers();
1302+
}
1303+
});
1304+
11491305
it('should retry on GLM rate limit StreamContentError with backoff delay', async () => {
11501306
vi.useFakeTimers();
11511307

packages/core/src/core/geminiChat.ts

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,53 @@ const RATE_LIMIT_RETRY_OPTIONS = {
8585
delayMs: 60000,
8686
};
8787

88+
/**
89+
* Creates a promise that resolves after the specified delay, but can be
90+
* resolved early by calling the returned `skip` function.
91+
*
92+
* If an `AbortSignal` is provided and it fires before the delay completes,
93+
* the promise rejects so the caller's `await` throws and normal error
94+
* propagation takes over (e.g. the retry loop breaks and the generator exits).
95+
*/
96+
function delay(
97+
delayMs: number,
98+
signal?: AbortSignal,
99+
): {
100+
promise: Promise<void>;
101+
skip: () => void;
102+
} {
103+
let resolveRef: () => void;
104+
let timeoutId: ReturnType<typeof setTimeout>;
105+
106+
const promise = new Promise<void>((resolve, reject) => {
107+
resolveRef = resolve;
108+
109+
if (signal?.aborted) {
110+
reject(signal.reason);
111+
return;
112+
}
113+
114+
timeoutId = setTimeout(resolve, delayMs);
115+
116+
signal?.addEventListener(
117+
'abort',
118+
() => {
119+
clearTimeout(timeoutId);
120+
reject(signal.reason);
121+
},
122+
{ once: true },
123+
);
124+
});
125+
126+
return {
127+
promise,
128+
skip: () => {
129+
clearTimeout(timeoutId);
130+
resolveRef();
131+
},
132+
};
133+
}
134+
88135
/**
89136
* Returns true if the response is valid, false otherwise.
90137
*
@@ -353,18 +400,23 @@ export class GeminiChat {
353400
`Rate limit throttling detected (retry ${rateLimitRetryCount}/${maxRateLimitRetries}). ` +
354401
`Waiting ${delayMs / 1000}s before retrying...`,
355402
);
403+
const { promise: delayPromise, skip } = delay(
404+
delayMs,
405+
params.config?.abortSignal,
406+
);
356407
yield {
357408
type: StreamEventType.RETRY,
358409
retryInfo: {
359410
message,
360411
attempt: rateLimitRetryCount,
361412
maxRetries: maxRateLimitRetries,
362413
delayMs,
414+
skipDelay: skip,
363415
},
364416
};
365417
// Don't count rate-limit retries against the content retry limit
366418
attempt--;
367-
await new Promise((res) => setTimeout(res, delayMs));
419+
await delayPromise;
368420
continue;
369421
}
370422

@@ -397,7 +449,7 @@ export class GeminiChat {
397449
yield { type: StreamEventType.RETRY };
398450
// Don't count transient retries against content retry limit.
399451
attempt--;
400-
await new Promise((res) => setTimeout(res, delayMs));
452+
await delay(delayMs, params.config?.abortSignal).promise;
401453
continue;
402454
}
403455
// Transient budget exhausted — stop immediately.
@@ -418,13 +470,10 @@ export class GeminiChat {
418470
model,
419471
),
420472
);
421-
await new Promise((res) =>
422-
setTimeout(
423-
res,
424-
INVALID_CONTENT_RETRY_OPTIONS.initialDelayMs *
425-
(attempt + 1),
426-
),
427-
);
473+
await delay(
474+
INVALID_CONTENT_RETRY_OPTIONS.initialDelayMs * (attempt + 1),
475+
params.config?.abortSignal,
476+
).promise;
428477
continue;
429478
}
430479
}

packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ export * from './utils/projectSummary.js';
233233
export * from './utils/promptIdContext.js';
234234
export * from './utils/proxyUtils.js';
235235
export * from './utils/quotaErrorDetection.js';
236+
export * from './utils/rateLimit.js';
236237
export * from './utils/readManyFiles.js';
237238
export * from './utils/request-tokenizer/supportedImageFormats.js';
238239
export { TextTokenizer } from './utils/request-tokenizer/textTokenizer.js';

packages/core/src/utils/rateLimit.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ export interface RetryInfo {
2222
maxRetries: number;
2323
/** Delay in milliseconds before the retry happens. */
2424
delayMs: number;
25+
/** When called, resolves the delay promise early so the retry happens immediately. */
26+
skipDelay: () => void;
2527
}
2628

2729
/**

0 commit comments

Comments
 (0)