google-gemini / google-gemini/gemini-cli

bug: mid-stream retry backoff is not abort-aware and retries content errors after cancellation

Open Beginner friendly
#29,027 0 comments 0 reactions 0 assignees View on GitHub
area/agent status/need-triage
Dominant language
TypeScript
Stars
107k
Forks
14.6k
Avg merge
2d 3h
Merged PRs (30d)
45

Description

## What happened?

The mid-stream retry loop in `GeminiChat` has two abort-handling gaps:

1. The backoff sleep uses a raw `setTimeout` promise that is not abort-aware — after the user cancels, the sleep still runs to completion (up to `initialDelayMs * 2^n`), and the retry `continue`s before the next attempt's entry check finally notices the abort.
2. For **content** errors (`isRetryableContentError`), the gating condition omits the `signal.aborted` check entirely, so an already-aborted stream consumes retry slots, emits `RETRY` stream events and `coreEvents.emitRetryAttempt` telemetry *after* cancellation, and re-enters `makeApiCallAndProcessStream()` (re-running hooks/model selection) before eventually throwing.

## Affected code

`packages/core/src/core/geminiChat.ts:668`:

```ts
if (isRetryableContentError || (isRetryable && !signal.aborted)) {
```

`packages/core/src/core/geminiChat.ts:707`:

```ts
await new Promise((res) => setTimeout(res, delayMs));
continue;
```

Note the repo already has an abort-aware helper used everywhere else, including `retry.ts`: `delay(ms, signal)` (`packages/core/src/utils/delay.ts`).

## How can this be reproduced?

1. Start a streaming request; make the first stream chunk fail with a retryable content error (or network error).
2. Abort the signal immediately (user presses ESC).
3. Observe: `emitRetryAttempt` fires post-abort, the raw timer keeps the loop alive for the full backoff delay, and another API call is attempted before unwinding.

## What did you expect to happen?

Cancellation should short-circuit immediately: no further retries, no retry telemetry/events after abort, no lingering timer.

## Impact

Cancellation lags seconds behind ESC in the failure window; spurious retry telemetry pollutes diagnostics; wasted model calls after the user gave up.

## Suggested direction

- Use `await delay(delayMs, signal)` instead of the raw promise.
- Gate both retry branches on `!signal.aborted` (i.e., `!signal.aborted && (isRetryable || isRetryableContentError)`).

---

*Found by source audit on current `main` (commit `5411f113c`); platform-independent. No open issue/PR covering this was found (searched: stream retry abort).*

Contributor guide

Open the contributing guide

Research direction

Start at packages/core/src/core/geminiChat.ts around lines 668 and 707, then read the abort-aware delay helper in packages/core/src/utils/delay.ts and its use in retry.ts. Verify the retry loop checks cancellation for both error types, aborts its backoff promptly, and produces no post-cancellation retry calls, events, or telemetry.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, cli
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
86/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.