fix(dingtalk): fire-and-forget message processing & session_webhook fallback#11518
Closed
kagura-agent wants to merge 1 commit into
Closed
fix(dingtalk): fire-and-forget message processing & session_webhook fallback#11518kagura-agent wants to merge 1 commit into
kagura-agent wants to merge 1 commit into
Conversation
…allback Fixes NousResearch#11463: DingTalk channel receives messages but fails to reply with 'No session_webhook available'. Two changes: 1. **Fire-and-forget message processing**: process() now dispatches _on_message as a background task via asyncio.create_task instead of awaiting it. This ensures the SDK ACK is returned immediately, preventing heartbeat timeouts and disconnections when message processing takes longer than the SDK's ACK deadline. 2. **session_webhook extraction fallback**: If ChatbotMessage.from_dict() fails to map the sessionWebhook field (possible across SDK versions), the handler now falls back to extracting it directly from the raw callback data dict using both 'sessionWebhook' and 'session_webhook' key variants. Added 3 tests covering webhook extraction, fallback behavior, and fire-and-forget ACK timing.
Contributor
|
Merged via #11605 (#11605). Your commit was cherry-picked with authorship preserved (47a0dd1). Thanks for the fire-and-forget fix + session_webhook fallback + 3 new tests — this is a real scalability issue for longer-running agents that would silently drop the WebSocket when the SDK heartbeat timed out. Conflict resolution note: your branch predated #11564 (the group-gating tests we merged earlier today), so This should also resolve @sgjeff's report in #11463 — their symptom ("No session_webhook available") matches exactly what your fallback handles. We'll ping them to retest. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Fixes #11463: DingTalk channel receives messages but fails to reply with
No session_webhook available.Changes
1. Fire-and-forget message processing
_IncomingHandler.process()now dispatches_on_messageas a background task viaasyncio.create_task()instead ofawaiting it directly. This ensures the SDK ACK is returned immediately, preventing heartbeat timeouts and WebSocket disconnections when message processing takes longer than the SDK's ACK deadline.2. session_webhook extraction fallback
If
ChatbotMessage.from_dict()fails to map thesessionWebhookfield (possible across SDK versions where the camelCase key name differs), the handler now falls back to extracting it directly from the raw callback data dict using bothsessionWebhookandsession_webhookkey variants.Testing
Added 3 new tests:
test_process_extracts_session_webhook— verifies webhook is populated from callback datatest_process_fallback_session_webhook_when_from_dict_misses_it— verifies fallback extractiontest_process_returns_ack_immediately— verifies ACK returns before message processing completesAll 31 DingTalk tests pass.