diff --git a/.changeset/fix-token-cache-timer-leak.md b/.changeset/fix-token-cache-timer-leak.md new file mode 100644 index 00000000000..6b3440a8fa6 --- /dev/null +++ b/.changeset/fix-token-cache-timer-leak.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Fix token cache refresh timer leak that caused accelerating token refresh requests after `session.touch()` or organization switching. diff --git a/integration/tests/session-token-cache/refresh-timer-cleanup.test.ts b/integration/tests/session-token-cache/refresh-timer-cleanup.test.ts new file mode 100644 index 00000000000..485ac39e383 --- /dev/null +++ b/integration/tests/session-token-cache/refresh-timer-cleanup.test.ts @@ -0,0 +1,77 @@ +import { expect, test } from '@playwright/test'; + +import { appConfigs } from '../../presets'; +import type { FakeUser } from '../../testUtils'; +import { createTestUtils, testAgainstRunningApps } from '../../testUtils'; + +/** + * Tests that the token cache's proactive refresh timer does not accumulate + * orphaned timers across set() calls. + * + * Background: Every API response that piggybacks client data triggers _updateClient, + * which reconstructs Session objects and calls #hydrateCache → SessionTokenCache.set(). + * Without proper timer cleanup, each set() call would leave the previous refresh timer + * running, causing the effective polling rate to accelerate over time. + */ +testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })( + 'Token refresh timer cleanup @generic', + ({ app }) => { + test.describe.configure({ mode: 'serial' }); + + let fakeUser: FakeUser; + + test.beforeAll(async () => { + const u = createTestUtils({ app }); + fakeUser = u.services.users.createFakeUser(); + await u.services.users.createBapiUser(fakeUser); + }); + + test.afterAll(async () => { + await fakeUser.deleteIfExists(); + await app.teardown(); + }); + + test('touch does not cause clustered token refresh requests', async ({ page, context }) => { + test.setTimeout(120_000); + const u = createTestUtils({ app, page, context }); + + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); + await u.po.expect.toBeSignedIn(); + + // Track token fetch requests with timestamps + const tokenRequests: number[] = []; + await page.route('**/v1/client/sessions/*/tokens**', async route => { + tokenRequests.push(Date.now()); + await route.continue(); + }); + + // Trigger multiple touch() calls — each causes _updateClient → Session constructor + // → #hydrateCache → set(), which previously leaked orphaned refresh timers + for (let i = 0; i < 5; i++) { + await page.evaluate(async () => { + await (window as any).Clerk?.session?.touch(); + }); + } + + // Wait 50s — enough for one refresh cycle (~43s) but not two + // eslint-disable-next-line playwright/no-wait-for-timeout + await page.waitForTimeout(50_000); + + await page.unrouteAll(); + + // With the fix: at most 1-2 refresh requests (one cycle at ~43s) + // Without the fix: 5+ requests from orphaned timers all firing at different offsets + expect(tokenRequests.length).toBeLessThanOrEqual(3); + + // If multiple requests occurred, verify they aren't clustered together + // (clustering = orphaned timers firing near-simultaneously) + if (tokenRequests.length >= 2) { + for (let i = 1; i < tokenRequests.length; i++) { + const gap = tokenRequests[i] - tokenRequests[i - 1]; + expect(gap).toBeGreaterThan(10_000); + } + } + }); + }, +); diff --git a/packages/clerk-js/src/core/__tests__/tokenCache.test.ts b/packages/clerk-js/src/core/__tests__/tokenCache.test.ts index 9cb018ce56a..bac583ec5f9 100644 --- a/packages/clerk-js/src/core/__tests__/tokenCache.test.ts +++ b/packages/clerk-js/src/core/__tests__/tokenCache.test.ts @@ -1189,6 +1189,284 @@ describe('SessionTokenCache', () => { }); }); + describe('timer cleanup on overwrite', () => { + /** + * This describe block tests the fix for a bug where calling set() multiple times + * for the same cache key would leak orphaned refresh timers. Each set() call creates + * a new value object with its own refresh timer, but previously the old value's timers + * were never cleared. This caused refresh callbacks to fire N times after N set() calls + * instead of just once, making the poller appear erratic. + * + * The root cause was that both `#hydrateCache` (via Session constructor from _updateClient) + * and `#refreshTokenInBackground` call set() for the same key during a single refresh cycle, + * doubling the number of active timers each cycle. + */ + + it('cancels old refresh timer when set() is called again for the same key', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token = new Token({ id: 'overwrite-token', jwt, object: 'token' }); + + const onRefresh1 = vi.fn(); + const onRefresh2 = vi.fn(); + const key = { tokenId: 'overwrite-token' }; + + // First set() — schedules refresh timer at ~43s + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token), onRefresh: onRefresh1 }); + await Promise.resolve(); + + // Second set() for SAME key — should cancel first timer and schedule new one + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token), onRefresh: onRefresh2 }); + await Promise.resolve(); + + // Advance past refresh fire time (43s) + vi.advanceTimersByTime(44 * 1000); + + // Only the SECOND callback should fire; the first was cancelled + expect(onRefresh1).not.toHaveBeenCalled(); + expect(onRefresh2).toHaveBeenCalledTimes(1); + }); + + it('cancels old expiration timer when set() is called again for the same key', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt1 = createJwtWithTtl(nowSeconds, 30); + const jwt2 = createJwtWithTtl(nowSeconds, 120); + const token1 = new Token({ id: 'exp-overwrite', jwt: jwt1, object: 'token' }); + const token2 = new Token({ id: 'exp-overwrite', jwt: jwt2, object: 'token' }); + + const key = { tokenId: 'exp-overwrite' }; + + // First set() with 30s TTL + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token1) }); + await Promise.resolve(); + + // Second set() with 120s TTL — old 30s expiration timer should be cancelled + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token2) }); + await Promise.resolve(); + + // After 30s the old timer would have deleted the entry, but it should still exist + vi.advanceTimersByTime(31 * 1000); + const result = SessionTokenCache.get(key); + expect(result).toBeDefined(); + expect(result?.entry.tokenId).toBe('exp-overwrite'); + }); + + it('does not accumulate refresh timers across multiple set() calls', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token = new Token({ id: 'accumulate-test', jwt, object: 'token' }); + + const onRefresh = vi.fn(); + const key = { tokenId: 'accumulate-test' }; + + // Simulate 10 rapid set() calls for the same key (as would happen with + // multiple _updateClient / hydration cycles) + for (let i = 0; i < 10; i++) { + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token), onRefresh }); + await Promise.resolve(); + } + + // Advance past refresh fire time + vi.advanceTimersByTime(44 * 1000); + + // Should fire exactly ONCE, not 10 times + expect(onRefresh).toHaveBeenCalledTimes(1); + }); + + it('simulates the hydration + background refresh double-set scenario', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token = new Token({ id: 'double-set-token', jwt, object: 'token' }); + + const hydrateRefresh = vi.fn(); + const backgroundRefresh = vi.fn(); + const key = { tokenId: 'double-set-token' }; + + // 1. Simulate #hydrateCache from Session constructor (called by _updateClient) + SessionTokenCache.set({ + ...key, + tokenResolver: Promise.resolve(token), + onRefresh: hydrateRefresh, + }); + await Promise.resolve(); + + // 2. Simulate #refreshTokenInBackground's .then() calling set() with resolved token + // This is what happens during a background refresh cycle — both _updateClient + // and the .then() callback call set() for the same cache key + SessionTokenCache.set({ + ...key, + tokenResolver: Promise.resolve(token), + onRefresh: backgroundRefresh, + }); + await Promise.resolve(); + + // Advance past refresh fire time + vi.advanceTimersByTime(44 * 1000); + + // Only the second (background refresh) callback should fire + expect(hydrateRefresh).not.toHaveBeenCalled(); + expect(backgroundRefresh).toHaveBeenCalledTimes(1); + }); + + it('simulates multiple refresh cycles without timer accumulation', async () => { + const key = { tokenId: 'multi-cycle-token' }; + const refreshCounts: number[] = []; + + // Simulate 5 consecutive refresh cycles + // refreshFireTime = 60 - 15 - 2 = 43s + for (let cycle = 0; cycle < 5; cycle++) { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token = new Token({ id: 'multi-cycle-token', jwt, object: 'token' }); + + const onRefresh = vi.fn(); + + // Each cycle does TWO set() calls (hydration + background refresh) + SessionTokenCache.set({ + ...key, + tokenResolver: Promise.resolve(token), + onRefresh, + }); + await Promise.resolve(); + + SessionTokenCache.set({ + ...key, + tokenResolver: Promise.resolve(token), + onRefresh, + }); + await Promise.resolve(); + + // Advance to 42s — just BEFORE the 43s refresh timer fires + vi.advanceTimersByTime(42 * 1000); + refreshCounts.push(onRefresh.mock.calls.length); + + // Advance 2 more seconds past the 43s fire time + vi.advanceTimersByTime(2 * 1000); + refreshCounts.push(onRefresh.mock.calls.length); + } + + // Each cycle should show: 0 calls before timer, 1 call after timer + // If timers were accumulating, later cycles would show more than 1 call + for (let i = 0; i < refreshCounts.length; i += 2) { + expect(refreshCounts[i]).toBe(0); // before timer (42s) + expect(refreshCounts[i + 1]).toBe(1); // after timer (44s) + } + }); + + it('set() with different key does not affect existing timers', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token1 = new Token({ id: 'key-a', jwt, object: 'token' }); + const token2 = new Token({ id: 'key-b', jwt, object: 'token' }); + + const onRefreshA = vi.fn(); + const onRefreshB = vi.fn(); + + SessionTokenCache.set({ + tokenId: 'key-a', + tokenResolver: Promise.resolve(token1), + onRefresh: onRefreshA, + }); + await Promise.resolve(); + + SessionTokenCache.set({ + tokenId: 'key-b', + tokenResolver: Promise.resolve(token2), + onRefresh: onRefreshB, + }); + await Promise.resolve(); + + vi.advanceTimersByTime(44 * 1000); + + // Both should fire independently — setting key-b should NOT cancel key-a's timer + expect(onRefreshA).toHaveBeenCalledTimes(1); + expect(onRefreshB).toHaveBeenCalledTimes(1); + }); + + it('only the latest set() callback fires after interleaved set/clear/set', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token = new Token({ id: 'interleaved-token', jwt, object: 'token' }); + + const onRefresh1 = vi.fn(); + const onRefresh2 = vi.fn(); + const key = { tokenId: 'interleaved-token' }; + + // set, clear, set again + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token), onRefresh: onRefresh1 }); + await Promise.resolve(); + + SessionTokenCache.clear(); + + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token), onRefresh: onRefresh2 }); + await Promise.resolve(); + + vi.advanceTimersByTime(44 * 1000); + + expect(onRefresh1).not.toHaveBeenCalled(); + expect(onRefresh2).toHaveBeenCalledTimes(1); + }); + + it('does not install timers when a pending tokenResolver resolves after being overwritten', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token = new Token({ id: 'stale-pending', jwt, object: 'token' }); + + const onRefreshStale = vi.fn(); + const onRefreshFresh = vi.fn(); + const key = { tokenId: 'stale-pending' }; + + // First set() with a slow-resolving promise + let resolveSlowPromise: (t: TokenResource) => void; + const slowPromise = new Promise(resolve => { + resolveSlowPromise = resolve; + }); + + SessionTokenCache.set({ ...key, tokenResolver: slowPromise, onRefresh: onRefreshStale }); + + // Second set() overwrites the key before the slow promise resolves + SessionTokenCache.set({ + ...key, + tokenResolver: Promise.resolve(token), + onRefresh: onRefreshFresh, + }); + await Promise.resolve(); + + // Now the slow promise resolves — but its entry is stale + resolveSlowPromise!(token); + await Promise.resolve(); + + // Advance past refresh fire time + vi.advanceTimersByTime(44 * 1000); + + // Only the fresh callback should fire; the stale one should be ignored + expect(onRefreshStale).not.toHaveBeenCalled(); + expect(onRefreshFresh).toHaveBeenCalledTimes(1); + }); + + it('overwriting with a token that has no onRefresh cancels the old refresh timer', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const jwt = createJwtWithTtl(nowSeconds, 60); + const token = new Token({ id: 'cancel-refresh', jwt, object: 'token' }); + + const onRefresh = vi.fn(); + const key = { tokenId: 'cancel-refresh' }; + + // First set with onRefresh + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token), onRefresh }); + await Promise.resolve(); + + // Second set WITHOUT onRefresh (like a broadcast-received token) + SessionTokenCache.set({ ...key, tokenResolver: Promise.resolve(token) }); + await Promise.resolve(); + + vi.advanceTimersByTime(44 * 1000); + + // The old onRefresh should have been cancelled, and no new one was scheduled + expect(onRefresh).not.toHaveBeenCalled(); + }); + }); + describe('multi-session isolation', () => { it('stores tokens from different session IDs separately without interference', async () => { const nowSeconds = Math.floor(Date.now() / 1000); diff --git a/packages/clerk-js/src/core/tokenCache.ts b/packages/clerk-js/src/core/tokenCache.ts index 98bfaa25fae..7cc9938bde1 100644 --- a/packages/clerk-js/src/core/tokenCache.ts +++ b/packages/clerk-js/src/core/tokenCache.ts @@ -350,6 +350,19 @@ const MemoryTokenCache = (prefix = KEY_PREFIX): TokenCache => { const key = cacheKey.toKey(); + // Clear timers from any existing entry for this key to prevent orphaned + // refresh timers from accumulating across set() calls (e.g., from + // #hydrateCache during _updateClient AND #refreshTokenInBackground). + const existing = cache.get(key); + if (existing) { + if (existing.timeoutId !== undefined) { + clearTimeout(existing.timeoutId); + } + if (existing.refreshTimeoutId !== undefined) { + clearTimeout(existing.refreshTimeoutId); + } + } + const nowSeconds = Math.floor(Date.now() / 1000); const createdAt = entry.createdAt ?? nowSeconds; const value: TokenCacheValue = { createdAt, entry, expiresIn: undefined }; @@ -369,6 +382,12 @@ const MemoryTokenCache = (prefix = KEY_PREFIX): TokenCache => { entry.tokenResolver .then(newToken => { + // If this entry was overwritten by a newer set() call while our promise + // was pending, bail out to avoid installing orphaned timers. + if (cache.get(key) !== value) { + return; + } + // Store resolved token for synchronous reads entry.resolvedToken = newToken;