Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/shared-retry-cancellation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/shared': patch
---

The `retry` helper now supports cancellation and error-dependent backoff. Passing an `AbortSignal` via the new `signal` option cancels retrying, immediately interrupting any pending backoff delay, and `initialDelay` now also accepts a function deriving the backoff base delay from the error that triggered the retry.
80 changes: 80 additions & 0 deletions packages/shared/src/__tests__/retry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,86 @@ describe('retry', () => {
expect(attempts).toBe(4);
});

test('rejects without calling the callback when the signal is already aborted', async () => {
const callback = vi.fn();
const controller = new AbortController();
controller.abort(new Error('cancelled'));

await expect(retry(callback, { signal: controller.signal })).rejects.toThrow('cancelled');
expect(callback).not.toHaveBeenCalled();
});

test('aborting during a delay rejects immediately and stops retrying', async () => {
let attempts = 0;
const controller = new AbortController();

const result = retry(
() => {
attempts++;
throw new Error('failed');
},
{
initialDelay: 10_000,
jitter: false,
shouldRetry: () => true,
signal: controller.signal,
},
);
const assertion = expect(result).rejects.toThrow('cancelled');

await vi.advanceTimersByTimeAsync(0);
expect(attempts).toBe(1);

controller.abort(new Error('cancelled'));
await assertion;

await vi.advanceTimersByTimeAsync(60_000);
expect(attempts).toBe(1);
});

test('rejects with the abort reason when the signal aborts while the callback is running', async () => {
const controller = new AbortController();
const onBeforeRetry = vi.fn();
let attempts = 0;

const result = retry(
() => {
attempts++;
controller.abort(new Error('cancelled'));
throw new Error('failed');
},
{ shouldRetry: () => true, signal: controller.signal, onBeforeRetry },
);

await expect(result).rejects.toThrow('cancelled');
expect(attempts).toBe(1);
expect(onBeforeRetry).not.toHaveBeenCalled();
});

test('a functional initialDelay derives the backoff base from the error', async () => {
let attempts = 0;
retry(
() => {
attempts++;
throw attempts === 1 ? new Error('fast') : new Error('slow');
},
{
factor: 2,
jitter: false,
shouldRetry: (_, count) => count <= 2,
initialDelay: error => ((error as Error).message === 'fast' ? 100 : 1000),
},
).catch(() => {});

expect(attempts).toBe(1);
// First retry: base 100 * 2^0
await vi.advanceTimersByTimeAsync(100);
expect(attempts).toBe(2);
// Second retry: base 1000 * 2^1
await vi.advanceTimersByTimeAsync(2_000);
expect(attempts).toBe(3);
});

test('applies jitter by default', async () => {
let attempts = 0;

Expand Down
72 changes: 55 additions & 17 deletions packages/shared/src/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ type Milliseconds = number;

type RetryOptions = Partial<{
/**
* The initial delay before the first retry.
* The base delay of the exponential backoff: either milliseconds, or a function
* deriving them from the error that triggered the retry and the iteration number,
* starting at 1 for the first retry.
* The exponential factor, max delay, and jitter still apply.
*
* @default 125
*/
initialDelay: Milliseconds;
initialDelay: Milliseconds | ((error: unknown, iteration: number) => Milliseconds);
Comment on lines +5 to +12

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document that iteration starts at 1.

This new public callback parameter otherwise has ambiguous indexing semantics.

Proposed documentation update
-   * deriving them from the error that triggered the retry and the iteration number.
+   * deriving them from the error that triggered the retry and the iteration number,
+   * starting at 1 for the first retry.

The Docs team may also need to review this generated reference-doc change. As per path instructions, JSDoc on public/reference-facing APIs is customer-facing documentation.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* The base delay of the exponential backoff: either milliseconds, or a function
* deriving them from the error that triggered the retry and the iteration number.
* The exponential factor, max delay, and jitter still apply.
*
* @default 125
*/
initialDelay: Milliseconds;
initialDelay: Milliseconds | ((error: unknown, iteration: number) => Milliseconds);
/**
* The base delay of the exponential backoff: either milliseconds, or a function
* deriving them from the error that triggered the retry and the iteration number,
* starting at 1 for the first retry.
* The exponential factor, max delay, and jitter still apply.
*
* `@default` 125
*/
initialDelay: Milliseconds | ((error: unknown, iteration: number) => Milliseconds);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/shared/src/retry.ts` around lines 5 - 11, Update the JSDoc for the
public initialDelay callback parameter in retry configuration to explicitly
state that iteration starts at 1, clarifying its indexing semantics without
changing the callback type or behavior.

Source: Path instructions

/**
* The maximum delay between retries.
* The delay between retries will never exceed this value.
Expand Down Expand Up @@ -50,6 +53,12 @@ type RetryOptions = Partial<{
* This can be used to modify request parameters, add headers, etc.
*/
onBeforeRetry?: (iteration: number) => void | Promise<void>;
/**
* An AbortSignal that cancels retrying.
* Aborting rejects the returned promise with the signal's abort reason, immediately
* interrupting any pending delay. It does not abort a callback that is already executing.
*/
signal: AbortSignal;
}>;

const defaultOptions = {
Expand All @@ -63,27 +72,46 @@ const defaultOptions = {

const RETRY_IMMEDIATELY_DELAY = 100;

const sleep = async (ms: Milliseconds) => new Promise(s => setTimeout(s, ms));
const abortReason = (signal: AbortSignal): Error => signal.reason ?? new Error('The operation was aborted');

const sleep = async (ms: Milliseconds, signal?: AbortSignal) =>
new Promise<void>((resolve, reject) => {
if (!signal) {
setTimeout(resolve, ms);
return;
}
if (signal.aborted) {
reject(abortReason(signal));
return;
}
const onAbort = () => {
clearTimeout(timer);
reject(abortReason(signal));
};
const timer = setTimeout(() => {
signal.removeEventListener('abort', onAbort);
resolve();
}, ms);
signal.addEventListener('abort', onAbort, { once: true });
});

const applyJitter = (delay: Milliseconds, jitter: boolean) => {
return jitter ? delay * (1 + Math.random()) : delay;
};

const createExponentialDelayAsyncFn = (
opts: Required<Pick<RetryOptions, 'initialDelay' | 'maxDelayBetweenRetries' | 'factor' | 'jitter'>>,
opts: Required<Pick<RetryOptions, 'maxDelayBetweenRetries' | 'factor' | 'jitter'>> & Pick<RetryOptions, 'signal'>,
) => {
let timesCalled = 0;

const calculateDelayInMs = () => {
const constant = opts.initialDelay;
const base = opts.factor;
let delay = constant * Math.pow(base, timesCalled);
const calculateDelayInMs = (base: Milliseconds) => {
let delay = base * Math.pow(opts.factor, timesCalled);
delay = applyJitter(delay, opts.jitter);
return Math.min(opts.maxDelayBetweenRetries || delay, delay);
};

return async (): Promise<void> => {
await sleep(calculateDelayInMs());
return async (base: Milliseconds): Promise<void> => {
await sleep(calculateDelayInMs(base), opts.signal);
timesCalled++;
};
};
Expand All @@ -94,22 +122,32 @@ const createExponentialDelayAsyncFn = (
*/
export const retry = async <T>(callback: () => T | Promise<T>, options: RetryOptions = {}): Promise<T> => {
let iterations = 0;
const { shouldRetry, initialDelay, maxDelayBetweenRetries, factor, retryImmediately, jitter, onBeforeRetry } = {
...defaultOptions,
...options,
};
const { shouldRetry, initialDelay, maxDelayBetweenRetries, factor, retryImmediately, jitter, onBeforeRetry, signal } =
{
...defaultOptions,
...options,
};

const resolveInitialDelay = typeof initialDelay === 'function' ? initialDelay : () => initialDelay;
const delay = createExponentialDelayAsyncFn({
initialDelay,
maxDelayBetweenRetries,
factor,
jitter,
signal,
});

while (true) {
if (signal?.aborted) {
throw abortReason(signal);
}

Comment on lines 139 to +143

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Recheck cancellation immediately after the callback rejects.

If the signal aborts while callback is running, the catch block can return the original error or invoke retry hooks before observing cancellation. This violates the documented guarantee that cancellation rejects with the abort reason.

Proposed fix
     } catch (e) {
+      if (signal?.aborted) {
+        throw abortReason(signal);
+      }
+
       iterations++;

Add a regression test where the signal aborts during an active callback and that callback subsequently rejects.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while (true) {
if (signal?.aborted) {
throw abortReason(signal);
}
if (signal?.aborted) {
throw abortReason(signal);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/shared/src/retry.ts` around lines 138 - 142, Update the retry loop
around the callback’s catch handling to recheck signal cancellation immediately
after the callback rejects, before returning the original error or invoking
retry hooks; when aborted, throw abortReason(signal) to preserve the
cancellation contract. Add a regression test covering a callback that aborts the
signal while running and then rejects.

try {
return await callback();
Comment on lines 144 to 145

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium src/retry.ts:144

If signal aborts while callback() is in flight and the callback then succeeds, retry returns the result normally instead of rejecting with the abort reason. This contradicts the documented contract that aborting rejects the returned promise. The loop awaits the callback at await callback() and returns immediately without rechecking signal.aborted, so a slow successful attempt resolves after cancellation. Recheck signal?.aborted after the await and throw abortReason(signal) before returning.

Suggested change
try {
return await callback();
try {
const result = await callback();
if (signal?.aborted) {
throw abortReason(signal);
}
return result;
} catch (e) {
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/shared/src/retry.ts around lines 144-145:

If `signal` aborts while `callback()` is in flight and the callback then succeeds, `retry` returns the result normally instead of rejecting with the abort reason. This contradicts the documented contract that aborting rejects the returned promise. The loop awaits the callback at `await callback()` and returns immediately without rechecking `signal.aborted`, so a slow successful attempt resolves after cancellation. Recheck `signal?.aborted` after the await and throw `abortReason(signal)` before returning.

} catch (e) {
if (signal?.aborted) {
throw abortReason(signal);
}

iterations++;
if (!shouldRetry(e, iterations)) {
throw e;
Expand All @@ -120,9 +158,9 @@ export const retry = async <T>(callback: () => T | Promise<T>, options: RetryOpt
}

if (retryImmediately && iterations === 1) {
await sleep(applyJitter(RETRY_IMMEDIATELY_DELAY, jitter));
await sleep(applyJitter(RETRY_IMMEDIATELY_DELAY, jitter), signal);
} else {
await delay();
await delay(resolveInitialDelay(e, iterations));
}
}
}
Expand Down
Loading