From 09918f4b38d7218b6abd8a6cf8fead35452bdcee Mon Sep 17 00:00:00 2001 From: Chris Parsons <438676+chrisparsons83@users.noreply.github.com> Date: Mon, 2 Mar 2026 20:55:42 -0800 Subject: [PATCH] =?UTF-8?q?fix:=20improve=20JWT=20expiry=20handling=20and?= =?UTF-8?q?=20revalidation=20logic=20in=20DraftRoom=20=E2=80=A6=20(#58)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: improve JWT expiry handling and revalidation logic in DraftRoom component * fix: ensure authRecoveryTimerRef is initialized to undefined to prevent potential errors --- .../leagues/$leagueId.draft.$seasonId.tsx | 91 +++++++++++++++---- 1 file changed, 72 insertions(+), 19 deletions(-) diff --git a/app/routes/leagues/$leagueId.draft.$seasonId.tsx b/app/routes/leagues/$leagueId.draft.$seasonId.tsx index 987667a..77f985f 100644 --- a/app/routes/leagues/$leagueId.draft.$seasonId.tsx +++ b/app/routes/leagues/$leagueId.draft.$seasonId.tsx @@ -288,17 +288,37 @@ export default function DraftRoom() { }; }, [reconnectCount, revalidate]); - // Guard against Clerk JWT expiry race: if the loader ran without a userId (expired - // short-lived JWT) but the Clerk client SDK has a valid session, revalidate once so - // the loader re-runs with a fresh token and returns correct userTeam/userQueue data. - // The ref prevents an infinite loop if the server consistently fails to return a - // userId (e.g. misconfigured secret key) — we only attempt this recovery once. - const authRevalidatedRef = useRef(false); + // Guard against Clerk JWT expiry race: if the loader ran without a userId + // (expired short-lived JWT) but the Clerk client SDK has a valid session, + // revalidate so the loader re-runs with a fresh token and returns correct + // userTeam/userQueue data. This can happen repeatedly on mobile when the + // user backgrounds and foregrounds the app, so we don't limit it to a + // single attempt. Instead we debounce to avoid rapid-fire revalidations. + // + // Cap at 3 attempts to avoid an infinite loop if the server consistently + // fails to resolve a userId (e.g. clock skew, misconfigured Clerk keys). + // After 3 failures we fall through to the authDegraded overlay. + const authRecoveryTimerRef = useRef | undefined>(undefined); + const authRecoveryAttemptsRef = useRef(0); + const MAX_AUTH_RECOVERY_ATTEMPTS = 3; useEffect(() => { - if (clerkUserId && !currentUserId && !authRevalidatedRef.current) { - authRevalidatedRef.current = true; - revalidate(); + if (clerkUserId && !currentUserId) { + if (authRecoveryAttemptsRef.current >= MAX_AUTH_RECOVERY_ATTEMPTS) { + setAuthDegraded(true); + return; + } + // Small debounce so we don't fire multiple times in quick succession + // (e.g. visibility change + socket reconnect both updating deps). + clearTimeout(authRecoveryTimerRef.current); + authRecoveryTimerRef.current = setTimeout(() => { + authRecoveryAttemptsRef.current += 1; + revalidate(); + }, 100); + } else if (currentUserId) { + // Auth recovered — reset the counter for the next background cycle. + authRecoveryAttemptsRef.current = 0; } + return () => clearTimeout(authRecoveryTimerRef.current); }, [clerkUserId, currentUserId, revalidate]); // Auth session protection: proactively refresh the Clerk JWT to prevent @@ -320,7 +340,10 @@ export default function DraftRoom() { // Refresh the Clerk JWT when the tab becomes visible again. Browsers // throttle/suspend intervals in background tabs, so the SDK's automatic // 60-second refresh may not have run. Getting a fresh token here prevents - // the next user action from hitting a 401. + // the next user action from hitting a 401. After a successful refresh we + // also revalidate the loader so it re-runs with the fresh token — without + // this, userTeam/userQueue stay stale from the previous (possibly + // unauthenticated) loader run. useEffect(() => { // Nothing to do once auth is fully degraded — avoid re-registering a // listener that would immediately exit on every future visibility change. @@ -337,21 +360,38 @@ export default function DraftRoom() { try { const token = await getToken({ skipCache: true }); if (aborted) return; - if (token === null) { - // Retry once after a short delay — the network may still be waking up. - await new Promise((r) => setTimeout(r, 1000)); + if (token !== null) { + // Token refreshed successfully. Only revalidate if the loader is + // missing auth (currentUserId is null) — if the loader already has + // a valid user there's no stale state to fix and the extra round + // trip just adds unnecessary server load. + if (!currentUserId) { + revalidate(); + } + } else { + // Retry after a longer delay — mobile networks may take a few + // seconds to fully wake up after background suspension. + await new Promise((r) => setTimeout(r, 2500)); if (aborted) return; const retryToken = await getToken({ skipCache: true }); if (aborted) return; - if (retryToken === null) setAuthDegraded(true); + if (retryToken !== null) { + revalidate(); + } else { + setAuthDegraded(true); + } } } catch { try { - await new Promise((r) => setTimeout(r, 1000)); + await new Promise((r) => setTimeout(r, 2500)); if (aborted) return; const retryToken = await getToken({ skipCache: true }); if (aborted) return; - if (retryToken === null) setAuthDegraded(true); + if (retryToken !== null) { + revalidate(); + } else { + setAuthDegraded(true); + } } catch { if (!aborted) setAuthDegraded(true); } @@ -364,7 +404,7 @@ export default function DraftRoom() { aborted = true; document.removeEventListener("visibilitychange", handleVisibilityChange); }; - }, [clerkUserId, getToken, authDegraded]); + }, [clerkUserId, currentUserId, getToken, authDegraded, revalidate]); // Track revalidation lifecycle to sync local state from fresh loader data. // @@ -424,7 +464,20 @@ export default function DraftRoom() { // been removed server-side, but we never received those // `participant-removed-from-queues` events. The loader re-fetches // userQueue fresh on every revalidation so we can trust it here. - setQueue(userQueue); + // + // Guard: only sync user-specific state if the loader still has auth. + // If the revalidation ran with an expired JWT, currentUserId will be + // null and userQueue will be [] — don't overwrite the local queue. + // + // Note: userTeam (which gates the queue tab) comes directly from + // useLoaderData() and is not synced through local state, so the queue + // tab will briefly disappear between a bad (unauthenticated) revalidation + // and the follow-up recovery revalidation triggered by the auth recovery + // effect above. This is acceptable — the tab reappears as soon as the + // second revalidation completes with a valid token. + if (currentUserId) { + setQueue(userQueue); + } // Sync timers — the server-side timer state may have changed while // we were disconnected (time bank adjustments, new timers, etc.) if (timers.length > 0) { @@ -446,7 +499,7 @@ export default function DraftRoom() { return status; }); } - }, [revalidatorState, draftPicks, season, userQueue, timers, autodraftSettings]); + }, [revalidatorState, draftPicks, season, userQueue, timers, autodraftSettings, currentUserId]); const [picks, setPicks] = useState(draftPicks); const [currentPick, setCurrentPick] = useState(season.currentPickNumber || 1);