feat: add new hardware errors #421
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ConnTimeout = 4002, | ||
| ConnBlocked = 4003, | ||
| ConnIframeMissing = 4010, | ||
| ConnSuiteMissing = 4011, |
There was a problem hiding this comment.
Should this be a "new" sub-category? 🤔 4020 instead of 4011?
There was a problem hiding this comment.
Its kind of the trezor iframe suite
There was a problem hiding this comment.
Should we use just use one instead and call it ConnBridgeMissing? Or maybe even ConnTransportBridgeMissing? (I also thought of "Gateway" but "Bridge" is the term we use for this purpose.
WDYT?
| ConnTimeout = 4002, | ||
| ConnBlocked = 4003, | ||
| ConnIframeMissing = 4010, | ||
| ConnSuiteMissing = 4011, |
There was a problem hiding this comment.
Should we use just use one instead and call it ConnBridgeMissing? Or maybe even ConnTransportBridgeMissing? (I also thought of "Gateway" but "Bridge" is the term we use for this purpose.
WDYT?
| originalName: 'Failure_UnknownCode', | ||
| }, | ||
| Init_IframeBlocked: { | ||
| customCode: ErrorCode.ConnBlocked, | ||
| message: 'Iframe blocked', | ||
| severity: Severity.Err, | ||
| category: Category.Connection, | ||
| retryStrategy: RetryStrategy.NoRetry, | ||
| userActionable: true, | ||
| userMessage: | ||
| 'Connection blocked. Please check your browser settings and allow iframes.', | ||
| sdkMessage: 'Iframe blocked', |
There was a problem hiding this comment.
What's the difference between originalName and sdkMessage here? 🤔
Also, we should re-use the constants from Trezor when possible, so 'Iframe blocked' should be Init_IframeBlocked (like the key being using the object here)
Co-authored-by: Charly Chevalier <charly.chevalier@consensys.net>
| (mapping) => mapping.customCode, | ||
| ); | ||
| expect(ledgerCustomCodes.length).toBeGreaterThan(0); | ||
| }); |
There was a problem hiding this comment.
Test claims uniqueness validation but doesn't verify it
Low Severity
The test named "should have unique error codes within each vendor" only asserts that ledgerCustomCodes.length is greater than zero. It doesn't actually verify uniqueness - that would require comparing the array length against a Set of the same values. Looking at the actual error mappings, multiple vendor codes intentionally map to the same customCode (e.g., both 0x6985 and 0x5501 map to UserRejected). The test name is misleading and could give false confidence that uniqueness is being validated when it's not.
| export const HARDWARE_ERROR_MAPPINGS = { | ||
| ledger: { | ||
| vendorName: 'Ledger', | ||
| errorMappings: { |
There was a problem hiding this comment.
Question: Maybe we could just use LEDGER_ERROR_MAPPING for this one?
This PR adds new hardware errors and error codes to the @metamask/keyring-utils.
See: https://consensyssoftware.atlassian.net/browse/MUL-1304?atlOrigin=eyJpIjoiZmYwZmRmNDg1NDkzNDMxMjg2ZTBmNmUwYzQzZTA2ZjgiLCJwIjoiaiJ9
Examples
Note
Adds structured hardware error handling and mappings.
ErrorCode,Severity, andCategoryenums for standardized hardware errorsHARDWARE_ERROR_MAPPINGSfor Ledger (0xcodes → internalErrorCode,Severity,Category,userMessage)HardwareWalletErrorwithid,timestamp,metadata,cause,isCritical(),isWarning(),withMetadata(),toJSON(),toString(), andtoDetailedString()index.tsWritten by Cursor Bugbot for commit ce66cea. This will update automatically on new commits. Configure here.