fix(cron): use real asyncio Task for live adapter delivery - #19099
Closed
zealonexp wants to merge 1 commit into
Closed
fix(cron): use real asyncio Task for live adapter delivery#19099zealonexp wants to merge 1 commit into
zealonexp wants to merge 1 commit into
Conversation
aiohttp ClientTimeout requires an asyncio Task context. The previous run_coroutine_threadsafe() wraps the callback in a Handle, not a Task, causing "Timeout context manager should be used inside a task" errors on Weixin delivery. Replace with call_soon_threadsafe + ensure_future to create a proper Task on the gateway event loop, preserving thread safety. Fixes: cron delivery to Weixin fails with timeout error Affected jobs: evolver-gene-extraction, evolver-gene-feedback
Collaborator
Collaborator
Contributor
|
Thanks for the attention to aiohttp ClientTimeout + asyncio interaction — that's a real concern. Unfortunately the implementation can't land as written: If you have a concrete reproducer of the Weixin/aiohttp timeout issue you were addressing, please open a fresh issue with the traceback and we'll dig in. Closing this PR so it doesn't become confusing in the queue. |
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.
What does this PR do?
Fixes cron job delivery to Weixin (and potentially other async platforms) failing with:
Root cause:
_deliver_result()incron/scheduler.pyusesasyncio.run_coroutine_threadsafe()to scheduleadapter.send()on the gateway event loop. However,run_coroutine_threadsafewraps the callback in a Handle, not a Task. aiohttp 3.13.5+ requiresasyncio.current_task()to return a valid Task when entering timeout contexts — which fails because the coroutine is not wrapped in a proper Task.The fix replaces
run_coroutine_threadsafewithloop.call_soon_threadsafe+asyncio.ensure_futureto wrap the coroutine in a real asyncio Task before scheduling on the gateway loop.Related Issue
Same root cause (aiohttp cross-loop Task semantics) but different fix location:
model_tools._run_async()(tool call bridge)weixin.pyadapter sideweixin.pywith session rebuildThis PR fixes the cron scheduler delivery path specifically.
Type of Change
Changes Made
cron/scheduler.py(+20/-4): Replacerun_coroutine_threadsafewithcall_soon_threadsafe+ensure_futurein_deliver_result()How to Test
gep-extractat 03:00)Timeout context manager should be used inside a taskVerified locally:
gep-extractandgep-feedbackcron jobs run and deliver successfully.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — not run (single file change, no test infrastructure changes)Documentation & Housekeeping
Screenshots / Logs
Cron delivery to Weixin now succeeds (previously timed out with the error above).