Skip to content

fix(wecom): raise on silent ws close to prevent 100% CPU reconnect spin - #54574

Open
leonxia1010 wants to merge 2 commits into
NousResearch:mainfrom
leonxia1010:pr/wecom-silent-ws-close-spin
Open

leonxia1010 wants to merge 2 commits into
NousResearch:mainfrom
leonxia1010:pr/wecom-silent-ws-close-spin

Conversation

@leonxia1010

Copy link
Copy Markdown

What does this PR do?

_read_events() in the WeCom adapter exited cleanly when self._ws became
None or closed between iterations without any CLOSE-typed message arriving.
_listen_loop treated this as a successful read, reset backoff_idx to 0, and
immediately re-entered _read_events — which returned cleanly again.

Result: a tight loop that burned ~100% CPU indefinitely with no asyncio.sleep
ever firing.

Observed in production as a single profile accumulating 90+ CPU-hours over
11 days
while gateway.log went silent.

This complements #28311 (fix(wecom): handle WSMsgType.CLOSING to prevent CPU spin), which covered graceful server-side shutdowns via a new terminal
message type. This PR covers the orthogonal case where the websocket
transitions to closed/None between iterations without emitting any terminal
message type at all.

Related Issue

No prior issue — surfaced in a local production deployment after an extended
silent-close interval. Reproducer included in the new test class.

Fixes # (none)

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/platforms/wecom/adapter.py_read_events() now raises
    RuntimeError("WeCom websocket closed (no message)") when the loop exits
    while self._running is True. _listen_loop already catches this and
    applies the existing [2, 5, 10, 30, 60] RECONNECT_BACKOFF.
  • tests/gateway/test_wecom.py — new regression class
    TestReadEventsRaisesOnSilentClose with three cases (ws=None, ws closed
    without close-typed message, clean shutdown when not running).
  • scripts/release.py — add huachi1990@hotmail.comleonxia1010
    mapping to AUTHOR_MAP so contributor-check.yml passes for this PR's
    fix commit.

How to Test

  1. scripts/run_tests.sh (or venv/bin/python -m pytest tests/gateway/test_wecom.py -v)
  2. Specifically the new class:
    pytest tests/gateway/test_wecom.py::TestReadEventsRaisesOnSilentClose -v
    should report 3 passed.
  3. Manual: run hermes connected to WeCom, force a silent ws close (e.g.
    break the tunnel without sending a CLOSE frame). With the fix, reconnect
    backoff kicks in ([2,5,10,30,60]s); without it, CPU pegs at 100% and
    gateway.log goes silent.

Checklist

Code

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A (pure asyncio control flow, no public API or doc surface)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — verified on macOS; pure asyncio + aiohttp behavior is identical across platforms
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

_read_events exited cleanly when self._ws became None or closed between
iterations (no CLOSE-typed message). _listen_loop treated this as a
successful read, reset backoff_idx to 0, and immediately re-entered
_read_events — which returned cleanly again. Result: a tight loop that
burned ~100% CPU indefinitely with no asyncio.sleep ever firing,
observed in production as a single profile accumulating 90+ CPU-hours
over 11 days while gateway.log went silent.

Raise explicitly when the loop exits while still _running so the caller
takes the except path and applies the existing [2, 5, 10, 30, 60]
RECONNECT_BACKOFF.
Required by .github/workflows/contributor-check.yml so the attribution
check on this PR's fix commit passes.
@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 Jun 29, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: this fixes issue #49918 (the _read_events() post-loop gap). Competing open PRs for the same WeCom CPU-spin: #49941 (empty-TEXT-frame streak guard) and #28306 (auth-fail closed-ws guard) — distinct mechanisms. This PR adds the direct post-loop raise for the silent-close (no CLOSE message) path. A maintainer should pick one canonical fix.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused regression fix. The premise remains present on current main: plugins/platforms/wecom/adapter.py:367 permits _read_events() to return normally when a websocket becomes closed without yielding a terminal frame, while plugins/platforms/wecom/adapter.py:340-352 treats that return as success and resets backoff. The proposed post-loop raise routes this path through the existing reconnect and sleep handling.

The approach also matches the established closed-websocket guard in gateway/platforms/qqbot/adapter.py:692-711. No blocking correctness issue was found in the reviewed diff.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 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: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