Skip to content

Commit debfc72

Browse files
authored
fix(js): wait up to 10s for the Device Intelligence result by default (#10275)
1 parent 690edf2 commit debfc72

4 files changed

Lines changed: 262 additions & 30 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
'@clerk/shared': patch
4+
---
5+
6+
Wait up to 10 seconds, instead of 5, for the Device Intelligence check to finish before sending a sign-in or sign-up without its result. This applies when an instance hasn't configured its own limit. Users on slow connections are less likely to have sign-ups blocked by protections that require a Device Intelligence result.
7+
8+
Keep the wait bounded when the device clock changes so later attempts can recover after another tab's lock is released.
9+
10+
The shared loader type reference now documents the same 10-second default.

‎packages/clerk-js/src/core/__tests__/protectSession.test.ts‎

Lines changed: 199 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ describe('ProtectSession deadlines', () => {
686686

687687
const startedAt = Date.now();
688688
await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
689-
// Misread, this would fall back to the 5s default and be two orders of magnitude out.
689+
// Misread, this would fall back to the 10s default and be two orders of magnitude out.
690690
expect(Date.now() - startedAt).toBeLessThan(1_000);
691691
});
692692

@@ -733,11 +733,204 @@ describe('ProtectSession deadlines', () => {
733733
// `tokenTimeoutMs` is server config with no upper bound of its own, so the ceiling is ours.
734734
expect(clampTimeout(120_000)).toBe(10_000);
735735
expect(clampTimeout(1_500)).toBe(1_500);
736-
expect(clampTimeout(undefined)).toBe(5_000);
737-
expect(clampTimeout(0)).toBe(5_000);
738-
expect(clampTimeout(-1)).toBe(5_000);
739-
expect(clampTimeout('soon')).toBe(5_000);
740-
expect(clampTimeout(Number.POSITIVE_INFINITY)).toBe(5_000);
736+
expect(clampTimeout(undefined)).toBe(10_000);
737+
expect(clampTimeout(0)).toBe(10_000);
738+
expect(clampTimeout(-1)).toBe(10_000);
739+
expect(clampTimeout('soon')).toBe(10_000);
740+
expect(clampTimeout(Number.POSITIVE_INFINITY)).toBe(10_000);
741+
});
742+
});
743+
744+
describe('ProtectSession default deadline', () => {
745+
const defaultLoader = () => loader({ token_timeout_ms: undefined });
746+
747+
const track = (params: Promise<unknown> | undefined) => {
748+
const state = { settled: false };
749+
void params?.then(() => {
750+
state.settled = true;
751+
});
752+
return state;
753+
};
754+
755+
beforeEach(() => {
756+
vi.useFakeTimers();
757+
});
758+
759+
afterEach(() => {
760+
vi.useRealTimers();
761+
delete (navigator as unknown as Record<string, unknown>).locks;
762+
});
763+
764+
it('accepts a token that arrives after 7s', async () => {
765+
const { session: created, elements } = session([defaultLoader()]);
766+
created?.start();
767+
const params = created?.getRequestParams();
768+
769+
await vi.advanceTimersByTimeAsync(7_000);
770+
serveInline(elements[0], { cid: created?.placeholders().cid });
771+
772+
await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'ok' });
773+
});
774+
775+
it('gives up at 10s when the loader never loads', async () => {
776+
const { session: created } = session([defaultLoader()]);
777+
created?.start();
778+
const params = created?.getRequestParams();
779+
const state = track(params);
780+
781+
await vi.advanceTimersByTimeAsync(9_999);
782+
expect(state.settled).toBe(false);
783+
await vi.advanceTimersByTimeAsync(1);
784+
await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
785+
});
786+
787+
it('holds a sign-in made mid-acquisition only for what is left of it', async () => {
788+
const { session: created } = session([defaultLoader()]);
789+
created?.start();
790+
await vi.advanceTimersByTimeAsync(6_000);
791+
792+
const params = created?.getRequestParams();
793+
const state = track(params);
794+
795+
await vi.advanceTimersByTimeAsync(3_999);
796+
expect(state.settled).toBe(false);
797+
await vi.advanceTimersByTimeAsync(1);
798+
await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
799+
});
800+
801+
it('never holds one sign-in past the deadline when the lock is granted late', async () => {
802+
Object.defineProperty(navigator, 'locks', {
803+
value: {
804+
request: vi.fn((_key: string, _options: unknown, callback: () => Promise<unknown>) =>
805+
new Promise(resolve => setTimeout(resolve, 4_900)).then(() => callback()),
806+
),
807+
},
808+
configurable: true,
809+
});
810+
811+
const { session: created } = session([defaultLoader()]);
812+
created?.start();
813+
const params = created?.getRequestParams();
814+
const state = track(params);
815+
816+
await vi.advanceTimersByTimeAsync(9_999);
817+
expect(state.settled).toBe(false);
818+
await vi.advanceTimersByTimeAsync(1);
819+
await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
820+
});
821+
822+
it('keeps waiting for a leader tab after SafeLock gives up, until its own deadline', async () => {
823+
Object.defineProperty(navigator, 'locks', {
824+
value: {
825+
request: vi.fn(
826+
(_key: string, options: { signal: AbortSignal }) =>
827+
new Promise((_resolve, reject) => {
828+
options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError')));
829+
}),
830+
),
831+
},
832+
configurable: true,
833+
});
834+
835+
const { session: created, elements } = session([defaultLoader()]);
836+
created?.start();
837+
const params = created?.getRequestParams();
838+
839+
await vi.advanceTimersByTimeAsync(7_000);
840+
localStorage.setItem('__clerk_protect_st', storedEntry({ token: 'v1.leader.mac', rid: 'c'.repeat(26) }));
841+
await vi.advanceTimersByTimeAsync(100);
842+
843+
await expect(params).resolves.toMatchObject({
844+
__clerk_protect_token: 'v1.leader.mac',
845+
__clerk_protect_status: 'ok',
846+
});
847+
expect(elements).toHaveLength(0);
848+
});
849+
850+
it('answers at once when the browser refuses the cross-tab lock', async () => {
851+
Object.defineProperty(navigator, 'locks', {
852+
value: { request: vi.fn(() => Promise.reject(new DOMException('denied', 'SecurityError'))) },
853+
configurable: true,
854+
});
855+
856+
const { session: created, elements } = session([defaultLoader()]);
857+
created?.start();
858+
const params = created?.getRequestParams();
859+
const state = track(params);
860+
861+
await vi.advanceTimersByTimeAsync(50);
862+
expect(state.settled).toBe(true);
863+
await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
864+
expect(elements).toHaveLength(0);
865+
});
866+
867+
it('stops at its own deadline when that comes before SafeLock gives up', async () => {
868+
Object.defineProperty(navigator, 'locks', {
869+
value: {
870+
request: vi.fn(
871+
(_key: string, options: { signal: AbortSignal }) =>
872+
new Promise((_resolve, reject) => {
873+
options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError')));
874+
}),
875+
),
876+
},
877+
configurable: true,
878+
});
879+
880+
const { session: created, elements } = session([loader({ token_timeout_ms: 3_000 })]);
881+
created?.start();
882+
const params = created?.getRequestParams();
883+
const state = track(params);
884+
885+
await vi.advanceTimersByTimeAsync(2_999);
886+
expect(state.settled).toBe(false);
887+
await vi.advanceTimersByTimeAsync(1);
888+
await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
889+
890+
localStorage.setItem('__clerk_protect_st', storedEntry({ token: 'v1.leader.mac', rid: 'c'.repeat(26) }));
891+
await vi.advanceTimersByTimeAsync(2_000);
892+
expect(vi.getTimerCount()).toBe(0);
893+
await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_token: 'v1.leader.mac' });
894+
expect(elements).toHaveLength(0);
895+
});
896+
897+
it.each([
898+
[2_000, -3_600_000],
899+
[6_000, -3_600_000],
900+
[2_000, 3_600_000],
901+
[6_000, 3_600_000],
902+
])('settles follower polling after a clock change at %s ms by %s ms', async (changeAt, correction) => {
903+
let available = false;
904+
const request = vi.fn((_key: string, options: { signal: AbortSignal }, callback: () => Promise<unknown>) => {
905+
if (available) {
906+
return callback();
907+
}
908+
return new Promise((_resolve, reject) => {
909+
options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError')));
910+
});
911+
});
912+
Object.defineProperty(navigator, 'locks', { configurable: true, value: { request } });
913+
const { session: created, elements } = session([defaultLoader()]);
914+
created?.start();
915+
const params = created?.getRequestParams();
916+
const state = track(params);
917+
918+
await vi.advanceTimersByTimeAsync(changeAt);
919+
vi.setSystemTime(Date.now() + correction);
920+
await vi.advanceTimersByTimeAsync(9_999 - changeAt);
921+
expect(state.settled).toBe(false);
922+
await vi.advanceTimersByTimeAsync(1);
923+
await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
924+
expect(vi.getTimerCount()).toBe(0);
925+
926+
available = true;
927+
await vi.advanceTimersByTimeAsync(30_000);
928+
const retry = created?.getRequestParams();
929+
await vi.advanceTimersByTimeAsync(0);
930+
expect(request).toHaveBeenCalledTimes(2);
931+
expect(elements).toHaveLength(1);
932+
serveInline(elements[0], { cid: created?.placeholders().cid });
933+
await expect(retry).resolves.toMatchObject({ __clerk_protect_status: 'ok' });
741934
});
742935
});
743936

‎packages/clerk-js/src/core/protectSession.ts‎

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ const INLINE_TOKEN_GLOBAL = '__clerk_specter';
3131
/** A token this close to expiry is treated as absent, so a fresh run starts before it lapses. */
3232
const TOKEN_REFRESH_MARGIN_MS = 60 * 1_000;
3333
/** Acquisition deadline when the instance does not configure one. */
34-
const DEFAULT_TOKEN_TIMEOUT_MS = 5 * 1_000;
34+
const DEFAULT_TOKEN_TIMEOUT_MS = 10 * 1_000;
3535
/** Ceiling on the instance-configured deadline, so no server value can stall a sign-in. */
3636
const MAX_TOKEN_TIMEOUT_MS = 10 * 1_000;
37+
const STORE_POLL_INTERVAL_MS = 100;
38+
const LOCK_REFUSAL_WINDOW_MS = 4_000;
3739
/** A stored token claiming more remaining life than this was not minted by us. */
3840
const MAX_TOKEN_LIFETIME_MS = 24 * 60 * 60 * 1_000;
3941
/** Longest token we will hand back, so a planted store entry cannot bloat a sign-in body. */
@@ -406,16 +408,17 @@ function isRetryableStatus(status: number): boolean {
406408
}
407409

408410
function sleep(ms: number, signal: AbortSignal): Promise<void> {
411+
if (signal.aborted) {
412+
return Promise.resolve();
413+
}
409414
return new Promise(resolve => {
410-
const timer = setTimeout(resolve, ms);
411-
signal.addEventListener(
412-
'abort',
413-
() => {
414-
clearTimeout(timer);
415-
resolve();
416-
},
417-
{ once: true },
418-
);
415+
const settle = () => {
416+
clearTimeout(timer);
417+
signal.removeEventListener('abort', settle);
418+
resolve();
419+
};
420+
const timer = setTimeout(settle, ms);
421+
signal.addEventListener('abort', settle, { once: true });
419422
});
420423
}
421424

@@ -624,7 +627,8 @@ export class ProtectSession {
624627
if (readStoredToken(this.#tokenStorageKey, TOKEN_SEND_MARGIN_MS, this.#floor)) {
625628
return;
626629
}
627-
if (Date.now() - this.#lastAttemptAt < this.#backoffMs()) {
630+
const elapsed = Date.now() - this.#lastAttemptAt;
631+
if (elapsed >= 0 && elapsed < this.#backoffMs()) {
628632
return;
629633
}
630634
this.#startAcquisition();
@@ -668,22 +672,47 @@ export class ProtectSession {
668672
return 'ok';
669673
}
670674

671-
const result = await this.#lock.acquireLockAndRun(async () => {
672-
// Double-checked inside the lock. Without this re-read every tab runs the loader in turn,
673-
// which is the exact failure this design exists to prevent.
674-
if (this.#freshStoredToken()) {
675-
return 'ok' satisfies ProtectStatus;
675+
const controller = new AbortController();
676+
const timer = setTimeout(() => controller.abort(), this.#timeoutMs);
677+
let lockWaitedOut = false;
678+
const lockTimer = setTimeout(() => {
679+
lockWaitedOut = true;
680+
}, LOCK_REFUSAL_WINDOW_MS);
681+
try {
682+
const result = await this.#lock.acquireLockAndRun(async () => {
683+
// Double-checked inside the lock. Without this re-read every tab runs the loader in turn,
684+
// which is the exact failure this design exists to prevent.
685+
if (this.#freshStoredToken()) {
686+
return 'ok' satisfies ProtectStatus;
687+
}
688+
return await this.#mint();
689+
});
690+
691+
if (typeof result === 'string') {
692+
return result as ProtectStatus;
676693
}
677-
return await this.#mint();
678-
});
679694

680-
if (typeof result === 'string') {
681-
return result as ProtectStatus;
695+
// Refused: nobody is minting for us. Waited out: a leader may still be, so poll the store until our deadline.
696+
if (!lockWaitedOut) {
697+
return this.#freshStoredToken() ? 'ok' : 'timeout';
698+
}
699+
return (await this.#awaitStoredToken(controller.signal)) ? 'ok' : 'timeout';
700+
} finally {
701+
clearTimeout(timer);
702+
clearTimeout(lockTimer);
682703
}
704+
}
683705

684-
// The lock was never held — `SafeLock` gives up waiting after ~5s so a wedged leader delays
685-
// nobody. Whatever a leader did manage to write is still ours to use.
686-
return this.#freshStoredToken() ? 'ok' : 'timeout';
706+
async #awaitStoredToken(signal: AbortSignal): Promise<boolean> {
707+
for (;;) {
708+
if (this.#freshStoredToken()) {
709+
return true;
710+
}
711+
if (signal.aborted) {
712+
return false;
713+
}
714+
await sleep(STORE_POLL_INTERVAL_MS, signal);
715+
}
687716
}
688717

689718
/** Runs the loader and takes the token it was served with. Never rejects. */

‎packages/shared/src/types/protectConfig.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export interface ProtectLoader {
3434
token_url?: string;
3535
/**
3636
* How long to wait for the token before giving up and reporting a status instead. Defaults to
37-
* 5000 and is capped at 10000 by the SDK, so this cannot stall a sign-in.
37+
* 10000 and is capped at 10000 by the SDK, so this cannot stall a sign-in.
3838
*/
3939
token_timeout_ms?: number;
4040
/**

0 commit comments

Comments
 (0)