fix: properly surface Treezor user rejection errors#8490
Open
Olivier-BB wants to merge 10 commits into
Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
|
@copilot resolve the merge conflicts in this pull request |
…ve-error-trezor # Conflicts: # packages/keyring-controller/package.json # packages/transaction-controller/src/TransactionController.ts Co-authored-by: dawnseeker8 <7315988+dawnseeker8@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MetaMask/core/sessions/64a5b953-7f1d-4e47-be62-0f18a2f761f9 Co-authored-by: dawnseeker8 <7315988+dawnseeker8@users.noreply.github.com>
Resolved the merge conflicts in commit
The yarn.lock was updated accordingly, and all 1,400+ tests across both affected packages continue to pass. |
Contributor
|
@Olivier-BB please update this branch and fix CI so we can get this reviewed and merged. |
Contributor
|
@copilot resolve the merge conflicts in this pull request |
…essage methods - Introduced tests to ensure that errors with code 4001 and string rejections are normalized to user rejection errors. - Added checks to verify that non-rejection errors are re-thrown unchanged, including those with circular references. - Included a test for normalizing cancellation-like errors in signPersonalMessage to a 4001 code.
- Consolidated multiple lines of jest.spyOn calls into single lines for improved readability. - Maintained existing test functionality while enhancing code clarity.
Document the user-rejection error normalization changes in keyring-controller and transaction-controller, satisfying the changelog check for PR #8490. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
@copilot resolve the merge conflicts in this pull request |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Treezor user rejection errors were not being surfaced clearly, which could result in uninformative error handling around rejected requests.
This change updates the relevant controller flows so Treezor user rejection errors are surfaced properly, and adds test coverage in both
keyring-controllerandtransaction-controllerto verify the behavior.KeyringControllernow catchessignMessage/signPersonalMessage/signTransactionerrors (and conditionallysignTypedMessage) and normalizes cancellation-like errors by recursively inspectingcode,message,stack,cause, andoriginalError.TransactionControllersimilarly detects cancellation-like errors (including a Trezor/"unknown error" string case), persists them asuserRejectedRequest, and updates tests to assert the persisted errorcode.The
TransactionControllertests for "cancelled"/"canceled" signing error normalization have been updated to use theKeyringController:signTransactionmessenger action handler (rather than the legacysignoption) to properly exercise the error path, reflecting the current signing architecture.References
Checklist
Note
Medium Risk
Changes the error type and message surfaced when users reject signing, which affects dapps and UI; heuristic string/code matching could theoretically mislabel non-rejection failures.
Overview
Signing flows now map hardware-wallet and other cancellation-like failures to a standard
userRejectedRequest(4001) provider error instead of opaque or wrapped errors.KeyringControlleradds@metamask/rpc-errorsand wrapssignMessage,signPersonalMessage,signTransaction, and (when applicable)signTypedMessageso rejections detected viacode4001, message/stack patterns (user rejected,cancelled/canceled,failure_actioncancelled), and nestedcause/originalErrorbecomeproviderErrors.userRejectedRequestwith a fixed MetaMask denial message (optionaldatapreserved). Non-rejection errors are unchanged;signTypedMessagestill wraps other failures inKeyringControllerError.TransactionControlleradds similar heuristics (including a Trezorunknown errorstring case) and, on failed txs, persists and emits the normalized rejection error ontransactionFailed. Tests cover both controllers and theKeyringController:signTransactionmessenger path.Reviewed by Cursor Bugbot for commit ae7c32a. Bugbot is set up for automated code reviews on this repo. Configure here.