From 20cdecdad11d98e5d2e11d66b7b3b5ef427cac13 Mon Sep 17 00:00:00 2001 From: Daniel Rocha <68558152+danroc@users.noreply.github.com> Date: Wed, 18 Oct 2023 01:24:23 +0200 Subject: [PATCH 1/4] fix: remove non-serializable properties from exceptions --- src/rpc-handler.ts | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/rpc-handler.ts b/src/rpc-handler.ts index 47b89f231..af29940ae 100644 --- a/src/rpc-handler.ts +++ b/src/rpc-handler.ts @@ -30,13 +30,14 @@ export class MethodNotSupportedError extends Error { } /** - * Handles a keyring JSON-RPC request. + * Inner function that dispatches JSON-RPC request to the associated Keyring + * methods. * * @param keyring - Keyring instance. * @param request - Keyring JSON-RPC request. * @returns A promise that resolves to the keyring response. */ -export async function handleKeyringRequest( +async function dispatchRequest( keyring: Keyring, request: JsonRpcRequest, ): Promise { @@ -128,3 +129,40 @@ export async function handleKeyringRequest( } } } + +/** + * Handles a keyring JSON-RPC request. + * + * This function is meant to be used as a handler for Keyring JSON-RPC requests + * in an Accounts Snap. + * + * Example: + * + * ```typescript + * export const onKeyringRequest: OnKeyringRequestHandler = async ({ + * origin, + * request, + * }) => { + * return await handleKeyringRequest(keyring, request); + * }; + * ``` + * + * @param keyring - Keyring instance. + * @param request - Keyring JSON-RPC request. + * @returns A promise that resolves to the keyring response. + */ +export async function handleKeyringRequest( + keyring: Keyring, + request: JsonRpcRequest, +): Promise { + try { + return await dispatchRequest(keyring, request); + } catch (error) { + const message = + error instanceof Error && typeof error.message === 'string' + ? error.message + : 'An unknown error occurred while handling the keyring request'; + + throw new Error(message); + } +} From abbfa876d4f1e98eccbbb9b9fa98854cb0bff692 Mon Sep 17 00:00:00 2001 From: Daniel Rocha <68558152+danroc@users.noreply.github.com> Date: Wed, 18 Oct 2023 17:47:05 +0200 Subject: [PATCH 2/4] test: update unit tests --- src/rpc-handler.test.ts | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/src/rpc-handler.test.ts b/src/rpc-handler.test.ts index b467fbd29..858cdfdee 100644 --- a/src/rpc-handler.test.ts +++ b/src/rpc-handler.test.ts @@ -1,7 +1,7 @@ import type { Keyring } from './api'; import { KeyringRpcMethod, isKeyringRpcMethod } from './internal/rpc'; import type { JsonRpcRequest } from './JsonRpcRequest'; -import { MethodNotSupportedError, handleKeyringRequest } from './rpc-handler'; +import { handleKeyringRequest } from './rpc-handler'; describe('keyringRpcDispatcher', () => { const keyring = { @@ -197,7 +197,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toStrictEqual(expected); }); - it('should throw MethodNotSupportedError if exportAccount is not implemented', async () => { + it('should throw an error if exportAccount is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -211,7 +211,7 @@ describe('keyringRpcDispatcher', () => { delete partialKeyring.exportAccount; await expect(handleKeyringRequest(partialKeyring, request)).rejects.toThrow( - MethodNotSupportedError, + 'Method not supported: keyring_exportAccount', ); }); @@ -229,7 +229,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('ListRequests result'); }); - it('should throw MethodNotSupportedError if listRequests is not implemented', async () => { + it('should throw an error if listRequests is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -242,7 +242,7 @@ describe('keyringRpcDispatcher', () => { delete partialKeyring.listRequests; await expect(handleKeyringRequest(partialKeyring, request)).rejects.toThrow( - MethodNotSupportedError, + 'Method not supported: keyring_listRequests', ); }); @@ -263,7 +263,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('GetRequest result'); }); - it('should throw MethodNotSupportedError if getRequest is not implemented', async () => { + it('should throw an error if getRequest is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -277,7 +277,7 @@ describe('keyringRpcDispatcher', () => { delete partialKeyring.getRequest; await expect(handleKeyringRequest(partialKeyring, request)).rejects.toThrow( - MethodNotSupportedError, + 'Method not supported: keyring_getRequest', ); }); @@ -328,7 +328,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('ApproveRequest result'); }); - it('should throw MethodNotSupportedError if approveRequest is not implemented', async () => { + it('should throw an error if approveRequest is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -342,7 +342,7 @@ describe('keyringRpcDispatcher', () => { delete partialKeyring.approveRequest; await expect(handleKeyringRequest(partialKeyring, request)).rejects.toThrow( - MethodNotSupportedError, + 'Method not supported: keyring_approveRequest', ); }); @@ -409,7 +409,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('RejectRequest result'); }); - it('should throw MethodNotSupportedError if rejectRequest is not implemented', async () => { + it('should throw an error if rejectRequest is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -423,11 +423,11 @@ describe('keyringRpcDispatcher', () => { delete partialKeyring.rejectRequest; await expect(handleKeyringRequest(partialKeyring, request)).rejects.toThrow( - MethodNotSupportedError, + 'Method not supported: keyring_rejectRequest', ); }); - it('should throw MethodNotSupportedError for an unknown method', async () => { + it('should throw an error for an unknown method', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -435,7 +435,22 @@ describe('keyringRpcDispatcher', () => { }; await expect(handleKeyringRequest(keyring, request)).rejects.toThrow( - MethodNotSupportedError, + 'Method not supported: unknown_method', + ); + }); + + it('should throw an "unknown error" if the error message is not a string', async () => { + const request: JsonRpcRequest = { + jsonrpc: '2.0', + id: '80c25a6b-4a76-44f4-88c5-7b3b76f72a74', + method: 'keyring_listAccounts', + }; + + const error = new Error(); + error.message = 1 as unknown as string; + keyring.listAccounts.mockRejectedValue(error); + await expect(handleKeyringRequest(keyring, request)).rejects.toThrow( + 'An unknown error occurred while handling the keyring request', ); }); }); From 03a1538b243ae625dac9142e8c21f24ed5aa79ed Mon Sep 17 00:00:00 2001 From: Daniel Rocha <68558152+danroc@users.noreply.github.com> Date: Thu, 23 May 2024 14:47:34 +0200 Subject: [PATCH 3/4] test: improve test-case messages --- src/rpc-handler.test.ts | 54 ++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/rpc-handler.test.ts b/src/rpc-handler.test.ts index 858cdfdee..5726915a9 100644 --- a/src/rpc-handler.test.ts +++ b/src/rpc-handler.test.ts @@ -3,7 +3,7 @@ import { KeyringRpcMethod, isKeyringRpcMethod } from './internal/rpc'; import type { JsonRpcRequest } from './JsonRpcRequest'; import { handleKeyringRequest } from './rpc-handler'; -describe('keyringRpcDispatcher', () => { +describe('handleKeyringRequest', () => { const keyring = { listAccounts: jest.fn(), getAccount: jest.fn(), @@ -23,7 +23,7 @@ describe('keyringRpcDispatcher', () => { jest.clearAllMocks(); }); - it('should call keyring_listAccounts', async () => { + it('calls `keyring_listAccounts`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -37,7 +37,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('ListAccounts result'); }); - it('should fail to call keyringRpcDispatcher with a non-JSON-RPC request', async () => { + it('fails to execute an mal-formatted JSON-RPC request', async () => { const request = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -51,7 +51,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should call keyring_getAccount', async () => { + it('calls `keyring_getAccount`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -68,7 +68,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('GetAccount result'); }); - it('should fail to call keyring_getAccount without the account ID', async () => { + it('fails to call `keyring_getAccount` without providing an account ID', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -81,7 +81,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should fail to call keyring_getAccount without params', async () => { + it('fails to call `keyring_getAccount` when the `params` is not provided', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -93,7 +93,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should call keyring_createAccount', async () => { + it('calls `keyring_createAccount`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -108,7 +108,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('CreateAccount result'); }); - it('should call keyring_filterAccountChains', async () => { + it('calls `keyring_filterAccountChains`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -131,7 +131,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('FilterSupportedChains result'); }); - it('should call keyring_updateAccount', async () => { + it('calls `keyring_updateAccount`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -160,7 +160,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('UpdateAccount result'); }); - it('should call keyring_deleteAccount', async () => { + it('calls `keyring_deleteAccount`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -177,7 +177,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('DeleteAccount result'); }); - it('should call keyring_exportAccount', async () => { + it('calls `keyring_exportAccount`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -197,7 +197,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toStrictEqual(expected); }); - it('should throw an error if exportAccount is not implemented', async () => { + it('throws an error if `keyring_exportAccount` is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -215,7 +215,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should call keyring_listRequests', async () => { + it('calls `keyring_listRequests`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -229,7 +229,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('ListRequests result'); }); - it('should throw an error if listRequests is not implemented', async () => { + it('throws an error if `keyring_listRequests` is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -246,7 +246,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should call keyring_getRequest', async () => { + it('calls `keyring_getRequest`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -263,7 +263,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('GetRequest result'); }); - it('should throw an error if getRequest is not implemented', async () => { + it('throws an error if `keyring_getRequest` is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -281,7 +281,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should call keyring_submitRequest', async () => { + it('calls `keyring_submitRequest`', async () => { const dappRequest = { id: 'c555de37-cf4b-4ff2-8273-39db7fb58f1c', scope: 'eip155:1', @@ -306,7 +306,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('SubmitRequest result'); }); - it('should call keyring_approveRequest', async () => { + it('calls `keyring_approveRequest`', async () => { const payload = { id: '59db4ff8-8eb3-4a75-8ef3-b80aff8fa780', data: { signature: '0x0123' }, @@ -328,7 +328,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('ApproveRequest result'); }); - it('should throw an error if approveRequest is not implemented', async () => { + it('throws an error if `keyring_approveRequest` is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -346,7 +346,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('calls the keyring with a non-UUIDv4 string request ID', async () => { + it('calls a method with a non-UUIDv4 string as the request ID', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: 'request-id', @@ -357,7 +357,7 @@ describe('keyringRpcDispatcher', () => { expect(await handleKeyringRequest(keyring, request)).toStrictEqual([]); }); - it('calls the keyring with a number request ID', async () => { + it('calls the keyring with a number as the request ID', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: 1, @@ -368,7 +368,7 @@ describe('keyringRpcDispatcher', () => { expect(await handleKeyringRequest(keyring, request)).toStrictEqual([]); }); - it('calls the keyring with a null request ID', async () => { + it('calls the keyring with null as the request ID', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: null, @@ -379,7 +379,7 @@ describe('keyringRpcDispatcher', () => { expect(await handleKeyringRequest(keyring, request)).toStrictEqual([]); }); - it('fails to call the keyring with a boolean request ID', async () => { + it('fails to call the keyring with a boolean as tne request ID', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: true as any, @@ -392,7 +392,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should call keyring_rejectRequest', async () => { + it('calls `keyring_rejectRequest`', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -409,7 +409,7 @@ describe('keyringRpcDispatcher', () => { expect(result).toBe('RejectRequest result'); }); - it('should throw an error if rejectRequest is not implemented', async () => { + it('throws an error if `keyring_rejectRequest` is not implemented', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -427,7 +427,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should throw an error for an unknown method', async () => { + it('throws an error if an unknown method is called', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '7c507ff0-365f-4de0-8cd5-eb83c30ebda4', @@ -439,7 +439,7 @@ describe('keyringRpcDispatcher', () => { ); }); - it('should throw an "unknown error" if the error message is not a string', async () => { + it('throws an "unknown error" if the error message is not a string', async () => { const request: JsonRpcRequest = { jsonrpc: '2.0', id: '80c25a6b-4a76-44f4-88c5-7b3b76f72a74', From c5a23f98865a205ca30d3c1a500f2b317dd69014 Mon Sep 17 00:00:00 2001 From: Daniel Rocha <68558152+danroc@users.noreply.github.com> Date: Thu, 23 May 2024 14:50:12 +0200 Subject: [PATCH 4/4] chore: update JSDoc --- src/rpc-handler.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/rpc-handler.ts b/src/rpc-handler.ts index af29940ae..99153fc0b 100644 --- a/src/rpc-handler.ts +++ b/src/rpc-handler.ts @@ -136,9 +136,11 @@ async function dispatchRequest( * This function is meant to be used as a handler for Keyring JSON-RPC requests * in an Accounts Snap. * - * Example: - * - * ```typescript + * @param keyring - Keyring instance. + * @param request - Keyring JSON-RPC request. + * @returns A promise that resolves to the keyring response. + * @example + * ```ts * export const onKeyringRequest: OnKeyringRequestHandler = async ({ * origin, * request, @@ -146,10 +148,6 @@ async function dispatchRequest( * return await handleKeyringRequest(keyring, request); * }; * ``` - * - * @param keyring - Keyring instance. - * @param request - Keyring JSON-RPC request. - * @returns A promise that resolves to the keyring response. */ export async function handleKeyringRequest( keyring: Keyring,