Viem Migration - use short message when canceling transaction#7473
Viem Migration - use short message when canceling transaction#7473brunota20 wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughThe PR enhances error handling in ChangesProvider Rejection Error Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Deploying swap-dev with
|
| Latest commit: |
498b672
|
| Status: | 🚫 Build failed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@apps/cowswap-frontend/src/modules/accountProxy/hooks/useRecoverFundsCallback.ts`:
- Around line 35-39: In useRecoverFundsCallback's catch block, avoid directly
reading e.shortMessage/e.message because caught value may be non-object; update
the handler (around isRejectRequestProviderError, handleSetError) to first check
typeof e === 'object' && e !== null (and optionally 'in' checks for
shortMessage/message) and then extract message = (e && (e as any).shortMessage)
|| (e && (e as any).message) || String(e); keep the existing branch for
isRejectRequestProviderError(e) and call handleSetError(message) using this safe
extraction so non-object throws don't cause a secondary crash.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 210b6b9e-aee3-41db-bf04-e2fe96d8c83b
📒 Files selected for processing (2)
apps/cowswap-frontend/src/locales/en-US.poapps/cowswap-frontend/src/modules/accountProxy/hooks/useRecoverFundsCallback.ts
| if (isRejectRequestProviderError(e)) { | ||
| handleSetError(t`User rejected the request.`) | ||
| } else { | ||
| const message = e.shortMessage || e.message || e.toString() | ||
| handleSetError(message) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether catch variables are treated strictly and locate similar unsafe patterns.
echo "== tsconfig useUnknownInCatchVariables =="
fd -i '^tsconfig.*\.json$' | xargs -I{} sh -c 'echo "--- {} ---"; cat "{}" | jq -r ".compilerOptions.useUnknownInCatchVariables // \"not-set\""' 2>/dev/null
echo
echo "== Potentially unsafe property reads in catch blocks =="
rg -nP --type=ts --type=tsx -C2 'catch\s*\(([^)]*)\)\s*\{[\s\S]{0,300}\b\1\.(shortMessage|message)\b'Repository: cowprotocol/cowswap
Length of output: 4384
🏁 Script executed:
cat -n apps/cowswap-frontend/src/modules/accountProxy/hooks/useRecoverFundsCallback.ts | head -50Repository: cowprotocol/cowswap
Length of output: 1779
🏁 Script executed:
# Search for catch blocks that access .message or .shortMessage without narrowing
rg --type=ts --type=ts -C3 'catch\s*\([^)]*\)' | grep -A3 -E '\.(shortMessage|message|toString)\(' | head -60Repository: cowprotocol/cowswap
Length of output: 2376
🏁 Script executed:
# Look for how .message is accessed in catch blocks across the repo
rg -C2 'catch\s*\(\s*(\w+)\s*\)' --type=ts --type=ts | rg -B2 -A2 '\1\.(message|shortMessage)' | head -80Repository: cowprotocol/cowswap
Length of output: 257
🏁 Script executed:
# Search for patterns accessing error.message or error.shortMessage in catch blocks
rg 'catch\s*\([^)]*\)' -A5 --type=ts --type=ts | rg -B3 '\.(message|shortMessage)' | head -100Repository: cowprotocol/cowswap
Length of output: 5958
🏁 Script executed:
# Check how isRejectRequestProviderError is defined to understand what it does
fd -i 'isRejectRequestProviderError' -t f | head -5 | xargs -I {} sh -c 'echo "=== {} ==="; head -30 "{}"'Repository: cowprotocol/cowswap
Length of output: 45
🏁 Script executed:
# Search for isRejectRequestProviderError definition
rg 'isRejectRequestProviderError' --type=ts --type=ts -B2 -A10 | head -80Repository: cowprotocol/cowswap
Length of output: 8233
🏁 Script executed:
# Find where getProviderErrorMessage is defined
rg 'export.*getProviderErrorMessage' --type=ts --type=ts -A10 | head -50Repository: cowprotocol/cowswap
Length of output: 790
🏁 Script executed:
# Check if getProviderErrorMessage is imported/available in the useRecoverFundsCallback file
head -50 apps/cowswap-frontend/src/modules/accountProxy/hooks/useRecoverFundsCallback.ts | grep -i providerRepository: cowprotocol/cowswap
Length of output: 181
Guard catch error before reading shortMessage/message to avoid secondary crash.
Line 38 accesses e.shortMessage || e.message without narrowing the catch variable, which will crash if a non-object value is thrown (since useUnknownInCatchVariables is not enabled in tsconfig). This breaks error handling itself.
Suggested fix
+type ErrorWithMessages = {
+ shortMessage?: string
+ message?: string
+}
+
+function getReadableErrorMessage(error: unknown): string {
+ if (typeof error === 'object' && error !== null) {
+ const candidate = error as ErrorWithMessages
+ if (candidate.shortMessage) return candidate.shortMessage
+ if (candidate.message) return candidate.message
+ }
+ return String(error)
+}
+
if (isRejectRequestProviderError(e)) {
handleSetError(t`User rejected the request.`)
} else {
- const message = e.shortMessage || e.message || e.toString()
+ const message = getReadableErrorMessage(e)
handleSetError(message)
}🤖 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
`@apps/cowswap-frontend/src/modules/accountProxy/hooks/useRecoverFundsCallback.ts`
around lines 35 - 39, In useRecoverFundsCallback's catch block, avoid directly
reading e.shortMessage/e.message because caught value may be non-object; update
the handler (around isRejectRequestProviderError, handleSetError) to first check
typeof e === 'object' && e !== null (and optionally 'in' checks for
shortMessage/message) and then extract message = (e && (e as any).shortMessage)
|| (e && (e as any).message) || String(e); keep the existing branch for
isRejectRequestProviderError(e) and call handleSetError(message) using this safe
extraction so non-object throws don't cause a secondary crash.
|
something broke on the cowfi vercel deployment |
…sage-for-recover-funds
|
Hey @brunota20 , could you please link a notion task of the issue you're fixing to the PR? |
4 participants
Summary
Shortens the error rejection message shown in the "Recover Funds" error modal. Previously, rejecting the wallet signature displayed the full verbose viem
error: "User rejected the request. Details: User rejected the request. Version: viem@2.47.1". Now it shows just "User rejected the request."
Uses
isRejectRequestProviderError()from@cowprotocol/common-utils— the same pattern already used in TWAP and swap error handling. For non-rejectionerrors, prefers viem's
shortMessageover the fullmessage.https://www.notion.so/cownation/Shorten-error-rejection-message-for-recover-funds-35a8da5f04ca80edbbb9e346fd381cdd?v=2fc8da5f04ca81ada4ca000c320d51ef&source=copy_link
To Test
Summary by CodeRabbit