Retry transport failures#8783
Closed
0xFirekeeper wants to merge 1 commit into
Closed
Conversation
Add transport-layer retrying for network failures and wire it into OTP auth. - Introduce fetchWithTransportRetry and isTransportError to retry fetch thunks that fail before an HTTP response (TypeError), while excluding aborts/timeouts (AbortError). Retries use jittered exponential backoff (3 attempts, 300ms base). - Update getClientFetch to perform each attempt with its own AbortController/timeout and call fetchWithTransportRetry. - Extend retry utility with backoff and shouldRetry options, ensure no sleep after final attempt, and add tests for retry behavior. - Update fetch and OTP tests to model abort behavior and verify retry semantics. - Use fetchWithTransportRetry for sendOtp and verifyOtp so transient transport errors are retried without causing duplicate side effects. Includes changeset note describing the behavior.
🦋 Changeset detectedLatest commit: 0aabc2f The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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.
|
Contributor
WalkthroughThis PR adds network-layer retry support to in-app wallet authentication requests. The retry utility is enhanced with exponential backoff and conditional retry logic, a fetch wrapper detects transport errors and retries them, timeouts are isolated per attempt, and OTP endpoints are integrated to prevent side-effect duplication during retries. ChangesIn-app wallet transport retry support
Sequence DiagramsequenceDiagram
participant OTPClient as OTP Client
participant fetchRetry as fetchWithTransportRetry
participant isTransport as isTransportError
participant retryFn as retry utility
participant doFetch
participant fetch as browser fetch
OTPClient->>fetchRetry: doFetch thunk for POST
fetchRetry->>retryFn: fn=doFetch, {backoff: true, shouldRetry: isTransportError}
retryFn->>doFetch: attempt 1
doFetch->>doFetch: create fresh AbortController
doFetch->>fetch: POST with timeout signal
fetch-->>doFetch: TypeError: Network request failed
doFetch-->>retryFn: error
retryFn->>isTransport: is transport error?
isTransport-->>retryFn: true (not AbortError)
retryFn->>retryFn: sleep exponential backoff + jitter
retryFn->>doFetch: attempt 2
doFetch->>doFetch: create fresh AbortController (isolated)
doFetch->>fetch: POST with fresh timeout signal
fetch-->>doFetch: 200 OK Response
doFetch-->>retryFn: Response
retryFn-->>fetchRetry: Response
fetchRetry-->>OTPClient: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/thirdweb/src/wallets/in-app/web/lib/auth/otp.ts`:
- Around line 48-56: The POST retries for sendOtp and verifyOtp use
fetchWithTransportRetry but generate the request payload inside the retry
closure, which can cause duplicate server-side side effects; generate a stable
idempotency key (e.g. via crypto.randomUUID() or a provided client id) once
before the call and include it in the request (Idempotency-Key header or an
idempotency field on body) so the same key is sent on every retry; ensure the
idempotency key is created outside the fetchWithTransportRetry closure and added
to headers/body used by the fetch(url, { body: stringify(body), headers, method:
"POST" }) calls in sendOtp and verifyOtp so server-side dedupe can detect
retries.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8eb45ec1-3dc1-4d62-9bfc-b78fe9c69a4d
📒 Files selected for processing (6)
.changeset/in-app-wallet-transport-retry.mdpackages/thirdweb/src/utils/fetch.test.tspackages/thirdweb/src/utils/fetch.tspackages/thirdweb/src/utils/retry.test.tspackages/thirdweb/src/utils/retry.tspackages/thirdweb/src/wallets/in-app/web/lib/auth/otp.ts
Comment on lines
+48
to
+56
| // Only retried when the request fails before a response is received, so a | ||
| // verification code is never sent more than once. | ||
| const response = await fetchWithTransportRetry(() => | ||
| fetch(url, { | ||
| body: stringify(body), | ||
| headers, | ||
| method: "POST", | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
Add idempotency protection before retrying these POST auth mutations.
These retries can still duplicate side effects when a request is processed server-side but the client never receives the response. Retrying sendOtp/verifyOtp without an idempotency key + server dedupe contract risks duplicate OTP sends or unintended code consumption.
Also applies to: 119-127
🤖 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/thirdweb/src/wallets/in-app/web/lib/auth/otp.ts` around lines 48 -
56, The POST retries for sendOtp and verifyOtp use fetchWithTransportRetry but
generate the request payload inside the retry closure, which can cause duplicate
server-side side effects; generate a stable idempotency key (e.g. via
crypto.randomUUID() or a provided client id) once before the call and include it
in the request (Idempotency-Key header or an idempotency field on body) so the
same key is sent on every retry; ensure the idempotency key is created outside
the fetchWithTransportRetry closure and added to headers/body used by the
fetch(url, { body: stringify(body), headers, method: "POST" }) calls in sendOtp
and verifyOtp so server-side dedupe can detect retries.
Contributor
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8783 +/- ##
==========================================
+ Coverage 52.78% 52.80% +0.02%
==========================================
Files 934 934
Lines 62976 63017 +41
Branches 4137 4143 +6
==========================================
+ Hits 33241 33278 +37
- Misses 29635 29639 +4
Partials 100 100
🚀 New features to boost your workflow:
|
0xFirekeeper
added
the
DO NOT MERGE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add transport-layer retrying for network failures and wire it into OTP auth.
PR-Codex overview
This PR introduces automatic retries for SDK and in-app wallet authentication requests during transient network failures, with a focus on avoiding duplicate side effects. It implements jittered exponential backoff for retries and updates relevant functions to utilize this new behavior.
Detailed summary
fetchWithTransportRetryto handle fetch requests with retries.sendOtpandverifyOtpfunctions to usefetchWithTransportRetry.retryfunction to include options for backoff and non-retryable errors.isTransportErrorto identify network errors for retry logic.Summary by CodeRabbit