-
Notifications
You must be signed in to change notification settings - Fork 1
fix: add special handling for user wallet error and don't log to sentry #139
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThree executor utility files are updated to centralize error message handling by importing and using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b7a38c9 to
eea3621
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: 1
📜 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 (6)
.claude/agents/react-19-knowledge.md(1 hunks).claude/commands/react-review.md(1 hunks)apps/agentic-chat/src/main.tsx(2 hunks)apps/agentic-chat/src/utils/sendExecutor.ts(2 hunks)apps/agentic-chat/src/utils/swapExecutor.ts(3 hunks)apps/agentic-chat/src/utils/walletErrors.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/agentic-chat/src/utils/swapExecutor.ts (1)
apps/agentic-chat/src/utils/walletErrors.ts (1)
getUserFriendlyErrorMessage(23-28)
apps/agentic-chat/src/main.tsx (1)
apps/agentic-chat/src/utils/walletErrors.ts (1)
isUserCancellation(5-21)
apps/agentic-chat/src/utils/sendExecutor.ts (1)
apps/agentic-chat/src/utils/walletErrors.ts (1)
getUserFriendlyErrorMessage(23-28)
🔇 Additional comments (12)
.claude/commands/react-review.md (1)
22-50: React 19 categories and checklist align well with best practices.The critical issues (lines 22–30), performance issues, and best-practice violations map cleanly to React 19 semantics and complement the companion knowledge document. The output format (lines 52–88) is clear and actionable for developers.
.claude/agents/react-19-knowledge.md (5)
1-90: Comprehensive and technically accurate React 19 feature reference.The documentation of new features (
use(),useActionState,useOptimistic,useFormStatus, ref-as-prop, context-as-provider,useDeferredValuewith initialValue, document metadata, and resource preloading) is well-structured and aligns with official React 19 guidance.
200-360: Rules of React and useEffect anti-patterns are clear and comprehensive.The "Rules of React" section correctly addresses component purity, hook rules, and immutability. The anti-patterns section thoroughly covers the five common mistakes (data transformation, caching, state reset, event handling, chained effects) with concrete examples. All guidance aligns with React 19 best practices.
386-464: Memoization guidance balances React Compiler capabilities with practical scenarios.The section correctly notes that the React Compiler handles most memoization, while providing clear guidance on when manual
memo,useMemo, anduseCallbackremain necessary. The anti-patterns (lines 451–464) flag common mistakes like objects in dependency arrays and hooks in loops.
586-622: Breaking changes and TypeScript migration guidance is accurate.The table of removed/deprecated APIs (lines 590–600) and TypeScript-specific changes (lines 604–612, ref callback no-implicit-returns,
useRefrequires argument) correctly reflect React 19 migration requirements. Error handling changes withonUncaughtError,onCaughtError, andonRecoverableErroroptions are also documented accurately.
626-655: Red/yellow/green flag checklist is practical and actionable.The quick-reference checklist provides a memorable way to identify issues during code review. Red flags cover must-fix items, yellow flags suggest improvements, and green flags highlight good patterns. This tiered approach is well-suited for code review automation and developer guidance.
apps/agentic-chat/src/main.tsx (1)
21-27: LGTM! Clean Sentry filtering implementation.The
beforeSendhook correctly filters out user wallet cancellations from Sentry reporting. This is appropriate since user cancellations are intentional actions, not errors worth tracking.apps/agentic-chat/src/utils/sendExecutor.ts (1)
37-39: LGTM! Consistent error handling.The error handling is now unified with the centralized
getUserFriendlyErrorMessageutility, providing consistent user-facing messages across send operations.apps/agentic-chat/src/utils/swapExecutor.ts (1)
41-43: LGTM! Unified error handling for swap operations.Both
executeApprovalandexecuteSwapnow use the centralized error utility with appropriate operation labels, ensuring consistent user-facing error messages.Also applies to: 50-52
apps/agentic-chat/src/utils/walletErrors.ts (3)
1-3: LGTM! Well-defined interface.The
WalletErrorinterface correctly extendsErrorwith an optionalcodeproperty to support EIP-1193 error codes.
23-28: Clean utility function.The function correctly delegates to
isUserCancellationand produces clear, consistent messages. The fallback toString(error)for non-Error types is a good defensive pattern.
5-21: Implementation is correct; "expired" messages are intentionally grouped with cancellations for UX simplification.The EIP-1193 code
4001check correctly identifies explicit user rejections. The message-based fallback patterns provide good coverage for various wallet implementations.One note:
'request expired'and'proposal expired'(lines 17-18) are technically timeout/network scenarios rather than explicit user cancellations. However, grouping them with user rejections in the Sentry filter is reasonable—these errors aren't actionable bugs and suppress noise in monitoring. The same grouping in user-facing messages viagetUserFriendlyErrorMessageworks for UX, though it slightly conflates two different failure modes. This is a sensible trade-off if simplified messaging is preferred over distinguishing timeout scenarios.
eea3621 to
b8bff62
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)
apps/agentic-chat/src/utils/swapExecutor.ts (1)
38-43: Preserve original error when rethrowing to avoid losing context and classificationCatching
errorand throwingnew Error(getUserFriendlyErrorMessage(...))improves user-facing messages but discards the original error object (stack, code, and any wallet-specific metadata). That can make debugging harder and may interfere with any higher-level logic (e.g., Sentry filtering or additionalisUserCancellationchecks) that relies on the original error shape rather than just the message.Consider a pattern that keeps the original error when possible while still surfacing the friendly message:
} catch (error) { - throw new Error(getUserFriendlyErrorMessage(error, 'Approval')) + const message = getUserFriendlyErrorMessage(error, 'Approval') + if (error instanceof Error) { + // Preserve original error details while updating the message + error.message = message + throw error + } + throw new Error(message) }…and similarly for the swap path:
} catch (error) { - throw new Error(getUserFriendlyErrorMessage(error, 'Swap')) + const message = getUserFriendlyErrorMessage(error, 'Swap') + if (error instanceof Error) { + error.message = message + throw error + } + throw new Error(message) }This keeps user-friendly messaging while retaining the original error instance for logging and classification.
Also applies to: 47-52
📜 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 (3)
apps/agentic-chat/src/main.tsx(1 hunks)apps/agentic-chat/src/utils/sendExecutor.ts(2 hunks)apps/agentic-chat/src/utils/swapExecutor.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/agentic-chat/src/main.tsx
- apps/agentic-chat/src/utils/sendExecutor.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/agentic-chat/src/utils/swapExecutor.ts (1)
apps/agentic-chat/src/utils/walletErrors.ts (1)
getUserFriendlyErrorMessage(23-28)
🔇 Additional comments (1)
apps/agentic-chat/src/utils/swapExecutor.ts (1)
6-6: Centralizing wallet error messaging viagetUserFriendlyErrorMessagelooks goodImporting
getUserFriendlyErrorMessagefrom@/utils/walletErrorskeeps swap error handling consistent with send/other executors and aligns with the PR goal of centralizing wallet error UX.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.