From a9deda6d5a40da952b221da03d781f85ea8f1b0a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Nov 2025 16:07:36 +0000 Subject: [PATCH 1/3] fix(ui): prevent WebSocket reconnection loop causing UI refresh The WebSocket reconnection dialog was causing an infinite reconnection loop that made the UI unusable. This was happening because: 1. WebSocket callback functions were being recreated on every render 2. When callbacks changed, the WebSocket hooks saw them as new dependencies 3. This triggered WebSocket disconnection and reconnection 4. State updates from reconnection triggered component re-renders 5. Back to step 1, creating an infinite loop Fixed by: - Using useRef to store WebSocket callbacks in both useWebSocket and useEnterpriseWebSocket hooks - Updating refs when callbacks change instead of recreating connections - Wrapping callbacks in useCallback in Dashboard, SessionViewer, and SharedSessions pages (defense in depth) - Removing callback dependencies from WebSocket connect() functions This ensures that callback changes don't trigger reconnection, breaking the loop and making the UI stable again. Fixes reconnection issues from PRs #60 and #61. --- ui/src/hooks/useEnterpriseWebSocket.ts | 43 +++++++++++++++++++------- ui/src/hooks/useWebSocket.ts | 33 +++++++++++++++++--- ui/src/pages/Dashboard.tsx | 16 +++++++--- ui/src/pages/SessionViewer.tsx | 9 ++++-- ui/src/pages/SharedSessions.tsx | 9 ++++-- 5 files changed, 82 insertions(+), 28 deletions(-) diff --git a/ui/src/hooks/useEnterpriseWebSocket.ts b/ui/src/hooks/useEnterpriseWebSocket.ts index a4f965ef..8048c460 100644 --- a/ui/src/hooks/useEnterpriseWebSocket.ts +++ b/ui/src/hooks/useEnterpriseWebSocket.ts @@ -52,6 +52,29 @@ export function useEnterpriseWebSocket( const reconnectTimeoutRef = useRef(null); const shouldReconnectRef = useRef(true); + // Store callbacks in refs to avoid reconnection when they change + const onMessageRef = useRef(onMessage); + const onErrorRef = useRef(onError); + const onOpenRef = useRef(onOpen); + const onCloseRef = useRef(onClose); + + // Update refs when callbacks change + useEffect(() => { + onMessageRef.current = onMessage; + }, [onMessage]); + + useEffect(() => { + onErrorRef.current = onError; + }, [onError]); + + useEffect(() => { + onOpenRef.current = onOpen; + }, [onOpen]); + + useEffect(() => { + onCloseRef.current = onClose; + }, [onClose]); + const getWebSocketUrl = useCallback(() => { const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; const host = window.location.host; @@ -84,8 +107,8 @@ export function useEnterpriseWebSocket( setReconnectAttempts(0); shouldReconnectRef.current = true; - if (onOpen) { - onOpen(); + if (onOpenRef.current) { + onOpenRef.current(); } }; @@ -96,8 +119,8 @@ export function useEnterpriseWebSocket( setLastMessage(message); - if (onMessage) { - onMessage(message); + if (onMessageRef.current) { + onMessageRef.current(message); } } catch (error) { console.error('[WebSocket] Failed to parse message:', error); @@ -108,8 +131,8 @@ export function useEnterpriseWebSocket( console.error('[WebSocket] Error:', error); setIsConnected(false); - if (onError) { - onError(error); + if (onErrorRef.current) { + onErrorRef.current(error); } }; @@ -117,8 +140,8 @@ export function useEnterpriseWebSocket( // console.log('[WebSocket] Connection closed'); setIsConnected(false); - if (onClose) { - onClose(); + if (onCloseRef.current) { + onCloseRef.current(); } // Attempt reconnection if enabled and within retry limit @@ -145,10 +168,6 @@ export function useEnterpriseWebSocket( } }, [ getWebSocketUrl, - onOpen, - onMessage, - onError, - onClose, autoReconnect, reconnectInterval, maxReconnectAttempts, diff --git a/ui/src/hooks/useWebSocket.ts b/ui/src/hooks/useWebSocket.ts index 10aa6728..580c7805 100644 --- a/ui/src/hooks/useWebSocket.ts +++ b/ui/src/hooks/useWebSocket.ts @@ -40,6 +40,29 @@ export function useWebSocket({ const shouldReconnectRef = useRef(true); const reconnectAttemptsRef = useRef(0); + // Store callbacks in refs to avoid reconnection when they change + const onMessageRef = useRef(onMessage); + const onErrorRef = useRef(onError); + const onOpenRef = useRef(onOpen); + const onCloseRef = useRef(onClose); + + // Update refs when callbacks change + useEffect(() => { + onMessageRef.current = onMessage; + }, [onMessage]); + + useEffect(() => { + onErrorRef.current = onError; + }, [onError]); + + useEffect(() => { + onOpenRef.current = onOpen; + }, [onOpen]); + + useEffect(() => { + onCloseRef.current = onClose; + }, [onClose]); + const connect = useCallback(() => { try { const ws = new WebSocket(url); @@ -49,13 +72,13 @@ export function useWebSocket({ setIsConnected(true); setReconnectAttempts(0); reconnectAttemptsRef.current = 0; - onOpen?.(); + onOpenRef.current?.(); }; ws.onmessage = (event) => { try { const data = JSON.parse(event.data); - onMessage(data); + onMessageRef.current(data); } catch (error) { console.error('Failed to parse WebSocket message:', error); } @@ -63,13 +86,13 @@ export function useWebSocket({ ws.onerror = (error) => { console.error('WebSocket error:', error); - onError?.(error); + onErrorRef.current?.(error); }; ws.onclose = () => { // console.log(`WebSocket closed: ${url}`); setIsConnected(false); - onClose?.(); + onCloseRef.current?.(); // Attempt reconnection with exponential backoff const currentAttempts = reconnectAttemptsRef.current; @@ -91,7 +114,7 @@ export function useWebSocket({ } catch (error) { console.error('Failed to create WebSocket connection:', error); } - }, [url, onMessage, onError, onOpen, onClose, reconnectInterval, maxReconnectAttempts]); + }, [url, reconnectInterval, maxReconnectAttempts]); const sendMessage = useCallback((message: any) => { if (wsRef.current?.readyState === WebSocket.OPEN) { diff --git a/ui/src/pages/Dashboard.tsx b/ui/src/pages/Dashboard.tsx index 57fcdfba..617eeb5c 100644 --- a/ui/src/pages/Dashboard.tsx +++ b/ui/src/pages/Dashboard.tsx @@ -1,4 +1,4 @@ -import { useState, useRef } from 'react'; +import { useState, useRef, useCallback } from 'react'; import { Grid, Paper, Typography, Box, Card, CardContent, Chip } from '@mui/material'; import { Computer as ComputerIcon, @@ -72,7 +72,8 @@ export default function Dashboard() { const { data: repositories = [], isLoading: reposLoading } = useRepositories(); // Real-time sessions updates via WebSocket with notifications - const baseSessionsWs = useSessionsWebSocket((updatedSessions) => { + // Wrap callback in useCallback to prevent reconnection loop + const handleSessionsUpdate = useCallback((updatedSessions: Session[]) => { // Filter to only show current user's sessions const userSessions = username ? updatedSessions.filter((s: Session) => s.user === username) @@ -93,15 +94,20 @@ export default function Dashboard() { }); setSessions(userSessions); - }); + }, [username, addNotification]); + + const baseSessionsWs = useSessionsWebSocket(handleSessionsUpdate); // Enhanced WebSocket with connection quality and manual reconnect const sessionsWs = useEnhancedWebSocket(baseSessionsWs); // Real-time metrics updates via WebSocket - const metricsWs = useMetricsWebSocket((updatedMetrics) => { + // Wrap callback in useCallback to prevent reconnection loop + const handleMetricsUpdate = useCallback((updatedMetrics: any) => { setMetrics(updatedMetrics); - }); + }, []); + + const metricsWs = useMetricsWebSocket(handleMetricsUpdate); const stats = [ { diff --git a/ui/src/pages/SessionViewer.tsx b/ui/src/pages/SessionViewer.tsx index 13a2e5d4..f180b33a 100644 --- a/ui/src/pages/SessionViewer.tsx +++ b/ui/src/pages/SessionViewer.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef } from 'react'; +import { useState, useEffect, useRef, useCallback } from 'react'; import { useParams, useNavigate } from 'react-router-dom'; import { Box, @@ -132,7 +132,8 @@ export default function SessionViewer() { const { addNotification } = useNotificationQueue(); // Real-time session updates via WebSocket with notifications - const baseWebSocket = useSessionsWebSocket((updatedSessions) => { + // Wrap callback in useCallback to prevent reconnection loop + const handleSessionUpdate = useCallback((updatedSessions: any[]) => { if (!sessionId) return; // Find this session in the update @@ -157,7 +158,9 @@ export default function SessionViewer() { prevStateRef.current = updatedSession.state; setSession(updatedSession); } - }); + }, [sessionId, session, addNotification]); + + const baseWebSocket = useSessionsWebSocket(handleSessionUpdate); // Enhanced WebSocket with connection quality and manual reconnect const enhanced = useEnhancedWebSocket(baseWebSocket); diff --git a/ui/src/pages/SharedSessions.tsx b/ui/src/pages/SharedSessions.tsx index ba96e6d3..f428ec6a 100644 --- a/ui/src/pages/SharedSessions.tsx +++ b/ui/src/pages/SharedSessions.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef } from 'react'; +import { useState, useEffect, useRef, useCallback } from 'react'; import { Box, Typography, @@ -112,7 +112,8 @@ export default function SharedSessions() { const { addNotification } = useNotificationQueue(); // Real-time session updates via WebSocket with notifications - const baseWebSocket = useSessionsWebSocket((updatedSessions) => { + // Wrap callback in useCallback to prevent reconnection loop + const handleSessionsUpdate = useCallback((updatedSessions: any[]) => { if (!currentUser?.id || sessions.length === 0) return; // Update shared sessions with real-time data and show notifications for changes @@ -144,7 +145,9 @@ export default function SharedSessions() { }); setSessions(updatedSharedSessions); - }); + }, [currentUser?.id, sessions, addNotification]); + + const baseWebSocket = useSessionsWebSocket(handleSessionsUpdate); // Enhanced WebSocket with connection quality and manual reconnect const enhanced = useEnhancedWebSocket(baseWebSocket); From 934c425fd8491c196bc7b7926aecac9bea381a20 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Nov 2025 16:12:00 +0000 Subject: [PATCH 2/3] fix(ui): prevent useEnterpriseWebSocket reconnection loop in degraded mode When the WebSocket backend is unavailable, the connect() function in useEnterpriseWebSocket was being recreated on every reconnection attempt because reconnectAttempts was in its dependency array. This caused a reconnection loop that made the UI unusable. The issue: 1. WebSocket closes and schedules reconnection 2. setReconnectAttempts() updates state 3. connect() function is recreated (reconnectAttempts in dependencies) 4. New WebSocket connection created with old closure values 5. Loop continues, making UI unresponsive Fixed by: - Added reconnectAttemptsRef to track attempts without triggering recreations - Updated onopen handler to reset the ref - Updated onclose handler to check ref instead of state variable - Removed reconnectAttempts from connect() dependency array This matches the pattern already used in useWebSocket.ts and ensures the UI remains usable even when WebSocket is in degraded mode, as reconnection attempts happen in the background without disrupting the UI. --- ui/src/hooks/useEnterpriseWebSocket.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ui/src/hooks/useEnterpriseWebSocket.ts b/ui/src/hooks/useEnterpriseWebSocket.ts index 8048c460..47e8815c 100644 --- a/ui/src/hooks/useEnterpriseWebSocket.ts +++ b/ui/src/hooks/useEnterpriseWebSocket.ts @@ -51,6 +51,7 @@ export function useEnterpriseWebSocket( const wsRef = useRef(null); const reconnectTimeoutRef = useRef(null); const shouldReconnectRef = useRef(true); + const reconnectAttemptsRef = useRef(0); // Store callbacks in refs to avoid reconnection when they change const onMessageRef = useRef(onMessage); @@ -105,6 +106,7 @@ export function useEnterpriseWebSocket( // console.log('[WebSocket] Connected to enterprise WebSocket'); setIsConnected(true); setReconnectAttempts(0); + reconnectAttemptsRef.current = 0; shouldReconnectRef.current = true; if (onOpenRef.current) { @@ -145,20 +147,22 @@ export function useEnterpriseWebSocket( } // Attempt reconnection if enabled and within retry limit + const currentAttempts = reconnectAttemptsRef.current; if ( shouldReconnectRef.current && autoReconnect && - reconnectAttempts < maxReconnectAttempts + currentAttempts < maxReconnectAttempts ) { // console.log( - // `[WebSocket] Attempting reconnection in ${reconnectInterval}ms (attempt ${reconnectAttempts + 1}/${maxReconnectAttempts})` + // `[WebSocket] Attempting reconnection in ${reconnectInterval}ms (attempt ${currentAttempts + 1}/${maxReconnectAttempts})` // ); reconnectTimeoutRef.current = setTimeout(() => { + reconnectAttemptsRef.current += 1; setReconnectAttempts((prev) => prev + 1); connect(); }, reconnectInterval); - } else if (reconnectAttempts >= maxReconnectAttempts) { + } else if (currentAttempts >= maxReconnectAttempts) { console.error('[WebSocket] Max reconnection attempts reached'); } }; @@ -171,7 +175,6 @@ export function useEnterpriseWebSocket( autoReconnect, reconnectInterval, maxReconnectAttempts, - reconnectAttempts, ]); const disconnect = useCallback(() => { From 2db15516bfa2ca1683323dc320e0e46b4a7a2c00 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Nov 2025 16:36:29 +0000 Subject: [PATCH 3/3] fix(ui): stop WebSocket reconnection loop from effect dependencies The UI was unusable in degraded WebSocket mode because the mount effects had connect/close functions in their dependency arrays, causing infinite reconnection loops: The problem: 1. useEffect runs and calls connect() on mount 2. connect() is in the dependency array 3. When connect() changes (due to its own dependencies), effect reruns 4. New WebSocket connection created while old one is still reconnecting 5. Loop continues infinitely, making UI completely unusable In useWebSocket.ts: - Removed connect and close from mount effect dependencies - Added empty dependency array with eslint-disable comment - Effect now only runs on mount/unmount as intended In useEnterpriseWebSocket.ts: - Fixed visibility change effect that had connect in dependencies - Stored isConnected in ref to avoid effect recreation - Visibility listener now set up once on mount, not on every state change - Removed connect and isConnected from visibility effect dependencies Result: - WebSocket connects once on mount - Reconnection attempts happen in background via setTimeout - After max attempts (10), reconnection stops completely - UI remains fully usable even when WebSocket backend is unavailable - No more "Reconnecting in 1s..." loop blocking the interface This fixes the degraded mode issue where users couldn't interact with the UI while WebSocket was attempting to reconnect. --- ui/src/hooks/useEnterpriseWebSocket.ts | 11 +++++++++-- ui/src/hooks/useWebSocket.ts | 3 ++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/ui/src/hooks/useEnterpriseWebSocket.ts b/ui/src/hooks/useEnterpriseWebSocket.ts index 47e8815c..3b89f94e 100644 --- a/ui/src/hooks/useEnterpriseWebSocket.ts +++ b/ui/src/hooks/useEnterpriseWebSocket.ts @@ -223,9 +223,15 @@ export function useEnterpriseWebSocket( }, []); // Empty dependency array - only run on mount/unmount // Handle page visibility changes (reconnect when page becomes visible) + // Store isConnected in ref to avoid effect recreation + const isConnectedRef = useRef(isConnected); + useEffect(() => { + isConnectedRef.current = isConnected; + }, [isConnected]); + useEffect(() => { const handleVisibilityChange = () => { - if (document.visibilityState === 'visible' && !isConnected) { + if (document.visibilityState === 'visible' && !isConnectedRef.current) { // console.log('[WebSocket] Page visible, attempting reconnection'); connect(); } @@ -236,7 +242,8 @@ export function useEnterpriseWebSocket( return () => { document.removeEventListener('visibilitychange', handleVisibilityChange); }; - }, [isConnected, connect]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // Only set up listener once on mount return { isConnected, diff --git a/ui/src/hooks/useWebSocket.ts b/ui/src/hooks/useWebSocket.ts index 580c7805..e6f53bf5 100644 --- a/ui/src/hooks/useWebSocket.ts +++ b/ui/src/hooks/useWebSocket.ts @@ -145,7 +145,8 @@ export function useWebSocket({ return () => { close(); }; - }, [connect, close]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // Only run on mount/unmount, not when connect/close change return { isConnected,