Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 470
feat(shared): Replace callWithRetry and runWithExponentialBackoff with retry#5144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
b1f5bc53f3284a6e9fb24c48f5d3File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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'; | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -79,7 +79,7 @@ | ||
| "authorization", | ||
| "authorization-errors", | ||
| "browser", | ||
| "callWithRetry", | ||
| "retry", | ||
| "color", | ||
| "cookie", | ||
| "date", | ||
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
| }); |
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ why do we need to cast here?