From 2844498681ee5d47c1b8e28c7bd64bef2895600f Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 11 Jul 2025 12:41:52 -0500 Subject: [PATCH 1/5] feat(shared): Detect clerk-js loading failure --- .changeset/cold-trees-hammer.md | 5 + integration/tests/resiliency.test.ts | 3 +- .../src/__tests__/loadClerkJsScript.test.ts | 108 +++++++++++--- packages/shared/src/loadClerkJsScript.ts | 141 +++++++++++++++--- packages/shared/src/telemetry/collector.ts | 6 +- 5 files changed, 220 insertions(+), 43 deletions(-) create mode 100644 .changeset/cold-trees-hammer.md diff --git a/.changeset/cold-trees-hammer.md b/.changeset/cold-trees-hammer.md new file mode 100644 index 00000000000..d2b553c5498 --- /dev/null +++ b/.changeset/cold-trees-hammer.md @@ -0,0 +1,5 @@ +--- +'@clerk/shared': minor +--- + +Add timeout-based mechanism to detect when clerk-js fails to load and create a minimal fallback Clerk instance to trigger error components. \ No newline at end of file diff --git a/integration/tests/resiliency.test.ts b/integration/tests/resiliency.test.ts index 30efae92ba8..b4d2505ae50 100644 --- a/integration/tests/resiliency.test.ts +++ b/integration/tests/resiliency.test.ts @@ -184,8 +184,9 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('resilienc await expect(page.getByText('Clerk is NOT loaded', { exact: true })).toBeVisible(); // Wait for loading to complete and verify final state + // Account for the new 15-second script loading timeout plus buffer for UI updates await expect(page.getByText('Status: error', { exact: true })).toBeVisible({ - timeout: 10_000, + timeout: 16_000, }); await expect(page.getByText('Clerk is out', { exact: true })).toBeVisible(); await expect(page.getByText('Clerk is ready or degraded (loaded)')).toBeHidden(); diff --git a/packages/shared/src/__tests__/loadClerkJsScript.test.ts b/packages/shared/src/__tests__/loadClerkJsScript.test.ts index 2e259e10292..bc8b58824ae 100644 --- a/packages/shared/src/__tests__/loadClerkJsScript.test.ts +++ b/packages/shared/src/__tests__/loadClerkJsScript.test.ts @@ -12,6 +12,12 @@ jest.mock('../loadScript'); setClerkJsLoadingErrorPackageName('@clerk/clerk-react'); const jsPackageMajorVersion = getMajorVersion(JS_PACKAGE_VERSION); +const mockClerk = { + status: 'ready', + loaded: true, + load: jest.fn(), +}; + describe('loadClerkJsScript(options)', () => { const mockPublishableKey = 'pk_test_Zm9vLWJhci0xMy5jbGVyay5hY2NvdW50cy5kZXYk'; @@ -19,17 +25,43 @@ describe('loadClerkJsScript(options)', () => { jest.clearAllMocks(); (loadScript as jest.Mock).mockResolvedValue(undefined); document.querySelector = jest.fn().mockReturnValue(null); + + (window as any).Clerk = undefined; + + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); }); test('throws error when publishableKey is missing', async () => { - await expect(() => loadClerkJsScript({} as any)).rejects.toThrow( + await expect(loadClerkJsScript({} as any)).rejects.toThrow( '@clerk/clerk-react: Missing publishableKey. You can get your key at https://dashboard.clerk.com/last-active?path=api-keys.', ); }); - test('loads script when no existing script is found', async () => { - await loadClerkJsScript({ publishableKey: mockPublishableKey }); + test('returns null immediately when Clerk is already loaded', async () => { + (window as any).Clerk = mockClerk; + const result = await loadClerkJsScript({ publishableKey: mockPublishableKey }); + expect(result).toBeNull(); + expect(loadScript).not.toHaveBeenCalled(); + }); + + test('loads script and waits for Clerk to be available', async () => { + const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); + + // Simulate Clerk becoming available after 250ms + setTimeout(() => { + (window as any).Clerk = mockClerk; + }, 250); + + // Advance timers to allow polling to detect Clerk + jest.advanceTimersByTime(300); + + const result = await loadPromise; + expect(result).toBeNull(); expect(loadScript).toHaveBeenCalledWith( expect.stringContaining( `https://foo-bar-13.clerk.accounts.dev/npm/@clerk/clerk-js@${jsPackageMajorVersion}/dist/clerk.browser.js`, @@ -42,34 +74,74 @@ describe('loadClerkJsScript(options)', () => { ); }); - test('uses existing script when found', async () => { - const mockExistingScript = document.createElement('script'); - document.querySelector = jest.fn().mockReturnValue(mockExistingScript); + test('times out and rejects when Clerk does not load', async () => { + let rejectedWith: any; - const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); - mockExistingScript.dispatchEvent(new Event('load')); + const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey, scriptLoadTimeout: 1000 }); - await expect(loadPromise).resolves.toBe(mockExistingScript); - expect(loadScript).not.toHaveBeenCalled(); + try { + jest.advanceTimersByTime(1000); + await loadPromise; + } catch (error) { + rejectedWith = error; + } + + expect(rejectedWith).toBeInstanceOf(Error); + expect(rejectedWith.message).toBe('Clerk: Failed to load Clerk'); + expect((window as any).Clerk).toBeUndefined(); }); - test('rejects when existing script fails to load', async () => { + test('waits for existing script with timeout', async () => { const mockExistingScript = document.createElement('script'); document.querySelector = jest.fn().mockReturnValue(mockExistingScript); const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); - mockExistingScript.dispatchEvent(new Event('error')); - await expect(loadPromise).rejects.toBe('Clerk: Failed to load Clerk'); + // Simulate Clerk becoming available after 250ms + setTimeout(() => { + (window as any).Clerk = mockClerk; + }, 250); + + // Advance timers to allow polling to detect Clerk + jest.advanceTimersByTime(300); + + const result = await loadPromise; + expect(result).toBeNull(); expect(loadScript).not.toHaveBeenCalled(); }); - test('throws error when loadScript fails', async () => { - (loadScript as jest.Mock).mockRejectedValue(new Error('Script load failed')); + test('handles race condition when Clerk loads just as timeout fires', async () => { + const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey, scriptLoadTimeout: 1000 }); - await expect(loadClerkJsScript({ publishableKey: mockPublishableKey })).rejects.toThrow( - 'Clerk: Failed to load Clerk', - ); + setTimeout(() => { + (window as any).Clerk = mockClerk; + }, 999); + + jest.advanceTimersByTime(1000); + + const result = await loadPromise; + expect(result).toBeNull(); + expect((window as any).Clerk).toBe(mockClerk); + }); + + test('validates Clerk is properly loaded with required methods', async () => { + const loadPromise = loadClerkJsScript({ publishableKey: mockPublishableKey }); + + setTimeout(() => { + (window as any).Clerk = { status: 'ready' }; + }, 100); + + jest.advanceTimersByTime(15000); + + try { + await loadPromise; + fail('Should have thrown error'); + } catch (error) { + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe('Clerk: Failed to load Clerk'); + // The malformed Clerk object should still be there since it was set + expect((window as any).Clerk).toEqual({ status: 'ready' }); + } }); }); diff --git a/packages/shared/src/loadClerkJsScript.ts b/packages/shared/src/loadClerkJsScript.ts index 97647e3f0bd..f9edff97eb4 100644 --- a/packages/shared/src/loadClerkJsScript.ts +++ b/packages/shared/src/loadClerkJsScript.ts @@ -16,8 +16,11 @@ const errorThrower = buildErrorThrower({ packageName: '@clerk/shared' }); /** * Sets the package name for error messages during ClerkJS script loading. * + * @param packageName - The name of the package to use in error messages (e.g., '@clerk/clerk-react'). * @example + * ```typescript * setClerkJsLoadingErrorPackageName('@clerk/clerk-react'); + * ``` */ export function setClerkJsLoadingErrorPackageName(packageName: string) { errorThrower.setPackageName({ packageName }); @@ -32,42 +35,125 @@ type LoadClerkJsScriptOptions = Without & { proxyUrl?: string; domain?: string; nonce?: string; + /** + * Timeout in milliseconds to wait for clerk-js to load before considering it failed. + * + * @default 15000 (15 seconds) + */ + scriptLoadTimeout?: number; }; /** - * Hotloads the Clerk JS script. + * Validates that window.Clerk exists and is properly initialized. + * This ensures we don't have false positives where the script loads but Clerk is malformed. * - * Checks for an existing Clerk JS script. If found, it returns a promise - * that resolves when the script loads. If not found, it uses the provided options to - * build the Clerk JS script URL and load the script. + * @returns `true` if window.Clerk exists and has the expected structure with a load method. + */ +function isClerkProperlyLoaded(): boolean { + if (typeof window === 'undefined' || !(window as any).Clerk) { + return false; + } + + // Basic validation that window.Clerk has the expected structure + const clerk = (window as any).Clerk; + return typeof clerk === 'object' && typeof clerk.load === 'function'; +} + +/** + * Waits for Clerk to be properly loaded with a timeout mechanism. + * Uses polling to check if Clerk becomes available within the specified timeout. + * + * @param timeoutMs - Maximum time to wait in milliseconds. + * @returns Promise that resolves with null if Clerk loads successfully, or rejects with an error if timeout is reached. + */ +function waitForClerkWithTimeout(timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + let resolved = false; + + const cleanup = (timeoutId: ReturnType, pollInterval: ReturnType) => { + clearTimeout(timeoutId); + clearInterval(pollInterval); + }; + + const checkAndResolve = () => { + if (resolved) return; + + if (isClerkProperlyLoaded()) { + resolved = true; + cleanup(timeoutId, pollInterval); + resolve(null); + } + }; + + const handleTimeout = () => { + if (resolved) return; + + resolved = true; + cleanup(timeoutId, pollInterval); + + if (!isClerkProperlyLoaded()) { + reject(new Error(FAILED_TO_LOAD_ERROR)); + } else { + resolve(null); + } + }; + + const timeoutId = setTimeout(handleTimeout, timeoutMs); + + checkAndResolve(); + + const pollInterval = setInterval(() => { + if (resolved) { + clearInterval(pollInterval); + return; + } + checkAndResolve(); + }, 100); + }); +} + +/** + * Hotloads the Clerk JS script with robust failure detection. + * + * Uses a timeout-based approach to ensure absolute certainty about load success/failure. + * If the script fails to load within the timeout period, or loads but doesn't create + * a proper Clerk instance, the promise rejects with an error. * * @param opts - The options used to build the Clerk JS script URL and load the script. * Must include a `publishableKey` if no existing script is found. + * @returns Promise that resolves with null if Clerk loads successfully, or rejects with an error. * * @example - * loadClerkJsScript({ publishableKey: 'pk_' }); + * ```typescript + * try { + * await loadClerkJsScript({ publishableKey: 'pk_test_...' }); + * console.log('Clerk loaded successfully'); + * } catch (error) { + * console.error('Failed to load Clerk:', error.message); + * } + * ``` */ -const loadClerkJsScript = async (opts?: LoadClerkJsScriptOptions) => { +const loadClerkJsScript = async (opts?: LoadClerkJsScriptOptions): Promise => { + const timeout = opts?.scriptLoadTimeout ?? 15000; + + if (isClerkProperlyLoaded()) { + return null; + } + const existingScript = document.querySelector('script[data-clerk-js-script]'); if (existingScript) { - return new Promise((resolve, reject) => { - existingScript.addEventListener('load', () => { - resolve(existingScript); - }); - - existingScript.addEventListener('error', () => { - reject(FAILED_TO_LOAD_ERROR); - }); - }); + return waitForClerkWithTimeout(timeout); } if (!opts?.publishableKey) { errorThrower.throwMissingPublishableKeyError(); - return; + return null; } - return loadScript(clerkJsScriptUrl(opts), { + const loadPromise = waitForClerkWithTimeout(timeout); + + loadScript(clerkJsScriptUrl(opts), { async: true, crossOrigin: 'anonymous', nonce: opts.nonce, @@ -75,15 +161,21 @@ const loadClerkJsScript = async (opts?: LoadClerkJsScriptOptions) => { }).catch(() => { throw new Error(FAILED_TO_LOAD_ERROR); }); + + return loadPromise; }; /** - * Generates a Clerk JS script URL. + * Generates a Clerk JS script URL based on the provided options. * * @param opts - The options to use when building the Clerk JS script URL. + * @returns The complete URL to the Clerk JS script. * * @example - * clerkJsScriptUrl({ publishableKey: 'pk_' }); + * ```typescript + * const url = clerkJsScriptUrl({ publishableKey: 'pk_test_...' }); + * // Returns: "https://example.clerk.accounts.dev/npm/@clerk/clerk-js@5/dist/clerk.browser.js" + * ``` */ const clerkJsScriptUrl = (opts: LoadClerkJsScriptOptions) => { const { clerkJSUrl, clerkJSVariant, clerkJSVersion, proxyUrl, domain, publishableKey } = opts; @@ -107,7 +199,10 @@ const clerkJsScriptUrl = (opts: LoadClerkJsScriptOptions) => { }; /** - * Builds an object of Clerk JS script attributes. + * Builds an object of Clerk JS script attributes based on the provided options. + * + * @param options - The options containing the values for script attributes. + * @returns An object containing data attributes to be applied to the script element. */ const buildClerkJsScriptAttributes = (options: LoadClerkJsScriptOptions) => { const obj: Record = {}; @@ -131,6 +226,12 @@ const buildClerkJsScriptAttributes = (options: LoadClerkJsScriptOptions) => { return obj; }; +/** + * Returns a function that applies Clerk JS script attributes to a script element. + * + * @param options - The options containing the values for script attributes. + * @returns A function that accepts a script element and applies the attributes to it. + */ const applyClerkJsScriptAttributes = (options: LoadClerkJsScriptOptions) => (script: HTMLScriptElement) => { const attributes = buildClerkJsScriptAttributes(options); for (const attribute in attributes) { diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index a9af381f151..522c47752a0 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -236,10 +236,8 @@ export class TelemetryCollector implements TelemetryCollectorInterface { version: this.#metadata.sdkVersion, }; - // @ts-expect-error -- The global window.Clerk type is declared in clerk-js, but we can't rely on that here - if (typeof window !== 'undefined' && window.Clerk) { - // @ts-expect-error -- The global window.Clerk type is declared in clerk-js, but we can't rely on that here - sdkMetadata = { ...sdkMetadata, ...window.Clerk.constructor.sdkMetadata }; + if (typeof window !== 'undefined' && (window as any).Clerk) { + sdkMetadata = { ...sdkMetadata, ...(window as any).Clerk.constructor.sdkMetadata }; } return sdkMetadata; From 3a09d9b77ee7883ac29cf680598229137c045b08 Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 11 Jul 2025 16:34:46 -0500 Subject: [PATCH 2/5] fix any cast --- packages/shared/src/telemetry/collector.ts | 52 ++++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 522c47752a0..f9d5ab170cc 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -22,6 +22,40 @@ import { isTruthy } from '../underscore'; import { TelemetryEventThrottler } from './throttler'; import type { TelemetryCollectorOptions } from './types'; +/** + * Interface describing the expected structure of window.Clerk + */ +interface WindowClerk { + constructor: { + sdkMetadata?: { + name?: string; + version?: string; + }; + }; +} + +/** + * Extend the Window interface to include the Clerk property + */ +declare global { + interface Window { + Clerk?: WindowClerk; + } +} + +/** + * Type guard to check if window.Clerk exists and has the expected structure + */ +function isWindowClerkWithMetadata(clerk: unknown): clerk is WindowClerk { + return ( + typeof clerk === 'object' && + clerk !== null && + 'constructor' in clerk && + typeof clerk.constructor === 'object' && + clerk.constructor !== null + ); +} + type TelemetryCollectorConfig = Pick< TelemetryCollectorOptions, 'samplingRate' | 'disabled' | 'debug' | 'maxBufferSize' @@ -231,13 +265,25 @@ export class TelemetryCollector implements TelemetryCollectorInterface { * This is necessary because the sdkMetadata can be set by the host SDK after the TelemetryCollector is instantiated. */ #getSDKMetadata() { - let sdkMetadata = { + const sdkMetadata = { name: this.#metadata.sdk, version: this.#metadata.sdkVersion, }; - if (typeof window !== 'undefined' && (window as any).Clerk) { - sdkMetadata = { ...sdkMetadata, ...(window as any).Clerk.constructor.sdkMetadata }; + if (typeof window !== 'undefined' && window.Clerk) { + const windowClerk = window.Clerk; + + if (isWindowClerkWithMetadata(windowClerk) && windowClerk.constructor.sdkMetadata) { + const { name, version } = windowClerk.constructor.sdkMetadata; + + // Only update properties if they exist to avoid overwriting with undefined + if (name !== undefined) { + sdkMetadata.name = name; + } + if (version !== undefined) { + sdkMetadata.version = version; + } + } } return sdkMetadata; From 82503980e3afd297124f7303782d0226994cc2ea Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 11 Jul 2025 16:35:15 -0500 Subject: [PATCH 3/5] wip --- packages/shared/src/telemetry/collector.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index f9d5ab170cc..7dca449cd1e 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -272,10 +272,10 @@ export class TelemetryCollector implements TelemetryCollectorInterface { if (typeof window !== 'undefined' && window.Clerk) { const windowClerk = window.Clerk; - + if (isWindowClerkWithMetadata(windowClerk) && windowClerk.constructor.sdkMetadata) { const { name, version } = windowClerk.constructor.sdkMetadata; - + // Only update properties if they exist to avoid overwriting with undefined if (name !== undefined) { sdkMetadata.name = name; From 1c6439b9f661bbbabc735cb625bf2e4917e45d6e Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 21 Jul 2025 09:38:34 -0500 Subject: [PATCH 4/5] use type assertion --- packages/shared/src/telemetry/collector.ts | 54 ++++++++++------------ 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 7dca449cd1e..ba99f92b824 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -1,6 +1,6 @@ /** * The `TelemetryCollector` class handles collection of telemetry events from Clerk SDKs. Telemetry is opt-out and can be disabled by setting a CLERK_TELEMETRY_DISABLED environment variable. - * The `ClerkProvider` also accepts a `telemetry` prop that will be passed to the collector during initialization: + * The `ClerkProvider` also accepts a `telemetry` prop that will be passed to the collector during initialization:. * * ```jsx * @@ -8,10 +8,11 @@ * * ``` * - * For more information, please see the telemetry documentation page: https://clerk.com/docs/telemetry + * For more information, please see the telemetry documentation page: https://clerk.com/docs/telemetry. */ import type { InstanceType, + SDKMetadata, TelemetryCollector as TelemetryCollectorInterface, TelemetryEvent, TelemetryEventRaw, @@ -23,30 +24,21 @@ import { TelemetryEventThrottler } from './throttler'; import type { TelemetryCollectorOptions } from './types'; /** - * Interface describing the expected structure of window.Clerk + * Local interface for window.Clerk to avoid global type pollution. + * This is only used within this module and doesn't affect other packages. */ -interface WindowClerk { - constructor: { - sdkMetadata?: { - name?: string; - version?: string; +interface WindowWithClerk extends Window { + Clerk?: { + constructor?: { + sdkMetadata?: SDKMetadata; }; }; } /** - * Extend the Window interface to include the Clerk property + * Type guard to check if window.Clerk exists and has the expected structure. */ -declare global { - interface Window { - Clerk?: WindowClerk; - } -} - -/** - * Type guard to check if window.Clerk exists and has the expected structure - */ -function isWindowClerkWithMetadata(clerk: unknown): clerk is WindowClerk { +function isWindowClerkWithMetadata(clerk: unknown): clerk is { constructor: { sdkMetadata?: SDKMetadata } } { return ( typeof clerk === 'object' && clerk !== null && @@ -270,18 +262,22 @@ export class TelemetryCollector implements TelemetryCollectorInterface { version: this.#metadata.sdkVersion, }; - if (typeof window !== 'undefined' && window.Clerk) { - const windowClerk = window.Clerk; + if (typeof window !== 'undefined') { + const windowWithClerk = window as WindowWithClerk; - if (isWindowClerkWithMetadata(windowClerk) && windowClerk.constructor.sdkMetadata) { - const { name, version } = windowClerk.constructor.sdkMetadata; + if (windowWithClerk.Clerk) { + const windowClerk = windowWithClerk.Clerk; - // Only update properties if they exist to avoid overwriting with undefined - if (name !== undefined) { - sdkMetadata.name = name; - } - if (version !== undefined) { - sdkMetadata.version = version; + if (isWindowClerkWithMetadata(windowClerk) && windowClerk.constructor.sdkMetadata) { + const { name, version } = windowClerk.constructor.sdkMetadata; + + // Only update properties if they exist to avoid overwriting with undefined + if (name !== undefined) { + sdkMetadata.name = name; + } + if (version !== undefined) { + sdkMetadata.version = version; + } } } } From c6e01216b4420b0db1734c17a0a3b05ed71ffc61 Mon Sep 17 00:00:00 2001 From: Jacek Radko Date: Mon, 21 Jul 2025 09:39:56 -0500 Subject: [PATCH 5/5] Apply suggestion from @jacekradko --- .changeset/cold-trees-hammer.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/cold-trees-hammer.md b/.changeset/cold-trees-hammer.md index d2b553c5498..48dff84abcb 100644 --- a/.changeset/cold-trees-hammer.md +++ b/.changeset/cold-trees-hammer.md @@ -2,4 +2,4 @@ '@clerk/shared': minor --- -Add timeout-based mechanism to detect when clerk-js fails to load and create a minimal fallback Clerk instance to trigger error components. \ No newline at end of file +Add timeout-based mechanism to detect when clerk-js fails to load and set the status to `error` on isomorphicClerk \ No newline at end of file