From d211c1c61d54539199ca8fa8c667743fb8295058 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Dec 2025 09:12:14 +0000 Subject: [PATCH] fix: Improve WebSocket persistence reliability and prevent memory leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add exponential backoff with jitter (1s→30s) for GraphQL client reconnection to prevent hammering the server on failures and avoid thundering herd - Replace mutable mounted variable with useRef in persistent-session-context for safer async callback handling in React concurrent mode - Clean up subscription refs on error/complete to prevent resource leaks - Consolidate duplicate useEffects in board-session-bridge that could cause unnecessary double session activations --- .../graphql-queue/graphql-client.ts | 20 ++++++++++++++- .../board-session-bridge.tsx | 17 +++---------- .../persistent-session-context.tsx | 25 +++++++++++++------ 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/packages/web/app/components/graphql-queue/graphql-client.ts b/packages/web/app/components/graphql-queue/graphql-client.ts index 3c87dec8..8ae98903 100644 --- a/packages/web/app/components/graphql-queue/graphql-client.ts +++ b/packages/web/app/components/graphql-queue/graphql-client.ts @@ -5,6 +5,12 @@ export type { Client }; const DEBUG = process.env.NODE_ENV === 'development'; const MUTATION_TIMEOUT_MS = 30_000; // 30 second timeout for mutations +// Retry configuration for WebSocket reconnection +const INITIAL_RETRY_DELAY_MS = 1000; // Start with 1 second +const MAX_RETRY_DELAY_MS = 30_000; // Cap at 30 seconds +const BACKOFF_MULTIPLIER = 2; +const JITTER_FACTOR = 0.3; // ±15% jitter to prevent thundering herd + let clientCounter = 0; // Cache for parsed operation names to avoid regex on every call @@ -61,8 +67,20 @@ export function createGraphQLClient( const client = createClient({ url, - retryAttempts: 5, + retryAttempts: 10, shouldRetry: () => true, + // Exponential backoff with jitter to prevent hammering the server on failures + retryWait: async (retryCount) => { + const baseDelay = Math.min( + INITIAL_RETRY_DELAY_MS * Math.pow(BACKOFF_MULTIPLIER, retryCount), + MAX_RETRY_DELAY_MS, + ); + // Add jitter: ±15% randomization to prevent thundering herd + const jitter = baseDelay * JITTER_FACTOR * (Math.random() * 2 - 1); + const delay = Math.round(baseDelay + jitter); + if (DEBUG) console.log(`[GraphQL] Client #${clientId} retry ${retryCount + 1}, waiting ${delay}ms`); + await new Promise((resolve) => setTimeout(resolve, delay)); + }, // Lazy connection - only connects when first subscription/mutation is made lazy: true, // Keep alive to detect disconnections diff --git a/packages/web/app/components/persistent-session/board-session-bridge.tsx b/packages/web/app/components/persistent-session/board-session-bridge.tsx index eaa548f6..ee07e7ce 100644 --- a/packages/web/app/components/persistent-session/board-session-bridge.tsx +++ b/packages/web/app/components/persistent-session/board-session-bridge.tsx @@ -27,9 +27,11 @@ const BoardSessionBridge: React.FC = ({ const { activeSession, activateSession } = usePersistentSession(); // Activate session when we have a session param and board details + // This handles both initial session joins (via shared link) and updates when + // board details change (e.g., angle change) while staying in the same session useEffect(() => { if (sessionIdFromUrl && boardDetails) { - // Activate session when URL has session param (joining via shared link) + // Activate when: joining a new session OR board path changed (e.g., angle change) if (activeSession?.sessionId !== sessionIdFromUrl || activeSession?.boardPath !== pathname) { activateSession({ sessionId: sessionIdFromUrl, @@ -51,19 +53,6 @@ const BoardSessionBridge: React.FC = ({ activateSession, ]); - // Update board details if they change (e.g., angle change) - useEffect(() => { - if (activeSession?.sessionId === sessionIdFromUrl && activeSession?.boardPath !== pathname) { - // Board path changed but session is the same - update the session info - activateSession({ - sessionId: sessionIdFromUrl!, - boardPath: pathname, - boardDetails, - parsedParams, - }); - } - }, [pathname, parsedParams, boardDetails, activeSession, sessionIdFromUrl, activateSession]); - return <>{children}; }; diff --git a/packages/web/app/components/persistent-session/persistent-session-context.tsx b/packages/web/app/components/persistent-session/persistent-session-context.tsx index c1ff7f02..3ea1777c 100644 --- a/packages/web/app/components/persistent-session/persistent-session-context.tsx +++ b/packages/web/app/components/persistent-session/persistent-session-context.tsx @@ -164,6 +164,7 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> const sessionRef = useRef(null); const isReconnectingRef = useRef(false); const activeSessionRef = useRef(null); + const mountedRef = useRef(true); // Track if component is mounted for async safety // Event subscribers const queueEventSubscribersRef = useRef void>>(new Set()); @@ -297,7 +298,8 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> return; } - let mounted = true; + // Reset mounted ref for this effect instance + mountedRef.current = true; let graphqlClient: Client | null = null; async function joinSession(clientToUse: Client): Promise { @@ -316,7 +318,7 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> } async function handleReconnect() { - if (!mounted || !graphqlClient) return; + if (!mountedRef.current || !graphqlClient) return; if (isReconnectingRef.current) { if (DEBUG) console.log('[PersistentSession] Reconnection already in progress'); return; @@ -326,7 +328,7 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> try { if (DEBUG) console.log('[PersistentSession] Reconnecting...'); const sessionData = await joinSession(graphqlClient); - if (sessionData && mounted) { + if (sessionData && mountedRef.current) { setSession(sessionData); if (DEBUG) console.log('[PersistentSession] Reconnected, clientId:', sessionData.clientId); } @@ -348,7 +350,7 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> onReconnect: handleReconnect, }); - if (!mounted) { + if (!mountedRef.current) { graphqlClient.dispose(); return; } @@ -357,7 +359,7 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> const sessionData = await joinSession(graphqlClient); - if (!mounted) { + if (!mountedRef.current) { graphqlClient.dispose(); return; } @@ -392,12 +394,14 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> }, error: (err) => { console.error('[PersistentSession] Queue subscription error:', err); - if (mounted) { + queueUnsubscribeRef.current = null; // Clean up ref on error + if (mountedRef.current) { setError(err instanceof Error ? err : new Error(String(err))); } }, complete: () => { if (DEBUG) console.log('[PersistentSession] Queue subscription completed'); + queueUnsubscribeRef.current = null; // Clean up ref on complete }, }, ); @@ -414,15 +418,17 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> }, error: (err) => { console.error('[PersistentSession] Session subscription error:', err); + sessionUnsubscribeRef.current = null; // Clean up ref on error }, complete: () => { if (DEBUG) console.log('[PersistentSession] Session subscription completed'); + sessionUnsubscribeRef.current = null; // Clean up ref on complete }, }, ); } catch (err) { console.error('[PersistentSession] Connection failed:', err); - if (mounted) { + if (mountedRef.current) { setError(err instanceof Error ? err : new Error(String(err))); setIsConnecting(false); } @@ -436,10 +442,13 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }> return () => { if (DEBUG) console.log('[PersistentSession] Cleaning up connection'); - mounted = false; + mountedRef.current = false; + // Clean up subscriptions and their refs queueUnsubscribeRef.current?.(); + queueUnsubscribeRef.current = null; sessionUnsubscribeRef.current?.(); + sessionUnsubscribeRef.current = null; if (graphqlClient) { if (sessionRef.current) {