Repository navigation
fix: preserve webhook callback responses - #9115
Conversation
dosubot
Bot
added
size:M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors webhook response handling to preserve raw platform responses, updates proactive messaging support across several adapters (WeCom, WeChat Official Account, and WeCom AI Bot) to raise exceptions instead of logging warnings on failure, and updates associated tests. The review feedback highlights critical issues where calling super().send_by_session before raising an exception in the WeCom and WeChat Official Account adapters could trigger unintended side-effects for failed messages. Additionally, it is recommended to catch UnicodeDecodeError alongside JSONDecodeError in the QQ Official Webhook server to prevent unhandled 500 errors on invalid payloads.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if hasattr(self.client, "kf_message"): | ||
| logger.warning("企业微信客服模式不支持 send_by_session 主动发送。") | ||
| await super().send_by_session(session, message_chain) | ||
| return | ||
| raise Exception("企业微信客服模式不支持 send_by_session 主动发送。") | ||
| if not self.agent_id: | ||
| logger.warning( | ||
| await super().send_by_session(session, message_chain) | ||
| raise Exception( | ||
| f"send_by_session 失败：无法为会话 {session.session_id} 推断 agent_id。", | ||
| ) |
There was a problem hiding this comment.
Calling await super().send_by_session(session, message_chain) before raising an exception is problematic. If the platform does not support proactive messages or if the agent_id is missing, the message sending has failed. Calling super().send_by_session will trigger base platform side-effects (such as recording the message in the database or history) even though the message was never actually sent, leading to inconsistent state.
The exception should be raised immediately without calling super().send_by_session.
if hasattr(self.client, "kf_message"):
raise Exception("企业微信客服模式不支持 send_by_session 主动发送。")
if not self.agent_id:
raise Exception(
f"send_by_session 失败：无法为会话 {session.session_id} 推断 agent_id。",
)| await super().send_by_session(session, message_chain) | ||
| raise Exception("微信公众号不支持发送主动消息") |
There was a problem hiding this comment.
Similar to the WeCom adapter, calling await super().send_by_session(session, message_chain) before raising an exception will trigger base platform side-effects (such as recording the message in the database or history) even though the message was never actually sent.
Since proactive messages are not supported for WeChat Official Accounts, the exception should be raised directly without calling super().send_by_session.
| await super().send_by_session(session, message_chain) | |
| raise Exception("微信公众号不支持发送主动消息") | |
| raise Exception("微信公众号不支持发送主动消息") |
| try: | ||
| msg = json.loads(body.decode("utf-8")) | ||
| except json.JSONDecodeError: |
There was a problem hiding this comment.
If the webhook payload contains invalid UTF-8 bytes, calling body.decode("utf-8") will raise a UnicodeDecodeError before json.loads is executed. Since only json.JSONDecodeError is caught, this will result in an unhandled exception and a 500 Internal Server Error.
Catching UnicodeDecodeError alongside json.JSONDecodeError ensures that any invalid encoding payloads are gracefully rejected with a 400 Bad Request.
| try: | |
| msg = json.loads(body.decode("utf-8")) | |
| except json.JSONDecodeError: | |
| try: | |
| msg = json.loads(body.decode("utf-8")) | |
| except (json.JSONDecodeError, UnicodeDecodeError): |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| result = await result | ||
| return _response_from_result(result) | ||
| return webhook_response_from_result(result) | ||
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the WeCom and Weixin adapters,
send_by_sessionnow callssuper().send_by_sessionand then raises, which means the message is still sent but reported as an error; consider either raising before doing any fallback send or using a non-exception signal so the caller can distinguish truly unsupported proactive messaging from successful fallback delivery. - The
support_proactive_messageflags for WeCom, WeCom AI Bot, and Weixin Official Account are set toTrueeven though the adapters immediately raise on proactive sends in some modes; it may be clearer to align these flags with actual proactive support or introduce a more nuanced capability indicator. - For webhook error handling you now wrap
PlatformServiceErrorinto a generic JSON error viawebhook_response_from_result; if different platforms need protocol-specific error formats, consider letting adapters return their own structured error responses instead of a uniform{"error": ...}payload.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the WeCom and Weixin adapters, `send_by_session` now calls `super().send_by_session` and then raises, which means the message is still sent but reported as an error; consider either raising before doing any fallback send or using a non-exception signal so the caller can distinguish truly unsupported proactive messaging from successful fallback delivery.
- The `support_proactive_message` flags for WeCom, WeCom AI Bot, and Weixin Official Account are set to `True` even though the adapters immediately raise on proactive sends in some modes; it may be clearer to align these flags with actual proactive support or introduce a more nuanced capability indicator.
- For webhook error handling you now wrap `PlatformServiceError` into a generic JSON error via `webhook_response_from_result`; if different platforms need protocol-specific error formats, consider letting adapters return their own structured error responses instead of a uniform `{"error": ...}` payload.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/weixin_official_account/weixin_offacc_adapter.py" line_range="404-408" />
<code_context>
"wecom 适配器",
id=self.config.get("id", "wecom"),
support_streaming_message=False,
- support_proactive_message=False,
+ support_proactive_message=True,
)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** `support_proactive_message=True` conflicts with an implementation that always raises in `send_by_session`.
With `support_proactive_message=True`, higher-level code will assume proactive sends are supported, but `send_by_session` still always raises. Either keep this flag `False` or update `send_by_session` so it can successfully send in at least some cases, keeping the advertised capability consistent with actual behavior.
```suggestion
"微信公众平台 适配器",
id=self.config.get("id", "weixin_official_account"),
support_streaming_message=False,
support_proactive_message=False,
)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| id=self.config.get("id", "weixin_official_account"), | ||
| support_streaming_message=False, | ||
| support_proactive_message=False, | ||
| support_proactive_message=True, | ||
| ) | ||
|
|
There was a problem hiding this comment.
suggestion (bug_risk): support_proactive_message=True conflicts with an implementation that always raises in send_by_session.
With support_proactive_message=True, higher-level code will assume proactive sends are supported, but send_by_session still always raises. Either keep this flag False or update send_by_session so it can successfully send in at least some cases, keeping the advertised capability consistent with actual behavior.
| id=self.config.get("id", "weixin_official_account"), | |
| support_streaming_message=False, | |
| support_proactive_message=False, | |
| support_proactive_message=True, | |
| ) | |
| "微信公众平台 适配器", | |
| id=self.config.get("id", "weixin_official_account"), | |
| support_streaming_message=False, | |
| support_proactive_message=False, | |
| ) |
2 participants
修复 QQ、微信客服等平台 Webhook 下无法正常校验的问题。
Summary by Sourcery
Preserve platform-specific webhook callback responses in unified webhook routes and tighten WeCom-related proactive messaging error handling while adjusting QQ Official webhook validation flow.
New Features:
Bug Fixes:
Enhancements:
Tests: