Skip to content

fix(qqbot): prevent Resume death-loop on Exception-disconnect path - #17990

Open
yunkai wants to merge 1 commit into
NousResearch:mainfrom
yunkai:fix/qqbot-exception-reconnect-death-loop
Open

yunkai wants to merge 1 commit into
NousResearch:mainfrom
yunkai:fix/qqbot-exception-reconnect-death-loop

Conversation

@yunkai

@yunkai yunkai commented Apr 30, 2026

Copy link
Copy Markdown

Problem

The except Exception branch in QQ Bot's _listen_loop handles WebSocket disconnects without a close code — typically triggered by QQ's TCP-layer idle timeout (~60s). This branch was missing three safety mechanisms present in the QQCloseError branch, causing the gateway to enter an infinite Resume death-loop: disconnect → Resume → disconnect every 60s, forever.

Pre-fix logs (April 29, 08:36–08:58)

WebSocket error → Reconnect → Resume sent (session_id=9aac..., seq=304)
WebSocket error → Reconnect → Resume sent (session_id=9aac..., seq=305)   # +60s
WebSocket error → Reconnect → Resume sent (session_id=9aac..., seq=306)   # +60s
...
Reconnect failed: Failed to get QQ Bot gateway URL   # rate-limited after ~22 min

Same session_id persists across all cycles, seq increments endlessly. QQ's TCP frontend does not reset its idle timer on WebSocket heartbeat frames, so Resume can never succeed without message traffic. Once rate-limited, the bot stays permanently offline — observed in production: 26+ hours of downtime.

Fix

Three mechanisms added to the except Exception branch:

  1. Exception-disconnect window tracking + forced Identify: Count disconnects in a 300-second sliding window. After 5 disconnects with the same session_id, clear the session to force a fresh Identify on the next reconnect — breaking the death-loop.

  2. Quick-disconnect detection: Mirror existing logic from the QQCloseError branch. Three disconnects within 5 seconds of connection triggers a fatal error with actionable guidance.

  3. Backoff with upper bound: backoff_idx now increments on reconnect failures and is checked against MAX_RECONNECT_ATTEMPTS (100). Previously backoff_idx was reset to 0 on every successful reconnect, making it impossible to ever give up.

Related: #15051

This PR builds on the direction of #15051 (adding quick-disconnect detection to the Exception path) and extends it with window counting + forced Identify + backoff cap — all three mechanisms together provide a complete defense against the death-loop.

Post-fix validation (April 30)

20:34  Identify → Ready (session_id=3ff5...)
21:04  WebSocket closed: code=4009 Session timed out
21:04  Session error (4009), clearing session for re-identify  ✅
21:04  Identify → Ready (session_id=3c5f...)                   ✅
21:34  Session timed out → clearing → Identify → Ready         ✅
22:04  Session timed out → clearing → Identify → Ready         ✅
22:34  Session timed out → clearing → Identify → Ready         ✅
  • ✅ No Resume death-loop (pre-fix: seq 304→324 continuously)
  • ✅ 4009 session timeout correctly triggers clearing → re-Identify
  • ✅ Connections rotate normally every ~30 minutes, zero exceptions
  • ✅ Fresh session_id on each Identify

The Exception handler (no close code) was missing:
1. Window-based disconnect counting — 5+ disconnects in 300s forces
   a fresh Identify instead of Resume, breaking the 60s idle-timeout
   death-loop on QQ's TCP frontend.
2. Quick-disconnect detection (mirrors QQCloseError branch).
3. Backoff with MAX_RECONNECT_ATTEMPTS upper bound — previously
   backoff_idx was reset to 0 on every successful reconnect,
   making it impossible to ever give up.

Root cause: QQ's TCP-layer idle timeout (~60s) is not reset by
WebSocket heartbeat frames on the load-balancer tier, so Resume
sessions with no message traffic enter an infinite reconnect cycle.
@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 Apr 30, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Related to #15051 (open PR fixing same exception-path disconnect loop) and #15490, #17703. This PR adds window counting + forced Identify + backoff cap on top of quick-disconnect detection.

@alt-glitch

Copy link
Copy Markdown
Contributor

Related to #15051 (open PR fixing same exception-path disconnect loop) and #15490, #17703.

jiqfu added a commit to jiqfu/hermes-agent that referenced this pull request May 1, 2026
Cherry-pick PR NousResearch#17990 from yunkai: fix exception-disconnect window tracking,
quick-disconnect detection, and backoff upper bound for the except Exception
branch in _listen_loop().

Fixes: 5+ exception-disconnects within 300s with same session_id now triggers
session clear → forced fresh Identify, breaking the infinite Resume death-loop.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for isolating the generic exception-path loop. Current main still resets reconnect state after every successful socket reopen (gateway/platforms/qqbot/adapter.py:651-655), so this remains worth salvaging.

Problems

  • The added quick-disconnect increment at PR line 579 is undone by the unchanged success arm at PR lines 606-608. A reconnect-success-immediate-disconnect cycle therefore never reaches MAX_QUICK_DISCONNECT_COUNT.
  • The advertised backoff cap has the same issue: PR line 607 resets backoff_idx, so the existing cap cannot bound a cycle with successful socket opens.
  • The diff changes only the adapter; it adds no regression test. Current tests/gateway/test_qqbot.py:2220-2241 covers the closed-socket raise, not repeated generic failures after successful reconnects.
  • Current main now requires _mark_transport_disconnected() for this path (gateway/platforms/qqbot/adapter.py:643; commit 8199ec380), so the older _mark_disconnected() context should not be carried forward.

Suggested changes

  • Preserve a loop-detection counter until a connection survives the configured threshold, and test successful reconnect followed by immediate generic failure.
  • Keep current transport/lifecycle separation while salvaging the recovery policy.

Automated hermes-sweeper review.

# --- Quick disconnect detection (mirror QQCloseError branch) ---
duration = time.monotonic() - connect_time
if duration < QUICK_DISCONNECT_THRESHOLD and connect_time > 0:
quick_disconnect_count += 1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This increment is reset by the unchanged successful-reconnect arm at lines 606-608. In the reported reconnect-success-immediate-close cycle, the counter therefore stays at 1 and never reaches MAX_QUICK_DISCONNECT_COUNT; retain it until a connection survives the threshold or reset it from a confirmed healthy-session event.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 12, 2026

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants