Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions packages/backend/src/api/__tests__/factory.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
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';
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',
Expand Down Expand Up @@ -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`,
Expand All @@ -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(
Expand Down
23 changes: 5 additions & 18 deletions packages/backend/src/api/request.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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,
Expand All @@ -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(),
Expand All @@ -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),
};
}

Expand Down Expand Up @@ -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),
};
}
};
Expand All @@ -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[];
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/tokens/__tests__/keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
Expand Down
4 changes: 3 additions & 1 deletion packages/backend/src/tokens/keys.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { with503Retry } from '@clerk/shared/utils';

import {
API_URL,
API_VERSION,
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion packages/clerk-js/src/core/resources/Base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,18 @@ export abstract class BaseResource {
assertProductionKeysOnDev(status, errors);

const apiResponseOptions: ConstructorParameters<typeof ClerkAPIResponseError>[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;
}
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions packages/clerk-js/src/core/resources/__tests__/Base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions packages/shared/src/utils/__tests__/retry.spec.ts
Original file line number Diff line number Diff line change
@@ -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
});
});
1 change: 1 addition & 0 deletions packages/shared/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export { noop } from './noop';
export * from './runtimeEnvironment';
export { handleValueOrFn } from './handleValueOrFn';
export { fastDeepMergeAndReplace, fastDeepMergeAndKeep } from './fastDeepMerge';
export * from './retry';
150 changes: 150 additions & 0 deletions packages/shared/src/utils/retry.ts
Original file line number Diff line number Diff line change
@@ -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<Response> {
// 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<void> {
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);
}
Loading