diff --git a/packages/keyring-controller/CHANGELOG.md b/packages/keyring-controller/CHANGELOG.md index d6c02c0fe6c..b906dab6170 100644 --- a/packages/keyring-controller/CHANGELOG.md +++ b/packages/keyring-controller/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added new `KeyringBuilder` type ([#7334](https://github.com/MetaMask/core/pull/7334)) - Added an action to call `removeAccount` ([#7241](https://github.com/MetaMask/core/pull/7241)) - This action is meant to be consumed by the `MultichainAccountService` to encapsulate the act of removing a wallet when seed phrase backup fails in the clients. +- Added new `KeyringControllerError` ([#7498](https://github.com/MetaMask/core/pull/7498)) + - All controller's errors are now using this error type. + - Keyring instance operation errors are also now also wrapped with this error type. ### Changed diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 174837a44e1..7cf33dff3f8 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -25,7 +25,8 @@ import { bytesToHex, isValidHexAddress } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; import sinon from 'sinon'; -import { KeyringControllerError } from './constants'; +import { KeyringControllerErrorMessage } from './constants'; +import { KeyringControllerError } from './errors'; import type { KeyringControllerEvents, KeyringControllerMessenger, @@ -48,6 +49,10 @@ import MockEncryptor, { SALT, } from '../tests/mocks/mockEncryptor'; import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring'; +import { + HardwareWalletError, + MockHardwareKeyring, +} from '../tests/mocks/mockHardwareKeyring'; import { MockKeyring } from '../tests/mocks/mockKeyring'; import MockShallowKeyring from '../tests/mocks/mockShallowKeyring'; import { buildMockTransaction } from '../tests/mocks/mockTransaction'; @@ -333,7 +338,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.addNewAccount()).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -405,7 +410,7 @@ describe('KeyringController', () => { hdPath: "m/44'/60'/0'/0", }); await expect(controller.addNewAccount()).rejects.toThrow( - KeyringControllerError.DuplicatedAccount, + KeyringControllerErrorMessage.DuplicatedAccount, ); }); }); @@ -531,7 +536,7 @@ describe('KeyringController', () => { await expect( controller.addNewAccountForKeyring(keyring as EthKeyring), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -573,7 +578,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.addNewKeyring(KeyringTypes.hd)).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -638,7 +643,7 @@ describe('KeyringController', () => { await withController(async ({ controller }) => { await expect( controller.createNewVaultAndRestore('', uint8ArraySeed), - ).rejects.toThrow(KeyringControllerError.InvalidEmptyPassword); + ).rejects.toThrow(KeyringControllerErrorMessage.InvalidEmptyPassword); }); }); @@ -723,7 +728,7 @@ describe('KeyringController', () => { // @ts-expect-error invalid password 123, ), - ).rejects.toThrow(KeyringControllerError.WrongPasswordType); + ).rejects.toThrow(KeyringControllerErrorMessage.WrongPasswordType); }, ); }); @@ -735,7 +740,51 @@ describe('KeyringController', () => { async ({ controller }) => { await expect( controller.createNewVaultAndKeychain(password), - ).rejects.toThrow(KeyringControllerError.NoFirstAccount); + ).rejects.toThrow(KeyringControllerErrorMessage.NoFirstAccount); + }, + ); + }); + + it('should throw error when HD keyring does not support generateRandomMnemonic', async () => { + // Create a custom HD keyring that doesn't support generateRandomMnemonic + class MockHdKeyringWithoutMnemonic { + static type = 'HD Key Tree'; + + type = 'HD Key Tree'; + + async getAccounts(): Promise { + return []; + } + + async addAccounts(): Promise { + return []; + } + + serialize = async (): Promise<{ type: string }> => ({ + type: this.type, + }); + + deserialize = async (): Promise => { + // noop + }; + } + + const mockBuilder = keyringBuilderFactory( + MockHdKeyringWithoutMnemonic as unknown as KeyringClass, + ); + + await withController( + { + skipVaultCreation: true, + keyringBuilders: [mockBuilder], + }, + async ({ controller }) => { + // Try to create a new vault, which will attempt to generate a mnemonic + await expect( + controller.createNewVaultAndKeychain(password), + ).rejects.toThrow( + KeyringControllerErrorMessage.UnsupportedGenerateRandomMnemonic, + ); }, ); }); @@ -801,7 +850,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.setLocked()).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -883,7 +932,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.exportSeedPhrase(password)).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -942,7 +991,9 @@ describe('KeyringController', () => { await expect( controller.exportAccount(password, address), - ).rejects.toThrow(KeyringControllerError.UnsupportedExportAccount); + ).rejects.toThrow( + KeyringControllerErrorMessage.UnsupportedExportAccount, + ); }, ); }); @@ -963,7 +1014,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.getAccounts()).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -1020,7 +1071,7 @@ describe('KeyringController', () => { await expect( controller.getEncryptionPublicKey(address), ).rejects.toThrow( - KeyringControllerError.UnsupportedGetEncryptionPublicKey, + KeyringControllerErrorMessage.UnsupportedGetEncryptionPublicKey, ); }, ); @@ -1035,7 +1086,7 @@ describe('KeyringController', () => { controller.getEncryptionPublicKey( initialState.keyrings[0].accounts[0], ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1109,7 +1160,9 @@ describe('KeyringController', () => { ciphertext: '0xabcdef1234567890', }, }), - ).rejects.toThrow(KeyringControllerError.UnsupportedDecryptMessage); + ).rejects.toThrow( + KeyringControllerErrorMessage.UnsupportedDecryptMessage, + ); }, ); }); @@ -1129,7 +1182,7 @@ describe('KeyringController', () => { ciphertext: '0xabcdef1234567890', }, }), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1193,7 +1246,7 @@ describe('KeyringController', () => { controller.getKeyringForAccount( '0x51253087e6f8358b5f10c0a94315d69db3357859', ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1204,7 +1257,7 @@ describe('KeyringController', () => { await expect( controller.getKeyringForAccount(initialState.keyrings[0].accounts[0]), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1239,7 +1292,7 @@ describe('KeyringController', () => { await controller.setLocked(); expect(() => controller.getKeyringsByType(KeyringTypes.hd)).toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -1264,7 +1317,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.persistAllKeyrings()).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -1420,7 +1473,7 @@ describe('KeyringController', () => { input, somePassword, ]), - ).rejects.toThrow(KeyringControllerError.DuplicatedAccount); + ).rejects.toThrow(KeyringControllerErrorMessage.DuplicatedAccount); }); }); }); @@ -1491,7 +1544,7 @@ describe('KeyringController', () => { AccountImportStrategy.privateKey, [input, 'password'], ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1512,7 +1565,7 @@ describe('KeyringController', () => { await withController(async ({ controller, initialState }) => { const account = initialState.keyrings[0].accounts[0] as Hex; await expect(controller.removeAccount(account)).rejects.toThrow( - KeyringControllerError.LastAccountInPrimaryKeyring, + KeyringControllerErrorMessage.LastAccountInPrimaryKeyring, ); expect(controller.state.keyrings).toHaveLength(1); expect(controller.state.keyrings[0].accounts).toHaveLength(1); @@ -1524,7 +1577,9 @@ describe('KeyringController', () => { await controller.addNewKeyring(KeyringTypes.hd); await expect( controller.removeAccount(controller.state.keyrings[0].accounts[0]), - ).rejects.toThrow(KeyringControllerError.LastAccountInPrimaryKeyring); + ).rejects.toThrow( + KeyringControllerErrorMessage.LastAccountInPrimaryKeyring, + ); }); }); @@ -1602,7 +1657,7 @@ describe('KeyringController', () => { await controller.addNewKeyring(MockKeyring.type); await expect(controller.removeAccount(address)).rejects.toThrow( - KeyringControllerError.UnsupportedRemoveAccount, + KeyringControllerErrorMessage.UnsupportedRemoveAccount, ); }, ); @@ -1615,7 +1670,7 @@ describe('KeyringController', () => { await expect( controller.removeAccount(initialState.keyrings[0].accounts[0]), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1675,7 +1730,7 @@ describe('KeyringController', () => { }; await expect(controller.signMessage(inputParams)).rejects.toThrow( - KeyringControllerError.UnsupportedSignMessage, + KeyringControllerErrorMessage.UnsupportedSignMessage, ); }, ); @@ -1692,7 +1747,7 @@ describe('KeyringController', () => { data: '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0', origin: 'https://metamask.github.io', }), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1761,7 +1816,7 @@ describe('KeyringController', () => { await expect( controller.signPersonalMessage(inputParams), ).rejects.toThrow( - KeyringControllerError.UnsupportedSignPersonalMessage, + KeyringControllerErrorMessage.UnsupportedSignPersonalMessage, ); }, ); @@ -1778,7 +1833,7 @@ describe('KeyringController', () => { data: '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0', origin: 'https://metamask.github.io', }), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -1838,7 +1893,7 @@ describe('KeyringController', () => { nonce, }), ).rejects.toThrow( - KeyringControllerError.MissingEip7702AuthorizationContractAddress, + KeyringControllerErrorMessage.MissingEip7702AuthorizationContractAddress, ); }); }, @@ -1862,7 +1917,7 @@ describe('KeyringController', () => { nonce, }), ).rejects.toThrow( - KeyringControllerError.UnsupportedSignEip7702Authorization, + KeyringControllerErrorMessage.UnsupportedSignEip7702Authorization, ); }, ); @@ -1893,7 +1948,7 @@ describe('KeyringController', () => { 'junk' as SignTypedDataVersion, ), ).rejects.toThrow( - "Keyring Controller signTypedMessage: Error: Unexpected signTypedMessage version: 'junk'", + "Keyring Controller signTypedMessage: KeyringControllerError: Unexpected signTypedMessage version: 'junk'", ); }); }); @@ -2119,7 +2174,7 @@ describe('KeyringController', () => { await expect( controller.signTypedMessage(inputParams, SignTypedDataVersion.V1), ).rejects.toThrow( - KeyringControllerError.UnsupportedSignTypedMessage, + KeyringControllerErrorMessage.UnsupportedSignTypedMessage, ); }, ); @@ -2150,7 +2205,7 @@ describe('KeyringController', () => { }, SignTypedDataVersion.V1, ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -2232,7 +2287,7 @@ describe('KeyringController', () => { await expect( controller.signTransaction(buildMockTransaction(), address), ).rejects.toThrow( - KeyringControllerError.UnsupportedSignTransaction, + KeyringControllerErrorMessage.UnsupportedSignTransaction, ); }, ); @@ -2248,7 +2303,7 @@ describe('KeyringController', () => { buildMockTransaction(), initialState.keyrings[0].accounts[0], ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -2324,7 +2379,7 @@ describe('KeyringController', () => { await expect( controller.prepareUserOperation(address, [], executionContext), ).rejects.toThrow( - KeyringControllerError.UnsupportedPrepareUserOperation, + KeyringControllerErrorMessage.UnsupportedPrepareUserOperation, ); }, ); @@ -2341,7 +2396,7 @@ describe('KeyringController', () => { [], executionContext, ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -2427,7 +2482,7 @@ describe('KeyringController', () => { await expect( controller.patchUserOperation(address, userOp, executionContext), ).rejects.toThrow( - KeyringControllerError.UnsupportedPatchUserOperation, + KeyringControllerErrorMessage.UnsupportedPatchUserOperation, ); }, ); @@ -2456,7 +2511,7 @@ describe('KeyringController', () => { }, executionContext, ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -2539,7 +2594,7 @@ describe('KeyringController', () => { await expect( controller.signUserOperation(address, userOp, executionContext), ).rejects.toThrow( - KeyringControllerError.UnsupportedSignUserOperation, + KeyringControllerErrorMessage.UnsupportedSignUserOperation, ); }, ); @@ -2568,7 +2623,7 @@ describe('KeyringController', () => { }, executionContext, ), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -2594,7 +2649,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(async () => controller.changePassword('')).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -2602,7 +2657,7 @@ describe('KeyringController', () => { it('should throw error if the new password is an empty string', async () => { await withController(async ({ controller }) => { await expect(controller.changePassword('')).rejects.toThrow( - KeyringControllerError.InvalidEmptyPassword, + KeyringControllerErrorMessage.InvalidEmptyPassword, ); }); }); @@ -2612,7 +2667,7 @@ describe('KeyringController', () => { await expect( // @ts-expect-error we are testing wrong input controller.changePassword(undefined), - ).rejects.toThrow(KeyringControllerError.WrongPasswordType); + ).rejects.toThrow(KeyringControllerErrorMessage.WrongPasswordType); }); }); @@ -2622,7 +2677,7 @@ describe('KeyringController', () => { await expect(async () => controller.changePassword('whatever'), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); }); }); }); @@ -2680,7 +2735,7 @@ describe('KeyringController', () => { }, async ({ controller }) => { await expect(controller.submitPassword(password)).rejects.toThrow( - KeyringControllerError.VaultDataError, + KeyringControllerErrorMessage.VaultDataError, ); }, ); @@ -2691,7 +2746,7 @@ describe('KeyringController', () => { { skipVaultCreation: true }, async ({ controller }) => { await expect(controller.submitPassword(password)).rejects.toThrow( - KeyringControllerError.VaultError, + KeyringControllerErrorMessage.VaultError, ); }, ); @@ -2699,7 +2754,13 @@ describe('KeyringController', () => { it('should throw an error if the encryptor returns an undefined encryption key', async () => { await withController( - { skipVaultCreation: true, state: { vault: createVault() } }, + { + skipVaultCreation: true, + state: { + vault: createVault(), + encryptionKey: 'existing-key', + } as KeyringControllerState, + }, async ({ controller, encryptor }) => { jest.spyOn(encryptor, 'decryptWithDetail').mockResolvedValueOnce({ vault: defaultKeyrings, @@ -2709,7 +2770,7 @@ describe('KeyringController', () => { }); await expect(controller.submitPassword(password)).rejects.toThrow( - KeyringControllerError.MissingCredentials, + KeyringControllerErrorMessage.MissingCredentials, ); }, ); @@ -3018,7 +3079,7 @@ describe('KeyringController', () => { await expect( // @ts-expect-error we are testing wrong input controller.submitPassword(123456), - ).rejects.toThrow(KeyringControllerError.WrongPasswordType); + ).rejects.toThrow(KeyringControllerErrorMessage.WrongPasswordType); }); }); @@ -3201,7 +3262,7 @@ describe('KeyringController', () => { MOCK_ENCRYPTION_KEY, initialState.encryptionSalt as string, ), - ).rejects.toThrow(KeyringControllerError.VaultDataError); + ).rejects.toThrow(KeyringControllerErrorMessage.VaultDataError); }); }); @@ -3209,7 +3270,7 @@ describe('KeyringController', () => { await withController(async ({ controller }) => { await expect( controller.submitEncryptionKey(MOCK_ENCRYPTION_KEY, '0x1234'), - ).rejects.toThrow(KeyringControllerError.ExpiredCredentials); + ).rejects.toThrow(KeyringControllerErrorMessage.ExpiredCredentials); }); }); @@ -3222,7 +3283,7 @@ describe('KeyringController', () => { 12341234, SALT, ), - ).rejects.toThrow(KeyringControllerError.WrongEncryptionKeyType); + ).rejects.toThrow(KeyringControllerErrorMessage.WrongEncryptionKeyType); }); }); }); @@ -3245,7 +3306,7 @@ describe('KeyringController', () => { await withController(async ({ controller }) => { await controller.setLocked(); await expect(controller.exportEncryptionKey()).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -3310,7 +3371,7 @@ describe('KeyringController', () => { { skipVaultCreation: true }, async ({ controller }) => { await expect(controller.verifySeedPhrase()).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }, ); @@ -3322,7 +3383,7 @@ describe('KeyringController', () => { const keyringId = controller.state.keyrings[1].metadata.id; await expect(controller.verifySeedPhrase(keyringId)).rejects.toThrow( - KeyringControllerError.UnsupportedVerifySeedPhrase, + KeyringControllerErrorMessage.UnsupportedVerifySeedPhrase, ); }); }); @@ -3337,7 +3398,7 @@ describe('KeyringController', () => { await controller.submitPassword(password); await expect(controller.verifySeedPhrase()).rejects.toThrow( - KeyringControllerError.KeyringNotFound, + KeyringControllerErrorMessage.KeyringNotFound, ); }, ); @@ -3348,7 +3409,7 @@ describe('KeyringController', () => { await controller.setLocked(); await expect(controller.verifySeedPhrase()).rejects.toThrow( - KeyringControllerError.ControllerLocked, + KeyringControllerErrorMessage.ControllerLocked, ); }); }); @@ -3369,7 +3430,7 @@ describe('KeyringController', () => { { skipVaultCreation: true }, async ({ controller }) => { await expect(controller.verifyPassword(password)).rejects.toThrow( - KeyringControllerError.VaultError, + KeyringControllerErrorMessage.VaultError, ); }, ); @@ -3394,7 +3455,7 @@ describe('KeyringController', () => { { skipVaultCreation: true }, async ({ controller }) => { await expect(controller.verifyPassword('123')).rejects.toThrow( - KeyringControllerError.VaultError, + KeyringControllerErrorMessage.VaultError, ); }, ); @@ -3458,7 +3519,9 @@ describe('KeyringController', () => { return keyring; }, ), - ).rejects.toThrow(KeyringControllerError.UnsafeDirectKeyringAccess); + ).rejects.toThrow( + KeyringControllerErrorMessage.UnsafeDirectKeyringAccess, + ); }); }); @@ -3469,7 +3532,7 @@ describe('KeyringController', () => { const fn = jest.fn(); await expect(controller.withKeyring(selector, fn)).rejects.toThrow( - KeyringControllerError.KeyringNotFound, + KeyringControllerErrorMessage.KeyringNotFound, ); expect(fn).not.toHaveBeenCalled(); }); @@ -3663,7 +3726,9 @@ describe('KeyringController', () => { controller.withKeyring(selector, async ({ keyring }) => { return keyring; }), - ).rejects.toThrow(KeyringControllerError.UnsafeDirectKeyringAccess); + ).rejects.toThrow( + KeyringControllerErrorMessage.UnsafeDirectKeyringAccess, + ); }); }); @@ -3676,7 +3741,7 @@ describe('KeyringController', () => { await expect( controller.withKeyring(selector, fn), - ).rejects.toThrow(KeyringControllerError.KeyringNotFound); + ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound); expect(fn).not.toHaveBeenCalled(); }, ); @@ -3690,13 +3755,44 @@ describe('KeyringController', () => { await expect( controller.withKeyring(selector, fn, { createIfMissing: true }), - ).rejects.toThrow(KeyringControllerError.KeyringNotFound); + ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound); expect(fn).not.toHaveBeenCalled(); }, ); }); }); }); + + it('should throw KeyringNotFound if keyring metadata is not found (internal consistency check)', async () => { + // This test verifies the defensive #getKeyringMetadata guard that ensures + // internal state consistency. In normal operation, this should never occur, + // but the guard exists to catch potential data corruption scenarios where + // a keyring exists but its metadata is not in the internal keyrings array. + await withController(async ({ controller }) => { + // Mock getKeyringForAccount to return a keyring that isn't in the internal array + // This simulates an inconsistent internal state + const mockOrphanKeyring: Partial = { + type: 'OrphanKeyring', + getAccounts: jest.fn().mockResolvedValue([]), + }; + + jest + .spyOn(controller, 'getKeyringForAccount') + .mockResolvedValue(mockOrphanKeyring as EthKeyring); + + const selector = { + address: '0x1234567890123456789012345678901234567890' as Hex, + }; + const fn = jest.fn(); + + // This should trigger the #getKeyringMetadata error because mockOrphanKeyring + // is not in the internal #keyrings array + await expect(controller.withKeyring(selector, fn)).rejects.toThrow( + KeyringControllerErrorMessage.KeyringNotFound, + ); + expect(fn).not.toHaveBeenCalled(); + }); + }); }); describe('isCustodyKeyring', () => { @@ -4123,12 +4219,19 @@ describe('KeyringController', () => { it('should not cause a deadlock when subscribing to state changes', async () => { await withController(async ({ controller, initialState, messenger }) => { - let executed = false; + let callCount = 0; + const noOp = async (): Promise => { + // No operation for subsequent calls + }; + const persistAction = async (): Promise => { + await controller.persistAllKeyrings(); + }; + const actions: (() => Promise)[] = [persistAction, noOp, noOp]; const listener = jest.fn(async () => { - if (!executed) { - executed = true; - await controller.persistAllKeyrings(); - } + callCount += 1; + // Only execute persistAllKeyrings on the first call to prevent infinite loops + const actionIndex = Math.min(callCount - 1, actions.length - 1); + await actions[actionIndex](); }); messenger.subscribe( @@ -4260,6 +4363,202 @@ describe('KeyringController', () => { ); }); }); + + describe('KeyringControllerError', () => { + describe('error features', () => { + it('should support error codes', () => { + const error = new KeyringControllerError('Test error', { + code: 'TEST_CODE', + }); + + expect(error.code).toBe('TEST_CODE'); + expect(error.message).toBe('Test error'); + expect(error.name).toBe('KeyringControllerError'); + }); + + it('should support additional data', () => { + const error = new KeyringControllerError('Test error', { + context: { key: 'value', number: 42 }, + }); + + expect(error.context).toStrictEqual({ key: 'value', number: 42 }); + }); + + it('should support error chaining with cause', () => { + const originalError = new Error('Original error'); + const error = new KeyringControllerError('Wrapped error', { + cause: originalError, + }); + + expect(error.cause).toBe(originalError); + expect(error.originalError).toBe(originalError); + }); + + it('should support backward compatibility with Error as second param', () => { + const originalError = new Error('Original error'); + const error = new KeyringControllerError( + 'Wrapped error', + originalError, + ); + + expect(error.cause).toBe(originalError); + expect(error.originalError).toBe(originalError); + }); + + it('should serialize to JSON correctly', () => { + const originalError = new Error('Original error'); + const error = new KeyringControllerError('Test error', { + code: 'TEST_CODE', + context: { key: 'value' }, + cause: originalError, + }); + + const json = error.toJSON(); + + expect(json.name).toBe('KeyringControllerError'); + expect(json.message).toBe('Test error'); + expect(json.code).toBe('TEST_CODE'); + expect(json.context).toStrictEqual({ key: 'value' }); + expect(json.cause).toStrictEqual({ + name: 'Error', + message: 'Original error', + stack: originalError.stack, + }); + }); + + it('should serialize to JSON without cause if not present', () => { + const error = new KeyringControllerError('Test error', { + code: 'TEST_CODE', + }); + + const json = error.toJSON(); + + expect(json.cause).toBeUndefined(); + }); + + it('should convert to string with code', () => { + const error = new KeyringControllerError('Test error', { + code: 'TEST_CODE', + }); + + const str = error.toString(); + + expect(str).toContain('KeyringControllerError'); + expect(str).toContain('Test error'); + expect(str).toContain('[TEST_CODE]'); + }); + + it('should convert to string with cause', () => { + const originalError = new Error('Original error'); + const error = new KeyringControllerError('Test error', { + cause: originalError, + }); + + const str = error.toString(); + + expect(str).toContain('KeyringControllerError: Test error'); + expect(str).toContain('Caused by: Error: Original error'); + }); + + it('should convert to string with both code and cause', () => { + const originalError = new Error('Original error'); + const error = new KeyringControllerError('Test error', { + code: 'TEST_CODE', + cause: originalError, + }); + + const str = error.toString(); + + expect(str).toContain('KeyringControllerError: Test error'); + expect(str).toContain('[TEST_CODE]'); + expect(str).toContain('Caused by: Error: Original error'); + }); + }); + }); + + describe('error handling', () => { + describe('when hardware wallet throws custom error', () => { + it('should preserve hardware wallet error in originalError property', async () => { + const mockHardwareKeyringBuilder = keyringBuilderFactory( + MockHardwareKeyring as unknown as KeyringClass, + ); + + await withController( + { + keyringBuilders: [mockHardwareKeyringBuilder], + }, + async ({ controller }) => { + // Add the hardware keyring + await controller.addNewKeyring('Mock Hardware'); + // Get all accounts - the hardware wallet should be the second keyring + const allAccounts = await controller.getAccounts(); + // Use the hardware wallet address (last one added) + const hardwareAddress = allAccounts[allAccounts.length - 1]; + + const typedData = { + types: { + EIP712Domain: [ + { name: 'name', type: 'string' }, + { name: 'version', type: 'string' }, + ], + Message: [{ name: 'content', type: 'string' }], + }, + primaryType: 'Message', + domain: { + name: 'Test', + version: '1', + }, + message: { + content: 'Hello!', + }, + }; + + await expect( + controller.signTypedMessage( + { data: JSON.stringify(typedData), from: hardwareAddress }, + SignTypedDataVersion.V4, + ), + ).rejects.toThrow(KeyringControllerError); + + // Verify the error details by catching it explicitly + let caughtError: unknown; + try { + await controller.signTypedMessage( + { data: JSON.stringify(typedData), from: hardwareAddress }, + SignTypedDataVersion.V4, + ); + } catch (error) { + caughtError = error; + } + + // Verify the error is a KeyringControllerError (wrapped by signTypedMessage) + expect(caughtError).toBeInstanceOf(KeyringControllerError); + + const keyringError = caughtError as KeyringControllerError; + + // Verify the error message contains information about the hardware wallet error + expect(keyringError.message).toContain( + 'Keyring Controller signTypedMessage', + ); + expect(keyringError.message).toContain('HardwareWalletError'); + expect(keyringError.message).toContain( + 'User rejected the request on hardware device', + ); + + // Verify the original hardware wallet error is preserved in originalError + expect(keyringError.cause).toBeInstanceOf(HardwareWalletError); + expect(keyringError.cause?.message).toBe( + 'User rejected the request on hardware device', + ); + expect(keyringError.cause?.name).toBe('HardwareWalletError'); + expect((keyringError.cause as HardwareWalletError).code).toBe( + 'USER_REJECTED', + ); + }, + ); + }); + }); + }); }); type WithControllerCallback = ({ diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 2b148798f6c..1c9e8ac5cd7 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -36,7 +36,8 @@ import { cloneDeep, isEqual } from 'lodash'; // When generating a ULID within the same millisecond, monotonicFactory provides some guarantees regarding sort order. import { ulid } from 'ulid'; -import { KeyringControllerError } from './constants'; +import { KeyringControllerErrorMessage } from './constants'; +import { KeyringControllerError } from './errors'; import type { Eip7702AuthorizationParams, PersonalMessageParams, @@ -581,7 +582,7 @@ function assertHasUint8ArrayMnemonic( hasProperty(keyring, 'mnemonic') && keyring.mnemonic instanceof Uint8Array ) ) { - throw new Error("Can't get mnemonic bytes from keyring"); + throw new KeyringControllerError("Can't get mnemonic bytes from keyring"); } } @@ -593,11 +594,15 @@ function assertHasUint8ArrayMnemonic( */ function assertIsValidPassword(password: unknown): asserts password is string { if (typeof password !== 'string') { - throw new Error(KeyringControllerError.WrongPasswordType); + throw new KeyringControllerError( + KeyringControllerErrorMessage.WrongPasswordType, + ); } if (!password?.length) { - throw new Error(KeyringControllerError.InvalidEmptyPassword); + throw new KeyringControllerError( + KeyringControllerErrorMessage.InvalidEmptyPassword, + ); } } @@ -611,7 +616,9 @@ function assertIsEncryptionKeySet( encryptionKey: string | undefined, ): asserts encryptionKey is string { if (!encryptionKey) { - throw new Error(KeyringControllerError.EncryptionKeyNotSet); + throw new KeyringControllerError( + KeyringControllerErrorMessage.EncryptionKeyNotSet, + ); } } @@ -814,19 +821,21 @@ export class KeyringController< | EthKeyring | undefined; if (!primaryKeyring) { - throw new Error('No HD keyring found'); + throw new KeyringControllerError('No HD keyring found'); } const oldAccounts = await primaryKeyring.getAccounts(); if (accountCount && oldAccounts.length !== accountCount) { if (accountCount > oldAccounts.length) { - throw new Error('Account out of sequence'); + throw new KeyringControllerError('Account out of sequence'); } // we return the account already existing at index `accountCount` const existingAccount = oldAccounts[accountCount]; if (!existingAccount) { - throw new Error(`Can't find account at index ${accountCount}`); + throw new KeyringControllerError( + `Can't find account at index ${accountCount}`, + ); } return existingAccount; @@ -861,7 +870,7 @@ export class KeyringController< if (accountCount && oldAccounts.length !== accountCount) { if (accountCount > oldAccounts.length) { - throw new Error('Account out of sequence'); + throw new KeyringControllerError('Account out of sequence'); } const existingAccount = oldAccounts[accountCount]; @@ -954,7 +963,9 @@ export class KeyringController< */ async verifyPassword(password: string): Promise { if (!this.state.vault) { - throw new Error(KeyringControllerError.VaultError); + throw new KeyringControllerError( + KeyringControllerErrorMessage.VaultError, + ); } await this.#encryptor.decrypt(password, this.state.vault); } @@ -983,7 +994,7 @@ export class KeyringController< await this.verifyPassword(password); const selectedKeyring = this.#getKeyringByIdOrDefault(keyringId); if (!selectedKeyring) { - throw new Error('Keyring not found'); + throw new KeyringControllerError('Keyring not found'); } assertHasUint8ArrayMnemonic(selectedKeyring); @@ -1002,7 +1013,9 @@ export class KeyringController< const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.exportAccount) { - throw new Error(KeyringControllerError.UnsupportedExportAccount); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedExportAccount, + ); } return await keyring.exportAccount(normalize(address) as Hex); @@ -1037,7 +1050,9 @@ export class KeyringController< const address = ethNormalize(account) as Hex; const keyring = (await this.getKeyringForAccount(account)) as EthKeyring; if (!keyring.getEncryptionPublicKey) { - throw new Error(KeyringControllerError.UnsupportedGetEncryptionPublicKey); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedGetEncryptionPublicKey, + ); } return await keyring.getEncryptionPublicKey(address, opts); @@ -1059,7 +1074,9 @@ export class KeyringController< const address = ethNormalize(messageParams.from) as Hex; const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.decryptMessage) { - throw new Error(KeyringControllerError.UnsupportedDecryptMessage); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedDecryptMessage, + ); } return keyring.decryptMessage(address, messageParams.data); @@ -1101,8 +1118,8 @@ export class KeyringController< } else if (!winners.length) { errorInfo = 'There are keyrings, but none match the address'; } - throw new Error( - `${KeyringControllerError.NoKeyring}. Error info: ${errorInfo}`, + throw new KeyringControllerError( + `${KeyringControllerErrorMessage.NoKeyring}. Error info: ${errorInfo}`, ); } @@ -1159,7 +1176,7 @@ export class KeyringController< case AccountImportStrategy.privateKey: { const [importedKey] = args; if (!importedKey) { - throw new Error('Cannot import an empty key.'); + throw new KeyringControllerError('Cannot import an empty key.'); } const prefixed = add0x(importedKey); @@ -1167,7 +1184,9 @@ export class KeyringController< try { bufferedPrivateKey = hexToBytes(prefixed); } catch { - throw new Error('Cannot import invalid private key.'); + throw new KeyringControllerError( + 'Cannot import invalid private key.', + ); } if ( @@ -1175,7 +1194,9 @@ export class KeyringController< // ensures that the key is 64 bytes long getBinarySize(prefixed) !== 64 + '0x'.length ) { - throw new Error('Cannot import invalid private key.'); + throw new KeyringControllerError( + 'Cannot import invalid private key.', + ); } privateKey = remove0x(prefixed); @@ -1193,7 +1214,9 @@ export class KeyringController< break; } default: - throw new Error(`Unexpected import strategy: '${String(strategy)}'`); + throw new KeyringControllerError( + `Unexpected import strategy: '${String(strategy)}'`, + ); } const newKeyring = await this.#newKeyring(KeyringTypes.simple, [ privateKey, @@ -1225,12 +1248,16 @@ export class KeyringController< // Primary keyring should never be removed, so we need to keep at least one account in it if (isPrimaryKeyring && shouldRemoveKeyring) { - throw new Error(KeyringControllerError.LastAccountInPrimaryKeyring); + throw new KeyringControllerError( + KeyringControllerErrorMessage.LastAccountInPrimaryKeyring, + ); } // Not all the keyrings support this, so we have to check if (!keyring.removeAccount) { - throw new Error(KeyringControllerError.UnsupportedRemoveAccount); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedRemoveAccount, + ); } // The `removeAccount` method of snaps keyring is async. We have to update @@ -1281,13 +1308,15 @@ export class KeyringController< this.#assertIsUnlocked(); if (!messageParams.data) { - throw new Error("Can't sign an empty message"); + throw new KeyringControllerError("Can't sign an empty message"); } const address = ethNormalize(messageParams.from) as Hex; const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.signMessage) { - throw new Error(KeyringControllerError.UnsupportedSignMessage); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedSignMessage, + ); } return await keyring.signMessage(address, messageParams.data); @@ -1308,8 +1337,8 @@ export class KeyringController< const keyring = (await this.getKeyringForAccount(from)) as EthKeyring; if (!keyring.signEip7702Authorization) { - throw new Error( - KeyringControllerError.UnsupportedSignEip7702Authorization, + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedSignEip7702Authorization, ); } @@ -1319,8 +1348,8 @@ export class KeyringController< | undefined; if (contractAddress === undefined) { - throw new Error( - KeyringControllerError.MissingEip7702AuthorizationContractAddress, + throw new KeyringControllerError( + KeyringControllerErrorMessage.MissingEip7702AuthorizationContractAddress, ); } @@ -1344,7 +1373,9 @@ export class KeyringController< const address = ethNormalize(messageParams.from) as Hex; const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.signPersonalMessage) { - throw new Error(KeyringControllerError.UnsupportedSignPersonalMessage); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedSignPersonalMessage, + ); } const normalizedData = normalize(messageParams.data) as Hex; @@ -1374,7 +1405,9 @@ export class KeyringController< SignTypedDataVersion.V4, ].includes(version) ) { - throw new Error(`Unexpected signTypedMessage version: '${version}'`); + throw new KeyringControllerError( + `Unexpected signTypedMessage version: '${version}'`, + ); } // Cast to `Hex` here is safe here because `messageParams.from` is not nullish. @@ -1382,7 +1415,9 @@ export class KeyringController< const address = ethNormalize(messageParams.from) as Hex; const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.signTypedData) { - throw new Error(KeyringControllerError.UnsupportedSignTypedMessage); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedSignTypedMessage, + ); } return await keyring.signTypedData( @@ -1394,9 +1429,14 @@ export class KeyringController< { version }, ); } catch (error) { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - throw new Error(`Keyring Controller signTypedMessage: ${error}`); + const errorMessage = + error instanceof Error + ? `${error.name}: ${error.message}` + : String(error); + throw new KeyringControllerError( + `Keyring Controller signTypedMessage: ${errorMessage}`, + error instanceof Error ? error : undefined, + ); } } @@ -1417,7 +1457,9 @@ export class KeyringController< const address = ethNormalize(from) as Hex; const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.signTransaction) { - throw new Error(KeyringControllerError.UnsupportedSignTransaction); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedSignTransaction, + ); } return await keyring.signTransaction(address, transaction, opts); @@ -1441,7 +1483,9 @@ export class KeyringController< const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.prepareUserOperation) { - throw new Error(KeyringControllerError.UnsupportedPrepareUserOperation); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedPrepareUserOperation, + ); } return await keyring.prepareUserOperation( @@ -1470,7 +1514,9 @@ export class KeyringController< const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.patchUserOperation) { - throw new Error(KeyringControllerError.UnsupportedPatchUserOperation); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedPatchUserOperation, + ); } return await keyring.patchUserOperation(address, userOp, executionContext); @@ -1494,7 +1540,9 @@ export class KeyringController< const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.signUserOperation) { - throw new Error(KeyringControllerError.UnsupportedSignUserOperation); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedSignUserOperation, + ); } return await keyring.signUserOperation(address, userOp, executionContext); @@ -1727,7 +1775,9 @@ export class KeyringController< } if (!keyring) { - throw new Error(KeyringControllerError.KeyringNotFound); + throw new KeyringControllerError( + KeyringControllerErrorMessage.KeyringNotFound, + ); } const result = await operation({ @@ -1740,7 +1790,9 @@ export class KeyringController< // should be discouraged, as it can lead to unexpected behavior. // This error is thrown to prevent consumers using `withKeyring` // as a way to get a reference to a keyring instance. - throw new Error(KeyringControllerError.UnsafeDirectKeyringAccess); + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsafeDirectKeyringAccess, + ); } return result; @@ -1891,7 +1943,9 @@ export class KeyringController< (candidate) => candidate.keyring === keyring, ); if (!keyringWithMetadata) { - throw new Error(KeyringControllerError.KeyringNotFound); + throw new KeyringControllerError( + KeyringControllerErrorMessage.KeyringNotFound, + ); } return keyringWithMetadata.metadata; } @@ -1934,7 +1988,7 @@ export class KeyringController< this.#assertControllerMutexIsLocked(); if (typeof password !== 'string') { - throw new TypeError(KeyringControllerError.WrongPasswordType); + throw new TypeError(KeyringControllerErrorMessage.WrongPasswordType); } this.update((state) => { @@ -1978,7 +2032,7 @@ export class KeyringController< const { vault } = this.state; if (typeof password !== 'string') { - throw new TypeError(KeyringControllerError.WrongPasswordType); + throw new TypeError(KeyringControllerErrorMessage.WrongPasswordType); } let serializedEncryptionKey: string, salt: string; @@ -2019,12 +2073,14 @@ export class KeyringController< typeof encryptionKey !== 'string' || typeof keyDerivationSalt !== 'string' ) { - throw new TypeError(KeyringControllerError.WrongEncryptionKeyType); + throw new TypeError(KeyringControllerErrorMessage.WrongEncryptionKeyType); } const { vault } = this.state; if (vault && JSON.parse(vault).salt !== keyDerivationSalt) { - throw new Error(KeyringControllerError.ExpiredCredentials); + throw new KeyringControllerError( + KeyringControllerErrorMessage.ExpiredCredentials, + ); } this.#encryptionKey = { @@ -2045,11 +2101,15 @@ export class KeyringController< const keyring = this.#getKeyringByIdOrDefault(keyringId); if (!keyring) { - throw new Error(KeyringControllerError.KeyringNotFound); + throw new KeyringControllerError( + KeyringControllerErrorMessage.KeyringNotFound, + ); } - if (keyring.type !== KeyringTypes.hd) { - throw new Error(KeyringControllerError.UnsupportedVerifySeedPhrase); + if (keyring.type !== (KeyringTypes.hd as string)) { + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedVerifySeedPhrase, + ); } assertHasUint8ArrayMnemonic(keyring); @@ -2058,7 +2118,7 @@ export class KeyringController< const accounts = await keyring.getAccounts(); /* istanbul ignore if */ if (accounts.length === 0) { - throw new Error('Cannot verify an empty keyring.'); + throw new KeyringControllerError('Cannot verify an empty keyring.'); } // The HD Keyring Builder is a default keyring builder @@ -2075,13 +2135,17 @@ export class KeyringController< const testAccounts = await hdKeyring.getAccounts(); /* istanbul ignore if */ if (testAccounts.length !== accounts.length) { - throw new Error('Seed phrase imported incorrect number of accounts.'); + throw new KeyringControllerError( + 'Seed phrase imported incorrect number of accounts.', + ); } testAccounts.forEach((account: string, i: number) => { /* istanbul ignore if */ if (account.toLowerCase() !== accounts[i].toLowerCase()) { - throw new Error('Seed phrase imported different accounts.'); + throw new KeyringControllerError( + 'Seed phrase imported different accounts.', + ); } }); @@ -2193,7 +2257,9 @@ export class KeyringController< }> { return this.#withVaultLock(async () => { if (!this.state.vault) { - throw new Error(KeyringControllerError.VaultError); + throw new KeyringControllerError( + KeyringControllerErrorMessage.VaultError, + ); } const parsedEncryptedVault = JSON.parse(this.state.vault); @@ -2208,7 +2274,9 @@ export class KeyringController< const encryptionKey = this.#encryptionKey?.serialized; if (!encryptionKey) { - throw new Error(KeyringControllerError.MissingCredentials); + throw new KeyringControllerError( + KeyringControllerErrorMessage.MissingCredentials, + ); } const key = await this.#encryptor.importKey(encryptionKey); @@ -2218,7 +2286,9 @@ export class KeyringController< ); if (!isSerializedKeyringsArray(vault)) { - throw new Error(KeyringControllerError.VaultDataError); + throw new KeyringControllerError( + KeyringControllerErrorMessage.VaultDataError, + ); } const { keyrings, newMetadata } = @@ -2247,7 +2317,9 @@ export class KeyringController< await this.#assertNoDuplicateAccounts(); if (!this.#encryptionKey) { - throw new Error(KeyringControllerError.MissingCredentials); + throw new KeyringControllerError( + KeyringControllerErrorMessage.MissingCredentials, + ); } const serializedKeyrings = await this.#getSerializedKeyrings(); @@ -2257,7 +2329,9 @@ export class KeyringController< (keyring) => keyring.type === (KeyringTypes.hd as string), ) ) { - throw new Error(KeyringControllerError.NoHdKeyring); + throw new KeyringControllerError( + KeyringControllerErrorMessage.NoHdKeyring, + ); } const key = await this.#encryptor.importKey( @@ -2349,7 +2423,9 @@ export class KeyringController< const [firstAccount] = await keyring.getAccounts(); if (!firstAccount) { - throw new Error(KeyringControllerError.NoFirstAccount); + throw new KeyringControllerError( + KeyringControllerErrorMessage.NoFirstAccount, + ); } return firstAccount; } @@ -2398,8 +2474,8 @@ export class KeyringController< const keyringBuilder = this.#getKeyringBuilderForType(type); if (!keyringBuilder) { - throw new Error( - `${KeyringControllerError.NoKeyringBuilder}. Keyring type: ${type}`, + throw new KeyringControllerError( + `${KeyringControllerErrorMessage.NoKeyringBuilder}. Keyring type: ${type}`, ); } @@ -2413,10 +2489,13 @@ export class KeyringController< await keyring.init(); } - if (type === KeyringTypes.hd && (!isObject(data) || !data.mnemonic)) { + if ( + type === (KeyringTypes.hd as string) && + (!isObject(data) || !data.mnemonic) + ) { if (!keyring.generateRandomMnemonic) { - throw new Error( - KeyringControllerError.UnsupportedGenerateRandomMnemonic, + throw new KeyringControllerError( + KeyringControllerErrorMessage.UnsupportedGenerateRandomMnemonic, ); } @@ -2537,7 +2616,9 @@ export class KeyringController< const accounts = await this.#getAccountsFromKeyrings(additionalKeyrings); if (new Set(accounts).size !== accounts.length) { - throw new Error(KeyringControllerError.DuplicatedAccount); + throw new KeyringControllerError( + KeyringControllerErrorMessage.DuplicatedAccount, + ); } } @@ -2563,7 +2644,9 @@ export class KeyringController< */ #assertIsUnlocked(): void { if (!this.state.isUnlocked) { - throw new Error(KeyringControllerError.ControllerLocked); + throw new KeyringControllerError( + KeyringControllerErrorMessage.ControllerLocked, + ); } } @@ -2625,7 +2708,9 @@ export class KeyringController< */ #assertControllerMutexIsLocked(): void { if (!this.#controllerOperationMutex.isLocked()) { - throw new Error(KeyringControllerError.ControllerLockRequired); + throw new KeyringControllerError( + KeyringControllerErrorMessage.ControllerLockRequired, + ); } } diff --git a/packages/keyring-controller/src/constants.ts b/packages/keyring-controller/src/constants.ts index 588d403d223..3e7355fac00 100644 --- a/packages/keyring-controller/src/constants.ts +++ b/packages/keyring-controller/src/constants.ts @@ -1,4 +1,4 @@ -export enum KeyringControllerError { +export enum KeyringControllerErrorMessage { NoKeyring = 'KeyringController - No keyring found', KeyringNotFound = 'KeyringController - Keyring not found.', UnsafeDirectKeyringAccess = 'KeyringController - Returning keyring instances is unsafe', diff --git a/packages/keyring-controller/src/errors.ts b/packages/keyring-controller/src/errors.ts new file mode 100644 index 00000000000..0ef506f1d58 --- /dev/null +++ b/packages/keyring-controller/src/errors.ts @@ -0,0 +1,132 @@ +/** + * Options for creating a KeyringControllerError. + */ +export type KeyringControllerErrorOptions = { + /** + * The underlying error that caused this error (for error chaining). + * Uses the standard Error.cause property (ES2022). + */ + cause?: Error; + /** + * Optional error code for programmatic error handling. + * This can be used to identify specific error types without string matching. + */ + code?: string; + /** + * Additional context data associated with the error. + * Useful for debugging and error reporting. + */ + context?: Record; +}; + +/** + * Error class for KeyringController-related errors. + * + * This error class extends the standard Error class and supports: + * - Error chaining via the `cause` property (ES2022 standard) + * - Optional error codes for programmatic error handling + * - Additional context data for debugging + * - Backward compatibility with the legacy `originalError` property + */ +export class KeyringControllerError extends Error { + /** + * Optional error code for programmatic error handling. + */ + code?: string; + + /** + * Additional context data associated with the error. + */ + context?: Record; + + /** + * The underlying error that caused this error (ES2022 standard). + * This is set manually for compatibility with older TypeScript versions. + */ + cause?: Error; + + /** + * @deprecated Use `cause` instead. This property is maintained for backward compatibility. + */ + originalError?: Error; + + /** + * Creates a new KeyringControllerError. + * + * @param message - The error message. + * @param options - Error options or an Error object for backward compatibility. + */ + constructor( + message: string, + options?: KeyringControllerErrorOptions | Error, + ) { + super(message); + this.name = 'KeyringControllerError'; + + // Support both new signature (options object) and legacy signature (Error as second param) + const cause = options instanceof Error ? options : options?.cause; + const code = options instanceof Error ? undefined : options?.code; + const context = options instanceof Error ? undefined : options?.context; + + // Set cause property for error chaining (ES2022 standard) + if (cause) { + this.cause = cause; + // Maintain backward compatibility with originalError + this.originalError = cause; + } + + // Set code and data if provided + if (code) { + this.code = code; + } + if (context) { + this.context = context; + } + + // Ensure proper prototype chain for instanceof checks + Object.setPrototypeOf(this, KeyringControllerError.prototype); + } + + /** + * Returns a JSON representation of the error. + * Useful for logging and error reporting. + * + * @returns JSON representation of the error. + */ + toJSON(): Record { + return { + name: this.name, + message: this.message, + code: this.code, + context: this.context, + stack: this.stack, + cause: this.cause + ? { + name: this.cause.name, + message: this.cause.message, + stack: this.cause.stack, + } + : undefined, + }; + } + + /** + * Returns a string representation of the error chain. + * Includes all chained errors for better debugging. + * + * @returns String representation of the error chain. + */ + toString(): string { + let result = `${this.name}: ${this.message}`; + + if (this.code) { + result += ` [${this.code}]`; + } + + if (this.cause) { + result += `\n Caused by: ${this.cause}`; + } + + return result; + } +} diff --git a/packages/keyring-controller/src/index.ts b/packages/keyring-controller/src/index.ts index 0176ac5c808..341ad18bac3 100644 --- a/packages/keyring-controller/src/index.ts +++ b/packages/keyring-controller/src/index.ts @@ -1,2 +1,3 @@ export * from './KeyringController'; export type * from './types'; +export * from './errors'; diff --git a/packages/keyring-controller/tests/mocks/mockHardwareKeyring.ts b/packages/keyring-controller/tests/mocks/mockHardwareKeyring.ts new file mode 100644 index 00000000000..bfaf3ae14f8 --- /dev/null +++ b/packages/keyring-controller/tests/mocks/mockHardwareKeyring.ts @@ -0,0 +1,43 @@ +import type { Hex } from '@metamask/utils'; + +export class HardwareWalletError extends Error { + code: string; + + constructor(message: string, code: string) { + super(message); + this.name = 'HardwareWalletError'; + this.code = code; + } +} + +/** + * Mock hardware keyring that supports signTypedData but throws an error. + */ +export class MockHardwareKeyring { + static type = 'Mock Hardware'; + + type = 'Mock Hardware'; + + async getAccounts(): Promise { + return ['0x9876543210987654321098765432109876543210']; + } + + async signTypedData( + _address: Hex, + _data: unknown, + _opts: unknown, + ): Promise { + throw new HardwareWalletError( + 'User rejected the request on hardware device', + 'USER_REJECTED', + ); + } + + serialize = async (): Promise<{ type: string }> => ({ + type: this.type, + }); + + deserialize = async (_opts: unknown): Promise => { + // noop + }; +}