Skip to content

fix: properly surface Treezor user rejection errors#8490

Open
Olivier-BB wants to merge 1 commit intomainfrom
fix/41184-uninformative-error-trezor
Open

fix: properly surface Treezor user rejection errors#8490
Olivier-BB wants to merge 1 commit intomainfrom
fix/41184-uninformative-error-trezor

Conversation

@Olivier-BB
Copy link
Copy Markdown

@Olivier-BB Olivier-BB commented Apr 16, 2026

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-controller and transaction-controller to verify the behavior.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Touches core signing/transaction error handling, so incorrect normalization could change downstream UX and retry logic, but changes are localized and covered by new tests.

Overview
Ensures signing cancellation/user-rejection failures are consistently surfaced as EIP-1193 userRejectedRequest (code 4001) across KeyringController message/typed-message/transaction signing and TransactionController transaction failure persistence.

Adds recursive detection/normalization for “cancelled/canceled/user rejected” style errors (including nested cause/originalError and a Treezor/Trezor-specific string case), updates tests to assert the normalized error shape/code, and introduces @metamask/rpc-errors as a keyring-controller dependency.

Reviewed by Cursor Bugbot for commit 6fae189. Bugbot is set up for automated code reviews on this repo. Configure here.

@Olivier-BB Olivier-BB requested review from a team as code owners April 16, 2026 14:05
@Olivier-BB Olivier-BB added this to Bugs Apr 16, 2026
@Olivier-BB Olivier-BB removed this from Bugs Apr 16, 2026
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6fae189. Configure here.

this.#hasUserRejectedMessage(
error as Error & { cause?: unknown; originalError?: unknown },
visited,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error cause/originalError never checked for Error instances

High Severity

In #hasUserRejectedMessage, the instanceof Error branch recurses with error as Error & { cause?: unknown; originalError?: unknown } — but that type assertion is compile-time only, so it passes the same object reference back. Since error was already added to visited, the recursive call immediately returns false, meaning cause and originalError are never inspected for Error instances. The typeof error === 'object' branch below correctly checks those properties, but it's unreachable for Errors. Compare with #isSigningUserRejectedError in KeyringController.ts which correctly accesses each property individually.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6fae189. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant