fix: surface weixin media send failures#8175
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider including the session/user identifier in the
RuntimeErrormessage (e.g.,self.meta().idortarget_user) so failures are easier to trace back in logs and error reporting. - Instead of raising a generic
RuntimeError, you might want to introduce or reuse a more specific exception type for outbound send failures so callers can distinguish this from other runtime issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider including the session/user identifier in the `RuntimeError` message (e.g., `self.meta().id` or `target_user`) so failures are easier to trace back in logs and error reporting.
- Instead of raising a generic `RuntimeError`, you might want to introduce or reuse a more specific exception type for outbound send failures so callers can distinguish this from other runtime issues.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/weixin_oc/weixin_oc_adapter.py" line_range="1651-1654" />
<code_context>
"weixin_oc(%s): outbound message ignored, no supported segments",
self.meta().id,
)
+ if failed_segments:
+ raise RuntimeError(
+ f"weixin_oc failed to send {failed_segments} message segment(s)"
+ )
</code_context>
<issue_to_address>
**suggestion:** Include contextual identifiers in the exception message for easier traceability.
Right now the `RuntimeError` only reports the number of failed segments. Please include key identifiers (e.g. `self.meta().id`, `session.session_id` or `target_user`) so operators can correlate this failure with logs and external systems, e.g.: `f"weixin_oc({self.meta().id}, session={session.session_id}) failed to send {failed_segments} segment(s)"`.
```suggestion
if failed_segments:
raise RuntimeError(
f"weixin_oc({self.meta().id}, target_user={target_user}) "
f"failed to send {failed_segments} message segment(s)"
)
```
</issue_to_address>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 improves error handling in the WeixinOC adapter by tracking failed message segments and raising a RuntimeError if any segment fails to send. It also updates the logging to include exception information for better debugging and adds a unit test to verify that the adapter correctly raises an error when media segments fail to send.
|
Addressed the review feedback in 34beb5f: the RuntimeError now includes the WeixinOC adapter id and target_user, and the regression assertion checks that context. Validation passed: focused WeixinOC adapter test, py_compile on the touched files, and ruff check on the touched files. |
* fix: surface weixin media send failures * fix: include weixin send failure context * Delete tests/unit/test_weixin_oc_adapter.py --------- Co-authored-by: Weilong Liao <37870767+Soulter@users.noreply.github.com>
* fix: surface weixin media send failures * fix: include weixin send failure context * Delete tests/unit/test_weixin_oc_adapter.py --------- Co-authored-by: Weilong Liao <37870767+Soulter@users.noreply.github.com>
Fixes #8160.
Summary
Test plan
Summary by Sourcery
Handle Weixin personal outbound message failures more robustly and ensure they are surfaced to callers.
Bug Fixes:
Tests: