Skip to content

fix(qqbot): check ws.closed in _read_events to prevent idle loop on reconnect failure - #25070

Closed
bighamx wants to merge 1 commit into
NousResearch:mainfrom
bighamx:fix/qqbot-websocket-reconnect
Closed

fix(qqbot): check ws.closed in _read_events to prevent idle loop on reconnect failure#25070
bighamx wants to merge 1 commit into
NousResearch:mainfrom
bighamx:fix/qqbot-websocket-reconnect

Conversation

@bighamx

@bighamx bighamx commented May 13, 2026

Copy link
Copy Markdown

Bug

QQ Bot WebSocket reconnect enters an infinite idle loop after a failed reconnection.

Root Cause

When a WebSocket reconnect fails (e.g. transient network error getting the gateway URL), the old _ws object remains but is closed. _read_events() at line 631 only checked if not self._ws:, so a closed-but-not-None WebSocket passed the guard. The while loop condition not self._ws.closed evaluated to False immediately, causing _read_events() to return successfully. The event loop then reset the backoff counter (backoff_idx = 0), entering an infinite idle loop with no further reconnection attempts and no log output.

Fix

Add self._ws.closed check in the guard:

# Before:
if not self._ws:

# After:
if not self._ws or self._ws.closed:

This causes _read_events() to raise RuntimeError("WebSocket not connected") when the WS is closed, properly triggering the reconnect logic in the event loop.

Observed Behavior

  • WebSocket disconnects with code 4009 (session timeout) — normal, reconnects fine
  • Non-normal WebSocket close (no close code) at 17:17:53
  • Reconnect attempt at 17:18:00 fails: Reconnect failed: Failed to get QQ Bot gateway URL: (empty error, transient)
  • After that: complete silence — no more logs, no more reconnect attempts, bot stays offline indefinitely

After Fix

The closed WebSocket triggers RuntimeError, the event loop catches it and uses the existing backoff logic to retry reconnection.

…econnect failure

When a WebSocket reconnect fails, the old _ws object remains but is closed.
_read_events() only checked 'if not self._ws', so a closed-but-not-None
WebSocket caused the method to return immediately without error. The event
loop then reset the backoff counter, entering an infinite idle loop with
no further reconnection attempts.

Fix: add 'or self._ws.closed' check so a closed WebSocket raises
RuntimeError and triggers proper reconnect logic.
Copilot AI review requested due to automatic review settings May 13, 2026 15:49

Copilot AI 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.

Pull request overview

Fixes a QQ Bot reconnect edge case where a failed reconnection could leave a non-None but closed WebSocket object in self._ws, causing _read_events() to return immediately and the outer loop to reset backoff and idle indefinitely.

Changes:

  • Treat a closed WebSocket the same as “not connected” in _read_events() by checking self._ws.closed.
  • Ensure _read_events() raises RuntimeError("WebSocket not connected") when the WS is closed, so the existing reconnect/backoff logic is engaged.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@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/qqbot QQ Bot adapter labels May 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #17704 — identical fix (adding self._ws.closed check in _read_events() guard). Also closely related to #20994 which takes an alternative approach (nulling self._ws on reconnect failure).

See also #19414 for a superset fix covering multiple QQBot reconnect bugs.

@teknium1

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: this QQBot reconnect fix is already implemented on current main.

Evidence:

  • gateway/platforms/qqbot/adapter.py:684 now checks self._ws.closed on entry to _read_events() and raises RuntimeError("WebSocket closed"), so a closed-but-non-None websocket no longer returns normally.
  • gateway/platforms/qqbot/adapter.py:639 routes that exception through the existing reconnect/backoff path instead of resetting backoff after a clean _read_events() return.
  • tests/gateway/test_qqbot.py:2210 covers the closed-on-entry websocket case with adapter._ws = SimpleNamespace(closed=True) and asserts _read_events() raises.
  • The implementation landed in 3eeca4613d618618093db416b564a2b9ef8dbe6a (fix(qqbot): stop 100% CPU spin when WebSocket is closed but not None (#31193, #31771) (#40574)).

Thanks for the focused report and patch. The current main branch has the same behavior covered by regression tests.

@teknium1 teknium1 closed this Jun 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 12, 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/qqbot QQ Bot 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.

4 participants