fix(weixin): schedule direct sends on adapter loop#17557
Closed
tonyxu-io wants to merge 1 commit into
Closed
Conversation
Collaborator
Author
|
Thanks for flagging the duplicates. I agree this is the same underlying Weixin/aiohttp cross-event-loop issue. I checked the related PRs:
This PR intentionally takes the “schedule on the adapter loop” variant rather than the “skip live adapter / fresh session” variant:
If maintainers prefer the simpler fresh-session fallback used in #12810/#15911/#13361, I’m happy to rework this PR in that direction or close it in favor of whichever duplicate you want to merge. The reason I kept this open is that it preserves live adapter reuse while still fixing the loop-affinity problem. |
Open
2 tasks
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
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
Why this is needed
send_weixin_direct()is used by one-shot delivery paths such assend_messageand cron delivery. When a live Weixin adapter is available, it reuses that adapter so outbound delivery can keep the adapter's context-token and media behavior.The live adapter's
_send_sessionis anaiohttp.ClientSessioncreated duringWeixinAdapter.connect(), so it belongs to the gateway adapter's asyncio event loop. Cron/tool delivery can callsend_weixin_direct()from a different worker thread/event loop. Before this change, that path directly awaitedlive_adapter.send(...)from the caller loop, which can use the live adapter's aiohttp session on the wrong loop.That cross-loop aiohttp session access can fail with event-loop/task errors such as:
In practice, this can make a scheduled job complete successfully but fail when delivering the result to Weixin.
Implementation details
This PR stores the adapter's loop at connect time:
Then, when
send_weixin_direct()finds a live adapter with an open send session:asyncio.run_coroutine_threadsafe(...)and awaits the wrapped futureThis preserves live adapter reuse while avoiding cross-loop use of the adapter's
aiohttp.ClientSession.Test Plan
/home/tonyxu/.hermes/hermes-agent/venv/bin/python -m pytest tests/gateway/test_weixin.py::TestWeixinSendMessageIntegration::test_send_weixin_direct_schedules_live_adapter_send_on_adapter_loop tests/gateway/test_weixin.py::TestWeixinSendMessageIntegration::test_send_to_platform_routes_weixin_media_to_native_helper -q -o 'addopts='/home/tonyxu/.hermes/hermes-agent/venv/bin/python -m pytest tests/gateway/test_weixin.py -q -o 'addopts='Results:
2 passed, 2 warnings50 passed, 2 warnings