fix(provider): add None check for completion.choices to prevent TypeError#6316
fix(provider): add None check for completion.choices to prevent TypeError#6316ccsang wants to merge 3 commits intoAstrBotDevs:devfrom
Conversation
…rror Fixes AstrBotDevs#6252 When using certain providers (e.g., OpenRouter), the API may return completion.choices as None instead of an empty list. This causes TypeError: object of type 'NoneType' has no len() when calling len(completion.choices). This fix adds None checks before len() calls in three locations: - Streaming chunk handler (line 314) - _extract_reasoning_content method (line 348) - _parse_openai_completion method (line 471)
Summary of ChangesHello, 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 Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
completion.choices is None or len(completion.choices) == 0pattern is now duplicated in three places; consider extracting a small helper (e.g.,has_choices(completion)) or using a single idiom likeif not completion.choices:to keep this logic consistent and easier to maintain. - For the streaming handler and
_extract_reasoning_content, you silently skip whenchoicesisNone; consider logging or otherwise surfacing this case so that an upstream provider returningNoneinstead of an empty list is more visible during debugging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `completion.choices is None or len(completion.choices) == 0` pattern is now duplicated in three places; consider extracting a small helper (e.g., `has_choices(completion)`) or using a single idiom like `if not completion.choices:` to keep this logic consistent and easier to maintain.
- For the streaming handler and `_extract_reasoning_content`, you silently skip when `choices` is `None`; consider logging or otherwise surfacing this case so that an upstream provider returning `None` instead of an empty list is more visible during debugging.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a TypeError that occurs when completion.choices is None by adding explicit None checks before accessing its length. The fix is applied in three different locations to prevent the crash. My review includes suggestions to make these checks more concise and Pythonic by using truthiness evaluation, which simplifies the code while maintaining the same logic.
| except Exception as e: | ||
| logger.warning("Saving chunk state error: " + str(e)) | ||
| if len(chunk.choices) == 0: | ||
| if chunk.choices is None or len(chunk.choices) == 0: |
There was a problem hiding this comment.
| """Extract reasoning content from OpenAI ChatCompletion if available.""" | ||
| reasoning_text = "" | ||
| if len(completion.choices) == 0: | ||
| if completion.choices is None or len(completion.choices) == 0: |
There was a problem hiding this comment.
| llm_response = LLMResponse("assistant") | ||
|
|
||
| if len(completion.choices) == 0: | ||
| if completion.choices is None or len(completion.choices) == 0: |
There was a problem hiding this comment.
Address Sourcery AI review comment: - Use 'if not completion.choices:' instead of explicit None check - This handles both None and empty list cases concisely - More Pythonic and easier to maintain
Fixes #6252
Problem
When using certain providers (e.g., OpenRouter), the API may return
completion.choicesasNoneinstead of an empty list. This causes:in three locations where
len(completion.choices)is called without checking forNonefirst.Solution
Add
Nonechecks beforelen()calls in three locations:Changes
Testing
choicesisNonechoicesis empty listSummary by Sourcery
Bug Fixes: