Skip to content

fix(weixin): asyncio.wait_for timeout in _api_post/_api_get (+ regression tests, cluster closure) - #35117

Merged
teknium1 merged 3 commits into
NousResearch:mainfrom
banditburai:worktree-fix+weixin-asyncio-loops
Jun 2, 2026
Merged

fix(weixin): asyncio.wait_for timeout in _api_post/_api_get (+ regression tests, cluster closure)#35117
teknium1 merged 3 commits into
NousResearch:mainfrom
banditburai:worktree-fix+weixin-asyncio-loops

Conversation

@banditburai

@banditburai banditburai commented May 30, 2026

Copy link
Copy Markdown
Contributor

Credit: the production fix is authored by @caojiguang (#31853), carried here with their git authorship preserved. This PR adds the regression suite and resolves the broader Weixin asyncio-timeout cluster. Independent fix + test work by @ryan-flow (#32179) is acknowledged.

Problem

Cron and other non-gateway WeChat sends fail with:

RuntimeError: Timeout context manager should be used inside a task

send_message runs _api_post/_api_get in a worker thread with its own event loop (via _run_async), while the aiohttp session is bound to the gateway loop. aiohttp.ClientTimeout is implemented through async_timeout.TimerContext, whose __enter__ calls asyncio.current_task(loop=session._loop) — the gateway loop, not the worker loop — which returns None and raises. The request never reaches the wire. The long-poll path was unaffected (already inside a Task), which is why this only bit cron / out-of-band sends.

Fix

Replace the per-request aiohttp.ClientTimeout with asyncio.wait_for(coro, timeout=timeout_ms/1000), wrapping each request body in a local coroutine. wait_for ensure_future-wraps the coroutine into a real Task and enforces the deadline via loop.call_later + fut.cancel() — it never calls current_task(), so it is event-loop-context-agnostic.

This completes the migration begun in #21196, which moved the sibling helpers _upload_ciphertext / _download_bytes / _download_remote_media to the same wait_for pattern; _api_post / _api_get were the last two helpers still on per-request ClientTimeout. All five Weixin HTTP helpers now use one timeout mechanism.

Supporting context (unchanged, already on main): the send session is built with aiohttp.ClientTimeout(total=None, connect=None, sock_connect=None, sock_read=None), so the session-level aiohttp timer is fully disabled and all timeout budget is owned by the per-request wait_for.

Scope / non-goals

Two files: gateway/platforms/weixin.py, tests/gateway/test_weixin.py. This PR does not touch:

Behavior / compatibility

  • Timeout budget unchanged — the per-request ClientTimeout already bounded these calls by the same timeout_ms (API_TIMEOUT_MS=15s, CONFIG_TIMEOUT_MS=10s, LONG_POLL_TIMEOUT_MS=35s); wait_for enforces the identical budget. No truncation of previously-successful calls.
  • On timeout, wait_for raises asyncio.TimeoutError; on Python 3.12 this is TimeoutError and is not a CancelledError subclass, so the poll loop's except CancelledError does not swallow it — real sends retry then re-raise.
  • _get_updates long-poll swallow is unchanged: it still catches asyncio.TimeoutError and returns the empty-batch sentinel {"ret": 0, "msgs": [], "get_updates_buf": sync_buf}.
  • On timeout, wait_for cancels the inner coroutine mid-async with session.post(...); aiohttp's response context manager releases the connection in __aexit__ — no leaked connection. Same pattern as the production sibling helpers.

Tests

python -m pytest tests/gateway/test_weixin.py tests/test_model_tools_async_bridge.py -q
# 73 passed

TestWeixinApiTimeout (7 cases):

Test Helper Contract
happy-path _api_post / _api_get returns parsed JSON; no timeout= kwarg forwarded to the session call (regression guard against reintroducing ClientTimeout)
slow response _api_post / _api_get wait_for actually fires and raises asyncio.TimeoutError (real timeout, not mocked)
non-2xx _api_post / _api_get raises RuntimeError carrying HTTP status + truncated body
long-poll _get_updates underlying asyncio.TimeoutError is swallowed into the empty-batch sentinel (loop survives)

Note: tests pin the observable contract (no timeout= forwarded; wait_for raises; _get_updates swallows) rather than reproducing the loop-context crash in-process, which requires real aiohttp request machinery and is impractical to unit-test.

Cluster closure

Resolves the Weixin asyncio-timeout cluster (15 active items). The eight issues below all report the timeout-context crash on the text-send path this fix removes:

Closes #13099, Closes #13281, Closes #16293, Closes #17347, Closes #18014, Closes #18836, Closes #20229, Closes #24142

Referenced, not closed by this diff:

Supersedes:

Known residuals / out of scope: #16570, #8043 — their timeout paths live in model_tools.py, untouched by this diff.

Infographic

gateway-reliability-three-fixes

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/wecom WeCom / WeChat Work adapter P2 Medium — degraded but workaround exists labels May 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This PR supersedes #31853 (by @caojiguang) — carries the same production fix with preserved authorship, adds regression tests, and resolves the broader Weixin asyncio-timeout cluster (#29037, #24248, #19050, #17911). Also acknowledges independent work in #32179 (by @ryan-flow, closed).

caojiguang and others added 3 commits May 30, 2026 11:51
…api_post/_api_get

Cron delivery to WeChat fails with 'Timeout context manager should
be used inside a task' because _api_post and _api_get use aiohttp's
ClientTimeout directly.  When the cron scheduler calls send() via
asyncio.run_coroutine_threadsafe(), aiohttp cannot find a running
task and raises RuntimeError.

_upload_media, _download_bytes, and _download_remote_media already
use asyncio.wait_for() to avoid this.  Apply the same pattern to
_api_post and _api_get — the two remaining iLink API helpers that
still use the raw ClientTimeout approach.

This fixes cron delivery errors seen on the WeChat platform adapter
when meyo-external cron jobs attempt to deliver output to WeChat.
The fix commit preserves @caojiguang's authorship (from NousResearch#31853); the
release-notes AUTHOR_MAP gate requires their email to map to a GitHub
username.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/wecom WeCom / WeChat Work adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants