fix: improve JWT expiry handling and revalidation logic in DraftRoom … (#58)

* fix: improve JWT expiry handling and revalidation logic in DraftRoom component

* fix: ensure authRecoveryTimerRef is initialized to undefined to prevent potential errors
This commit is contained in:
Chris Parsons 2026-03-02 20:55:42 -08:00 committed by GitHub
parent 599bba8949
commit 09918f4b38
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -288,17 +288,37 @@ export default function DraftRoom() {
}; };
}, [reconnectCount, revalidate]); }, [reconnectCount, revalidate]);
// Guard against Clerk JWT expiry race: if the loader ran without a userId (expired // Guard against Clerk JWT expiry race: if the loader ran without a userId
// short-lived JWT) but the Clerk client SDK has a valid session, revalidate once so // (expired short-lived JWT) but the Clerk client SDK has a valid session,
// the loader re-runs with a fresh token and returns correct userTeam/userQueue data. // revalidate so the loader re-runs with a fresh token and returns correct
// The ref prevents an infinite loop if the server consistently fails to return a // userTeam/userQueue data. This can happen repeatedly on mobile when the
// userId (e.g. misconfigured secret key) — we only attempt this recovery once. // user backgrounds and foregrounds the app, so we don't limit it to a
const authRevalidatedRef = useRef(false); // 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<ReturnType<typeof setTimeout> | undefined>(undefined);
const authRecoveryAttemptsRef = useRef(0);
const MAX_AUTH_RECOVERY_ATTEMPTS = 3;
useEffect(() => { useEffect(() => {
if (clerkUserId && !currentUserId && !authRevalidatedRef.current) { if (clerkUserId && !currentUserId) {
authRevalidatedRef.current = true; if (authRecoveryAttemptsRef.current >= MAX_AUTH_RECOVERY_ATTEMPTS) {
revalidate(); 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]); }, [clerkUserId, currentUserId, revalidate]);
// Auth session protection: proactively refresh the Clerk JWT to prevent // 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 // Refresh the Clerk JWT when the tab becomes visible again. Browsers
// throttle/suspend intervals in background tabs, so the SDK's automatic // throttle/suspend intervals in background tabs, so the SDK's automatic
// 60-second refresh may not have run. Getting a fresh token here prevents // 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(() => { useEffect(() => {
// Nothing to do once auth is fully degraded — avoid re-registering a // Nothing to do once auth is fully degraded — avoid re-registering a
// listener that would immediately exit on every future visibility change. // listener that would immediately exit on every future visibility change.
@ -337,21 +360,38 @@ export default function DraftRoom() {
try { try {
const token = await getToken({ skipCache: true }); const token = await getToken({ skipCache: true });
if (aborted) return; if (aborted) return;
if (token === null) { if (token !== null) {
// Retry once after a short delay — the network may still be waking up. // Token refreshed successfully. Only revalidate if the loader is
await new Promise((r) => setTimeout(r, 1000)); // 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; if (aborted) return;
const retryToken = await getToken({ skipCache: true }); const retryToken = await getToken({ skipCache: true });
if (aborted) return; if (aborted) return;
if (retryToken === null) setAuthDegraded(true); if (retryToken !== null) {
revalidate();
} else {
setAuthDegraded(true);
}
} }
} catch { } catch {
try { try {
await new Promise((r) => setTimeout(r, 1000)); await new Promise((r) => setTimeout(r, 2500));
if (aborted) return; if (aborted) return;
const retryToken = await getToken({ skipCache: true }); const retryToken = await getToken({ skipCache: true });
if (aborted) return; if (aborted) return;
if (retryToken === null) setAuthDegraded(true); if (retryToken !== null) {
revalidate();
} else {
setAuthDegraded(true);
}
} catch { } catch {
if (!aborted) setAuthDegraded(true); if (!aborted) setAuthDegraded(true);
} }
@ -364,7 +404,7 @@ export default function DraftRoom() {
aborted = true; aborted = true;
document.removeEventListener("visibilitychange", handleVisibilityChange); document.removeEventListener("visibilitychange", handleVisibilityChange);
}; };
}, [clerkUserId, getToken, authDegraded]); }, [clerkUserId, currentUserId, getToken, authDegraded, revalidate]);
// Track revalidation lifecycle to sync local state from fresh loader data. // 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 // been removed server-side, but we never received those
// `participant-removed-from-queues` events. The loader re-fetches // `participant-removed-from-queues` events. The loader re-fetches
// userQueue fresh on every revalidation so we can trust it here. // 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 // Sync timers — the server-side timer state may have changed while
// we were disconnected (time bank adjustments, new timers, etc.) // we were disconnected (time bank adjustments, new timers, etc.)
if (timers.length > 0) { if (timers.length > 0) {
@ -446,7 +499,7 @@ export default function DraftRoom() {
return status; return status;
}); });
} }
}, [revalidatorState, draftPicks, season, userQueue, timers, autodraftSettings]); }, [revalidatorState, draftPicks, season, userQueue, timers, autodraftSettings, currentUserId]);
const [picks, setPicks] = useState(draftPicks); const [picks, setPicks] = useState(draftPicks);
const [currentPick, setCurrentPick] = useState(season.currentPickNumber || 1); const [currentPick, setCurrentPick] = useState(season.currentPickNumber || 1);