fix: throttle/debounce high-frequency WebSocket events to eliminate UI stuttering#73
Conversation
…I stuttering (suresh1319#67) Problem: The client emits state updates on every individual micro-change (keystrokes, cursor moves, awareness changes) without any rate limiting. With multiple users, this floods the WebSocket channel and causes main-thread blocking, UI stuttering, high CPU usage, and frame drops. Solution — three layers of rate limiting: 1. Client-side (Editor.js): - Throttle cursor-position awareness broadcasts (50ms / ~20 fps) - Throttle awareness-change DOM bookmark rebuilds (80ms) - Debounce code-change React state snapshots (300ms) 2. Parent component (EditorPage.js): - Document the debounced data flow from Editor → EditorPage 3. Server-side (server.js): - Per-socket sliding-window rate limiter on CODE_CHANGE events (max 30/sec). Excess events are silently dropped — the Yjs CRDT layer handles eventual consistency. 4. New utility (src/utils/throttle.js): - Zero-dependency throttle and debounce factories with proper cleanup (cancel/flush) to prevent memory leaks on unmount. Closes suresh1319#67
Confidence Score: 3/5 - Review RecommendedLikely safe but review recommended — the PR's goal of throttling/debouncing high-frequency WebSocket events to reduce UI stuttering is well-motivated, but the rate limiter implementation in Key Findings:
Files requiring special attention
|
Update regarding AI Review Bot FeedbackI have updated the PR description to accurately reflect the implementation details:
The fixed-window implementation efficiently caps the |
…x PERMISSION_DENIED bug (suresh1319#67) - Enhanced createThrottle with leading+trailing edge semantics, input validation, and pending() introspection. - Enhanced createDebounce with input validation, pending(), and safe flush() on cleanup. - Added 19 focused unit tests covering edge cases, argument forwarding, cancel/flush lifecycle, and input validation. - Fixed PERMISSION_DENIED bug: added missing action constant to Actions.js and replaced raw error emits in server.js so permission-denial toasts actually reach the client. - Refined Editor.js cleanup ordering: flush code-change debounce before cancelling cursor/awareness throttles.
EntelligenceAI PR SummaryRefactors error signaling in the file upload socket event handler to use a single communication channel, preventing double-toast UX regressions.
Confidence Score: 3/5 - Review RecommendedLikely safe but review recommended — this PR makes a targeted and sensible fix by removing duplicate Key Findings:
Files requiring special attention
|
Hardened createThrottle / createDebounce utilities (src/utils/throttle.js)
Comprehensive test suite (src/utils/throttle.test.js) -- 19 tests
Fixed PERMISSION_DENIED bug (src/Actions.js + server.js)
Refined Editor.js cleanup ordering
node --check src/components/Editor.js -> ok |
…iding-window rate limiter, remove dead import - Removed unused useMemo import from Editor.js (maintainability nit). - Added ACTIONS.INVALID_PAYLOAD constant in Actions.js — semantically separates input-validation errors from auth/authz rejections so client handlers can distinguish between the two. - Changed server.js !Array.isArray(nodes) check to emit INVALID_PAYLOAD instead of PERMISSION_DENIED — fixes the correctness issue where a malformed request was mislabeled as a permission failure. - Replaced fixed-window rate limiter in server.js with a true sliding-window implementation using a timestamp queue — the old approach allowed up to 2x the limit at window boundaries. - Added INVALID_PAYLOAD listener and cleanup in EditorPage.js so malformed-request toasts are surfaced to the user.
| socketRef.current.on(ACTIONS.INVALID_PAYLOAD, (payload) => { | ||
| const message = | ||
| typeof payload === 'string' | ||
| ? payload | ||
| : payload && typeof payload === 'object' && 'message' in payload | ||
| ? payload.message | ||
| : 'Invalid request.'; | ||
| toast.error(message); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Double error toast when FS_UPLOAD_BATCH receives invalid nodes payload
server.js:205-208 emits INVALID_PAYLOAD AND calls reply(false, …) for the same validation failure. EditorPage.js handles both: the INVALID_PAYLOAD socket listener (line 245) fires toast.error, and the FS_UPLOAD_BATCH ack callback (line 567) also fires toast.error — showing two identical toasts.
| socketRef.current.on(ACTIONS.INVALID_PAYLOAD, (payload) => { | |
| const message = | |
| typeof payload === 'string' | |
| ? payload | |
| : payload && typeof payload === 'object' && 'message' in payload | |
| ? payload.message | |
| : 'Invalid request.'; | |
| toast.error(message); | |
| }); | |
| socketRef.current.on(ACTIONS.INVALID_PAYLOAD, (payload) => { | |
| // Only handle INVALID_PAYLOAD events that are NOT from FS_UPLOAD_BATCH, | |
| // which already surfaces the error via its ack callback. | |
| const message = | |
| typeof payload === 'string' | |
| ? payload | |
| : payload && typeof payload === 'object' && 'message' in payload | |
| ? payload.message | |
| : 'Invalid request.'; | |
| toast.warn(message); | |
| }); |
Prompt to fix with AI
Copy this prompt into your AI coding assistant to fix this issue.
In server.js the FS_UPLOAD_BATCH invalid-nodes branch (line 205-208) fires both a socket.emit(ACTIONS.INVALID_PAYLOAD, …) and reply(false, …). In EditorPage.js both the new INVALID_PAYLOAD listener (line 245) and the FS_UPLOAD_BATCH ack callback (line 563-568) call toast.error, so the user sees two toasts. Fix by removing socket.emit(ACTIONS.INVALID_PAYLOAD, …) from the FS_UPLOAD_BATCH handler in server.js and relying solely on the ack reply to surface this error, keeping INVALID_PAYLOAD reserved for fire-and-forget events that have no ack channel.
@Pratyush-Panda-2006 please refer to this |
…double toasts - Remove socket.emit(PERMISSION_DENIED) from upload permission check, keeping only the ack reply callback. EditorPage.js ack handler already shows an error toast, so emitting both caused duplicate toast notifications. - Remove socket.emit(INVALID_PAYLOAD) from upload payload validation, keeping only the ack reply callback. Same dual-signal issue caused a double-toast UX regression for invalid payloads. Both error paths now use exactly one signal (the ack callback) per failure, ensuring the user sees a single, clear error toast.
|
@suresh1319 fixed it |
|
@Pratyush-Panda-2006 solve the conflicts |
Summary
Fixes #67 - Eliminates UI stuttering, frame drops, and high CPU usage caused by unthrottled real-time WebSocket events during multi-user collaborative sessions.
Problem
The client application emits state update payloads to the WebSocket server on every individual micro-change (keystrokes, cursor movements, awareness state changes). With multiple active users in a room, this creates a flood of network traffic that blocks the main thread and degrades the UI.
Solution
Implements a three-layer rate-limiting strategy across client and server:
1. Client - Cursor broadcasts (
Editor.js)createThrottle()2. Client - Awareness-change DOM updates (
Editor.js)changehandler rebuilds DOM bookmark widgets (cursor labels) for every peer3. Client - Code-change state snapshots (
Editor.jstoEditorPage.js)createDebounce()withflush()on unmountfileContentsRef(used for downloads/runs) is still updated synchronouslysetStatecall (which triggers a re-render) is deferred4. Server - Per-socket CODE_CHANGE rate limiter (
server.js)5. New utility -
src/utils/throttle.jscreateThrottle()andcreateDebounce()factoriescancel()andflush()methods to prevent memory leaks on component unmountFiles Changed
src/utils/throttle.jssrc/components/Editor.jssrc/pages/EditorPage.jsserver.jsPerformance Impact
Testing