Skip to content

fix(web): prevent WebSocket reconnection storm in session stream#1386

Merged
RealKai42 merged 3 commits intomainfrom
fix/web-websocket-reconnect-storm
Mar 10, 2026
Merged

fix(web): prevent WebSocket reconnection storm in session stream#1386
RealKai42 merged 3 commits intomainfrom
fix/web-websocket-reconnect-storm

Conversation

@YoungY620
Copy link
Copy Markdown
Collaborator

@YoungY620 YoungY620 commented Mar 10, 2026

Related Issue

Internal discovery — no linked issue.

Description

useSessionStream hook suffered from a WebSocket reconnection storm caused by unstable callback identity chains:

  1. resetState depended on slashCommands.length in its useCallback dependency array
  2. When the initialize response set slashCommands, resetState identity changed
  3. This cascaded through connectuseLayoutEffect → disconnect/reconnect → loop

Fixes:

  • Stabilize resetState identity: Replace slashCommands.length dependency with a slashCommandsLenRef ref, and use a resetStateRef to access resetState from effects without adding it to dependency arrays
  • Preserve slash commands across session switches: Pass preserveSlashCommands=true in useLayoutEffect and clearMessages so available commands are not cleared during the gap before the initialize response arrives
  • Guard null refs on unmount: Add optional chaining (?.) on Map.clear() calls inside resetState to prevent TypeError: Cannot read properties of null (reading 'clear') when React nulls out refs during component teardown

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked the related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run make gen-changelog to update the changelog.
  • I have run make gen-docs to update the user documentation.

Open with Devin

…switches

- Add optional chaining on Map.clear() calls in resetState to prevent
  TypeError during React unmount when refs are already null
- Pass preserveSlashCommands=true in useLayoutEffect and clearMessages
  to avoid clearing available commands during session switches

Entire-Checkpoint: 2977bc73228c
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@RealKai42 RealKai42 merged commit 6dceb24 into main Mar 10, 2026
14 checks passed
@RealKai42 RealKai42 deleted the fix/web-websocket-reconnect-storm branch March 10, 2026 10:00
howardpen9 added a commit to howardpen9/kimi-cli that referenced this pull request Mar 12, 2026
…n storms

The `connect` callback had many unstable dependencies (handleMessage,
onError, sendInitialize, etc.) that frequently changed identity due to
their own deep dependency chains. When connect changed, it cascaded to
reconnect → sendMessage → queue auto-send effects, potentially causing
rapid connect/close/reconnect cycles.

This extends the ref-based pattern from MoonshotAI#1386 to all callbacks used
inside connect(). By reading from refs (handleMessageRef, onErrorRef,
etc.) instead of capturing callbacks directly, connect() only depends on
sessionId and stable state setters, breaking the cascade.

Closes MoonshotAI#1409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants