Skip to content

fix(core): Ensure zero-quota limits fail fast to prevent retry loop hang#27698

Open
luisfelipe-alt wants to merge 8 commits into
google-gemini:mainfrom
luisfelipe-alt:bugfix/WT-107_477249167
Open

fix(core): Ensure zero-quota limits fail fast to prevent retry loop hang#27698
luisfelipe-alt wants to merge 8 commits into
google-gemini:mainfrom
luisfelipe-alt:bugfix/WT-107_477249167

Conversation

@luisfelipe-alt

Copy link
Copy Markdown
Contributor

Fail fast on zero-quota limits to prevent retry loop hang

Summary

This PR fixes a critical bug in the error classification logic where the Gemini-CLI gets trapped in a futile 10-attempt retry loop when hitting a hard quota limit of 0 (e.g., on unbilled free-tier accounts). This bug causes the CLI to hang for up to 10 minutes before failing, severely degrading the user experience.

Details

  • The Bug: When the Gemini API returns a 429 error with limit: 0 and a "Please retry in 59.906331105s." message, classifyGoogleError matches the retry delay and incorrectly classifies it as a RetryableQuotaError.
  • The Fix: We added a defensive check in classifyGoogleError (inside packages/core/src/utils/googleQuotaErrors.ts) to intercept any error message containing "limit: 0" or "limit:0" and immediately classify it as a TerminalQuotaError. This check is executed before the retry delay regex is evaluated.
  • Impact: The CLI now fails fast on zero-quota limits, immediately propagating the error to the user instead of hanging for 10 minutes. Standard rate limits (which do not contain limit: 0) are unaffected and will still be retried with backoff as expected.

Related Issues

How to Validate

  1. Run the newly added unit test case to verify that limit: 0 errors are correctly classified as TerminalQuotaError:

    npx vitest run packages/core/src/utils/googleQuotaErrors.test.ts

    Expected Result: All 35 tests pass successfully, including the new test case should return TerminalQuotaError when limit is 0 even if message contains "Please retry in Xs".

  2. Run the retry utility tests to ensure no regressions in the core retry logic:

    npx vitest run packages/core/src/utils/retry.test.ts

    Expected Result: All 41 tests pass successfully.

Pre-Merge Checklist

  • Updated relevant documentation and README (if needed)
  • Added/updated tests (if needed)
  • Noted breaking changes (if any)
  • Validated on required platforms/methods:
    • MacOS
      • npm run
      • npx
      • Docker
      • Podman
      • Seatbelt
    • Windows
      • npm run
      • npx
      • Docker
    • Linux
      • npm run
      • npx
      • Docker

The error classification logic was matching the "Please retry in..." string and returning a RetryableQuotaError even when the error explicitly stated "limit: 0". This forced retryWithBackoff into a futile 10-attempt loop, causing a 10-minute hang on unbilled free-tier accounts.
This change intercepts "limit: 0" and "limit:0" errors in classifyGoogleError and classifies them as TerminalQuotaError immediately, bypassing the retryable classification and failing fast.
@luisfelipe-alt luisfelipe-alt requested a review from a team as a code owner June 5, 2026 16:51
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a performance issue where the Gemini-CLI would incorrectly classify hard quota limits (limit: 0) as retryable errors. By intercepting these specific error messages before the retry logic evaluates them, the CLI now terminates immediately, significantly improving the user experience by avoiding unnecessary and futile retry attempts.

Highlights

  • Error Classification Logic: Added a defensive check in classifyGoogleError to identify 'limit: 0' quota errors and immediately classify them as TerminalQuotaError.
  • Retry Loop Prevention: Prevents the CLI from entering a 10-minute retry loop when encountering hard quota limits by failing fast instead of attempting retries.
  • Testing: Added a new unit test case to verify that errors containing 'limit: 0' are correctly handled even when they include retry delay messages.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions github-actions Bot added the size/s A small PR label Jun 5, 2026
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

📊 PR Size: size/M

  • Lines changed: 157
  • Additions: +148
  • Deletions: -9
  • Files changed: 2

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a check to classify Google API errors with a limit of 0 as terminal quota errors, even if they contain a retry message, and adds a corresponding unit test. The feedback suggests replacing the simple string inclusion check with a regular expression to avoid false positives on fractional or padded limits (e.g., 'limit: 0.5').

Comment thread packages/core/src/utils/googleQuotaErrors.ts Outdated
@gemini-cli gemini-cli Bot added the status/need-issue Pull requests that need to have an associated issue. label Jun 5, 2026
Applying changes suggested by bot

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Comment thread packages/core/src/utils/googleQuotaErrors.ts Outdated
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a check to classify Google API errors with a limit of 0 as a TerminalQuotaError, preventing infinite retry loops when a quota is completely exhausted. It also adds a unit test to verify this behavior. The reviewer identified a critical issue where the check is placed inside a fallback block, meaning structured errors with details will bypass it and still be incorrectly classified as retryable. Additionally, the reviewer pointed out that the new test masks this issue by mocking the error parser to return null, and suggested moving the check earlier in the logic and adding a test case with structured RetryInfo details.

Comment thread packages/core/src/utils/googleQuotaErrors.ts Outdated
Comment thread packages/core/src/utils/googleQuotaErrors.test.ts
@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from 26dbf1c to 5c32c2b Compare June 5, 2026 19:23
@github-actions github-actions Bot added the size/m A medium sized PR label Jun 5, 2026
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Google quota error classification logic in packages/core/src/utils/googleQuotaErrors.ts by extracting the errorMessage variable to the top of the function and moving the universal limit: 0 check before the fallback block. It also adds corresponding unit tests in packages/core/src/utils/googleQuotaErrors.test.ts to ensure TerminalQuotaError is correctly returned when the limit is 0, even if retry information is present. There are no review comments provided, and I have no feedback to provide.

@galz10 galz10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: PR #27698

Intent Summary: This PR introduces a fail-fast mechanism for Gemini API errors that indicate a zero-quota limit (limit: 0). By intercepting this specific error signature early, it correctly classifies it as a TerminalQuotaError and prevents the CLI from entering a futile, long-running retry loop.

🚨 Critical Concerns (P0/P1)

Action required before merging.

  • packages/core/src/utils/googleQuotaErrors.ts:244: The condition status === undefined paired with the loose regex /limit:\s*0(?!\d|\.)/ is dangerously greedy. Because classifyGoogleError takes error: unknown, any unrelated application error containing the string "limit: 0" (e.g., new Error("Validation failed: array length limit: 0")) will lack a status and be incorrectly swallowed and misclassified as a TerminalQuotaError (a Google API quota issue).
      if (
        (status === 429 ||
          status === 499 ||
          status === 503) &&
        /limit:\s*0(?!\d|\.)/.test(errorMessage)
      ) {
    
    Note: If you absolutely must catch zero-quota errors that lack an HTTP status, the regex must be significantly tightened to avoid false positives (e.g., `/Quota exceeded.limit:\s0/).

🧹 Refactoring & Nits (P2/P3)

Recommended improvements.

  • packages/core/src/utils/googleQuotaErrors.test.ts:832: AI Slop. Passing an empty new Error() and relying purely on the mock makes the test non-intuitive. Pass a representative error to make the test self-documenting.
        const result = classifyGoogleError(new Error('Quota exceeded for limit: 0'));
    
  • packages/core/src/utils/googleQuotaErrors.ts:238: Minor DRY violation. If you move the errorMessage variable definition to the very top of the function (before the status === 404 check), you can reuse it for the 404 error fallback (errorMessage || 'Model not found'), removing duplicated message extraction logic.

📝 Metadata Review

Feedback on PR description or commit message clarity.

  • The PR description is excellent. It clearly defines the bug, the exact fix, the impact, and provides explicit instructions for test validation.

@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from 5c32c2b to 737a5e4 Compare June 8, 2026 18:32
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

Code Review: PR #27698

Intent Summary: This PR introduces a fail-fast mechanism for Gemini API errors that indicate a zero-quota limit (limit: 0). By intercepting this specific error signature early, it correctly classifies it as a TerminalQuotaError and prevents the CLI from entering a futile, long-running retry loop.

🚨 Critical Concerns (P0/P1)

Action required before merging.

  • packages/core/src/utils/googleQuotaErrors.ts:244: The condition status === undefined paired with the loose regex /limit:\s*0(?!\d|\.)/ is dangerously greedy. Because classifyGoogleError takes error: unknown, any unrelated application error containing the string "limit: 0" (e.g., new Error("Validation failed: array length limit: 0")) will lack a status and be incorrectly swallowed and misclassified as a TerminalQuotaError (a Google API quota issue).

      if (
        (status === 429 ||
          status === 499 ||
          status === 503) &&
        /limit:\s*0(?!\d|\.)/.test(errorMessage)
      ) {
    

    _Note: If you absolutely must catch zero-quota errors that lack an HTTP status, the regex must be significantly tightened to avoid false positives (e.g., `/Quota exceeded.limit:\s_0/).

🧹 Refactoring & Nits (P2/P3)

Recommended improvements.

  • packages/core/src/utils/googleQuotaErrors.test.ts:832: AI Slop. Passing an empty new Error() and relying purely on the mock makes the test non-intuitive. Pass a representative error to make the test self-documenting.
        const result = classifyGoogleError(new Error('Quota exceeded for limit: 0'));
    
  • packages/core/src/utils/googleQuotaErrors.ts:238: Minor DRY violation. If you move the errorMessage variable definition to the very top of the function (before the status === 404 check), you can reuse it for the 404 error fallback (errorMessage || 'Model not found'), removing duplicated message extraction logic.

📝 Metadata Review

Feedback on PR description or commit message clarity.

  • The PR description is excellent. It clearly defines the bug, the exact fix, the impact, and provides explicit instructions for test validation.

Done, I have attended the comments:

  1. Tightened Regex: We tightened the regular expression to /(?:Quota exceeded|quota_exceeded).limit:\s0(?!\d|.)/i. This ensures that even if status is undefined, the error must contain both "Quota exceeded" (or "quota_exceeded") and "limit: 0" to be classified as a quota error, completely avoiding false positives on unrelated application errors (like "Validation failed: array length limit: 0").

  2. DRY Refactoring: We moved the errorMessage extraction to the very top of classifyGoogleError, allowing us to reuse it in the status === 404 check and removing duplicated logic.

  3. Self-Documenting Test Case: We updated the test case to pass a representative error message (new Error('Quota exceeded for limit: 0')) to classifyGoogleError, making the test self-documenting and intuitive.

@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors classifyGoogleError to extract a common errorMessage variable and introduces a universal check to return a TerminalQuotaError when a quota limit of 0 is detected, accompanied by new unit tests. The reviewer identified a high-severity issue in the regular expression used for this check, noting that .* does not match newline characters in JavaScript, which could lead to failures in production. They provided a code suggestion to use [\s\S]* instead for robust matching.

Comment thread packages/core/src/utils/googleQuotaErrors.ts Outdated
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors Google quota error classification by extracting the error message extraction logic and introducing a universal check for "limit: 0" to classify errors as TerminalQuotaError, supported by new unit tests. However, a critical syntax error was introduced in the newly added if statement with a duplicate closing parenthesis and opening brace, which will cause compilation to fail.

Comment thread packages/core/src/utils/googleQuotaErrors.ts
@shaman2527

Copy link
Copy Markdown

si me pasa igual cuando entro con la cuenta en gemini cli hay que esperar como 10 min para que funcione

@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from 0d323c5 to 5a54543 Compare June 8, 2026 19:43
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a universal check for 'limit: 0' errors in classifyGoogleError to return a TerminalQuotaError early, along with corresponding unit tests. The review feedback highlights two critical issues: first, moving the errorMessage definition to the top of the function causes a regression in 404 error handling where plain object errors evaluate to '[object Object]' instead of falling back to 'Model not found'; second, the negative lookahead in the regex incorrectly rejects matches when 'limit: 0' is followed by a period at the end of a sentence, which can be resolved by updating the lookahead pattern to allow periods not followed by a digit.

Comment thread packages/core/src/utils/googleQuotaErrors.ts
Comment thread packages/core/src/utils/googleQuotaErrors.ts Outdated
@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from 1db28df to 89bbd6e Compare June 9, 2026 15:05
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors classifyGoogleError in packages/core/src/utils/googleQuotaErrors.ts to centralize error message extraction and introduces a universal check for zero-limit quota errors (limit: 0) to classify them as TerminalQuotaError. It also updates the 404 status handling to trim error messages and adds comprehensive unit tests in googleQuotaErrors.test.ts to verify these changes under various error formats. There are no review comments, and I have no feedback to provide.

@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a universal 'limit: 0' check in classifyGoogleError to immediately classify such errors as TerminalQuotaError, alongside comprehensive unit tests to validate this behavior. It also refactors the error message extraction and trims 404 error messages. The feedback suggests removing the restrictive 'quota exceeded' substring checks from the new logic, as other variations like 'Rate limit exceeded' could also occur, and instead relying solely on the status codes and the 'limit: 0' regex.

Comment thread packages/core/src/utils/googleQuotaErrors.ts
auto-merge was automatically disabled June 10, 2026 15:05

Head branch was pushed to by a user without write access

@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from 0381e5d to f476048 Compare June 10, 2026 15:05
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors Google API error classification in googleQuotaErrors.ts by extracting a common errorMessage variable, trimming 404 error messages, and introducing a universal check to classify errors with limit: 0 as TerminalQuotaError. It also adds comprehensive unit tests covering these scenarios. The reviewer recommends removing status === undefined from the limit: 0 check to prevent non-HTTP errors from being misclassified, suggesting instead to restrict the check to HTTP statuses 429, 499, and 503, and updating the tests accordingly.

Comment thread packages/core/src/utils/googleQuotaErrors.ts
Comment thread packages/core/src/utils/googleQuotaErrors.test.ts Outdated
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from f476048 to 3774315 Compare June 10, 2026 15:24
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a universal check for limit: 0 to classify certain errors as TerminalQuotaError and refactors classifyGoogleError to extract the error message earlier, supported by several new test cases. The review feedback identifies a critical issue where plain object errors (which are not instances of Error) will have their messages evaluated as "[object Object]", bypassing the limit: 0 check and obscuring error messages. A robust extraction of the message property from plain objects is suggested to resolve this.

Comment thread packages/core/src/utils/googleQuotaErrors.ts Outdated
@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from 25cbc30 to f343b5e Compare June 10, 2026 16:07
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Google error classification logic in packages/core/src/utils/googleQuotaErrors.ts by centralizing the extraction of errorMessage and introducing a universal check to classify limit: 0 errors (for status codes 429, 499, and 503) as TerminalQuotaError. It also adds comprehensive unit tests in googleQuotaErrors.test.ts to validate this new classification behavior under various conditions. There are no review comments, and I have no feedback to provide.

Comment thread packages/core/src/utils/googleQuotaErrors.ts Outdated
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@luisfelipe-alt luisfelipe-alt force-pushed the bugfix/WT-107_477249167 branch from 30cda22 to 68e8736 Compare June 11, 2026 18:47
@luisfelipe-alt luisfelipe-alt requested a review from galz10 June 11, 2026 18:47
@luisfelipe-alt

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Google error classification logic in packages/core/src/utils/googleQuotaErrors.ts by introducing a helper function extractErrorMessage to consistently extract error messages from various error formats. It also implements a universal check to classify quota errors with a limit of 0 as TerminalQuotaError across status codes 429, 499, and 503, ensuring fractional limits (like 0.5) are still treated as retryable. Additionally, comprehensive unit tests have been added to validate these changes under different error structures and edge cases. No review comments were provided, so there is no further feedback to address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m A medium sized PR size/s A small PR status/need-issue Pull requests that need to have an associated issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants