Skip to content

fix(weixin): remove _LIVE_ADAPTERS session reuse to avoid aiohttp timeout error - #22235

Closed
Scott1743 wants to merge 1 commit into
NousResearch:mainfrom
Scott1743:pr/weixin-session-fix
Closed

fix(weixin): remove _LIVE_ADAPTERS session reuse to avoid aiohttp timeout error#22235
Scott1743 wants to merge 1 commit into
NousResearch:mainfrom
Scott1743:pr/weixin-session-fix

Conversation

@Scott1743

Copy link
Copy Markdown

Problem

In aiohttp 3.13.5, context manager fails when a session created with is reused from a context via (where ).

The optimization that reuses an existing adapter+session across calls causes intermittent errors:

Fix

Always create a fresh per call instead of reusing . This is slightly less efficient but eliminates the non-deterministic timeout failures.

Changes

  • : remove session reuse logic in , always create fresh session

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

Copy link
Copy Markdown
Collaborator

Likely duplicate of #14481 and #13350 — same root cause: aiohttp session reuse in weixin direct send causes timeout errors across event loops. Multiple prior PRs target the same fix (#12810, #13361, #18890).

…eout error

aiohttp 3.13.5's asyncio.timeout() context guard fails when a session
created with trust_env=True is reused from a ThreadPoolExecutor context
via _run_async (loop.is_running()=True). The _LIVE_ADAPTERS optimization
that reuses an existing adapter+session causes intermittent
'Timeout context manager should be used inside a task' errors.

Fix: always create a fresh aiohttp.ClientSession per send_weixin_direct
call instead of reusing _LIVE_ADAPTERS. This is slightly less efficient
but eliminates the non-deterministic timeout failures.
@Scott1743
Scott1743 force-pushed the pr/weixin-session-fix branch from b75fdf7 to 2eb8624 Compare May 9, 2026 03:27
@Scott1743

Copy link
Copy Markdown
Author

@alt-glitch Thanks for the context. Agreed on the root cause — this is the same pattern. If any of the prior PRs (#12810, #13361, #18890) is already merged or close to merge, happy to close this as duplicate and defer. If not, this PR stands.

@teknium1

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: current main already fixes the Weixin direct-send aiohttp timeout failure this PR targets.

Evidence:

  • gateway/platforms/weixin.py:2286 now gates _LIVE_ADAPTERS reuse: the live adapter is reused only when its _send_session is open and send_session._loop is asyncio.get_running_loop(); otherwise send_weixin_direct() falls through to the fresh one-shot aiohttp.ClientSession path at gateway/platforms/weixin.py:2315.
  • gateway/platforms/weixin.py:1287 disables aiohttp's built-in ClientTimeout for the Weixin send session, with the inline comment tying it to the exact Timeout context manager should be used inside a task failure mode.
  • gateway/platforms/weixin.py:381 and gateway/platforms/weixin.py:405 enforce API timeouts with asyncio.wait_for() instead of aiohttp timeout kwargs.
  • tests/gateway/test_weixin.py:1106 covers the timeout-contract migration by asserting Weixin API helpers no longer forward aiohttp timeout kwargs and still raise on slow responses.

The PR's broader approach of removing live-adapter reuse entirely is no longer needed for the reported cross-event-loop timeout failure.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 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 platform/wecom WeCom / WeChat Work adapter 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