Skip to content

fix(weixin): replace aiohttp timeout with asyncio.wait_for for cross-thread safety - #17911

Closed
rj-chenlinfeng wants to merge 1 commit into
NousResearch:mainfrom
rj-chenlinfeng:fix/weixin-aiohttp-timeout-cross-thread
Closed

rj-chenlinfeng wants to merge 1 commit into
NousResearch:mainfrom
rj-chenlinfeng:fix/weixin-aiohttp-timeout-cross-thread

Conversation

@rj-chenlinfeng

Copy link
Copy Markdown

Problem

When cron jobs deliver messages via asyncio.run_coroutine_threadsafe(), the WeChat iLink platform adapter intermittently fails with:

Timeout context manager should be used inside a task

This is because aiohttp.ClientTimeout relies on BaseTimerContext which calls asyncio.current_task() internally. When a coroutine is submitted from a non-event-loop thread (e.g., cron scheduler thread), a race condition can cause current_task() to return None, triggering the error.

Solution

Replace aiohttp.ClientTimeout with asyncio.wait_for() in _api_post() and _api_get().

asyncio.wait_for() is a pure-asyncio timeout mechanism that does not depend on task context and is safe for cross-thread coroutine submission via run_coroutine_threadsafe().

Changes

  • gateway/platforms/weixin.py: _api_post() and _api_get() now wrap the HTTP call in an inner coroutine and use asyncio.wait_for() for timeout control instead of passing timeout= to session.post()/session.get().

Testing

@alt-glitch

Copy link
Copy Markdown
Contributor

Related to #16074, #14530, #12810, #17267 — all address the same WeChat cross-thread aiohttp timeout crash. Maintainer should pick one approach and close the rest.

@teknium1

Copy link
Copy Markdown
Collaborator

This looks implemented on current main; closing as an automated hermes-sweeper review.

Evidence:

  • gateway/platforms/weixin.py:370 / :393 now implement _api_post() and _api_get() with inner coroutines and asyncio.wait_for(...), without forwarding timeout= to session.post() / session.get().
  • Commit 566669013f3f9c0b52cb1392250b76d510d99dc7 is the matching fix: fix(weixin): replace aiohttp ClientTimeout with asyncio.wait_for in _api_post/_api_get, and its message describes the same cron run_coroutine_threadsafe() / Timeout context manager should be used inside a task failure.
  • tests/gateway/test_weixin.py:1106 adds regression coverage for the wait_for migration, including no aiohttp timeout kwarg and timeout behavior.
  • The fix is contained in release tag v2026.6.5.

The maintainer comment here correctly noted this belonged to a cluster of related WeChat timeout PRs; main appears to have picked and shipped this approach.

@teknium1 teknium1 closed this Jun 10, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 10, 2026
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 sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants