[SDK] Add error tracking for payment execution failures #8486
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 66e3f43 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 |
WalkthroughAdds analytics tracking calls across the Bridge UI and the useStepExecutor hook, includes wallet info in several tracking payloads, normalizes token address comparison in getWalletBalance, and adds a changeset entry for a patch release. No public API signatures were changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (1)
631-647: Isolate analytics failures so they never break payment error handlingRight now, if
trackPayEventorstringifywere to throw synchronously, it would short‑circuit the rest of thiscatchblock and potentially preventsetError(...)from running. Given this runs in the failure path, the analytics call should be best‑effort and never affect user‑visible error handling.Consider wrapping the tracking call in its own
try/catch:- } catch (err) { - console.error("Error executing payment", err); - trackPayEvent({ - client, - error: err instanceof Error ? err.message : stringify(err), - event: "ub:ui:error", - }); + } catch (err) { + console.error("Error executing payment", err); + try { + trackPayEvent({ + client, + error: err instanceof Error ? err.message : stringify(err), + event: "ub:ui:error", + }); + } catch (trackingError) { + console.error("Failed to track payment error", trackingError); + }This keeps analytics robust without risking changes to the existing error UX.
🧹 Nitpick comments (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (1)
653-665: Addclienttoexecute’s dependency array
executenow closes overclient(viatrackPayEvent), butclientis not listed in the dependency array. For correctness and to satisfy React hook linting, it should typically be included:}, [ executionState, wallet, currentTxIndex, flatTxs, executeSingleTx, executeBatch, executeSendCalls, onrampStatus, executeOnramp, onComplete, - preparedQuote, + preparedQuote, + client, ]);Assuming
clientis stable in practice, this is mostly about keeping dependencies accurate and avoiding future surprises.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Vercel Agent Review
- GitHub Check: Lint Packages
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (1)
16-17: Analytics and JSON helper imports look correctThe new imports for
trackPayEventandstringifyare consistent with existing relative.jsimports and are used appropriately in the error path. No changes needed here.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8486 +/- ##
==========================================
- Coverage 54.70% 54.67% -0.04%
==========================================
Files 920 920
Lines 61046 61106 +60
Branches 4143 4143
==========================================
+ Hits 33398 33408 +10
- Misses 27546 27596 +50
Partials 102 102
🚀 New features to boost your workflow:
|
size-limit report 📦
|
joaquim-verges
force-pushed
the
_SDK_Add_error_tracking_for_payment_execution_failures
branch
from
a8140e8 to
66e3f43
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/react/core/hooks/useStepExecutor.ts (1)
2-14: Use origin chain forchainIdin all execution tracking eventsThe execution tracking events (
ub:ui:execution:start,ub:ui:execution:success,ub:ui:execution:error) are inconsistent with the rest of the Bridge analytics layer:
- Currently:
chainId: preparedQuote.steps[0]?.destinationToken.chainId- Should be:
chainId: preparedQuote.steps[0]?.originToken.chainIdAcross
QuoteLoader,PaymentDetails, andSuccessScreen,chainIdconsistently represents the origin chain whiletoChainIdrepresents the destination. UsingdestinationToken.chainIdforchainIdhere creates an inconsistent(fromToken, chainId)pair and will skew analytics downstream.Apply this fix to all three events:
- chainId: preparedQuote.steps[0]?.destinationToken.chainId, + chainId: preparedQuote.steps[0]?.originToken.chainId,This applies to the start event (line ~496), success event (line ~657), and error event (line ~676).
🧹 Nitpick comments (2)
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx (1)
132-145: Payment-selection analytics now capture wallet context (note snapshot timing)Using
payerWallet?.getAccount()?.address/payerWallet?.idforwalletAddressandwalletTypeis sound and avoids runtime issues with the later declaration ofpayerWallet. Do note this event only runs for the static["payment_selection"]key, so it captures whatever wallet is active on initial mount rather than tracking later wallet/step changes; if you ever need per-wallet granular analytics, you’d want to key or enable the query offpayerWalletas well.If you intend this event to reflect only first-load state, no change is needed; otherwise, consider:
- useQuery({ + useQuery({ queryFn: () => { trackPayEvent({ client, event: "payment_selection", toChainId: destinationToken.chainId, toToken: destinationToken.address, walletAddress: payerWallet?.getAccount()?.address, walletType: payerWallet?.id, }); return true; }, - queryKey: ["payment_selection"], + queryKey: ["payment_selection", payerWallet?.id], });Also applies to: 147-160
packages/thirdweb/src/wallets/utils/getWalletBalance.ts (1)
13-14: Canonical address comparison fix is correct (with slightly earlier failure on invalid input)Normalizing both
tokenAddressandNATIVE_TOKEN_ADDRESSviagetAddressbefore comparing correctly fixes the “native token passed via address” bug and aligns with case-insensitive address semantics. Note this will now throw immediately for malformed addresses instead of failing later during ERC20 calls, which is generally preferable but is a small behaviour change for callers passing invalid input.If you prefer to treat malformed
tokenAddressas “native” instead of erroring, you could defensively wrap the comparison:let isErc20 = false; if (tokenAddress) { try { isErc20 = getAddress(tokenAddress) !== getAddress(NATIVE_TOKEN_ADDRESS); } catch { isErc20 = false; // fall back to native path on invalid address } } if (isErc20) { // ERC20 branch ... }Also applies to: 48-51
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/fifty-moons-punch.md(1 hunks)packages/thirdweb/src/react/core/hooks/useStepExecutor.ts(5 hunks)packages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsx(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx(2 hunks)packages/thirdweb/src/wallets/utils/getWalletBalance.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/fifty-moons-punch.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/wallets/utils/getWalletBalance.tspackages/thirdweb/src/react/core/hooks/useStepExecutor.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Vercel Agent Review
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx (1)
66-88: Success-screen tracking payload extension looks consistentIncluding
walletAddress: preparedQuote.intent.senderfor bothbuy/sellandtransferevents is consistent with how the rest of the Bridge flow models the sender and doesn’t affect control flow. No issues from a correctness or typing perspective.packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx (1)
85-118: Payment-details analytics enrichment is safe and consistentAdding
walletAddress/walletTypefrompaymentMethod.payerWalletto thepayment_detailsevent fits the existing typing and matches how payer wallet is used later (e.g., insenderfallback). Optional chaining keeps this from impacting error paths.packages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsx (1)
110-127: Quote-loader analytics enrichment is straightforwardIncluding
walletAddress: senderin theub:ui:loading_quote:${mode}payload reuses an existing, already-validated prop and doesn’t alter the request/quote flow. No correctness issues here.
joaquim-verges
deleted the
_SDK_Add_error_tracking_for_payment_execution_failures
branch
PR-Codex overview
This PR focuses on enhancing wallet balance retrieval and payment execution tracking in the
thirdwebpackage, particularly by improving how wallet addresses are managed and adding event tracking for payment execution states.Detailed summary
getWalletBalanceto compare token addresses usinggetAddress.walletAddressandwalletTypein payment-related functions.useStepExecutor.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.