fix(gemini-cli): enhance 429 retry delay parsing #449
Conversation
Add fallback parsing for quota reset delay when RetryInfo is not present: - Try ErrorInfo.metadata.quotaResetDelay (e.g., "373.801628ms") - Parse from error.message "Your quota will reset after Xs." This ensures proper cooldown timing for rate-limited requests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @sususu98, 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 significantly improves the Gemini CLI's ability to handle API rate limits by introducing multiple fallback strategies for determining the required retry delay. By parsing delay information from Highlights
Using Gemini Code AssistThe 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
Customization To customize 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the parsing of 429 retry delays by adding fallback mechanisms. The changes correctly implement fallbacks to parse the delay from ErrorInfo.metadata.quotaResetDelay and from the error message text. My review includes suggestions to improve efficiency by combining loops and optimizing regular expression compilation. Overall, this is a good improvement for handling rate-limiting responses more robustly.
| if details.Exists() && details.IsArray() { | ||
| for _, detail := range details.Array() { | ||
| typeVal := detail.Get("@type").String() | ||
| if typeVal == "type.googleapis.com/google.rpc.RetryInfo" { | ||
| retryDelay := detail.Get("retryDelay").String() | ||
| if retryDelay != "" { | ||
| // Parse duration string like "0.847655010s" | ||
| duration, err := time.ParseDuration(retryDelay) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse duration") | ||
| } | ||
| return &duration, nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Fallback: try ErrorInfo.metadata.quotaResetDelay (e.g., "373.801628ms") | ||
| for _, detail := range details.Array() { | ||
| typeVal := detail.Get("@type").String() | ||
| if typeVal == "type.googleapis.com/google.rpc.ErrorInfo" { | ||
| quotaResetDelay := detail.Get("metadata.quotaResetDelay").String() | ||
| if quotaResetDelay != "" { | ||
| duration, err := time.ParseDuration(quotaResetDelay) | ||
| if err == nil { | ||
| return &duration, nil | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This implementation iterates over the details array twice: once for RetryInfo and a second time for ErrorInfo. This is inefficient. You can combine these into a single loop to improve performance and code clarity. By iterating once, you can check for RetryInfo and, if not found, use a fallback value from ErrorInfo that you've stored from the same loop.
if details.Exists() && details.IsArray() {
var errorInfoDelay string
for _, detail := range details.Array() {
typeVal := detail.Get("@type").String()
if typeVal == "type.googleapis.com/google.rpc.RetryInfo" {
if retryDelay := detail.Get("retryDelay").String(); retryDelay != "" {
duration, err := time.ParseDuration(retryDelay)
if err != nil {
return nil, fmt.Errorf("failed to parse duration")
}
return &duration, nil
}
}
if typeVal == "type.googleapis.com/google.rpc.ErrorInfo" {
if d := detail.Get("metadata.quotaResetDelay").String(); d != "" {
errorInfoDelay = d
}
}
}
if errorInfoDelay != "" {
if duration, err := time.ParseDuration(errorInfoDelay); err == nil {
return &duration, nil
}
}
}| // Fallback: parse from error.message "Your quota will reset after Xs." | ||
| message := gjson.GetBytes(errorBody, "error.message").String() | ||
| if message != "" { | ||
| re := regexp.MustCompile(`after\s+(\d+)s\.?`) |
There was a problem hiding this comment.
Compiling a regular expression with regexp.MustCompile inside a function that can be called multiple times is inefficient. It's a Go best practice to compile regular expressions once at the package level and reuse the compiled object. This avoids the performance overhead of recompilation on every call to parseRetryDelay.
Consider defining it as a package-level variable:
var quotaResetMsgRegex = regexp.MustCompile(`after\s+(\d+)s\.?`)1 participant
Summary
RetryInfois not presentErrorInfo.metadata.quotaResetDelay(e.g.,"373.801628ms")error.messagecontaining"Your quota will reset after Xs."This ensures proper cooldown timing for rate-limited requests, as most Gemini CLI 429 responses only contain
ErrorInfowithoutRetryInfo.Test plan
🤖 Generated with Claude Code