Skip to content

Commit 857823e

Browse files
jacekradkoEphem
andauthored
fix(clerk-js): Fix token cache refresh timer leak (clerk#8098)
Co-authored-by: Fredrik Höglund <fredrik@clerk.dev>
1 parent 1cf76d1 commit 857823e

6 files changed

Lines changed: 502 additions & 3 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Fix token cache refresh timer leak that caused accelerating token refresh requests after `session.touch()` or organization switching.

integration/tests/session-token-cache/multi-session.test.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,5 +226,106 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSessionTasks] })(
226226
expect(tab1FinalInfo.userId).toBe(user1SessionInfo.userId);
227227
expect(tab1FinalInfo.activeSessionId).toBe(user1SessionInfo.sessionId);
228228
});
229+
230+
/**
231+
* Test Flow:
232+
* 1. Tab1: Sign in as user1
233+
* 2. Tab2: Inherits user1's session, then signs in as user2 (multi-session)
234+
* 3. Tab1 has user1's active session; tab2 has user2's active session
235+
* 4. Each tab's active session independently hydrates its token cache
236+
* 5. Start counting /tokens requests, wait for both refresh timers to fire
237+
* 6. Assert exactly 2 /tokens requests (one per session), with each session
238+
* represented exactly once
239+
*
240+
* Expected Behavior:
241+
* - Two different sessions produce two independent refresh requests
242+
* - BroadcastChannel does NOT deduplicate across sessions (different tokenIds)
243+
* - Each session refreshes exactly once
244+
*
245+
* Note that this test does not currently assert in which tab the updates happen,
246+
* this might be something we want to add in the future, but currently it is not
247+
* deterministic.
248+
*/
249+
test('multi-session scheduled refreshes produce one request per session', async ({ context }) => {
250+
test.setTimeout(90_000);
251+
252+
const page1 = await context.newPage();
253+
await page1.goto(app.serverUrl);
254+
await page1.waitForFunction(() => (window as any).Clerk?.loaded);
255+
256+
const u1 = createTestUtils({ app, page: page1 });
257+
await u1.po.signIn.goTo();
258+
await u1.po.signIn.setIdentifier(fakeUser1.email);
259+
await u1.po.signIn.continue();
260+
await u1.po.signIn.setPassword(fakeUser1.password);
261+
await u1.po.signIn.continue();
262+
await u1.po.expect.toBeSignedIn();
263+
264+
const user1SessionId = await page1.evaluate(() => (window as any).Clerk?.session?.id);
265+
expect(user1SessionId).toBeDefined();
266+
267+
const page2 = await context.newPage();
268+
await page2.goto(app.serverUrl);
269+
await page2.waitForFunction(() => (window as any).Clerk?.loaded);
270+
271+
// eslint-disable-next-line playwright/no-wait-for-timeout
272+
await page2.waitForTimeout(1000);
273+
274+
const u2 = createTestUtils({ app, page: page2 });
275+
await u2.po.expect.toBeSignedIn();
276+
277+
// Sign in as user2 on tab2, creating a second session
278+
const signInResult = await page2.evaluate(
279+
async ({ email, password }) => {
280+
const clerk = (window as any).Clerk;
281+
const signIn = await clerk.client.signIn.create({ identifier: email, password });
282+
await clerk.setActive({ session: signIn.createdSessionId });
283+
return {
284+
sessionCount: clerk?.client?.sessions?.length || 0,
285+
sessionId: clerk?.session?.id,
286+
success: true,
287+
};
288+
},
289+
{ email: fakeUser2.email, password: fakeUser2.password },
290+
);
291+
292+
expect(signInResult.success).toBe(true);
293+
expect(signInResult.sessionCount).toBe(2);
294+
295+
const user2SessionId = signInResult.sessionId;
296+
expect(user2SessionId).toBeDefined();
297+
expect(user2SessionId).not.toBe(user1SessionId);
298+
299+
// Tab1 has user1's active session; tab2 has user2's active session.
300+
// Start counting /tokens requests.
301+
const refreshRequests: Array<{ sessionId: string; url: string }> = [];
302+
await context.route('**/v1/client/sessions/*/tokens*', async route => {
303+
const url = route.request().url();
304+
const match = url.match(/sessions\/([^/]+)\/tokens/);
305+
refreshRequests.push({ sessionId: match?.[1] || 'unknown', url });
306+
await route.continue();
307+
});
308+
309+
// Wait for proactive refresh timers to fire.
310+
// Default token TTL is 60s; onRefresh fires at 60 - 15 - 2 = 43s from iat.
311+
// Uses page.evaluate to avoid the global actionTimeout (10s) capping the wait.
312+
await page1.evaluate(() => new Promise(resolve => setTimeout(resolve, 50_000)));
313+
314+
// Two different sessions should each produce exactly one refresh request.
315+
// BroadcastChannel deduplication is per-tokenId, so different sessions refresh independently.
316+
expect(refreshRequests.length).toBe(2);
317+
318+
const refreshedSessionIds = new Set(refreshRequests.map(r => r.sessionId));
319+
expect(refreshedSessionIds.has(user1SessionId)).toBe(true);
320+
expect(refreshedSessionIds.has(user2SessionId)).toBe(true);
321+
322+
// Both tabs should still have valid tokens after the refresh cycle
323+
const page1Token = await page1.evaluate(() => (window as any).Clerk.session?.getToken());
324+
const page2Token = await page2.evaluate(() => (window as any).Clerk.session?.getToken());
325+
326+
expect(page1Token).toBeTruthy();
327+
expect(page2Token).toBeTruthy();
328+
expect(page1Token).not.toBe(page2Token);
329+
});
229330
},
230331
);
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { expect, test } from '@playwright/test';
2+
3+
import { appConfigs } from '../../presets';
4+
import type { FakeUser } from '../../testUtils';
5+
import { createTestUtils, testAgainstRunningApps } from '../../testUtils';
6+
7+
/**
8+
* Tests that the token cache's proactive refresh timer does not accumulate
9+
* orphaned timers across set() calls.
10+
*
11+
* Background: Every API response that piggybacks client data triggers _updateClient,
12+
* which reconstructs Session objects and calls #hydrateCache → SessionTokenCache.set().
13+
* Without proper timer cleanup, each set() call would leave the previous refresh timer
14+
* running, causing the effective polling rate to accelerate over time.
15+
*/
16+
testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })(
17+
'Token refresh timer cleanup @generic',
18+
({ app }) => {
19+
test.describe.configure({ mode: 'serial' });
20+
21+
let fakeUser: FakeUser;
22+
23+
test.beforeAll(async () => {
24+
const u = createTestUtils({ app });
25+
fakeUser = u.services.users.createFakeUser();
26+
await u.services.users.createBapiUser(fakeUser);
27+
});
28+
29+
test.afterAll(async () => {
30+
await fakeUser.deleteIfExists();
31+
await app.teardown();
32+
});
33+
34+
test('touch does not cause clustered token refresh requests', async ({ page, context }) => {
35+
test.setTimeout(120_000);
36+
const u = createTestUtils({ app, page, context });
37+
38+
await u.po.signIn.goTo();
39+
await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password });
40+
await u.po.expect.toBeSignedIn();
41+
42+
// Track token fetch requests with timestamps
43+
const tokenRequests: number[] = [];
44+
await page.route('**/v1/client/sessions/*/tokens**', async route => {
45+
tokenRequests.push(Date.now());
46+
await route.continue();
47+
});
48+
49+
// Trigger multiple touch() calls — each causes _updateClient → Session constructor
50+
// → #hydrateCache → set(), which previously leaked orphaned refresh timers.
51+
// Note: This works because the test instance is multi-session, so it doesn't
52+
// hit the 5s single-session touch throttle.
53+
for (let i = 0; i < 5; i++) {
54+
await page.evaluate(async () => {
55+
await (window as any).Clerk?.session?.touch();
56+
});
57+
}
58+
59+
// Wait 50s — enough for one refresh cycle (~43s) but not two
60+
// eslint-disable-next-line playwright/no-wait-for-timeout
61+
await page.waitForTimeout(50_000);
62+
63+
await page.unrouteAll();
64+
65+
// With the fix: at most 1-2 refresh requests (one cycle at ~43s)
66+
// Without the fix: 5+ requests from orphaned timers all firing at different offsets
67+
expect(tokenRequests.length).toBeLessThanOrEqual(3);
68+
69+
// If multiple requests occurred, verify they aren't clustered together
70+
// (clustering = orphaned timers firing near-simultaneously)
71+
if (tokenRequests.length >= 2) {
72+
for (let i = 1; i < tokenRequests.length; i++) {
73+
const gap = tokenRequests[i] - tokenRequests[i - 1];
74+
expect(gap).toBeGreaterThan(10_000);
75+
}
76+
}
77+
});
78+
},
79+
);

integration/tests/session-token-cache/single-session.test.ts

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })(
4646
* - Only ONE network request is made (from tab1)
4747
* - Tab2 gets the token via BroadcastChannel, proving cross-tab cache sharing
4848
*/
49-
test('MemoryTokenCache multi-tab token sharing', async ({ context }) => {
49+
test('multi-tab token sharing works when clearing the cache', async ({ context }) => {
5050
const page1 = await context.newPage();
5151
const page2 = await context.newPage();
5252

@@ -128,5 +128,75 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })(
128128
// Verify only one token fetch happened (page1), proving page2 got it from BroadcastChannel
129129
expect(tokenRequests.length).toBe(1);
130130
});
131+
132+
/**
133+
* Test Flow:
134+
* 1. Open two tabs with the same browser context (shared cookies)
135+
* 2. Sign in on tab1, reload tab2 to pick up the session
136+
* 3. Both tabs hydrate their token cache with the session token
137+
* 4. Start counting /tokens requests, then wait for the timers to fire
138+
* 5. Assert only 1 /tokens request was made (not 2)
139+
*/
140+
test('multi-tab scheduled refreshes are deduped to a single request', async ({ context }) => {
141+
test.setTimeout(90_000);
142+
143+
const page1 = await context.newPage();
144+
const page2 = await context.newPage();
145+
146+
await page1.goto(app.serverUrl);
147+
await page2.goto(app.serverUrl);
148+
149+
await page1.waitForFunction(() => (window as any).Clerk?.loaded);
150+
await page2.waitForFunction(() => (window as any).Clerk?.loaded);
151+
152+
const u1 = createTestUtils({ app, page: page1 });
153+
await u1.po.signIn.goTo();
154+
await u1.po.signIn.setIdentifier(fakeUser.email);
155+
await u1.po.signIn.continue();
156+
await u1.po.signIn.setPassword(fakeUser.password);
157+
await u1.po.signIn.continue();
158+
await u1.po.expect.toBeSignedIn();
159+
160+
// eslint-disable-next-line playwright/no-wait-for-timeout
161+
await page1.waitForTimeout(1000);
162+
163+
await page2.reload();
164+
await page2.waitForFunction(() => (window as any).Clerk?.loaded);
165+
166+
const u2 = createTestUtils({ app, page: page2 });
167+
await u2.po.expect.toBeSignedIn();
168+
169+
// Both tabs are now signed in and have hydrated their token caches
170+
// via Session constructor -> #hydrateCache, each with an independent
171+
// onRefresh timer that fires at ~43s (TTL 60s - 15s leeway - 2s lead).
172+
// Start counting /tokens requests from this point.
173+
const refreshRequests: string[] = [];
174+
await context.route('**/v1/client/sessions/*/tokens*', async route => {
175+
refreshRequests.push(route.request().url());
176+
await route.continue();
177+
});
178+
179+
// Wait for proactive refresh timers to fire.
180+
// Default token TTL is 60s; onRefresh fires at 60 - 15 - 2 = 43s from iat.
181+
// We wait 50s to give comfortable buffer, this includes the broadcast delay.
182+
//
183+
// Uses page.evaluate instead of page.waitForTimeout to avoid
184+
// the global actionTimeout (10s) silently capping the wait.
185+
await page1.evaluate(() => new Promise(resolve => setTimeout(resolve, 50_000)));
186+
187+
// Only one tab should have made a /tokens request; the other tab should have
188+
// received the refreshed token via BroadcastChannel.
189+
expect(refreshRequests.length).toBe(1);
190+
191+
// Both tabs should still have valid tokens after the refresh cycle
192+
const [page1Token, page2Token] = await Promise.all([
193+
page1.evaluate(() => (window as any).Clerk.session?.getToken()),
194+
page2.evaluate(() => (window as any).Clerk.session?.getToken()),
195+
]);
196+
197+
expect(page1Token).toBeTruthy();
198+
expect(page2Token).toBeTruthy();
199+
expect(page1Token).toBe(page2Token);
200+
});
131201
},
132202
);

0 commit comments

Comments
 (0)