Swap failed events: only reject committed txs with code !== 0 #3301
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
WalkthroughRecent updates to the codebase have enhanced error handling in the Changes
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Additional context usedBiome
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (4)
packages/web/components/swap-tool/index.tsx (3)
Line range hint
362-385: Add keyboard accessibility for clickable elements.- <button onClick={sendSwapTx}> + <button onClick={sendSwapTx} onKeyUp={sendSwapTx} onKeyDown={sendSwapTx}>Ensure that actions triggered by mouse clicks are also accessible via keyboard events to improve accessibility.
Also applies to: 390-409
Line range hint
976-977: Remove redundant Boolean conversion.- Boolean(swapState.error) + swapState.errorDirectly use the expression without the
Booleancall for cleaner and more efficient code.
Line range hint
386-386: Specify button type to prevent form submission.- <button onClick={...}> + <button type="button" onClick={...}>Explicitly set the button type to "button" to avoid unintended form submissions when the button is within a form element.
Also applies to: 595-616, 758-770
packages/web/hooks/use-swap.tsx (1)
Line range hint
513-513: Remove redundant Boolean conversion.The
Booleanconversion here is redundant as the expression itself is sufficient for a truthy/falsy evaluation.- Boolean(quote?.amount.toDec().isPositive()) && + quote?.amount.toDec().isPositive() &&
packages/web/hooks/use-swap.tsx
Outdated
| ({ code }) => { | ||
| if (code) | ||
| reject( | ||
| "Failed to send split route swap exact amount in message" | ||
| ); | ||
| else resolve("multiroute"); |
There was a problem hiding this comment.
Refactor error handling for clarity and maintainability.
Similar to the previous comment, the error handling here can be improved by providing more information about the failure.
- reject("Failed to send split route swap exact amount in message");
+ reject(`Failed to send split route swap exact amount in message with error code: ${code}`);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ({ code }) => { | |
| if (code) | |
| reject( | |
| "Failed to send split route swap exact amount in message" | |
| ); | |
| else resolve("multiroute"); | |
| ({ code }) => { | |
| if (code) | |
| reject(`Failed to send split route swap exact amount in message with error code: ${code}`); | |
| else resolve("multiroute"); |
packages/web/hooks/use-swap.tsx
Outdated
| ({ code }) => { | ||
| if (code) | ||
| reject("Failed to send swap exact amount in message"); | ||
| else resolve(pools.length === 1 ? "exact-in" : "multihop"); |
There was a problem hiding this comment.
Refactor error handling for clarity and maintainability.
The error handling in the sendTradeTokenInTx function could be made clearer by using a more descriptive error message. Consider including details about the error code when the transaction fails.
- reject("Failed to send swap exact amount in message");
+ reject(`Failed to send swap exact amount in message with error code: ${code}`);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ({ code }) => { | |
| if (code) | |
| reject("Failed to send swap exact amount in message"); | |
| else resolve(pools.length === 1 ? "exact-in" : "multihop"); | |
| ({ code }) => { | |
| if (code) | |
| reject(`Failed to send swap exact amount in message with error code: ${code}`); | |
| else resolve(pools.length === 1 ? "exact-in" : "multihop"); |
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (2)
packages/web/hooks/use-swap.tsx (2)
Line range hint
200-200: Remove redundantBooleancall for cleaner code.- Boolean(swapAssets.fromAsset) && Boolean(swapAssets.toAsset); + swapAssets.fromAsset && swapAssets.toAsset;
Line range hint
510-510: Remove redundantBooleancall for cleaner code.- Boolean(quoteForCurrentBalance) && !account?.txTypeInProgress; + quoteForCurrentBalance && !account?.txTypeInProgress;
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (3)
packages/web/components/swap-tool/index.tsx (3)
Line range hint
363-386: Add keyboard event handlers for accessibility.Actions triggered by mouse events should also be accessible via keyboard events to support keyboard-only users. Consider adding
onKeyUp,onKeyDown, oronKeyPresshandlers to these elements.Also applies to: 391-410
Line range hint
977-978: Remove redundant Boolean conversion.The
Booleancall here is unnecessary as the value is already coerced into a boolean by the conditional context. Simplify the code by removing theBooleancall.- Boolean(swapState.error) + swapState.error
Line range hint
387-387: Specify button types explicitly.The default type of a button in HTML is 'submit', which can cause unintended form submissions if placed inside a form. Specify the type explicitly to 'button' to prevent this behavior.
+ type="button"Add this property to all buttons that do not intend to submit a form.
Also applies to: 596-617, 759-771
| ({ code }) => { | ||
| if (code) | ||
| reject( | ||
| new Error("Failed to send swap exact amount in message") | ||
| ); | ||
| else resolve(pools.length === 1 ? "exact-in" : "multihop"); |
There was a problem hiding this comment.
Refactor error handling to include error codes for better diagnostics.
- reject(new Error("Failed to send swap exact amount in message"));
+ reject(new Error(`Failed to send swap exact amount in message with error code: ${code}`));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ({ code }) => { | |
| if (code) | |
| reject( | |
| new Error("Failed to send swap exact amount in message") | |
| ); | |
| else resolve(pools.length === 1 ? "exact-in" : "multihop"); | |
| ({ code }) => { | |
| if (code) | |
| reject( | |
| new Error(`Failed to send swap exact amount in message with error code: ${code}`) | |
| ); | |
| else resolve(pools.length === 1 ? "exact-in" : "multihop"); |
Before, we logged events if the broadcast failed (could be network error or signature rejection). Now, the promise that rejects that causes the error to be logged should largely come from failed txs on chain. Other lower priority errors, like random network errors or signature rejections, should be ignored.