-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore: refactor send events to be more consistent #5351
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: cd36fdd The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 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.
10 Skipped Deployments
|
Visual Regression Test Results ❌ FailedChromatic Build: https://www.chromatic.com/build?appId=6493191bf4b10fed8ca7353f&number=495 👉 Please review the visual changes in Chromatic and accept or reject them. |
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.
Pull Request Overview
This PR refactors send-related event tracking to improve consistency by centralizing all event firing in the SendController.sendToken() method rather than having events scattered across UI components and various helper methods.
Key Changes:
- Moved
SEND_INITIATED,SEND_SUCCESS,SEND_REJECTED, andSEND_ERRORevent tracking from UI and helper methods into the mainsendToken()method - Replaced
returnstatements withbreakstatements in the chain namespace switch to allow the success event to fire after send completion - Removed duplicate event tracking calls from
sendEvmToken(),sendNativeToken(),sendERC20Token(), andsendSolanaToken()
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/scaffold-ui/src/views/w3m-wallet-send-preview-view/index.ts | Removed error event tracking from UI layer (SEND_REJECTED/SEND_ERROR), moving responsibility to controller |
| packages/controllers/src/controllers/SendController.ts | Centralized all send-related event tracking in sendToken() method; added Event type import; replaced return with break statements in switch cases; removed duplicate event calls from helper methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Bundle Size Check✅ All bundles are within size limits 📊 View detailed bundle sizes> @reown/appkit-monorepo@1.7.1 size /home/runner/work/appkit/appkit > size-limit |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
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.
Bug: Stale Hash Corrupts Transaction Events
The sendEvmToken method only updates state.hash when hash is truthy, but resetSend doesn't clear state.hash. If a transaction returns no hash, the SEND_SUCCESS event will incorrectly report the hash from a previous transaction instead of an empty string, causing event tracking to associate the wrong transaction hash with the current send operation.
packages/controllers/src/controllers/SendController.ts#L208-L228
appkit/packages/controllers/src/controllers/SendController.ts
Lines 208 to 228 in 24da3af
| if (SendController.state.token?.address) { | |
| const { hash } = await SendController.sendERC20Token({ | |
| receiverAddress: SendController.state.receiverAddress, | |
| tokenAddress: SendController.state.token.address, | |
| sendTokenAmount: SendController.state.sendTokenAmount, | |
| decimals: SendController.state.token.quantity.decimals | |
| }) | |
| if (hash) { | |
| state.hash = hash | |
| } | |
| } else { | |
| const { hash } = await SendController.sendNativeToken({ | |
| receiverAddress: SendController.state.receiverAddress, | |
| sendTokenAmount: SendController.state.sendTokenAmount, | |
| decimals: SendController.state.token.quantity.decimals | |
| }) | |
| if (hash) { | |
| state.hash = hash | |
| } |
| event: 'SEND_REJECTED' | ||
| }) | ||
| } else { | ||
| EventsController.sendEvent(event) |
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.
Bug: EVM user rejection shows generic error instead of specific message
The refactoring removes the conversion of user rejection errors to UserRejectedRequestError (which was done via throw new UserRejectedRequestError(err) in the old code). The new code just re-throws the original error (throw err). However, the UI in w3m-wallet-send-preview-view checks error.originalName === 'UserRejectedRequestError' to decide whether to show the original error message. Since the error is no longer wrapped, originalName won't match for EVM chains, causing users to see "Failed to send transaction. Please try again." instead of the actual rejection message like "User rejected request".
Description
Type of change
Associated Issues
For Linear issues: Closes APKT-4271
Checklist
Note
Centralizes all send-flow telemetry in
SendController.sendToken, removes event emissions from UI, and updates tests to cover new event lifecycle and error handling.packages/controllers)SendController.sendToken: emitsSEND_INITIATED,SEND_SUCCESS,SEND_REJECTED/SEND_ERRORwith unified properties viagetSdkEventProperties.sendEvmToken/sendNativeToken/sendERC20Token/sendSolanaToken; keep balance updates; callresetSendonly after overall success; preservestate.hashon reset.setLoadingaround send), minor control-flow fixes, and add error-path telemetry.fetchTokenBalance: add retry gating andSnackController.showError('Token Balance Unavailable')on failure.packages/scaffold-ui)w3m-wallet-send-preview-view; delegate toSendController; show success/error snacks only.SEND_INITIATED→SEND_SUCCESS/SEND_REJECTED/SEND_ERRORlifecycle, Solana path viasendToken, loading state, and UI snack errors.@reown/appkit-controllersand@reown/appkit-scaffold-uiwith improved send-flow events handling.Written by Cursor Bugbot for commit cd36fdd. This will update automatically on new commits. Configure here.