-
-
Notifications
You must be signed in to change notification settings - Fork 672
feat: 😽增加重试机制, 解决模型不稳定的报错 #1344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Sourcery 评审者指南此 Pull Request 引入了 LLM 请求的重试机制,以解决某些提供商/模型和网络相关问题导致的不稳定性。它增加了最大重试次数和重试延迟的配置选项,并确保错误消息被记录而不是直接返回给用户。 更新后的提供商设置类图classDiagram
class ProviderSettings {
streaming_response: bool
max_retries: int
retry_delay: float
}
note for ProviderSettings "Added max_retries and retry_delay attributes"
文件级别变更
可能相关的 issue
提示和命令与 Sourcery 互动
自定义您的体验访问您的 仪表板 以:
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request introduces a retry mechanism for LLM requests to address instability issues with certain providers/models and network-related errors. It adds configuration options for maximum retry attempts and retry delay, and it ensures that error messages are logged instead of being directly returned to the user. Updated class diagram for provider settingsclassDiagram
class ProviderSettings {
streaming_response: bool
max_retries: int
retry_delay: float
}
note for ProviderSettings "Added max_retries and retry_delay attributes"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
嘿 @anka-afk - 我已经审查了你的更改 - 这里有一些反馈:
总体评论:
- 考虑使用一种补偿策略,该策略会随着每次重试而增加延迟。
- 定义一个用于可重试错误的自定义异常可能很有用,以避免依赖字符串匹配。
以下是我在审查期间查看的内容
- 🟢 一般问题:一切看起来都不错
- 🟢 安全性:一切看起来都不错
- 🟢 测试:一切看起来都不错
- 🟡 复杂性:发现 1 个问题
- 🟢 文档:一切看起来都不错
帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进你的评论。
Original comment in English
Hey @anka-afk - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a backoff strategy that increases the delay with each retry.
- It might be helpful to define a custom exception for retryable errors to avoid relying on string matching.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -158,94 +169,132 @@ async def process( | |||
req.session_id = event.unified_msg_origin | |||
|
|||
async def requesting(req: ProviderRequest): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
问题 (复杂度): 考虑重构 requesting
函数,通过将请求执行和重试逻辑提取到单独的辅助函数中,以提高可读性并降低嵌套复杂度,而无需更改功能。
考虑将重试逻辑和内部处理提取到单独的辅助函数中。这将扁平化嵌套循环和 try/except 块,以提高可读性,而无需更改行为。例如,将执行单个请求的逻辑移动到其自己的函数中,然后用重试循环包装它:
async def _execute_request(self, req: ProviderRequest, event, provider) -> Optional[LLMResponse]:
logger.debug(f"提供商请求 Payload: {req}")
final_llm_response = None
if self.streaming_response:
stream = provider.text_chat_stream(**req.__dict__)
async for llm_response in stream:
if llm_response.is_chunk:
if llm_response.result_chain:
yield llm_response.result_chain # MessageChain
else:
yield MessageChain().message(llm_response.completion_text)
else:
final_llm_response = llm_response
else:
final_llm_response = await provider.text_chat(**req.__dict__)
if not final_llm_response:
raise Exception("LLM response is None.")
# Execute post-response event hooks
await self._handle_event_hooks(event, final_llm_response)
# Handle functions/streaming responses
if self.streaming_response:
async for result in self._handle_llm_stream_response(event, req, final_llm_response):
yield result
else:
async for result in self._handle_llm_response(event, req, final_llm_response):
yield result
然后用重试逻辑包装此执行:
async def requesting(self, req: ProviderRequest, event, provider):
retry_count = 0
while True:
try:
async for result in self._execute_request(req, event, provider):
if isinstance(result, ProviderRequest):
req = result # new LLM request
break # re-enter execution with modified req
else:
yield result
else:
# Only break out if no inner loop reset happened.
break
retry_count = 0 # Reset retry_count if a new req was processed successfully.
except Exception as e:
retry_count += 1
logger.error(f"LLM请求失败 (尝试 {retry_count}/{self.max_retries}): {type(e).__name__} : {str(e)}")
logger.error(traceback.format_exc())
if retry_count < self.max_retries and any(err in str(e).lower() for err in ["timeout", "connection", "rate limit", "server error", "500", "503"]):
logger.info(f"将在 {self.retry_delay} 秒后重试 LLM 请求 >﹏<")
await asyncio.sleep(self.retry_delay)
else:
logger.error(f"LLM 请求失败, 重试次数({retry_count - 1})用尽: {type(e).__name__} : {str(e)}")
break
最后,更新您的调用站点以使用此重构的 requesting
函数,保持所有功能完整,同时减少嵌套。
Original comment in English
issue (complexity): Consider refactoring the requesting
function by extracting the request execution and retry logic into separate helper functions to improve readability and reduce nesting complexity without altering the functionality.
Consider extracting the retry logic and inner handling into separate helper functions. This would flatten the nested loops and try/except blocks to improve readability without changing behavior. For example, move the logic that executes a single request into its own function and then wrap that with the retry loop:
async def _execute_request(self, req: ProviderRequest, event, provider) -> Optional[LLMResponse]:
logger.debug(f"提供商请求 Payload: {req}")
final_llm_response = None
if self.streaming_response:
stream = provider.text_chat_stream(**req.__dict__)
async for llm_response in stream:
if llm_response.is_chunk:
if llm_response.result_chain:
yield llm_response.result_chain # MessageChain
else:
yield MessageChain().message(llm_response.completion_text)
else:
final_llm_response = llm_response
else:
final_llm_response = await provider.text_chat(**req.__dict__)
if not final_llm_response:
raise Exception("LLM response is None.")
# Execute post-response event hooks
await self._handle_event_hooks(event, final_llm_response)
# Handle functions/streaming responses
if self.streaming_response:
async for result in self._handle_llm_stream_response(event, req, final_llm_response):
yield result
else:
async for result in self._handle_llm_response(event, req, final_llm_response):
yield result
Then wrap this execution with retry logic:
async def requesting(self, req: ProviderRequest, event, provider):
retry_count = 0
while True:
try:
async for result in self._execute_request(req, event, provider):
if isinstance(result, ProviderRequest):
req = result # new LLM request
break # re-enter execution with modified req
else:
yield result
else:
# Only break out if no inner loop reset happened.
break
retry_count = 0 # Reset retry_count if a new req was processed successfully.
except Exception as e:
retry_count += 1
logger.error(f"LLM请求失败 (尝试 {retry_count}/{self.max_retries}): {type(e).__name__} : {str(e)}")
logger.error(traceback.format_exc())
if retry_count < self.max_retries and any(err in str(e).lower() for err in ["timeout", "connection", "rate limit", "server error", "500", "503"]):
logger.info(f"将在 {self.retry_delay} 秒后重试 LLM 请求 >﹏<")
await asyncio.sleep(self.retry_delay)
else:
logger.error(f"LLM 请求失败, 重试次数({retry_count - 1})用尽: {type(e).__name__} : {str(e)}")
break
Finally, update your call sites to use this refactored requesting
function, keeping all functionality intact while reducing nesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a configurable retry mechanism for LLM requests to improve stability and prevent excessive user-facing error messages in case of transient failures. The changes include the addition of new configuration options, implementation of retry logic in the LLM request flow, and updates to the default configuration settings.
- Added "max_retries" and "retry_delay" options in provider settings.
- Implemented a retry loop for LLM requests with error logging and conditional delays.
- Updated the default configuration file to include new retry parameters.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
astrbot/core/pipeline/process_stage/method/llm_request.py | Implemented retry mechanism for LLM requests with detailed logging |
astrbot/core/config/default.py | Added default values for "max_retries" and "retry_delay" |
Comments suppressed due to low confidence (2)
astrbot/core/pipeline/process_stage/method/llm_request.py:295
- The log message subtracts 1 from retry_count, which may confuse readers about the actual number of attempts. Consider logging the actual retry count to improve clarity.
logger.error(f"LLM 请求失败, 重试次数({retry_count - 1})用尽: {type(e).__name__} : {str(e)}")
astrbot/core/pipeline/process_stage/method/llm_request.py:288
- [nitpick] The log message contains an informal emoticon, which might be inappropriate for production logs. Consider using a more neutral tone.
logger.info(f"将在 {self.retry_delay} 秒后重试 LLM 请求 >﹏<")
感觉这个 process() 方法现在已经过分复杂了()我抽空仔细 check 一下吧~ 感觉重试机制得用装饰器来包装了 |
修复了 #1300
Motivation
Modifications
Check
好的,这是翻译成中文的 pull request 总结:
Sourcery 总结
为 LLM 请求添加重试机制,以提高稳定性和处理瞬时错误
新特性:
增强功能:
杂项:
Original summary in English
Summary by Sourcery
Add retry mechanism for LLM requests to improve stability and handle transient errors
New Features:
Enhancements:
Chores: