diff --git a/packages/backend/src/api/__tests__/factory.test.ts b/packages/backend/src/api/__tests__/factory.test.ts index e3e0f697461..75924e05772 100644 --- a/packages/backend/src/api/__tests__/factory.test.ts +++ b/packages/backend/src/api/__tests__/factory.test.ts @@ -1,5 +1,5 @@ import { http, HttpResponse } from 'msw'; -import { describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import jwksJson from '../../fixtures/jwks.json'; import userJson from '../../fixtures/user.json'; @@ -7,6 +7,11 @@ import { server, validateHeaders } from '../../mock-server'; import { createBackendApiClient } from '../factory'; describe('api.client', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(0).getTime()); // set to epoch start for consistent retry-after calculations + }); + const apiClient = createBackendApiClient({ apiUrl: 'https://api.clerk.test', secretKey: 'deadbeef', @@ -153,7 +158,7 @@ describe('api.client', () => { expect(errResponse.clerkTraceId).toBe('mock_cf_ray'); }); - it('executes a failed backend API request and includes Retry-After header', async () => { + it('executes a failed backend API request and includes Retry-After header for non 503', async () => { server.use( http.get( `https://api.clerk.test/v1/users/user_deadbeef`, @@ -169,6 +174,25 @@ describe('api.client', () => { expect(errResponse.retryAfter).toBe(123); }); + it('executes a failed backend API request and includes Retry-After header RFC1123 date for non 503', async () => { + server.use( + http.get( + `https://api.clerk.test/v1/users/user_deadbeef`, + validateHeaders(() => { + return HttpResponse.json( + { errors: [] }, + { status: 429, headers: { 'retry-after': new Date(new Date().getTime() + 60000).toUTCString() } }, + ); + }), + ), + ); + + const errResponse = await apiClient.users.getUser('user_deadbeef').catch(err => err); + + expect(errResponse.status).toBe(429); + expect(errResponse.retryAfter).not.toBeNaN(); + }); + it('executes a failed backend API request and ignores invalid Retry-After header', async () => { server.use( http.get( diff --git a/packages/backend/src/api/request.ts b/packages/backend/src/api/request.ts index c9d321f505e..6ab9256f5c0 100644 --- a/packages/backend/src/api/request.ts +++ b/packages/backend/src/api/request.ts @@ -1,4 +1,5 @@ import { ClerkAPIResponseError, parseError } from '@clerk/shared/error'; +import { getRetryAfterSeconds, with503Retry } from '@clerk/shared/utils'; import type { ClerkAPIError, ClerkAPIErrorJSON } from '@clerk/types'; import snakecaseKeys from 'snakecase-keys'; @@ -154,7 +155,7 @@ export function buildRequest(options: BuildRequestOptions) { let res: Response | undefined; try { if (formData) { - res = await runtime.fetch(finalUrl.href, { + res = await with503Retry(runtime.fetch)(finalUrl.href, { method, headers, body: formData, @@ -177,7 +178,7 @@ export function buildRequest(options: BuildRequestOptions) { }; }; - res = await runtime.fetch(finalUrl.href, { + res = await with503Retry(runtime.fetch)(finalUrl.href, { method, headers, ...buildBody(), @@ -196,7 +197,7 @@ export function buildRequest(options: BuildRequestOptions) { status: res?.status, statusText: res?.statusText, clerkTraceId: getTraceId(responseBody, res?.headers), - retryAfter: getRetryAfter(res?.headers), + retryAfter: getRetryAfterSeconds(res?.headers), }; } @@ -224,7 +225,7 @@ export function buildRequest(options: BuildRequestOptions) { status: res?.status, statusText: res?.statusText, clerkTraceId: getTraceId(err, res?.headers), - retryAfter: getRetryAfter(res?.headers), + retryAfter: getRetryAfterSeconds(res?.headers), }; } }; @@ -243,20 +244,6 @@ function getTraceId(data: unknown, headers?: Headers): string { return cfRay || ''; } -function getRetryAfter(headers?: Headers): number | undefined { - const retryAfter = headers?.get('Retry-After'); - if (!retryAfter) { - return; - } - - const value = parseInt(retryAfter, 10); - if (isNaN(value)) { - return; - } - - return value; -} - function parseErrors(data: unknown): ClerkAPIError[] { if (!!data && typeof data === 'object' && 'errors' in data) { const errors = data.errors as ClerkAPIErrorJSON[]; diff --git a/packages/backend/src/tokens/__tests__/keys.test.ts b/packages/backend/src/tokens/__tests__/keys.test.ts index f4b301c9d53..52ba6fe983e 100644 --- a/packages/backend/src/tokens/__tests__/keys.test.ts +++ b/packages/backend/src/tokens/__tests__/keys.test.ts @@ -168,7 +168,7 @@ describe('tokens.loadClerkJWKFromRemote(options)', () => { kid: 'ins_whatever', skipJwksCache: true, }); - void vi.advanceTimersByTimeAsync(10000); + void vi.advanceTimersByTimeAsync(60000); await promise; }).rejects.toThrowError('Error loading Clerk JWKS from https://api.clerk.com/v1/jwks with code=503'); }); diff --git a/packages/backend/src/tokens/keys.ts b/packages/backend/src/tokens/keys.ts index 64d487a8760..51cca4a00ec 100644 --- a/packages/backend/src/tokens/keys.ts +++ b/packages/backend/src/tokens/keys.ts @@ -1,3 +1,5 @@ +import { with503Retry } from '@clerk/shared/utils'; + import { API_URL, API_VERSION, @@ -185,7 +187,7 @@ async function fetchJWKSFromBAPI(apiUrl: string, key: string, apiVersion: string const url = new URL(apiUrl); url.pathname = joinPaths(url.pathname, apiVersion, '/jwks'); - const response = await runtime.fetch(url.href, { + const response = await with503Retry(runtime.fetch)(url.href, { headers: { Authorization: `Bearer ${key}`, 'Clerk-API-Version': SUPPORTED_BAPI_VERSION, diff --git a/packages/clerk-js/src/core/resources/Base.ts b/packages/clerk-js/src/core/resources/Base.ts index cc1b5db9819..593f03b15bb 100644 --- a/packages/clerk-js/src/core/resources/Base.ts +++ b/packages/clerk-js/src/core/resources/Base.ts @@ -146,12 +146,18 @@ export abstract class BaseResource { assertProductionKeysOnDev(status, errors); const apiResponseOptions: ConstructorParameters[1] = { data: errors, status }; - if (status === 429 && headers) { + if ((status === 429 || status == 503) && headers) { const retryAfter = headers.get('retry-after'); if (retryAfter) { const value = parseInt(retryAfter, 10); if (!isNaN(value)) { apiResponseOptions.retryAfter = value; + } else { + const date = new Date(retryAfter); + if (!isNaN(date.getTime())) { + const value = Math.ceil((date.getTime() - Date.now()) / 1000); + apiResponseOptions.retryAfter = value > 0 ? value : 0; + } } } } diff --git a/packages/clerk-js/src/core/resources/__tests__/Base.test.ts b/packages/clerk-js/src/core/resources/__tests__/Base.test.ts index 0eb8022b2a2..526e7ebc771 100644 --- a/packages/clerk-js/src/core/resources/__tests__/Base.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/Base.test.ts @@ -38,6 +38,27 @@ describe('BaseResource', () => { expect(errResponse.retryAfter).toBe(60); }); + it('populates retryAfter on 503 error responses', async () => { + BaseResource.clerk = { + // @ts-expect-error - We're not about to mock the entire FapiClient + getFapiClient: () => { + return { + request: vi.fn().mockResolvedValue({ + payload: {}, + status: 503, + statusText: 'Service Unavailable', + headers: new Headers({ 'Retry-After': new Date(new Date().getTime() + 60000).toUTCString() }), + }), + }; + }, + __internal_setCountry: vi.fn(), + }; + const resource = new TestResource(); + const errResponse = await resource.fetch().catch(err => err); + console.dir(errResponse); + expect(errResponse.retryAfter).toBe(60); + }); + it('does not populate retryAfter on invalid header', async () => { BaseResource.clerk = { // @ts-expect-error - We're not about to mock the entire FapiClient diff --git a/packages/shared/src/utils/__tests__/retry.spec.ts b/packages/shared/src/utils/__tests__/retry.spec.ts new file mode 100644 index 00000000000..b1a8312641f --- /dev/null +++ b/packages/shared/src/utils/__tests__/retry.spec.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest'; + +import { getRetryAfterMs } from '../retry'; + +describe('api.retry', () => { + it('parses retry-after values correctly', () => { + expect(getRetryAfterMs(new Headers({ 'Retry-After': '120' }))).toBe(120000); + expect(getRetryAfterMs(new Headers({ 'Retry-After': '0' }))).toBe(0); + expect(getRetryAfterMs(new Headers({ 'Retry-After': ' 45 ' }))).toBe(45000); + expect( + getRetryAfterMs(new Headers({ 'Retry-After': new Date(new Date().getTime() + 60000).toUTCString() })), + ).toBeGreaterThan(0); + expect(getRetryAfterMs(new Headers({ 'Retry-After': 'Wed, 21 Oct 2000 07:28:00 GMT' }))).toBe(0); // past date + expect(getRetryAfterMs(new Headers({ 'Retry-After': 'invalid-date' }))).toBeUndefined(); + expect(getRetryAfterMs(new Headers({}))).toBeUndefined(); + expect(getRetryAfterMs(new Headers({ 'Retry-After': '60, 120' }))).toBe(60000); // multiple headers, use first + }); +}); diff --git a/packages/shared/src/utils/index.ts b/packages/shared/src/utils/index.ts index 0d2d3bdbf12..c6172237ddd 100644 --- a/packages/shared/src/utils/index.ts +++ b/packages/shared/src/utils/index.ts @@ -6,3 +6,4 @@ export { noop } from './noop'; export * from './runtimeEnvironment'; export { handleValueOrFn } from './handleValueOrFn'; export { fastDeepMergeAndReplace, fastDeepMergeAndKeep } from './fastDeepMerge'; +export * from './retry'; diff --git a/packages/shared/src/utils/retry.ts b/packages/shared/src/utils/retry.ts new file mode 100644 index 00000000000..cc240eaa4ac --- /dev/null +++ b/packages/shared/src/utils/retry.ts @@ -0,0 +1,150 @@ +// maximum number of retries on 503 responses +const MAX_RETRIES = 5; + +// base delay in ms for exponential backoff +const BASE_DELAY_MS = 500; + +// longest delay we will allow even if more is specified by Retry-After header is 1 minute. +const MAX_DELAY_MS = 60000; + +/** + * wraps a fetch function to retry on 503 responses only, passing all other responses through and + * respecting the abort controller signal. + * + * if server provides a Retry-After header, that is respected (within reason), otherwise we use exponential + * backoff based on the number of attempts. + * + * retry is attempted up to MAX_RETRIES times with exponential backoff between 0 and 2^n * BASE_DELAY_MS. + * + */ +export function with503Retry(fetch: typeof globalThis.fetch) { + return async function fetchWithRetry503(input: RequestInfo | URL, init?: RequestInit): Promise { + // want to respect abort signals if provided + const abortSignal = init?.signal; + + for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { + // fetch will throw if already aborted + const response = await fetch(input, init); + + if (response.status !== 503) { + // If not a 503, return the response immediately + return response; + } + + if (attempt >= MAX_RETRIES) { + // return the last response + return response; + } + + // if there is a retry after, we'll respect that as the server is explicitly indicating when to retry. + const retryAfterMs = getRetryAfterMs(response.headers); + + if (retryAfterMs !== undefined) { + // if the value is 0, we retry immediately as it is explicitly indicated by server. Jitter is not + // applied in this case, as the server is instructing the wait time due to some knowledge it has. + if (retryAfterMs > 0) { + // note that we clamp the maximum delay to avoid excessively long waits, even if server indicates longer + // that could result in a muisconfiguration or error on server side causing request to delay for years + // (or millennia?) + await waitForTimeoutOrCancel(Math.min(retryAfterMs, MAX_DELAY_MS), abortSignal); + } + continue; // Proceed to next attempt + } + + // no Retry-After header, so we use exponential backoff. + + // Calculate delay + const delay = Math.random() * Math.pow(2, attempt) * BASE_DELAY_MS; + + // Wait for the delay before retrying, but abort if signal is triggered + await waitForTimeoutOrCancel(delay, abortSignal); + + // Proceed to next attempt + } + + // This point should never be reached + throw new Error('Unexpected error in fetchWithRetry503'); + }; +} + +/** + * Helper function to wait for a timeout or abort with Aborted if signal is triggered + */ +async function waitForTimeoutOrCancel(delay: number, signal: AbortSignal | null | undefined): Promise { + if (!signal) { + return new Promise(resolve => setTimeout(resolve, delay)); + } + return await new Promise((resolve, reject) => { + const onAbort = () => { + signal.removeEventListener('abort', onAbort); + clearTimeout(timeoutId); // timeoutId is defined, hoisting. + // Reject the promise if aborted using standard DOMException for AbortError + reject(new DOMException('Aborted', 'AbortError')); + }; + signal.addEventListener('abort', onAbort); + const timeoutId = setTimeout(() => { + signal.removeEventListener('abort', onAbort); + resolve(); + }, delay); + }); +} + +/** + * either returns number of milliseconds to wait as instructed by the server, or undefined + * if no valid Retry-After header is present. + * + * note that 0 is a valid retry-after value, and explicitly indicates no wait and that the + * client should retry immediately. + * + * Handles both delta-seconds and HTTP-date formats, returning the number of milliseconds to + * wait from now if HTTP-date is provided. If it is in the past, returns 0. + * + * does not clamp the upper bound value, that must be handled by the caller. + * + */ +export function getRetryAfterMs(headers?: Headers): number | undefined { + const retryAfter = headers?.get('Retry-After'); + if (!retryAfter) { + return; + } + + const intValue = parseInt(retryAfter, 10); + if (!isNaN(intValue)) { + if (intValue < 0) { + // invalid, treat as no header present + return; + } else if (intValue === 0) { + // explicit immediate retry + return 0; + } + return Math.ceil(intValue) * 1000; // return whole integers as milliseconds only. + } + + // reminder: https://jsdate.wtf/ + const date = new Date(retryAfter); + if (!isNaN(date.getTime())) { + const value = Math.ceil(date.getTime() - Date.now()); + if (value < 0) { + // date is in the past, so we return 0 + return 0; + } + return value; + } + + // otherwise the date was invalid so we treat as no header present + return; +} + +/** + * returns number of full seconds to wait as instructed by the server, or undefined + * if no valid Retry-After header is present. + * + * @see getRetryAfterMs + */ +export function getRetryAfterSeconds(headers?: Headers): number | undefined { + const ms = getRetryAfterMs(headers); + if (ms === undefined) { + return; + } + return Math.ceil(ms / 1000); +}