Improve OpenAI error handling #164
Conversation
WalkthroughThe pull request enhances error handling and retry logic in the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
app/llm/external/openai_embeddings.py (1)
22-25: LGTM: Improved retry logic with exponential backoffThe changes to the retry logic are well-implemented:
- Reducing retries from 10 to 5 is reasonable and aligns with the PR objectives.
- Introducing an exponential backoff strategy is a good practice for API requests.
- The comment explaining the maximum wait time is helpful.
Consider extracting these retry parameters into class-level constants for easier configuration and testing. For example:
class OpenAIEmbeddingModel(EmbeddingModel): MAX_RETRIES = 5 BACKOFF_FACTOR = 2 INITIAL_DELAY = 1 # ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- app/llm/external/openai_chat.py (2 hunks)
- app/llm/external/openai_embeddings.py (3 hunks)
🧰 Additional context used
🔇 Additional comments (5)
app/llm/external/openai_embeddings.py (5)
3-9: LGTM: Improved error handling importsThe addition of specific OpenAI error types (
APIError,APITimeoutError,RateLimitError, andInternalServerError) aligns well with the PR objectives. This change enables more precise error handling in theembedmethod.
35-40: LGTM: Precise error handling for OpenAI-specific errorsThe updated
exceptclause now catches specific OpenAI-related errors (APIError,APITimeoutError,RateLimitError, andInternalServerError). This change:
- Aligns perfectly with the PR objectives.
- Follows OpenAI's documentation recommendations.
- Prevents unnecessary retries for unrelated exceptions, improving overall efficiency.
42-43: LGTM: Improved logging for OpenAI errorsThe updated logging message is more appropriate and informative:
- It now covers all potential OpenAI errors, not just rate limit issues.
- The message provides clearer feedback during API interactions.
- It includes the attempt number, which is helpful for debugging.
45-45: LGTM: Informative final error loggingThe updated final error log message now includes the number of retries attempted. This change:
- Provides more context in the error log.
- Aligns well with the modifications made to the retry logic.
- Helps in debugging by clearly indicating when all retry attempts have been exhausted.
Line range hint
1-77: Summary: Excellent improvements to OpenAI error handling and retry logicOverall, the changes in this file significantly enhance the error handling and retry mechanism for OpenAI API interactions:
- The introduction of specific OpenAI error types allows for more precise error handling.
- The refined retry logic with exponential backoff is a good practice for API requests.
- Updated logging provides clearer and more informative feedback during API interactions.
- The changes align well with the PR objectives and OpenAI's documentation.
These improvements will lead to more robust and efficient API interactions. Great job!
MichaelOwenDyer
temporarily deployed
to
Iris Test
GitHub Actions
Inactive
|
Going to go ahead and merge this |
Currently, our OpenAI chat and embedding wrappers will retry requests up to 10 times if any
Exceptionoccurs. This is bad practice - there are exceptions we shouldn't retry on, and exceptions we shouldn't catch at all.This PR changes the error handling of
OpenAIChatModelandOpenAIEmbeddingModelto only retry if one of the following openai errors is raised:APIError,APITimeoutError,RateLimitError,InternalServerError. This is in alignment with the suggestions in the documentation here. Otherwise, we do not catch the exception and let it raise all the way up to the web layer, where the global error handler reports a fatal error to the client.This PR also adds special error handling in
OpenAIChatModelfor the OpenAI content filter. Unfortunately, no error is raised automatically by the openai library in this case. So we check to see if thefinish_reasonof the returnedchoiceiscontent_filter, and if so we manually raise anExceptionto the global error handler. This means that if the user's input is flagged by the content filter, the user will not receive a response from Iris and will instead see "Fatal error".Also, the maximum retries has been lowered from 10 to 5, which results in a maximum wait time of 31 seconds. If after this long no request has been successful, we give up and raise a fatal exception (so that the user isn't waiting indefinitely).
Closes #136.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation