-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
feat: log swap failures to amplitude #6789
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -441,6 +441,11 @@ export function Swap({ | |||
}) | |||
}) | |||
.catch((error) => { | |||
if (!didUserReject(error)) { | |||
sendAnalyticsEvent(SwapEventName.SWAP_ERROR, { | |||
confirmedTrade: tradeToConfirm, |
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.
are you sure that sending a full object will be okay here? if yes then all good
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.
there should be a way to test sending into the amplitude dev environment if you want to try that. i worry you won't be able to filter properly this way
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.
1 flaky tests on run #12029 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
Permit2 > approval process (with intermediate screens) > swaps after completing full permit2 approval process |
Output
Screenshots
Video
|
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
Description
logs swap failures to amplitude for trend tracking.
Linear ticket: https://linear.app/uniswap/issue/WEB-2307/log-swap-failures-to-amplitude
Screen capture
Test plan
Reproducing the error
tradeToConfirm
is properly included in theQA (ie manual testing)