Skip to content

fix: Improve WebSocket persistence reliability and prevent memory leaks #411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marcodejongh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Improve WebSocket persistence reliability and prevent memory leaks #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion packages/web/app/components/graphql-queue/graphql-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 3 additions & 14 deletions packages/web/app/components/persistent-session/board-session-bridge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ const BoardSessionBridge: React.FC<BoardSessionBridgeProps> = ({
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,
Expand All @@ -51,19 +53,6 @@ const BoardSessionBridge: React.FC<BoardSessionBridgeProps> = ({
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}</>;
};

Expand Down
25 changes: 17 additions & 8 deletions packages/web/app/components/persistent-session/persistent-session-context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }>
const sessionRef = useRef<Session | null>(null);
const isReconnectingRef = useRef(false);
const activeSessionRef = useRef<ActiveSessionInfo | null>(null);
const mountedRef = useRef(true); // Track if component is mounted for async safety

// Event subscribers
const queueEventSubscribersRef = useRef<Set<(event: ClientQueueEvent) => void>>(new Set());
Expand Down Expand Up @@ -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<Session | null> {
Expand All @@ -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;
Expand All @@ -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);
}
Expand All @@ -348,7 +350,7 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }>
onReconnect: handleReconnect,
});

if (!mounted) {
if (!mountedRef.current) {
graphqlClient.dispose();
return;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
},
},
);
Expand All @@ -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);
}
Expand All @@ -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) {
Expand Down