diff --git a/.changeset/metal-bobcats-wash.md b/.changeset/metal-bobcats-wash.md new file mode 100644 index 00000000000..c9fdcc1ae58 --- /dev/null +++ b/.changeset/metal-bobcats-wash.md @@ -0,0 +1,27 @@ +--- +'@clerk/shared': major +--- +This new version introduces the following breaking changes: +- Introduced a new `retry` utility function to replace the deprecated `callWithRetry`. +- Removed the `callWithRetry` function and its associated tests. +- Renamed `runWithExponentialBackOff` to `retry` for consistency. + +Migration steps: +- Replace any usage of `callWithRetry` with the new `retry` function. +- Update import statements from: + ```typescript + import { callWithRetry } from '@clerk/shared/callWithRetry'; + ``` + to: + ```typescript + import { retry } from '@clerk/shared/retry'; + ``` +- Replace any usage of `runWithExponentialBackOff` with `retry`. +- Update import statements from: + ```typescript + import { runWithExponentialBackOff } from '@clerk/shared/utils'; + ``` + to: + ```typescript + import { retry } from '@clerk/shared/retry'; + ``` diff --git a/packages/backend/src/tokens/keys.ts b/packages/backend/src/tokens/keys.ts index f17feb902f6..5158fa9d6ea 100644 --- a/packages/backend/src/tokens/keys.ts +++ b/packages/backend/src/tokens/keys.ts @@ -13,7 +13,7 @@ import { } from '../errors'; import { runtime } from '../runtime'; import { joinPaths } from '../util/path'; -import { callWithRetry } from '../util/shared'; +import { retry } from '../util/shared'; type JsonWebKeyWithKid = JsonWebKey & { kid: string }; @@ -137,8 +137,8 @@ export async function loadClerkJWKFromRemote({ reason: TokenVerificationErrorReason.RemoteJWKFailedToLoad, }); } - const fetcher = () => fetchJWKSFromBAPI(apiUrl, secretKey, apiVersion); - const { keys } = await callWithRetry<{ keys: JsonWebKeyWithKid[] }>(fetcher); + const fetcher = () => fetchJWKSFromBAPI(apiUrl, secretKey, apiVersion) as Promise<{ keys: JsonWebKeyWithKid[] }>; + const { keys } = await retry(fetcher); if (!keys || !keys.length) { throw new TokenVerificationError({ diff --git a/packages/backend/src/util/shared.ts b/packages/backend/src/util/shared.ts index c5c941594ea..f588a1d57fa 100644 --- a/packages/backend/src/util/shared.ts +++ b/packages/backend/src/util/shared.ts @@ -1,5 +1,5 @@ export { addClerkPrefix, getScriptUrl, getClerkJsMajorVersionOrTag } from '@clerk/shared/url'; -export { callWithRetry } from '@clerk/shared/callWithRetry'; +export { retry } from '@clerk/shared/retry'; export { isDevelopmentFromSecretKey, isProductionFromSecretKey, @@ -10,8 +10,8 @@ export { export { deprecated, deprecatedProperty } from '@clerk/shared/deprecated'; import { buildErrorThrower } from '@clerk/shared/error'; +import { createDevOrStagingUrlCache } from '@clerk/shared/keys'; // TODO: replace packageName with `${PACKAGE_NAME}@${PACKAGE_VERSION}` from tsup.config.ts export const errorThrower = buildErrorThrower({ packageName: '@clerk/backend' }); -import { createDevOrStagingUrlCache } from '@clerk/shared/keys'; export const { isDevOrStagingUrl } = createDevOrStagingUrlCache(); diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index 82f669023d6..f996ccd8db4 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -1,6 +1,6 @@ import { isBrowserOnline } from '@clerk/shared/browser'; +import { retry } from '@clerk/shared/retry'; import { camelToSnake } from '@clerk/shared/underscore'; -import { runWithExponentialBackOff } from '@clerk/shared/utils'; import type { ClerkAPIErrorJSON, ClientJSON, InstanceType } from '@clerk/types'; import { buildEmailAddress as buildEmailAddressUtil, buildURL as buildUrlUtil, stringifyQueryParams } from '../utils'; @@ -235,9 +235,9 @@ export function createFapiClient(options: FapiClientOptions): FapiClient { response = // retry only on GET requests for safety overwrittenRequestMethod === 'GET' - ? await runWithExponentialBackOff(() => fetch(urlStr, fetchOpts), { - firstDelay: 500, - maxDelay: 3000, + ? await retry(() => fetch(urlStr, fetchOpts), { + initialDelay: 500, + maxDelayBetweenRetries: 3000, shouldRetry: (_: unknown, iterationsCount: number) => { return iterationsCount < maxTries; }, diff --git a/packages/clerk-js/src/core/resources/Session.ts b/packages/clerk-js/src/core/resources/Session.ts index 2a6c1b6e3aa..d3eb8534873 100644 --- a/packages/clerk-js/src/core/resources/Session.ts +++ b/packages/clerk-js/src/core/resources/Session.ts @@ -1,6 +1,6 @@ import { createCheckAuthorization } from '@clerk/shared/authorization'; import { is4xxError } from '@clerk/shared/error'; -import { runWithExponentialBackOff } from '@clerk/shared/utils'; +import { retry } from '@clerk/shared/retry'; import type { ActJWTClaim, CheckAuthorization, @@ -84,7 +84,7 @@ export class Session extends BaseResource implements SessionResource { }; getToken: GetToken = async (options?: GetTokenOptions): Promise => { - return runWithExponentialBackOff(() => this._getToken(options), { + return retry(() => this._getToken(options), { shouldRetry: (error: unknown, currentIteration: number) => !is4xxError(error) && currentIteration < 4, }); }; diff --git a/packages/shared/package.json b/packages/shared/package.json index 58ea863e8d6..9ac388e885d 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -79,7 +79,7 @@ "authorization", "authorization-errors", "browser", - "callWithRetry", + "retry", "color", "cookie", "date", diff --git a/packages/shared/src/__tests__/callWithRetry.test.ts b/packages/shared/src/__tests__/callWithRetry.test.ts deleted file mode 100644 index bb276d769c8..00000000000 --- a/packages/shared/src/__tests__/callWithRetry.test.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { callWithRetry } from '../callWithRetry'; - -describe('callWithRetry', () => { - test('should return the result of the function if it succeeds', async () => { - const fn = jest.fn().mockResolvedValue('result'); - const result = await callWithRetry(fn); - expect(result).toBe('result'); - expect(fn).toHaveBeenCalledTimes(1); - }); - - test('should retry the function if it fails', async () => { - const fn = jest.fn().mockRejectedValueOnce(new Error('error')).mockResolvedValueOnce('result'); - const result = await callWithRetry(fn, 1, 2); - expect(result).toBe('result'); - expect(fn).toHaveBeenCalledTimes(2); - }); - - test('should throw an error if the function fails too many times', async () => { - const fn = jest.fn().mockRejectedValue(new Error('error')); - await expect(callWithRetry(fn, 1, 2)).rejects.toThrow('error'); - expect(fn).toHaveBeenCalledTimes(2); - }); -}); diff --git a/packages/shared/src/__tests__/retry.test.ts b/packages/shared/src/__tests__/retry.test.ts new file mode 100644 index 00000000000..2848b0869b8 --- /dev/null +++ b/packages/shared/src/__tests__/retry.test.ts @@ -0,0 +1,208 @@ +import { retry } from '../retry'; + +describe('retry', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + test('resolves with the result of the callback', async () => { + const result = await retry(() => Promise.resolve('success')); + expect(result).toBe('success'); + }); + + test('retries the callback until it succeeds', async () => { + let attempts = 0; + const result = retry( + () => { + attempts++; + if (attempts < 2) { + throw new Error('failed'); + } + return Promise.resolve('success'); + }, + { + initialDelay: 100, + factor: 1, + jitter: false, + }, + ); + await jest.advanceTimersByTimeAsync(200); + expect(await result).toBe('success'); + expect(attempts).toBe(2); + }); + + test('maxDelayBetweenRetries prevents delays from growing beyond the limit', async () => { + jest.useFakeTimers(); + let attempts = 0; + + retry( + () => { + attempts++; + throw new Error('failed'); + }, + { + maxDelayBetweenRetries: 300, + initialDelay: 100, + factor: 3, + jitter: false, + shouldRetry: (_, count) => count <= 4, + }, + ).catch(e => { + expect(e.message).toBe('failed'); + }); + + // Run all timer advances before testing the promise + await jest.advanceTimersByTimeAsync(100); + await jest.advanceTimersByTimeAsync(300); + await jest.advanceTimersByTimeAsync(300); + await jest.advanceTimersByTimeAsync(300); + expect(attempts).toBe(1 + 4); + jest.useRealTimers(); + }); + + test('respects initialDelay option', async () => { + let attempts = 0; + retry( + () => { + attempts++; + throw new Error('failed'); + }, + { initialDelay: 200, jitter: false, shouldRetry: (_, count) => count <= 2 }, + ).catch(() => {}); + + expect(attempts).toBe(1); + await jest.advanceTimersByTimeAsync(200); + expect(attempts).toBe(2); + await jest.advanceTimersByTimeAsync(400); + expect(attempts).toBe(3); + }); + + test('respects retryImmediately option', async () => { + let attempts = 0; + retry( + () => { + attempts++; + throw new Error('failed'); + }, + { + initialDelay: 1000, + retryImmediately: true, + jitter: false, + shouldRetry: (_, count) => count <= 2, + }, + ).catch(() => {}); + + expect(attempts).toBe(1); + await jest.advanceTimersByTimeAsync(101); + expect(attempts).toBe(2); + await jest.advanceTimersByTimeAsync(1000); + expect(attempts).toBe(3); + }); + + test('disables immediate retry when retryImmediately is false', async () => { + let attempts = 0; + retry( + () => { + attempts++; + throw new Error('failed'); + }, + { + initialDelay: 200, + retryImmediately: false, + jitter: false, + shouldRetry: (_, count) => count <= 2, + }, + ).catch(() => {}); + + expect(attempts).toBe(1); + await jest.advanceTimersByTimeAsync(200); + expect(attempts).toBe(2); + await jest.advanceTimersByTimeAsync(400); + expect(attempts).toBe(3); + }); + + test('respects shouldRetry custom logic', async () => { + let attempts = 0; + const error = new Error('special error'); + + await retry( + () => { + attempts++; + throw error; + }, + { + initialDelay: 100, + jitter: false, + shouldRetry: e => (e as Error).message !== 'special error', + }, + ).catch(e => { + expect(e).toBe(error); + }); + + expect(attempts).toBe(1); + }); + + test('respects factor for exponential backoff', async () => { + let attempts = 0; + retry( + () => { + attempts++; + throw new Error('failed'); + }, + { + initialDelay: 100, + factor: 4, + jitter: false, + shouldRetry: (_, count) => count <= 3, + }, + ).catch(() => {}); + + expect(attempts).toBe(1); + await jest.advanceTimersByTimeAsync(100); + expect(attempts).toBe(2); + await jest.advanceTimersByTimeAsync(400); + expect(attempts).toBe(3); + await jest.advanceTimersByTimeAsync(1600); + expect(attempts).toBe(4); + }); + + test('applies jitter by default', async () => { + let attempts = 0; + + jest.spyOn(Math, 'random').mockReturnValue(0.5); + + retry( + () => { + attempts++; + throw new Error('failed'); + }, + { + initialDelay: 100, + factor: 1, + shouldRetry: (_, count) => count <= 2, + }, + ).catch(() => {}); + + // First attempt that triggers the retry + expect(attempts).toBe(1); + // Flush all microtasks + await Promise.resolve(); + // Normal delay without jitter + await jest.advanceTimersByTimeAsync(100); + // But the attempt is still 1 because with the jitter enabled, + // the delay is now 150 + expect(attempts).toBe(1); + // Wait for 50ms more (100 + 50) + await jest.advanceTimersByTimeAsync(50); + // Should now reach the second attempt + expect(attempts).toBe(2); + await jest.advanceTimersByTimeAsync(150); + expect(attempts).toBe(3); + await jest.advanceTimersByTimeAsync(150); + expect(attempts).toBe(3); + }); +}); diff --git a/packages/shared/src/callWithRetry.ts b/packages/shared/src/callWithRetry.ts deleted file mode 100644 index e2723a3ba57..00000000000 --- a/packages/shared/src/callWithRetry.ts +++ /dev/null @@ -1,28 +0,0 @@ -function wait(ms: number) { - return new Promise(res => setTimeout(res, ms)); -} - -const MAX_NUMBER_OF_RETRIES = 5; - -/** - * Retry callback function every few hundred ms (with an exponential backoff - * based on the current attempt) until the maximum attempts has reached or - * the callback is executed successfully. The default number of maximum - * attempts is 5 and retries are triggered when callback throws an error. - */ -export async function callWithRetry( - fn: (...args: unknown[]) => Promise, - attempt = 1, - maxAttempts = MAX_NUMBER_OF_RETRIES, -): Promise { - try { - return await fn(); - } catch (e) { - if (attempt >= maxAttempts) { - throw e; - } - await wait(2 ** attempt * 100); - - return callWithRetry(fn, attempt + 1, maxAttempts); - } -} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 6c0d63672b1..a8b853eff53 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -12,7 +12,6 @@ export * from './utils'; export { apiUrlFromPublishableKey } from './apiUrlFromPublishableKey'; export * from './browser'; -export { callWithRetry } from './callWithRetry'; export * from './color'; export * from './constants'; export * from './date'; diff --git a/packages/shared/src/loadScript.ts b/packages/shared/src/loadScript.ts index 620c855b68a..fd8b9077e1e 100644 --- a/packages/shared/src/loadScript.ts +++ b/packages/shared/src/loadScript.ts @@ -1,4 +1,4 @@ -import { runWithExponentialBackOff } from './utils'; +import { retry } from './retry'; const NO_DOCUMENT_ERROR = 'loadScript cannot be called when document does not exist'; const NO_SRC_ERROR = 'loadScript cannot be called without a src'; @@ -47,5 +47,5 @@ export async function loadScript(src = '', opts: LoadScriptOptions): Promise iterations < 5 }); + return retry(load, { shouldRetry: (_, iterations) => iterations <= 5 }); } diff --git a/packages/shared/src/retry.ts b/packages/shared/src/retry.ts new file mode 100644 index 00000000000..8b98bab0874 --- /dev/null +++ b/packages/shared/src/retry.ts @@ -0,0 +1,111 @@ +type Milliseconds = number; + +type RetryOptions = Partial<{ + /** + * The initial delay before the first retry. + * @default 125 + */ + initialDelay: Milliseconds; + /** + * The maximum delay between retries. + * The delay between retries will never exceed this value. + * If set to 0, the delay will increase indefinitely. + * @default 0 + */ + maxDelayBetweenRetries: Milliseconds; + /** + * The multiplier for the exponential backoff. + * @default 2 + */ + factor: number; + /** + * A function to determine if the operation should be retried. + * The callback accepts the error that was thrown and the number of iterations. + * The iterations variable references the number of retries AFTER attempt + * that caused the error and starts at 1 (as in, this is the 1st, 2nd, nth retry). + * @default (error, iterations) => iterations < 5 + */ + shouldRetry: (error: unknown, iterations: number) => boolean; + /** + * Controls whether the helper should retry the operation immediately once before applying exponential backoff. + * The delay for the immediate retry is 100ms. + * @default true + */ + retryImmediately: boolean; + /** + * If true, the intervals will be multiplied by a factor in the range of [1,2]. + * @default true + */ + jitter: boolean; +}>; + +const defaultOptions: Required = { + initialDelay: 125, + maxDelayBetweenRetries: 0, + factor: 2, + shouldRetry: (_: unknown, iteration: number) => iteration < 5, + retryImmediately: true, + jitter: true, +}; + +const RETRY_IMMEDIATELY_DELAY = 100; + +const sleep = async (ms: Milliseconds) => new Promise(s => setTimeout(s, ms)); + +const applyJitter = (delay: Milliseconds, jitter: boolean) => { + return jitter ? delay * (1 + Math.random()) : delay; +}; + +const createExponentialDelayAsyncFn = ( + opts: Required>, +) => { + let timesCalled = 0; + + const calculateDelayInMs = () => { + const constant = opts.initialDelay; + const base = opts.factor; + let delay = constant * Math.pow(base, timesCalled); + delay = applyJitter(delay, opts.jitter); + return Math.min(opts.maxDelayBetweenRetries || delay, delay); + }; + + return async (): Promise => { + await sleep(calculateDelayInMs()); + timesCalled++; + }; +}; + +/** + * Retries a callback until it succeeds or the shouldRetry function returns false. + * See {@link RetryOptions} for the available options. + */ +export const retry = async (callback: () => T | Promise, options: RetryOptions = {}): Promise => { + let iterations = 0; + const { shouldRetry, initialDelay, maxDelayBetweenRetries, factor, retryImmediately, jitter } = { + ...defaultOptions, + ...options, + }; + + const delay = createExponentialDelayAsyncFn({ + initialDelay, + maxDelayBetweenRetries, + factor, + jitter, + }); + + while (true) { + try { + return await callback(); + } catch (e) { + iterations++; + if (!shouldRetry(e, iterations)) { + throw e; + } + if (retryImmediately && iterations === 1) { + await sleep(applyJitter(RETRY_IMMEDIATELY_DELAY, jitter)); + } else { + await delay(); + } + } + } +}; diff --git a/packages/shared/src/utils/__tests__/runWithExponentialBackOff.test.ts b/packages/shared/src/utils/__tests__/runWithExponentialBackOff.test.ts deleted file mode 100644 index c80ec1c0232..00000000000 --- a/packages/shared/src/utils/__tests__/runWithExponentialBackOff.test.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { runWithExponentialBackOff } from '../runWithExponentialBackOff'; - -describe('runWithExponentialBackOff', () => { - test('resolves with the result of the callback', async () => { - const result = await runWithExponentialBackOff(() => Promise.resolve('success')); - expect(result).toBe('success'); - }); - - test('retries the callback until it succeeds', async () => { - let attempts = 0; - const result = await runWithExponentialBackOff(() => { - attempts++; - if (attempts < 3) { - throw new Error('failed'); - } - return Promise.resolve('success'); - }); - expect(result).toBe('success'); - expect(attempts).toBe(3); - }); -}); diff --git a/packages/shared/src/utils/index.ts b/packages/shared/src/utils/index.ts index 2bca1129d18..c6f6c129dfb 100644 --- a/packages/shared/src/utils/index.ts +++ b/packages/shared/src/utils/index.ts @@ -2,7 +2,6 @@ export * from './createDeferredPromise'; export { isStaging } from './instance'; export { logErrorInDevMode } from './logErrorInDevMode'; export { noop } from './noop'; -export * from './runWithExponentialBackOff'; export * from './runtimeEnvironment'; export { handleValueOrFn } from './handleValueOrFn'; export { fastDeepMergeAndReplace, fastDeepMergeAndKeep } from './fastDeepMerge'; diff --git a/packages/shared/src/utils/runWithExponentialBackOff.ts b/packages/shared/src/utils/runWithExponentialBackOff.ts deleted file mode 100644 index 992b69e425d..00000000000 --- a/packages/shared/src/utils/runWithExponentialBackOff.ts +++ /dev/null @@ -1,61 +0,0 @@ -type Milliseconds = number; - -type BackoffOptions = Partial<{ - firstDelay: Milliseconds; - maxDelay: Milliseconds; - timeMultiple: number; - shouldRetry: (error: unknown, iterationsCount: number) => boolean; -}>; - -const defaultOptions: Required = { - firstDelay: 125, - maxDelay: 0, - timeMultiple: 2, - shouldRetry: () => true, -}; - -const sleep = async (ms: Milliseconds) => new Promise(s => setTimeout(s, ms)); - -const createExponentialDelayAsyncFn = (opts: { - firstDelay: Milliseconds; - maxDelay: Milliseconds; - timeMultiple: number; -}) => { - let timesCalled = 0; - - const calculateDelayInMs = () => { - const constant = opts.firstDelay; - const base = opts.timeMultiple; - const delay = constant * Math.pow(base, timesCalled); - return Math.min(opts.maxDelay || delay, delay); - }; - - return async (): Promise => { - await sleep(calculateDelayInMs()); - timesCalled++; - }; -}; - -export const runWithExponentialBackOff = async ( - callback: () => T | Promise, - options: BackoffOptions = {}, -): Promise => { - let iterationsCount = 0; - const { shouldRetry, firstDelay, maxDelay, timeMultiple } = { - ...defaultOptions, - ...options, - }; - const delay = createExponentialDelayAsyncFn({ firstDelay, maxDelay, timeMultiple }); - - while (true) { - try { - return await callback(); - } catch (e) { - iterationsCount++; - if (!shouldRetry(e, iterationsCount)) { - throw e; - } - await delay(); - } - } -};