Skip to content

fix(weixin): propagate asyncio.TimeoutError from _get_updates to prevent zombie connections - #23529

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/weixin-zombie-connection-23523
Open

fix(weixin): propagate asyncio.TimeoutError from _get_updates to prevent zombie connections#23529
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/weixin-zombie-connection-23523

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 11, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes WeChat gateway zombie connection issue where network drops (WiFi reconnect, macOS sleep/wake) cause the adapter to report "connected" indefinitely but no messages can be sent or received.

Root Cause

_get_updates() caught asyncio.TimeoutError and returned an empty success response (ret=0). This caused the poll loop to:

  1. Reset consecutive_failures to 0
  2. Continue polling on a dead network connection
  3. Never detect the connection was lost
  4. Report weixin.state=connected forever in /health/detailed

The connection was dead, but the gateway never detected it.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • See commit messages for detailed changes

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

@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 11, 2026
@liuhao1024
liuhao1024 force-pushed the fix/weixin-zombie-connection-23523 branch from 820ad18 to 9ed9aa7 Compare May 11, 2026 03:36
…ent zombie connections

Previously, _get_updates() caught asyncio.TimeoutError and returned an empty
success response (ret=0). This caused the poll loop to reset
consecutive_failures to 0 and continue polling indefinitely on a dead
network connection, creating a zombie state where the gateway reported
'connected' but no messages could be sent or received.

Now, _get_updates() propagates asyncio.TimeoutError, allowing the poll
loop's exception handler to treat it as a failure, increment
consecutive_failures, log the error, and retry with backoff.

Fixes NousResearch#23523

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the timeout-as-success conversion; current main still has that behavior in gateway/platforms/weixin.py:434-435.

Problems

  • Propagation alone does not resolve the reported connected-forever state. The timeout would reach the generic handler at gateway/platforms/weixin.py:1390-1395, which retries and resets the counter but does not call _set_fatal_error, disconnect, or notify the reconnect path. _mark_connected() remains active from gateway/platforms/weixin.py:1295-1296.
  • The test hunk is stale: current tests/gateway/test_weixin.py:1194-1207 explicitly asserts the old sentinel behavior. It must be replaced, not supplemented, or the salvaged change leaves contradictory tests.

Suggested changes

  • After the selected consecutive-failure threshold, transition through the existing fatal/reconnect lifecycle or rebuild the poll session, and add a poll-loop-level regression for that behavior.
  • Update the current sentinel test to require asyncio.TimeoutError propagation.

Automated hermes-sweeper review.

base_url=base_url,
endpoint=EP_GET_UPDATES,
payload={"get_updates_buf": sync_buf},
token=token,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Propagating the timeout is necessary, but _poll_loop currently catches it generically and only retries/resets its counter; it never clears connected state or enters the fatal/reconnect lifecycle. Please add the threshold transition/rebuild path, otherwise the reported health-state zombie remains.

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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants