fix: prevent ValueError when removing already-removed API key in retry loop

[mango766]

Summary

In _handle_api_error(), when a 429 rate-limit error is caught, the code calls available_api_keys.remove(chosen_key) to rotate to a different key. However, if the same key was already removed in a previous retry iteration, list.remove() raises ValueError, crashing the entire LLM request with an opaque error instead of properly retrying or falling back.

This fix adds a membership check (if chosen_key in available_api_keys) before calling remove().

Changes

File	Change
astrbot/core/provider/sources/openai_source.py	Guard remove() with in check

Test plan

• Trigger multiple 429 errors with the same API key to verify no ValueError crash
• Verify key rotation still works correctly when multiple keys are available

Summary by Sourcery

Bug Fixes:

• Prevent ValueError crashes when attempting to remove an API key that has already been removed during 429 retry handling.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 improves the robustness of the API key rotation logic within the _handle_api_error function. It prevents a specific crash scenario where a ValueError could occur if an API key was attempted to be removed multiple times during rate-limit retries, ensuring smoother operation and proper key rotation.

Highlights

• API Key Rotation Error Fix: Addressed a ValueError that occurred in the _handle_api_error() function when attempting to remove an API key that had already been removed from the available_api_keys list during a 429 rate-limit retry.
• Enhanced Robustness: Improved the retry mechanism to prevent crashes by ensuring an API key is only removed if it is still present in the list, enhancing the stability of LLM requests under rate-limiting conditions.

Changelog

• astrbot/core/provider/sources/openai_source.py
  • Implemented a membership check before removing an API key to prevent ValueError.

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 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 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

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. ↩

[sourcery-ai]

Hey - I've left some high level feedback:

• Instead of doing an in check followed by remove() (which traverses the list twice), consider wrapping available_api_keys.remove(chosen_key) in a try/except ValueError to handle the missing key in a single pass.
• If available_api_keys is shared across concurrent tasks, you may still hit race conditions on membership and removal; consider copying the list per request or protecting modifications with an appropriate synchronization mechanism.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Instead of doing an `in` check followed by `remove()` (which traverses the list twice), consider wrapping `available_api_keys.remove(chosen_key)` in a `try/except ValueError` to handle the missing key in a single pass.
- If `available_api_keys` is shared across concurrent tasks, you may still hit race conditions on membership and removal; consider copying the list per request or protecting modifications with an appropriate synchronization mechanism.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[gemini-code-assist]

Code Review

This pull request correctly fixes a ValueError that could occur in the API error handling logic when an API key is removed more than once from the list of available keys. The change introduces a check to ensure the key exists before attempting removal, which effectively resolves the crash. I've provided one suggestion to use a try...except block, which is a more idiomatic Python pattern for this scenario and can be slightly more efficient. Overall, this is a solid fix for an important reliability issue.

[gemini-code-assist]

While this check is correct and solves the ValueError, a more idiomatic Python approach is to use a try...except block. This pattern, often called 'Easier to Ask for Forgiveness than Permission' (EAFP), avoids the need to check for the key's existence before attempting removal. It can be slightly more efficient as it avoids a second scan of the list when the key is present.

            try:
                available_api_keys.remove(chosen_key)
            except ValueError:
                # Key was already removed in a previous retry, which is fine.
                pass