diff --git a/.changeset/easy-bars-type.md b/.changeset/easy-bars-type.md new file mode 100644 index 00000000000..b2cfbafeac0 --- /dev/null +++ b/.changeset/easy-bars-type.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Refactors Clerk initialization retry logic to use a dedicated withRetry utility with exponential backoff diff --git a/packages/clerk-js/src/core/__tests__/clerk.test.ts b/packages/clerk-js/src/core/__tests__/clerk.test.ts index 521f9c4b55e..36a242ffa69 100644 --- a/packages/clerk-js/src/core/__tests__/clerk.test.ts +++ b/packages/clerk-js/src/core/__tests__/clerk.test.ts @@ -1,4 +1,4 @@ -import { ClerkOfflineError, EmailLinkErrorCodeStatus } from '@clerk/shared/error'; +import { ClerkOfflineError, ClerkRuntimeError, EmailLinkErrorCodeStatus } from '@clerk/shared/error'; import { ERROR_CODES } from '@clerk/shared/internal/clerk-js/constants'; import type { ActiveSessionResource, @@ -14,8 +14,11 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, test, import { mockJwt } from '@/test/core-fixtures'; import { mockNativeRuntime } from '../../test/utils'; +import * as localStorageModule from '../../utils/localStorage'; +import { AuthCookieService } from '../auth/AuthCookieService'; import type { DevBrowser } from '../auth/devBrowser'; import { Clerk } from '../clerk'; +import * as errorsModule from '../errors'; import { eventBus, events } from '../events'; import type { DisplayConfig, Organization } from '../resources/internal'; import { BaseResource, Client, Environment, SignIn, SignUp } from '../resources/internal'; @@ -159,6 +162,183 @@ describe('Clerk singleton', () => { }); }); + describe('load retry behavior', () => { + let originalMountComponentRenderer: typeof Clerk.mountComponentRenderer; + + const createMockAuthService = () => ({ + decorateUrlWithDevBrowserToken: vi.fn((url: URL) => url), + getSessionCookie: vi.fn(() => null), + handleUnauthenticatedDevBrowser: vi.fn(() => Promise.resolve()), + isSignedOut: vi.fn(() => false), + setClientUatCookieForDevelopmentInstances: vi.fn(), + startPollingForToken: vi.fn(), + stopPollingForToken: vi.fn(), + }); + + const createMockComponentControls = () => { + const componentInstance = { + mountImpersonationFab: vi.fn(), + updateProps: vi.fn(), + }; + + return { + ensureMounted: vi.fn().mockResolvedValue(componentInstance), + prioritizedOn: vi.fn(), + }; + }; + + beforeEach(() => { + originalMountComponentRenderer = Clerk.mountComponentRenderer; + }); + + afterEach(() => { + Clerk.mountComponentRenderer = originalMountComponentRenderer; + vi.useRealTimers(); + }); + + it('retries once when dev browser authentication is lost', async () => { + vi.useFakeTimers(); + + const mockAuthService = createMockAuthService(); + const authCreateSpy = vi + .spyOn(AuthCookieService, 'create') + .mockResolvedValue(mockAuthService as unknown as AuthCookieService); + + const componentControls = createMockComponentControls(); + const devBrowserError = Object.assign(new Error('dev browser unauthenticated'), { + errors: [{ code: 'dev_browser_unauthenticated' }], + status: 401, + }); + + const mountSpy = vi + .fn>() + .mockImplementationOnce(() => { + throw devBrowserError; + }) + .mockReturnValue(componentControls); + + Clerk.mountComponentRenderer = mountSpy; + mockClientFetch.mockClear(); + + const sut = new Clerk(productionPublishableKey); + + try { + const loadPromise = sut.load(); + + await vi.runAllTimersAsync(); + await loadPromise; + } finally { + authCreateSpy.mockRestore(); + } + + expect(mountSpy).toHaveBeenCalledTimes(2); + expect(mockAuthService.handleUnauthenticatedDevBrowser).toHaveBeenCalledTimes(1); + expect(mockClientFetch).toHaveBeenCalledTimes(2); + }); + + it('surfaces network errors after exhausting retries', async () => { + vi.useFakeTimers(); + + const mockAuthService = createMockAuthService(); + const authCreateSpy = vi + .spyOn(AuthCookieService, 'create') + .mockResolvedValue(mockAuthService as unknown as AuthCookieService); + + const networkError = new ClerkRuntimeError('Network failure', { code: 'network_error' }); + const mountSpy = vi.fn>().mockImplementation(() => { + throw networkError; + }); + + Clerk.mountComponentRenderer = mountSpy; + mockClientFetch.mockClear(); + + const errorSpy = vi.spyOn(errorsModule, 'clerkErrorInitFailed'); + const sut = new Clerk(productionPublishableKey); + + try { + const loadPromise = sut.load(); + + // Attach rejection handler before advancing timers to avoid unhandled rejection + const expectation = expect(loadPromise).rejects.toThrow(/Something went wrong initializing Clerk/); + await vi.runAllTimersAsync(); + + const err = await loadPromise.catch(e => e); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/Something went wrong initializing Clerk/); + const cause = (err as Error).cause as any; + expect(cause).toBeDefined(); + expect(cause.code).toBe('network_error'); + expect(cause.clerkRuntimeError).toBe(true); + + await expectation; + + expect(mountSpy).toHaveBeenCalledTimes(2); + expect(mockClientFetch).toHaveBeenCalledTimes(2); + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy).toHaveBeenLastCalledWith(networkError); + expect(mockAuthService.handleUnauthenticatedDevBrowser).not.toHaveBeenCalled(); + } finally { + authCreateSpy.mockRestore(); + errorSpy.mockRestore(); + } + }); + + it('retries when environment fetch fails and no cached snapshot exists', async () => { + vi.useFakeTimers(); + + const mockAuthService = createMockAuthService(); + const authCreateSpy = vi + .spyOn(AuthCookieService, 'create') + .mockResolvedValue(mockAuthService as unknown as AuthCookieService); + + const localStorageSpy = vi.spyOn(localStorageModule.SafeLocalStorage, 'getItem').mockReturnValue(null); + + mockEnvironmentFetch.mockReset().mockRejectedValueOnce(new Error('Network error')).mockResolvedValue({}); + + const componentControls = createMockComponentControls(); + const mountSpy = vi.fn>().mockReturnValue(componentControls); + Clerk.mountComponentRenderer = mountSpy; + mockClientFetch.mockClear(); + + const sut = new Clerk(productionPublishableKey); + + try { + const loadPromise = sut.load(); + await vi.runAllTimersAsync(); + await loadPromise; + } finally { + authCreateSpy.mockRestore(); + localStorageSpy.mockRestore(); + } + + expect(mockEnvironmentFetch).toHaveBeenCalledTimes(2); + expect(mockClientFetch).toHaveBeenCalledTimes(2); + }); + + it('uses cached environment when fetch fails but cache exists', async () => { + const cachedEnv = { + userSettings: { signUp: { captcha_enabled: false } }, + displayConfig: {}, + }; + const localStorageSpy = vi.spyOn(localStorageModule.SafeLocalStorage, 'getItem').mockReturnValue(cachedEnv); + + mockEnvironmentFetch.mockReset().mockRejectedValue(new Error('Network error')); + mockClientFetch.mockClear().mockResolvedValue({ signedInSessions: [] }); + + const sut = new Clerk(productionPublishableKey); + + try { + await sut.load(); + } finally { + localStorageSpy.mockRestore(); + } + + // Should only fetch once since cache is used as fallback (no retry needed) + expect(mockEnvironmentFetch).toHaveBeenCalledTimes(1); + expect(mockClientFetch).toHaveBeenCalledTimes(1); + }); + }); + describe('__internal_oauthTransport', () => { it('defaults to null with no getter access errors', () => { const sut = new Clerk(productionPublishableKey); diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 2264ca42f58..7234a2039e0 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -169,6 +169,7 @@ import { } from '../utils'; import { CLERK_ENVIRONMENT_STORAGE_ENTRY, SafeLocalStorage } from '../utils/localStorage'; import { memoizeListenerCallback } from '../utils/memoizeStateListenerCallback'; +import { withRetry } from '../utils/retry'; import { AuthCookieService } from './auth/AuthCookieService'; import { CaptchaHeartbeat } from './auth/CaptchaHeartbeat'; import { @@ -194,7 +195,6 @@ import { BaseResource, Client, Environment, Organization, Waitlist } from './res import { State } from './state'; type SetActiveHook = (intent?: 'sign-out') => void | Promise; - declare global { interface Window { Clerk?: Clerk; @@ -3153,97 +3153,115 @@ export class Clerk implements ClerkInterface { let initializationDegradedCounter = 0; - let retries = 0; - while (retries < 2) { - retries++; + const initializeClerk = async (): Promise => { + const initEnvironmentPromise = Environment.getInstance() + .fetch({ touch: shouldTouchEnv }) + .then(res => this.updateEnvironment(res)) + .catch(err => { + if (is4xxError(err)) { + throw err; + } - try { - const initEnvironmentPromise = Environment.getInstance() - .fetch({ touch: shouldTouchEnv }) - .then(res => this.updateEnvironment(res)) - .catch(() => { - ++initializationDegradedCounter; - const environmentSnapshot = SafeLocalStorage.getItem( - CLERK_ENVIRONMENT_STORAGE_ENTRY, - null, - ); + ++initializationDegradedCounter; + const environmentSnapshot = SafeLocalStorage.getItem( + CLERK_ENVIRONMENT_STORAGE_ENTRY, + null, + ); - if (environmentSnapshot) { - this.updateEnvironment(new Environment(environmentSnapshot)); + if (environmentSnapshot) { + this.updateEnvironment(new Environment(environmentSnapshot)); + } else { + throw new ClerkRuntimeError('Failed to fetch environment', { code: 'network_error' }); + } + }); + + const initClient = async () => { + return Client.getOrCreateInstance() + .fetch() + .then(res => this.updateClient(res)) + .catch(async e => { + if (is4xxError(e)) { + throw e; } - }); - const initClient = async () => { - return Client.getOrCreateInstance() - .fetch() - .then(res => this.updateClient(res)) - .catch(async e => { - /** - * Only handle non 4xx errors, like 5xx errors and network errors. - */ - if (is4xxError(e)) { - // bubble it up - throw e; - } + ++initializationDegradedCounter; - ++initializationDegradedCounter; + const jwtInCookie = this.#authService?.getSessionCookie(); + const localClient = createClientFromJwt(jwtInCookie); - const jwtInCookie = this.#authService?.getSessionCookie(); - const localClient = createClientFromJwt(jwtInCookie); + this.updateClient(localClient); - this.updateClient(localClient); + this.#authService?.stopPollingForToken(); - /** - * In most scenarios we want the poller to stop while we are fetching a fresh token during an outage. - * We want to avoid having the below `getToken()` retrying at the same time as the poller. - */ - this.#authService?.stopPollingForToken(); + await this.session + ?.getToken({ skipCache: true }) + .catch(() => null) + .finally(() => { + this.#authService?.startPollingForToken(); + }); - // Attempt to grab a fresh token - await this.session - ?.getToken({ skipCache: true }) - // If the token fetch fails, let Clerk be marked as loaded and leave it up to the poller. - .catch(() => null) - .finally(() => { - this.#authService?.startPollingForToken(); - }); + return null; + }); + }; - // Allows for Clerk to be marked as loaded with the client and session created from the JWT. - return null; - }); - }; + const initComponents = () => { + void this.#clerkUI?.then(ui => ui.ensureMounted()); + }; - const [, clientResult] = await allSettled([initEnvironmentPromise, initClient()]); - if (clientResult.status === 'rejected') { - const e = clientResult.reason; + const [envResult, clientResult] = await allSettled([initEnvironmentPromise, initClient()]); - if (isError(e, 'requires_captcha')) { - await initClient(); - } else { - throw e; - } - } + if (envResult.status === 'rejected') { + throw envResult.reason; + } - this.#authService?.setClientUatCookieForDevelopmentInstances(); + if (clientResult.status === 'rejected') { + const e = clientResult.reason; - if (await this.#redirectFAPIInitiatedFlow()) { - return; - } - break; - } catch (err) { - if (isError(err, 'dev_browser_unauthenticated')) { - await this.#authService.handleUnauthenticatedDevBrowser(); - } else if (!isValidBrowserOnline()) { - console.warn(err); - return; + if (isError(e, 'requires_captcha')) { + initComponents(); + await initClient(); } else { - throw err; + throw e; } } - if (retries >= 2) { - clerkErrorInitFailed(); + this.#authService?.setClientUatCookieForDevelopmentInstances(); + + if (await this.#redirectFAPIInitiatedFlow()) { + return; + } + + initComponents(); + }; + + try { + await withRetry(initializeClerk, { + jitter: true, + maxAttempts: 2, + shouldRetry: async error => { + if (!isValidBrowserOnline()) { + console.warn(error); + return false; + } + + const isDevBrowserUnauthenticated = isError(error as any, 'dev_browser_unauthenticated'); + const isNetworkError = isClerkRuntimeError(error) && error.code === 'network_error'; + + if (isDevBrowserUnauthenticated && this.#authService) { + await this.#authService.handleUnauthenticatedDevBrowser(); + return true; + } + + return isNetworkError; + }, + }); + } catch (err) { + if (!isValidBrowserOnline()) { + console.warn(err); + return; } + + clerkErrorInitFailed(err); } this.#captchaHeartbeat = new CaptchaHeartbeat(this); diff --git a/packages/clerk-js/src/utils/__tests__/retry.test.ts b/packages/clerk-js/src/utils/__tests__/retry.test.ts new file mode 100644 index 00000000000..974f0fbfbd0 --- /dev/null +++ b/packages/clerk-js/src/utils/__tests__/retry.test.ts @@ -0,0 +1,205 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { withRetry } from '../retry'; + +describe('withRetry', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('throws when maxAttempts is zero', async () => { + const fn = vi.fn().mockResolvedValue('success'); + + await expect( + withRetry(fn, { + maxAttempts: 0, + shouldRetry: () => true, + }), + ).rejects.toThrow('withRetry: maxAttempts must be a positive integer'); + + expect(fn).not.toHaveBeenCalled(); + }); + + it('throws when maxAttempts is negative', async () => { + const fn = vi.fn().mockResolvedValue('success'); + + await expect( + withRetry(fn, { + maxAttempts: -1, + shouldRetry: () => true, + }), + ).rejects.toThrow('withRetry: maxAttempts must be a positive integer'); + + expect(fn).not.toHaveBeenCalled(); + }); + + it('throws when maxAttempts is not an integer', async () => { + const fn = vi.fn().mockResolvedValue('success'); + + await expect( + withRetry(fn, { + maxAttempts: 1.5, + shouldRetry: () => true, + }), + ).rejects.toThrow('withRetry: maxAttempts must be a positive integer'); + + expect(fn).not.toHaveBeenCalled(); + }); + + it('returns result on first successful attempt', async () => { + const fn = vi.fn().mockResolvedValue('success'); + + const promise = withRetry(fn, { + maxAttempts: 3, + shouldRetry: () => true, + }); + + await vi.runAllTimersAsync(); + const result = await promise; + + expect(result).toBe('success'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('retries on failure when shouldRetry returns true', async () => { + const fn = vi + .fn() + .mockRejectedValueOnce(new Error('first failure')) + .mockRejectedValueOnce(new Error('second failure')) + .mockResolvedValue('success'); + + const promise = withRetry(fn, { + maxAttempts: 3, + shouldRetry: () => true, + }); + + await vi.runAllTimersAsync(); + const result = await promise; + + expect(result).toBe('success'); + expect(fn).toHaveBeenCalledTimes(3); + }); + + it('does not retry when shouldRetry returns false', async () => { + const error = new Error('failure'); + const fn = vi.fn().mockRejectedValue(error); + + const promise = withRetry(fn, { + maxAttempts: 3, + shouldRetry: () => false, + }); + + // Attach rejection handler before advancing timers to avoid unhandled rejection + const expectation = expect(promise).rejects.toThrow('failure'); + await vi.runAllTimersAsync(); + await expectation; + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('throws after exhausting all retry attempts', async () => { + const error = new Error('persistent failure'); + const fn = vi.fn().mockRejectedValue(error); + + const promise = withRetry(fn, { + maxAttempts: 3, + shouldRetry: () => true, + }); + + // Attach rejection handler before advancing timers to avoid unhandled rejection + const expectation = expect(promise).rejects.toThrow('persistent failure'); + await vi.runAllTimersAsync(); + await expectation; + expect(fn).toHaveBeenCalledTimes(3); + }); + + it('applies exponential backoff between retries', async () => { + const fn = vi + .fn() + .mockRejectedValueOnce(new Error('first failure')) + .mockRejectedValueOnce(new Error('second failure')) + .mockResolvedValue('success'); + + const promise = withRetry(fn, { + jitter: false, + maxAttempts: 3, + shouldRetry: () => true, + }); + + await vi.advanceTimersByTimeAsync(0); + expect(fn).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(2000); + expect(fn).toHaveBeenCalledTimes(2); + + await vi.advanceTimersByTimeAsync(4000); + expect(fn).toHaveBeenCalledTimes(3); + + const result = await promise; + expect(result).toBe('success'); + }); + + it('supports async shouldRetry predicate', async () => { + const error = new Error('failure'); + const fn = vi.fn().mockRejectedValue(error); + + const shouldRetry = vi.fn().mockResolvedValue(false); + + const promise = withRetry(fn, { + maxAttempts: 3, + shouldRetry, + }); + + // Attach rejection handler before advancing timers to avoid unhandled rejection + const expectation = expect(promise).rejects.toThrow('failure'); + await vi.runAllTimersAsync(); + await expectation; + + expect(shouldRetry).toHaveBeenCalledTimes(1); + expect(shouldRetry).toHaveBeenCalledWith(error); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('calls shouldRetry with the error for each attempt', async () => { + const errors = [new Error('first'), new Error('second')]; + const fn = vi.fn().mockRejectedValueOnce(errors[0]).mockRejectedValueOnce(errors[1]).mockResolvedValue('success'); + + const shouldRetry = vi.fn().mockReturnValue(true); + + const promise = withRetry(fn, { + maxAttempts: 3, + shouldRetry, + }); + + await vi.runAllTimersAsync(); + const result = await promise; + + expect(result).toBe('success'); + expect(shouldRetry).toHaveBeenCalledTimes(2); + expect(shouldRetry).toHaveBeenNthCalledWith(1, errors[0]); + expect(shouldRetry).toHaveBeenNthCalledWith(2, errors[1]); + }); + + it('applies jitter to backoff by default', async () => { + const fn = vi.fn().mockRejectedValue(new Error('failure')); + + const promise = withRetry(fn, { + maxAttempts: 2, + shouldRetry: () => true, + }); + + // Attach rejection handler before advancing timers to avoid unhandled rejection + const expectation = expect(promise).rejects.toThrow('failure'); + + await vi.advanceTimersByTimeAsync(0); + expect(fn).toHaveBeenCalledTimes(1); + + await vi.runAllTimersAsync(); + expect(fn).toHaveBeenCalledTimes(2); + + await expectation; + }); +}); diff --git a/packages/clerk-js/src/utils/retry.ts b/packages/clerk-js/src/utils/retry.ts new file mode 100644 index 00000000000..d9e32545055 --- /dev/null +++ b/packages/clerk-js/src/utils/retry.ts @@ -0,0 +1,50 @@ +export interface RetryOptions { + jitter?: boolean; + maxAttempts: number; + shouldRetry: (error: unknown) => boolean | Promise; +} + +const sleep = (ms: number): Promise => new Promise(resolve => setTimeout(resolve, ms)); + +const calculateBackoff = (attempt: number, jitter: boolean): number => { + const baseDelay = Math.pow(2, attempt + 1) * 1_000; + + if (!jitter) { + return baseDelay; + } + + return baseDelay * (0.5 + Math.random() * 0.5); +}; + +export async function withRetry(fn: () => Promise, options: RetryOptions): Promise { + if (!Number.isInteger(options.maxAttempts) || options.maxAttempts <= 0) { + throw new Error('withRetry: maxAttempts must be a positive integer'); + } + + let lastError: unknown; + + for (let attempt = 0; attempt < options.maxAttempts; attempt++) { + try { + return await fn(); + } catch (error) { + lastError = error; + + const shouldRetry = await Promise.resolve(options.shouldRetry(error)); + + if (!shouldRetry) { + throw error; + } + + const isLastAttempt = attempt === options.maxAttempts - 1; + + if (isLastAttempt) { + throw error; + } + + const backoffMs = calculateBackoff(attempt, options.jitter ?? true); + await sleep(backoffMs); + } + } + + throw lastError; +} diff --git a/packages/clerk-js/tsconfig.json b/packages/clerk-js/tsconfig.json index 87bcab5f15f..3f9cbc6205e 100644 --- a/packages/clerk-js/tsconfig.json +++ b/packages/clerk-js/tsconfig.json @@ -7,7 +7,7 @@ "isolatedModules": true, "jsx": "react-jsx", "jsxImportSource": "@emotion/react", - "lib": ["dom", "dom.iterable", "es2020", "es2021.intl"], + "lib": ["dom", "dom.iterable", "es2020", "es2021.intl", "es2022.error"], "module": "esnext", "moduleResolution": "Bundler", "noEmit": true, diff --git a/packages/shared/src/internal/clerk-js/errors.ts b/packages/shared/src/internal/clerk-js/errors.ts index 77345b56d3e..8ffdc7c5a65 100644 --- a/packages/shared/src/internal/clerk-js/errors.ts +++ b/packages/shared/src/internal/clerk-js/errors.ts @@ -61,8 +61,8 @@ export function clerkNetworkError(url: string, e: Error): never { /** * */ -export function clerkErrorInitFailed(): never { - throw new Error(`${errorPrefix} Something went wrong initializing Clerk.`); +export function clerkErrorInitFailed(cause?: unknown): never { + throw new Error(`${errorPrefix} Something went wrong initializing Clerk.`, { cause }); } /**