diff --git a/.changeset/protect-check-load-timeout.md b/.changeset/protect-check-load-timeout.md new file mode 100644 index 00000000000..b02dfbe772e --- /dev/null +++ b/.changeset/protect-check-load-timeout.md @@ -0,0 +1,7 @@ +--- +'@clerk/clerk-js': patch +'@clerk/shared': patch +'@clerk/ui': patch +--- + +Fix an issue where a verification that was still progressing normally could be cancelled and reported to the user as having timed out. diff --git a/packages/clerk-js/src/core/__tests__/protectChallengeLoadTimeout.test.ts b/packages/clerk-js/src/core/__tests__/protectChallengeLoadTimeout.test.ts new file mode 100644 index 00000000000..348cf62919a --- /dev/null +++ b/packages/clerk-js/src/core/__tests__/protectChallengeLoadTimeout.test.ts @@ -0,0 +1,77 @@ +import type { ProtectLoader } from '@clerk/shared/types'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { Protect } from '../protect'; +import { __internal_resetProtectStorage } from '../protectSession'; +import type { Environment } from '../resources'; + +const environment = (loaders: unknown[]): Environment => ({ protectConfig: { loaders } }) as unknown as Environment; + +/** No `src`, so jsdom does not fetch a real URL and race the assertions here. */ +const loader = (overrides: Partial = {}): ProtectLoader => ({ + target: 'head', + type: 'script', + ...overrides, +}); + +describe('Protect.challengeLoadTimeoutMs', () => { + beforeEach(() => { + document.head.innerHTML = ''; + document.body.innerHTML = ''; + __internal_resetProtectStorage(); + vi.restoreAllMocks(); + }); + + it('is undefined when no loader asks for one, so the caller falls back', () => { + const protect = new Protect(); + protect.load(environment([loader()])); + + expect(protect.challengeLoadTimeoutMs).toBeUndefined(); + }); + + it('reports the value from the loader this browser was assigned', () => { + const protect = new Protect(); + protect.load(environment([loader({ challenge_load_timeout_ms: 25_000 })])); + + expect(protect.challengeLoadTimeoutMs).toBe(25_000); + }); + + // The whole point of putting it on the loader: while a new loader ramps, two are live for the + // same instance, and only the one this browser actually got may speak for it. + it('ignores a loader the rollout dice excluded', () => { + vi.spyOn(Math, 'random').mockReturnValue(0.9); + + const protect = new Protect(); + protect.load( + environment([ + // A 10% ramp, excluded at random() 0.9. + loader({ rollout: 0.1, challenge_load_timeout_ms: 25_000 }), + // The incumbent, which this browser does get. + loader({ challenge_load_timeout_ms: 45_000 }), + ]), + ); + + expect(protect.challengeLoadTimeoutMs).toBe(45_000); + }); + + it('reports the ramping loader once the dice include it', () => { + vi.spyOn(Math, 'random').mockReturnValue(0.01); + + const protect = new Protect(); + protect.load(environment([loader({ rollout: 0.1, challenge_load_timeout_ms: 25_000 }), loader()])); + + expect(protect.challengeLoadTimeoutMs).toBe(25_000); + }); + + it('ignores a non-positive or non-numeric value rather than passing it on', () => { + const protect = new Protect(); + protect.load( + environment([ + loader({ challenge_load_timeout_ms: 0 }), + loader({ challenge_load_timeout_ms: 'soon' as unknown as number }), + ]), + ); + + expect(protect.challengeLoadTimeoutMs).toBeUndefined(); + }); +}); diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 7fee3cd05f4..878d73b5452 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -327,6 +327,16 @@ export class Clerk implements ClerkInterface { return this.#oauthTransport; } + /** + * The verification-module load timeout asked for by the loader THIS browser was assigned, or + * undefined when it asked for nothing. Exposed because the assignment is a random draw per page + * load and cannot be recomputed from the environment config; callers fall back to the + * instance-wide value on that config, and then to the SDK default. + */ + get __internal_protectChallengeLoadTimeoutMs(): number | undefined { + return this.#protect?.challengeLoadTimeoutMs; + } + public __internal_getCachedResources: | (() => Promise<{ client: ClientJSONSnapshot | null; environment: EnvironmentJSONSnapshot | null }>) | undefined; diff --git a/packages/clerk-js/src/core/protect.ts b/packages/clerk-js/src/core/protect.ts index 259de41ee36..181ae2755f5 100644 --- a/packages/clerk-js/src/core/protect.ts +++ b/packages/clerk-js/src/core/protect.ts @@ -8,6 +8,21 @@ import type { Environment } from './resources'; export class Protect { #initialized: boolean = false; #session?: ProtectSession; + #challengeLoadTimeoutMs?: number; + + /** + * The verification-module LOAD timeout asked for by the loader this browser was assigned, or + * undefined when none asked for one — in which case the caller falls back to the instance-wide + * value and then to the SDK default. + * + * Resolved from the APPLIED loaders rather than from the config, because rollout is decided by + * a random draw per page load: a loader being ramped can carry its own value without changing + * anything for browsers still on the loader it replaces, and which one a browser got cannot be + * recomputed afterwards. + */ + get challengeLoadTimeoutMs(): number | undefined { + return this.#challengeLoadTimeoutMs; + } load(env: Environment): void { const config = env?.protectConfig; @@ -40,6 +55,19 @@ export class Protect { // loaders we are actually going to apply. const loaders = configured.filter(loader => isLoader(loader) && inRollout(loader)); + // Read off the applied set, before any of them run: this only describes config, so a loader + // that later fails to be placed has still spoken for the browser it was assigned to. + // + // First one that asks for it wins, and the instance-wide value applies only when none does — + // the precedence is across the applied set rather than per loader, because there is no single + // "the" loader once more than one is live. Documented on the field. + this.#challengeLoadTimeoutMs = loaders.find( + loader => + typeof loader.challenge_load_timeout_ms === 'number' && + Number.isFinite(loader.challenge_load_timeout_ms) && + loader.challenge_load_timeout_ms > 0, + )?.challenge_load_timeout_ms; + // Only an instance whose loaders reference the correlation id gets a session, so an instance // not using it is unaffected and stores nothing in the browser. const applyLoader: ApplyLoader = (loader, placeholders) => this.applyLoader(loader, placeholders); diff --git a/packages/clerk-js/src/core/resources/ProtectConfig.ts b/packages/clerk-js/src/core/resources/ProtectConfig.ts index 2a5b1a7cfb7..b5a093ede9b 100644 --- a/packages/clerk-js/src/core/resources/ProtectConfig.ts +++ b/packages/clerk-js/src/core/resources/ProtectConfig.ts @@ -11,6 +11,7 @@ export class ProtectConfig extends BaseResource implements ProtectConfigResource id: string = ''; loaders?: ProtectLoader[]; tokens_invalid_before?: number; + challenge_load_timeout_ms?: number; rollout?: number; public constructor(data: ProtectConfigJSON | ProtectConfigJSONSnapshot | null = null) { @@ -27,6 +28,7 @@ export class ProtectConfig extends BaseResource implements ProtectConfigResource this.id = this.withDefault(data.id, this.id); this.loaders = this.withDefault(data.loaders, this.loaders); this.tokens_invalid_before = this.withDefault(data.tokens_invalid_before, this.tokens_invalid_before); + this.challenge_load_timeout_ms = this.withDefault(data.challenge_load_timeout_ms, this.challenge_load_timeout_ms); return this; } @@ -37,6 +39,7 @@ export class ProtectConfig extends BaseResource implements ProtectConfigResource id: this.id, loaders: this.loaders, tokens_invalid_before: this.tokens_invalid_before, + challenge_load_timeout_ms: this.challenge_load_timeout_ms, }; } } diff --git a/packages/shared/src/internal/clerk-js/__tests__/protectCheck.test.ts b/packages/shared/src/internal/clerk-js/__tests__/protectCheck.test.ts index 1d0ac1cfdb6..cc74414ad43 100644 --- a/packages/shared/src/internal/clerk-js/__tests__/protectCheck.test.ts +++ b/packages/shared/src/internal/clerk-js/__tests__/protectCheck.test.ts @@ -1,8 +1,8 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { ProtectCheckResource } from '@/types'; -import { executeProtectCheck } from '../protectCheck'; +import { DEFAULT_PROTECT_CHECK_LOAD_TIMEOUT_MS, executeProtectCheck } from '../protectCheck'; const fakeContainer = (): HTMLDivElement => ({}) as HTMLDivElement; @@ -199,4 +199,131 @@ describe('executeProtectCheck', () => { }); }); }); + + describe('the load bound covers the handoff and nothing after it', () => { + beforeEach(() => { + // Fake only the timer functions under test. Faking the whole clock also stalls the + // machinery that settles a dynamic import, so the module would never finish loading. + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + // A network that accepts the connection and then never answers is the one load failure that + // cannot report itself — every other one rejects the import on its own. + it('rejects as a load failure when the module never arrives', async () => { + vi.doMock('https://protect.example.com/sdk-hangs.js', () => new Promise(() => {})); + + const running = executeProtectCheck( + protectCheck({ sdkUrl: 'https://protect.example.com/sdk-hangs.js' }), + fakeContainer(), + ); + const assertion = expect(running).rejects.toMatchObject({ code: 'protect_check_script_load_failed' }); + + await vi.advanceTimersByTimeAsync(DEFAULT_PROTECT_CHECK_LOAD_TIMEOUT_MS + 1); + await assertion; + }); + + it('honours a per-instance loadTimeoutMs override instead of the default', async () => { + vi.doMock('https://protect.example.com/sdk-hangs-2.js', () => new Promise(() => {})); + + // Expressed relative to the default, and LONGER than it, so that outliving the default is + // itself the proof the override replaced it rather than racing alongside it. + const override = DEFAULT_PROTECT_CHECK_LOAD_TIMEOUT_MS * 2; + const running = executeProtectCheck( + protectCheck({ sdkUrl: 'https://protect.example.com/sdk-hangs-2.js' }), + fakeContainer(), + { loadTimeoutMs: override }, + ); + let settled = false; + const watch = running.then( + () => (settled = true), + () => (settled = true), + ); + + await vi.advanceTimersByTimeAsync(DEFAULT_PROTECT_CHECK_LOAD_TIMEOUT_MS + 1); + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(override); + await watch; + expect(settled).toBe(true); + }); + + // An abort mid-load has to settle the operation, not leave it pending for the whole bound + // holding its closures and timer, and it is a cancellation rather than a load failure. + it('settles as an abort when the caller aborts during a stalled load', async () => { + vi.doMock('https://protect.example.com/sdk-hangs-3.js', () => new Promise(() => {})); + + const controller = new AbortController(); + const running = executeProtectCheck( + protectCheck({ sdkUrl: 'https://protect.example.com/sdk-hangs-3.js' }), + fakeContainer(), + { signal: controller.signal }, + ); + const assertion = expect(running).rejects.toMatchObject({ code: 'protect_check_aborted' }); + + controller.abort(); + // No timer advanced: the abort alone must settle it, well before the load bound. + await assertion; + }); + + // setTimeout stores its delay in a signed 32-bit int, so an oversized value overflows and + // fires immediately — failing every load instantly, the opposite of what was configured. + it('clamps an oversized loadTimeoutMs instead of overflowing the timer', async () => { + vi.doMock('https://protect.example.com/sdk-hangs-4.js', () => new Promise(() => {})); + + const running = executeProtectCheck( + protectCheck({ sdkUrl: 'https://protect.example.com/sdk-hangs-4.js' }), + fakeContainer(), + { loadTimeoutMs: 2_147_483_648 }, + ); + let settled = false; + const watch = running.then( + () => (settled = true), + () => (settled = true), + ); + + // An overflowed timer would already have fired by now. + await vi.advanceTimersByTimeAsync(1_000); + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(600_000); + await watch; + expect(settled).toBe(true); + }); + + // The point of the whole change: the challenge owns its own duration. A challenge running far + // longer than any load bound must still resolve — proof-of-transfer moves a server-chosen + // number of bytes, and a host-side wall would abort it as a "timeout". + it('never bounds the challenge once the module has taken control', async () => { + let running = false; + let finish!: (proofToken: string) => void; + vi.doMock('https://protect.example.com/sdk-slow.js', () => ({ + default: () => { + running = true; + return new Promise(resolve => (finish = resolve)); + }, + })); + + const loadTimeoutMs = 5_000; + const execution = executeProtectCheck( + protectCheck({ sdkUrl: 'https://protect.example.com/sdk-slow.js' }), + fakeContainer(), + { loadTimeoutMs }, + ); + const assertion = expect(execution).resolves.toBe('proof-after-ages'); + + // Wait on the real event loop until the module has taken control, so what follows is + // unambiguously time spent in the CHALLENGE rather than in the load. + while (!running) { + await new Promise(resolve => setImmediate(resolve)); + } + + // Burn far more time than the load bound. Nothing may abort the challenge for it. + await vi.advanceTimersByTimeAsync(360 * loadTimeoutMs); + finish('proof-after-ages'); + await assertion; + }); + }); }); diff --git a/packages/shared/src/internal/clerk-js/protectCheck.ts b/packages/shared/src/internal/clerk-js/protectCheck.ts index bcdcec43007..f44e33596cd 100644 --- a/packages/shared/src/internal/clerk-js/protectCheck.ts +++ b/packages/shared/src/internal/clerk-js/protectCheck.ts @@ -26,6 +26,83 @@ export interface ExecuteProtectCheckOptions { * Scripts that don't honor the signal will continue to run; this is best-effort by design. */ signal?: AbortSignal; + /** + * Overrides how long to wait for the challenge module to LOAD. Per-instance and per-loader + * config, since the right value depends on the population an instance serves; a non-positive + * or non-numeric value falls back to {@link DEFAULT_PROTECT_CHECK_LOAD_TIMEOUT_MS}. + * + * Bounds the handoff only — never the challenge. See the note on the constant. + */ + loadTimeoutMs?: number; +} + +/** + * Default bound on LOADING the challenge module. + * + * Its only job is a network that accepts a connection and then never answers, because every + * other load failure — a CSP block, DNS, a 404, a body that isn't a valid module — rejects the + * dynamic import on its own and needs no timer to notice. That makes a generous value the safe + * one: nothing legitimate is waiting on this timer, while a value tight enough to fire on a + * genuinely slow connection would fail a load that was going to succeed. + */ +export const DEFAULT_PROTECT_CHECK_LOAD_TIMEOUT_MS = 60_000; + +/** + * Ceiling on the configured load bound. `setTimeout` stores its delay in a signed 32-bit int, so a + * larger value overflows and fires immediately — which would make every load fail instantly, the + * exact opposite of what an operator asking for a long timeout wanted. Clamping rather than + * rejecting keeps a fat-fingered config from breaking sign-in. + */ +const MAX_PROTECT_CHECK_LOAD_TIMEOUT_MS = 600_000; + +function resolveLoadTimeoutMs(configured: number | undefined): number { + if (typeof configured !== 'number' || !Number.isFinite(configured) || configured <= 0) { + return DEFAULT_PROTECT_CHECK_LOAD_TIMEOUT_MS; + } + return Math.min(configured, MAX_PROTECT_CHECK_LOAD_TIMEOUT_MS); +} + +/** + * Races the dynamic import against `timeoutMs`, always clearing the timer so a fast load does not + * leave one pending. + * + * The bound stops at the import on purpose. Once `mod.default` is called the challenge owns its + * own deadline and the host imposes none: the host cannot know an honest duration for a challenge + * whose type is chosen server-side, per decision, and whose work it deliberately knows nothing + * about — waiting on a person, or moving a server-chosen number of bytes over an unknown link. A + * host-side wall over execution aborts legitimate challenges and reports them as timeouts, and + * since a re-run restarts the work, retrying cannot win on any connection slow enough to trip it. + * + * The abort signal is raced too. A stalled import cannot itself be cancelled, but without this the + * caller's abort would not settle anything: an unmounted component would keep this promise, its + * closures and its timer alive for the whole load bound, and then report a load failure for what + * was really a cancellation. + */ +function importWithTimeout( + url: string, + timeoutMs: number, + signal: AbortSignal | undefined, +): Promise> { + let timeoutId: ReturnType | undefined; + let onAbort: (() => void) | undefined; + + const expiry = new Promise((_, reject) => { + timeoutId = setTimeout(() => reject(new Error('Protect check script load timed out')), timeoutMs); + }); + const aborted = new Promise((_, reject) => { + if (!signal) { + return; + } + onAbort = () => reject(new Error('Protect check aborted during load')); + signal.addEventListener('abort', onAbort, { once: true }); + }); + + return Promise.race([import(/* webpackIgnore: true */ url), expiry, aborted]).finally(() => { + clearTimeout(timeoutId); + if (signal && onAbort) { + signal.removeEventListener('abort', onAbort); + } + }) as Promise>; } interface ScriptInitOptions { @@ -99,7 +176,7 @@ export async function executeProtectCheck( container: HTMLDivElement, options: ExecuteProtectCheckOptions = {}, ): Promise { - const { signal, setWidgetVisible } = options; + const { signal, setWidgetVisible, loadTimeoutMs } = options; const { sdkUrl, token, uiHints } = protectCheck; const validated = assertValidSdkUrl(sdkUrl); @@ -110,8 +187,14 @@ export async function executeProtectCheck( let mod: Record; try { - mod = await import(/* webpackIgnore: true */ validated.toString()); + mod = await importWithTimeout(validated.toString(), resolveLoadTimeoutMs(loadTimeoutMs), signal); } catch { + // An abort that landed mid-load is a cancellation, not a load failure. Checked first so the + // caller gets the same contract it does everywhere else: if you aborted, you never see + // anything but `protect_check_aborted`. + if (signal?.aborted) { + throw new ClerkRuntimeError('Protect check aborted by caller', { code: 'protect_check_aborted' }); + } // Surface a generic message and deliberately omit the original error: Chromium/Firefox embed // the sdk_url in the dynamic-import failure text, which a tampered response could plant in the UI. throw new ClerkRuntimeError( diff --git a/packages/shared/src/types/clerk.ts b/packages/shared/src/types/clerk.ts index 73e25a4950b..0f57591517d 100644 --- a/packages/shared/src/types/clerk.ts +++ b/packages/shared/src/types/clerk.ts @@ -332,6 +332,16 @@ export interface Clerk { */ __internal_moduleManager: ModuleManager | undefined; + /** + * The verification-module load timeout asked for by the loader this browser was assigned, or + * undefined when it asked for nothing. The assignment is randomized per page load, so it cannot + * be recomputed from the environment config; callers fall back to the instance-wide value on + * that config, and then to the SDK default. + * + * @internal + */ + __internal_protectChallengeLoadTimeoutMs?: number; + frontendApi: string; /** Your Clerk [Publishable Key](!publishable-key). */ diff --git a/packages/shared/src/types/protectConfig.ts b/packages/shared/src/types/protectConfig.ts index 8ea7fdf30a6..c7275ecc43f 100644 --- a/packages/shared/src/types/protectConfig.ts +++ b/packages/shared/src/types/protectConfig.ts @@ -37,12 +37,34 @@ export interface ProtectLoader { * 5000 and is capped at 10000 by the SDK, so this cannot stall a sign-in. */ token_timeout_ms?: number; + /** + * Overrides {@link ProtectConfigJSON.challenge_load_timeout_ms} for browsers that got this + * loader. + * + * Per loader because loaders roll out gradually: while a new one ramps, two are live for the + * same instance at once, and the new one may need a different value from the one it replaces. + * + * Precedence is across the APPLIED SET, not per loader: the first applied loader that specifies + * one wins, and the instance-wide value applies only when none of them does. An instance running + * two loaders at once should therefore either set this on both or on neither — setting it on one + * makes it apply to browsers that got the other, which is rarely what is meant. + */ + challenge_load_timeout_ms?: number; } export interface ProtectConfigJSON { object: 'protect_config'; id: string; loaders?: ProtectLoader[]; + /** + * How long to wait for a verification module to LOAD before giving up, in milliseconds. Absent + * means "use the SDK default". + * + * It bounds only the load. Once a verification module is running it governs its own duration, + * so this is not a bound on how long verification may take — the two are deliberately separate + * budgets, and only the first is the SDK's to set. + */ + challenge_load_timeout_ms?: number; /** * Unix seconds. A session token acquired while an older value was configured is discarded and * re-acquired, so raising this makes every browser fetch a fresh one as its environment @@ -61,6 +83,8 @@ export interface ProtectConfigResource extends ClerkResource { loaders?: ProtectLoader[]; /** See {@link ProtectConfigJSON.tokens_invalid_before}. */ tokens_invalid_before?: number; + /** See {@link ProtectConfigJSON.challenge_load_timeout_ms}. */ + challenge_load_timeout_ms?: number; __internal_toSnapshot: () => ProtectConfigJSONSnapshot; } diff --git a/packages/ui/src/hooks/useProtectCheckRunner.ts b/packages/ui/src/hooks/useProtectCheckRunner.ts index e80173033d8..04b82a7388a 100644 --- a/packages/ui/src/hooks/useProtectCheckRunner.ts +++ b/packages/ui/src/hooks/useProtectCheckRunner.ts @@ -1,9 +1,11 @@ import { ClerkRuntimeError, isClerkAPIResponseError } from '@clerk/shared/error'; import { ERROR_CODES } from '@clerk/shared/internal/clerk-js/constants'; +import { useClerk } from '@clerk/shared/react'; import type { ProtectCheckResource } from '@clerk/shared/types'; import React from 'react'; import { flushSync } from 'react-dom'; +import { useEnvironment } from '@/ui/contexts/EnvironmentContext'; import { useCardState } from '@/ui/elements/contexts'; import { handleError } from '@/ui/utils/errorHandler'; @@ -17,9 +19,6 @@ import { handleError } from '@/ui/utils/errorHandler'; */ const MAX_EXPIRED_RELOADS = 2; -/** Upper bound on how long we wait for the challenge SDK to settle before failing loud. */ -const PROTECT_CHECK_SCRIPT_TIMEOUT_MS = 60_000; - export interface ProtectCheckRunnerParams { /** * Reads the current protect_check off the resource. Called fresh on each effect run because @@ -68,6 +67,15 @@ export interface ProtectCheckRunner { export function useProtectCheckRunner(params: ProtectCheckRunnerParams): ProtectCheckRunner { const card = useCardState(); + // Override for the module-LOAD bound only (see `executeProtectCheck`), resolved loader first + // and instance second: a loader being rolled out gradually can carry its own value without + // changing anything for browsers still on the loader it replaces. Undefined at both levels + // leaves the SDK default in force. Read here rather than inside the effect so the effect keeps + // depending on primitives. + const loaderTimeoutMs = useClerk().__internal_protectChallengeLoadTimeoutMs; + const instanceTimeoutMs = useEnvironment().protectConfig?.challenge_load_timeout_ms; + const loadTimeoutMs = loaderTimeoutMs ?? instanceTimeoutMs; + // `handleError` re-throws what it does not recognise, and this runner awaits caller code that // raises plain errors (a transient fetch failure, an OAuth continuation that did not complete). const reportError = (err: any) => { @@ -227,7 +235,6 @@ export function useProtectCheckRunner(params: ProtectCheckRunnerParam setIsRunning(true); const runChallenge = async () => { - let timeoutId: ReturnType | undefined; try { // Load the Protect SDK loader lazily, gated on the same compile-time flag as the // fail-closed guard above. In no-RHC builds `__BUILD_DISABLE_RHC__` is `true`, so this @@ -238,20 +245,19 @@ export function useProtectCheckRunner(params: ProtectCheckRunnerParam return; } const { executeProtectCheck } = await import('@clerk/shared/internal/clerk-js/protectCheck'); - const proofToken = await Promise.race([ - executeProtectCheck(protectCheck, container, { signal: abortController.signal, setWidgetVisible }), - new Promise((_, reject) => { - timeoutId = setTimeout(() => { - // Stop the (possibly hung) SDK and surface a retryable timeout error. - abortController.abort(); - reject( - new ClerkRuntimeError('Protect verification timed out', { - code: ERROR_CODES.PROTECT_CHECK_TIMED_OUT, - }), - ); - }, PROTECT_CHECK_SCRIPT_TIMEOUT_MS); - }), - ]); + // Deliberately unraced. `executeProtectCheck` bounds LOADING the challenge module and + // nothing after it: once control passes to the challenge, the challenge owns its own + // deadline. We cannot know an honest duration for it — the challenge type is chosen + // server-side, per decision, long after this bundle shipped, and its work may be waiting + // on a person or moving a server-chosen number of bytes over a link we know nothing + // about. The wall that used to be here aborted valid challenges and reported them to the + // user as a timeout, and since a re-run restarts the work from the beginning, retrying + // could never win on any connection slow enough to trip it in the first place. + const proofToken = await executeProtectCheck(protectCheck, container, { + signal: abortController.signal, + setWidgetVisible, + loadTimeoutMs, + }); if (cancelled) { return; } @@ -286,9 +292,6 @@ export function useProtectCheckRunner(params: ProtectCheckRunnerParam } reportError(err); } finally { - if (timeoutId) { - clearTimeout(timeoutId); - } if (!cancelled) { isRunningRef.current = false; setIsRunning(false);