Repository navigation
fix(errors): show friendly message when wallet rejects order signing - #7781
Conversation
Safe/WalletConnect wallets (via viem) wrap a user's signing rejection in a TransactionExecutionError whose top-level shortMessage is "An unknown RPC error occurred."; the real 4001 code and "User rejected" message only live on error.cause. isRejectRequestProviderError only inspected the top-level error, so these rejections fell through to getSwapErrorMessage's generic branch and surfaced the raw "An unknown RPC error" to the user instead of "User rejected signing the order". Walk the error.cause chain (depth-capped to guard against cyclic causes), mirroring the recursive detection the TWAP flow already uses. Every consumer (swap, limit, eth, approval) now maps the wrapped rejection correctly.
|
@tenderdeve is attempting to deploy a commit to the cow-dev Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdates ChangesRejection Detection Recursion
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/common-utils/src/misc.ts (1)
186-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
anyusage violates the no-anyguideline.
error: anyis explicitly disallowed for production code, even though it's flagged with a TODO/eslint-disable. Consider typing asunknownand narrowing (typeof error === 'object' && error !== null) similar togetProviderErrorMessage, to avoid unchecked property access onerror.code/error.cause.As per coding guidelines, "MUST NOT use
anyor non-null assertions (!) in production code."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/common-utils/src/misc.ts` around lines 186 - 188, The `isRejectRequestProviderError` helper still uses `error: any`, which violates the no-`any` guideline. Change the `error` parameter to `unknown` and narrow it before accessing `code` or `cause`, following the same object-check pattern used in `getProviderErrorMessage`. Remove the `@typescript-eslint/no-explicit-any` suppression and keep the recursive `depth` logic intact.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@libs/common-utils/src/misc.ts`:
- Around line 186-188: The `isRejectRequestProviderError` helper still uses
`error: any`, which violates the no-`any` guideline. Change the `error`
parameter to `unknown` and narrow it before accessing `code` or `cause`,
following the same object-check pattern used in `getProviderErrorMessage`.
Remove the `@typescript-eslint/no-explicit-any` suppression and keep the
recursive `depth` logic intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ca7d40b8-4dd0-4c94-a250-c85a7c65771c
📒 Files selected for processing (2)
libs/common-utils/src/misc.test.tslibs/common-utils/src/misc.ts
elena-zh
added
the
trigger-preview
|
Cloudflare Pages preview mirror Preview branch URL: https://github.com/cowprotocol/cowswap/tree/cf-preview/pr-7781 Source fork branch:
|
github-actions
Bot
removed
the
trigger-preview
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
✅ AI Review (Codex GPT-5, worked 4m): no new non-duplicate findings
Review completed. I found no new non-duplicate comments worth posting.
Review scope and related context
- Checked
libs/common-utils/src/misc.tsandlibs/common-utils/src/misc.test.tsagainstorigin/develop...origin/pr/7781. - The PR matches the linked Safe/WalletConnect rejection issue and adds focused coverage for direct, nested, non-rejection,
null/undefined, and direct cyclic cause cases. - Existing CodeRabbit feedback already covers the still-valid root
AGENTS.mdissue:error: anyinisRejectRequestProviderErroratlibs/common-utils/src/misc.ts:188. I’m not duplicating that. - No active review threads were returned; the PR already has one human approval.
Generated using the pr-review skill from the CoW Protocol skills repo.
Fixes #7774
When a Safe/WalletConnect wallet rejects the order-signing request, viem wraps the rejection in a
TransactionExecutionErrorwhose top-levelshortMessageis"An unknown RPC error occurred.". The real4001code and "User rejected" message only live onerror.cause.isRejectRequestProviderErroronly looked at the top-level error, so those rejections missed detection andgetSwapErrorMessagesurfaced the raw "An unknown RPC error" instead of "User rejected signing the order".Now it walks the
error.causechain (depth-capped against cyclic causes), the same recursive approach the TWAP flow already uses. Fixes it once for every flow that relies on the util — swap, limit, eth, approval.Added
libs/common-utils/src/misc.test.tscovering the top-level code/message cases, the nested-cause case from the report, non-rejection errors, and the cyclic-cause guard.Summary by CodeRabbit
Bug Fixes
false.Tests