-
Notifications
You must be signed in to change notification settings - Fork 602
SDK: Show error in BuyWidget and SwapWidget UI if fetching token details fails #8215
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
Conversation
🦋 Changeset detectedLatest commit: 62cfbb7 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.
|
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. |
WalkthroughAdds explicit error propagation for token detail/price fetches across Bridge UI components. Selected token objects gain an isError flag, balance shape is expanded, SwapWidget disables price-query retries and surfaces granular error messages. A patch changeset was added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Bridge UI
participant TF as Token Fetcher
participant Q as Quote Preparer
User->>UI: Open Swap/Fund flow
UI->>TF: Fetch sell/buy token details & prices
alt Token fetch succeeds
TF-->>UI: Token data (with prices)
UI->>Q: Prepare quote (if applicable)
alt Quote succeeds
Q-->>UI: Quote
UI-->>User: Render token info and quote
else Quote fails
Q-->>UI: Error
UI-->>User: Show "Failed to prepare quote" error
end
else Token fetch fails or unsupported
TF-->>UI: Error (isError = true)
UI-->>User: Show "Failed to fetch token details" error (skip token visuals)
end
sequenceDiagram
autonumber
participant UI as SwapWidget
participant ST as Sell Token Query
participant BT as Buy Token Query
UI->>ST: Query sell token (price query: retry=false)
UI->>BT: Query buy token (price query: retry=false)
par Collect states
ST-->>UI: { data | isError }
BT-->>UI: { data | isError }
end
alt ST.isError
UI-->>User: Show "Failed to fetch sell token details"
else BT.isError
UI-->>User: Show "Failed to fetch buy token details"
else No errors
UI-->>User: Render token visuals and proceed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx (1)
395-412
: Simplify the nested ternary for better readability.While the error handling logic is correct and differentiates between quote preparation failure and token fetch failures, the deeply nested ternary reduces readability.
Apply this diff to simplify the error message logic:
- > - {preparedResultQuery.error - ? "Failed to get a quote" - : buyTokenQuery.isError - ? "Failed to fetch buy token details" - : sellTokenQuery.isError - ? "Failed to fetch sell token details" - : "Failed to get a quote"} - </Text> + > + {(() => { + if (preparedResultQuery.error) return "Failed to get a quote"; + if (buyTokenQuery.isError) return "Failed to fetch buy token details"; + if (sellTokenQuery.isError) return "Failed to fetch sell token details"; + return "Failed to get a quote"; + })()} + </Text>Alternatively, extract the error message to a helper function:
function getErrorMessage( preparedResultError: Error | null, buyTokenError: boolean, sellTokenError: boolean, ): string { if (preparedResultError) return "Failed to get a quote"; if (buyTokenError) return "Failed to fetch buy token details"; if (sellTokenError) return "Failed to fetch sell token details"; return "Failed to get a quote"; }
📜 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 (4)
.changeset/rare-buckets-fly.md
(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
(5 hunks)packages/thirdweb/src/react/web/ui/Bridge/common/selected-token-button.tsx
(3 hunks)packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/thirdweb/src/react/web/ui/Bridge/common/selected-token-button.tsx
🧰 Additional context used
📓 Path-based instructions (4)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/*.md
: Each change inpackages/*
must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API
Files:
.changeset/rare-buckets-fly.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}
: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/types
or localtypes.ts
barrels
Prefer type aliases over interface except for nominal shapes
Avoidany
andunknown
unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial
,Pick
, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}
: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/types
where applicable
Prefertype
aliases overinterface
except for nominal shapes
Avoidany
andunknown
unless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}
: Every public symbol must have comprehensive TSDoc with at least one compiling@example
and a custom tag (@beta
,@internal
,@experimental
, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf")
)
Files:
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-24T11:08:43.783Z
Learnt from: MananTank
PR: thirdweb-dev/js#8106
File: packages/thirdweb/src/react/web/ui/Bridge/bridge-widget/bridge-widget.tsx:34-41
Timestamp: 2025-09-24T11:08:43.783Z
Learning: In BridgeWidgetProps for packages/thirdweb/src/react/web/ui/Bridge/bridge-widget/bridge-widget.tsx, the Swap onError callback signature requires a non-undefined SwapPreparedQuote parameter (unlike Buy's onError which allows undefined quote). This is intentional - SwapWidget's onError is only called when a quote is available.
Applied to files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
📚 Learning: 2025-10-03T23:36:00.631Z
Learnt from: MananTank
PR: thirdweb-dev/js#8181
File: packages/thirdweb/src/react/web/ui/Bridge/BuyWidget.tsx:27-27
Timestamp: 2025-10-03T23:36:00.631Z
Learning: In packages/thirdweb/src/react/web/ui/Bridge/BuyWidget.tsx, the component intentionally uses a hardcoded English locale (connectLocaleEn) rather than reading from the provider, as BuyWidget is designed to be English-only and does not require internationalization support.
Applied to files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
🧬 Code graph analysis (1)
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx (5)
packages/thirdweb/src/react/core/design-system/index.ts (3)
radius
(188-196)spacing
(174-186)fontSize
(164-172)packages/thirdweb/src/react/web/ui/components/text.tsx (1)
Text
(18-34)packages/thirdweb/src/bridge/types/Token.ts (1)
TokenWithPrices
(14-16)packages/thirdweb/src/react/web/ui/components/Skeleton.tsx (1)
Skeleton
(10-28)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/formatTokenBalance.ts (1)
formatTokenAmount
(26-35)
⏰ 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). (8)
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
.changeset/rare-buckets-fly.md (1)
1-5
: LGTM!The changeset correctly documents this as a patch-level change. The UI improvement for error handling in BuyWidget and SwapWidget does not modify the public API, making patch the appropriate version bump level.
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx (3)
99-99
: LGTM! Good UX decision.Disabling automatic retries allows immediate error feedback to the user rather than waiting through multiple retry attempts. This aligns well with the PR objective of showing errors promptly.
319-327
: LGTM! Consistent error state propagation.The
isError
flag is correctly propagated from token queries to the selectedToken payloads for both sell and buy paths, enabling downstream components to render appropriate error states.Also applies to: 372-380
628-653
: LGTM! Type definition updated correctly.The
TokenSectionProps
type now includesisError: boolean
in the selectedToken union, which is consistent with the changes throughout the file and aligns with the error handling enhancements.packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx (5)
254-257
: LGTM! Comprehensive error detection.The
isError
flag correctly captures both token query failures and unsupported token scenarios, ensuring proper error state propagation to child components.
285-299
: LGTM! Clear error feedback for users.The global error banner provides consistent, visible feedback when token details fail to fetch. The styling with borders and spacing aligns with the design system, and the error message is clear and actionable.
404-411
: LGTM! Type definition matches implementation.The balance type definition has been updated to reflect the new object shape with
value
,decimals
,symbol
, andname
properties, maintaining type safety throughout the component.
595-605
: LGTM! Rendering logic updated correctly.The balance rendering now correctly accesses the nested properties (
balance.data.value
,balance.data.decimals
,balance.data.symbol
) from the new object shape, and the usage informatTokenAmount
is consistent with the function signature.
260-263
: Balance data shape is correctuseTokenBalance
(viauseWalletBalance
) returns aUseQueryResult<GetBalanceResult>
whereGetBalanceResult
includesvalue
,decimals
,symbol
, andname
as expected.
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8215 +/- ##
==========================================
- Coverage 55.03% 55.02% -0.02%
==========================================
Files 919 919
Lines 60569 60583 +14
Branches 4126 4126
==========================================
Hits 33333 33333
- Misses 27132 27146 +14
Partials 104 104
🚀 New features to boost your workflow:
|
Merge activity
|
…ils fails (#8215) <!-- ## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes" If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000): ## Notes for the reviewer Anything important to call out? Be sure to also clarify these in your comments. ## How to test Unit tests, playground, etc. --> <!-- start pr-codex --> --- ## PR-Codex overview This PR introduces error handling in the `BuyWidget` and `SwapWidget` UIs, displaying appropriate error messages when fetching token details fails. ### Detailed summary - Added `isError` state to handle errors in token fetching. - Updated `SelectedTokenButton` to check for errors before rendering. - Enhanced `SwapUI` to show error messages based on different query states. - Implemented error display in `FundWallet` for unsupported tokens. - Modified balance display logic to accommodate new error handling. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Inline error banners in Buy and Swap widgets when token details fail to load. - Clear, specific messages for quote, buy-token, and sell-token fetch failures. - Improvements - Selected token UI now falls back gracefully on errors instead of showing partial data. - Balance display uses richer token info (value, decimals, symbol, name) for accuracy. - More consistent handling of unsupported tokens across the flow. - Bug Fixes - Prevents stale token icons/details from appearing during error states. - Disables auto-retry on token price fetches to reduce UI flicker. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
9612ee8
to
62cfbb7
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx (1)
285-299
: Disable the CTA when token details erroredWe now surface the “Failed to fetch token details” banner, but the main Continue button stays enabled because it only depends on
receiver
. In this error state the click path is a no-op (we immediately return whendestinationToken
isundefined
), which is confusing. Please gate the button on the same error condition—e.g. includetokenQuery.isError || tokenQuery.data?.type === "unsupported_token"
in thedisabled
prop—so the UI feedback and CTA state stay in sync.
📜 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 (4)
.changeset/rare-buckets-fly.md
(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
(5 hunks)packages/thirdweb/src/react/web/ui/Bridge/common/selected-token-button.tsx
(3 hunks)packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/*.md
: Each change inpackages/*
must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API
Files:
.changeset/rare-buckets-fly.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}
: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/types
or localtypes.ts
barrels
Prefer type aliases over interface except for nominal shapes
Avoidany
andunknown
unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial
,Pick
, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}
: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/types
where applicable
Prefertype
aliases overinterface
except for nominal shapes
Avoidany
andunknown
unless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/react/web/ui/Bridge/common/selected-token-button.tsx
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/react/web/ui/Bridge/common/selected-token-button.tsx
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}
: Every public symbol must have comprehensive TSDoc with at least one compiling@example
and a custom tag (@beta
,@internal
,@experimental
, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf")
)
Files:
packages/thirdweb/src/react/web/ui/Bridge/common/selected-token-button.tsx
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-24T11:08:43.783Z
Learnt from: MananTank
PR: thirdweb-dev/js#8106
File: packages/thirdweb/src/react/web/ui/Bridge/bridge-widget/bridge-widget.tsx:34-41
Timestamp: 2025-09-24T11:08:43.783Z
Learning: In BridgeWidgetProps for packages/thirdweb/src/react/web/ui/Bridge/bridge-widget/bridge-widget.tsx, the Swap onError callback signature requires a non-undefined SwapPreparedQuote parameter (unlike Buy's onError which allows undefined quote). This is intentional - SwapWidget's onError is only called when a quote is available.
Applied to files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
📚 Learning: 2025-10-03T23:36:00.631Z
Learnt from: MananTank
PR: thirdweb-dev/js#8181
File: packages/thirdweb/src/react/web/ui/Bridge/BuyWidget.tsx:27-27
Timestamp: 2025-10-03T23:36:00.631Z
Learning: In packages/thirdweb/src/react/web/ui/Bridge/BuyWidget.tsx, the component intentionally uses a hardcoded English locale (connectLocaleEn) rather than reading from the provider, as BuyWidget is designed to be English-only and does not require internationalization support.
Applied to files:
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx
⏰ 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). (4)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
PR-Codex overview
This PR focuses on enhancing the error handling in the
BuyWidget
andSwapWidget
UIs by displaying error messages when fetching token details fails.Detailed summary
isError
state to various components to track errors in fetching token details.SelectedTokenButton
,SwapUI
, andFundWallet
.Summary by CodeRabbit
New Features
Improvements
Bug Fixes