refactor(connection): improve retry and cleanup behavior in project events

- Refined retry delay logic for clarity and correctness in `getNextRetryDelay`.
- Added `connectRef` to ensure latest `connect` function is called in retries.
- Separated cleanup and connection management effects to prevent premature disconnections.
- Enhanced inline comments for maintainability.
This commit is contained in:
2026-01-03 18:36:51 +01:00
parent caf283bed2
commit 746fb7b181
2 changed files with 28 additions and 23 deletions

View File

@@ -130,6 +130,8 @@ export function useProjectEvents(
const isManualDisconnectRef = useRef(false); const isManualDisconnectRef = useRef(false);
const mountedRef = useRef(true); const mountedRef = useRef(true);
const pingHandlerRef = useRef<(() => void) | null>(null); const pingHandlerRef = useRef<(() => void) | null>(null);
// Ref to hold latest connect function to avoid stale closure in scheduleReconnect
const connectRef = useRef<(() => void) | null>(null);
/** /**
* Update connection state and notify callback * Update connection state and notify callback
@@ -206,11 +208,13 @@ export function useProjectEvents(
/** /**
* Calculate next retry delay with exponential backoff * Calculate next retry delay with exponential backoff
* Returns current delay, then increases for next call
*/ */
const getNextRetryDelay = useCallback(() => { const getNextRetryDelay = useCallback(() => {
const nextDelay = currentRetryDelayRef.current * BACKOFF_MULTIPLIER; const delay = currentRetryDelayRef.current;
currentRetryDelayRef.current = Math.min(nextDelay, maxRetryDelay); // Increase for next call (capped at max)
return currentRetryDelayRef.current; currentRetryDelayRef.current = Math.min(delay * BACKOFF_MULTIPLIER, maxRetryDelay);
return delay;
}, [maxRetryDelay]); }, [maxRetryDelay]);
/** /**
@@ -234,7 +238,8 @@ export function useProjectEvents(
retryTimeoutRef.current = setTimeout(() => { retryTimeoutRef.current = setTimeout(() => {
if (!mountedRef.current || isManualDisconnectRef.current) return; if (!mountedRef.current || isManualDisconnectRef.current) return;
setRetryCount((prev) => prev + 1); setRetryCount((prev) => prev + 1);
connect(); // Use ref to always call latest connect function (avoids stale closure)
connectRef.current?.();
}, delay); }, delay);
}, [retryCount, maxRetryAttempts, getNextRetryDelay, updateConnectionState]); }, [retryCount, maxRetryAttempts, getNextRetryDelay, updateConnectionState]);
@@ -344,6 +349,9 @@ export function useProjectEvents(
initialRetryDelay, initialRetryDelay,
]); ]);
// Keep ref updated with latest connect function for scheduleReconnect
connectRef.current = connect;
/** /**
* Manually disconnect from SSE * Manually disconnect from SSE
*/ */
@@ -371,11 +379,22 @@ export function useProjectEvents(
clearProjectEvents(projectId); clearProjectEvents(projectId);
}, [clearProjectEvents, projectId]); }, [clearProjectEvents, projectId]);
// Consolidated connection management effect // Effect 1: Unmount cleanup only
// Handles both initial mount and auth state changes to prevent race conditions // Separated from connection management to prevent cleanup running on every state change
// This fixes: when connectionState changes, the old effect cleanup would close EventSource
useEffect(() => { useEffect(() => {
mountedRef.current = true; mountedRef.current = true;
return () => {
mountedRef.current = false;
cleanup();
};
}, [cleanup]);
// Effect 2: Connection management
// No cleanup here - only Effect 1 handles cleanup on unmount
// This prevents: cleanup closing EventSource immediately after connect() creates it
useEffect(() => {
// Connect when authenticated with a project and not manually disconnected // Connect when authenticated with a project and not manually disconnected
if (autoConnect && isAuthenticated && accessToken && projectId) { if (autoConnect && isAuthenticated && accessToken && projectId) {
if (connectionState === 'disconnected' && !isManualDisconnectRef.current) { if (connectionState === 'disconnected' && !isManualDisconnectRef.current) {
@@ -385,21 +404,7 @@ export function useProjectEvents(
// Disconnect when auth is lost // Disconnect when auth is lost
disconnect(); disconnect();
} }
}, [autoConnect, isAuthenticated, accessToken, projectId, connectionState, connect, disconnect]);
return () => {
mountedRef.current = false;
cleanup();
};
}, [
autoConnect,
isAuthenticated,
accessToken,
projectId,
connectionState,
connect,
disconnect,
cleanup,
]);
return { return {
events, events,

View File

@@ -62,8 +62,8 @@ function isValidToken(token: string): boolean {
* @returns Unix timestamp * @returns Unix timestamp
*/ */
function calculateExpiry(expiresIn?: number): number { function calculateExpiry(expiresIn?: number): number {
// Default to 15 minutes if not provided or invalid // Use nullish coalescing - 0 is invalid but should trigger warning, not silent fallback
let seconds = expiresIn || 900; let seconds = expiresIn ?? 900;
// Validate positive number and prevent overflow // Validate positive number and prevent overflow
if (seconds <= 0 || seconds > 31536000) { if (seconds <= 0 || seconds > 31536000) {