feat(shared): Add cancellation and per-error delay options to retry#9182
feat(shared): Add cancellation and per-error delay options to retry#9182wobsoriano wants to merge 9 commits into
Conversation
Adds an optional signal (AbortSignal) option so callers can cancel retrying promptly, and allows initialDelay to be a function deriving the backoff base delay from the error that triggered the retry.
🦋 Changeset detectedLatest commit: 1e1868f The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The `retry` helper now supports cancellation and error-dependent backoff. Added `signal` option for cancellation and modified `initialDelay` to accept a function for backoff base delay.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe shared ChangesRetry cancellation and dynamic backoff
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)packages/shared/src/retry.tsFile contains syntax errors that prevent linting: Line 169: Expected a statement but instead found '}'. Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/shared/src/retry.ts`:
- Around line 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.
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: fa8bdc4b-3b48-4f3e-8b60-0c1aca0aa78a
📒 Files selected for processing (3)
.changeset/shared-retry-cancellation.mdpackages/shared/src/__tests__/retry.spec.tspackages/shared/src/retry.ts
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
Co-authored-by: macroscopeapp[bot] <170038800+macroscopeapp[bot]@users.noreply.github.com>
ApprovabilityVerdict: Approved Additive feature adding optional You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/shared/src/retry.ts`:
- Around line 150-151: Remove the duplicated catch clause in the retry logic so
the surrounding try statement has only one `catch (e)` block. Preserve the
existing catch body and error-handling behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 103d6fe9-86f8-4115-80a0-a299e85963ce
📒 Files selected for processing (2)
packages/shared/src/__tests__/retry.spec.tspackages/shared/src/retry.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description
Follow-up to #9173, which added a hand-rolled exponential backoff to Expo's initialization resource recovery because the shared retry helper couldn't express two things it needed. This adds them:
signal?: AbortSignal: cancels retrying. Aborting rejects the returned promise with the signal's abort reason and immediately interrupts a pending backoff delay. It does not abort a callback that is already executing.initialDelaynow also accepts a function(error, iteration) => number, deriving the backoff base delay from the error that triggered the retry (e.g. 2s for network errors, 10s for 5xx). The exponential factor, cap, and jitter still apply.Both changes are optional and additive. Defaults and existing behavior are unchanged, and no current retry consumer is affected.
A follow-up PR will migrate the Expo recovery logic from #9173 onto this helper, gaining jitter on its retry delays.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change