-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(core): improve error handling of command parser and sync #4161
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
Merged
+17
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Soulter
approved these changes
Dec 22, 2025
Member
|
LGTM |
Contributor
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.
Hey - 我发现了 1 个问题,并给出了一些总体反馈:
- 在
_collect_descriptors中,包裹_build_descriptor的宽泛except Exception可能会掩盖真正的编程错误;建议收窄异常类型,或者至少记录完整的回溯信息(例如使用exc_info=True或logger.exception),以便更容易排查问题。 - 在
load中,目前是先记录错误信息,再调用traceback.format_exc();你可以通过使用logger.exception("同步指令配置失败")来简化并统一日志风格,这样会自动包含堆栈信息。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- In `_collect_descriptors`, the broad `except Exception` around `_build_descriptor` could hide real programming errors; consider narrowing the exception type or at least logging the full traceback (e.g., with `exc_info=True` or `logger.exception`) so issues are easier to diagnose.
- In `load`, instead of logging the error message and then `traceback.format_exc()`, you can simplify and make the log more consistent by using `logger.exception("同步指令配置失败")`, which will automatically include the stack trace.
## Individual Comments
### Comment 1
<location> `astrbot/core/star/command_management.py:202-205` </location>
<code_context>
+ if not include_sub_commands and desc.is_sub_command:
+ continue
+ descriptors.append(desc)
+ except Exception as e:
+ logger.warning(
+ f"解析指令处理函数 {handler.handler_full_name} 失败,跳过该指令。原因: {e!s}"
+ )
</code_context>
<issue_to_address>
**suggestion:** Capture and log the full traceback when a handler descriptor fails to build.
Catching `Exception` to avoid one bad handler breaking collection makes sense, but logging only `str(e)` limits diagnosability. Prefer `logger.exception(...)` (or include `traceback.format_exc()`) so the full stack trace is captured while keeping behavior unchanged:
```python
except Exception:
logger.exception(
"解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
)
```
```suggestion
except Exception:
logger.exception(
"解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
)
```
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
_collect_descriptors, the broadexcept Exceptionaround_build_descriptorcould hide real programming errors; consider narrowing the exception type or at least logging the full traceback (e.g., withexc_info=Trueorlogger.exception) so issues are easier to diagnose. - In
load, instead of logging the error message and thentraceback.format_exc(), you can simplify and make the log more consistent by usinglogger.exception("同步指令配置失败"), which will automatically include the stack trace.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_collect_descriptors`, the broad `except Exception` around `_build_descriptor` could hide real programming errors; consider narrowing the exception type or at least logging the full traceback (e.g., with `exc_info=True` or `logger.exception`) so issues are easier to diagnose.
- In `load`, instead of logging the error message and then `traceback.format_exc()`, you can simplify and make the log more consistent by using `logger.exception("同步指令配置失败")`, which will automatically include the stack trace.
## Individual Comments
### Comment 1
<location> `astrbot/core/star/command_management.py:202-205` </location>
<code_context>
+ if not include_sub_commands and desc.is_sub_command:
+ continue
+ descriptors.append(desc)
+ except Exception as e:
+ logger.warning(
+ f"解析指令处理函数 {handler.handler_full_name} 失败,跳过该指令。原因: {e!s}"
+ )
</code_context>
<issue_to_address>
**suggestion:** Capture and log the full traceback when a handler descriptor fails to build.
Catching `Exception` to avoid one bad handler breaking collection makes sense, but logging only `str(e)` limits diagnosability. Prefer `logger.exception(...)` (or include `traceback.format_exc()`) so the full stack trace is captured while keeping behavior unchanged:
```python
except Exception:
logger.exception(
"解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
)
```
```suggestion
except Exception:
logger.exception(
"解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
在命令描述符收集和命令配置同步周围添加错误处理,以在命令解析和加载过程中提高健壮性。
Bug 修复:
Original summary in English
Summary by Sourcery
Add error handling around command descriptor collection and command config synchronization to improve robustness during command parsing and loading.
Bug Fixes: