diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index db8688b0b9..f221ff2b12 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add the `mm daemon list` command, which prints the messenger actions the running daemon can dispatch via `daemon call`, enumerated from the live messenger so the list cannot drift from what `call` accepts ([#9339](https://github.com/MetaMask/core/pull/9339)) - Add the `mm daemon` command suite (`start`, `stop`, `status`, `purge`, and `call`) for running the wallet daemon and dispatching messenger actions over its socket ([#9255](https://github.com/MetaMask/core/pull/9255)) - Add a wallet factory and daemon entry point that construct a `@metamask/wallet` `Wallet` backed by the SQLite key-value store, hydrate it from persisted state, run controller initialization (aborting startup if any step fails), import the secret recovery phrase on first run, and expose a `dispose` teardown handle ([#9226](https://github.com/MetaMask/core/pull/9226)) - Add a daemon transport layer: a JSON-RPC client and server over a Unix socket, plus daemon spawn/stop lifecycle helpers ([#9108](https://github.com/MetaMask/core/pull/9108)) @@ -17,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Report daemon socket connection errors consistently across `mm daemon call` and `mm daemon list` ([#9339](https://github.com/MetaMask/core/pull/9339)) - Bump `@metamask/wallet` from `^3.0.0` to `^6.0.0` ([#9218](https://github.com/MetaMask/core/pull/9218), [#9263](https://github.com/MetaMask/core/pull/9263), [#9349](https://github.com/MetaMask/core/pull/9349)) [Unreleased]: https://github.com/MetaMask/core/ diff --git a/packages/wallet-cli/README.md b/packages/wallet-cli/README.md index ef6c44b847..dd32016dd8 100644 --- a/packages/wallet-cli/README.md +++ b/packages/wallet-cli/README.md @@ -20,13 +20,21 @@ Start the daemon (flags may also be supplied as the `INFURA_PROJECT_ID`, `MM_WAL mm daemon start --infura-project-id --password --srp "" ``` +Discover what the running wallet can do — `list` prints every messenger action currently dispatchable via `call`. This surface grows as more controllers are wired, so treat it as evolving rather than a stability contract: + +```sh +mm daemon list +``` + Call any messenger action on the running wallet (positional JSON array for arguments, optional `--timeout`): ```sh -mm daemon call AccountsController:listAccounts -mm daemon call KeyringController:getState --timeout 10000 +mm daemon call KeyringController:getState +mm daemon call NetworkController:getState --timeout 10000 ``` +For the exact parameters and return shape of a given action, see the TypeDoc/README of the controller that owns it (e.g. [`@metamask/keyring-controller`](https://github.com/MetaMask/core/tree/main/packages/keyring-controller#readme)). + Inspect or tear it down: ```sh diff --git a/packages/wallet-cli/src/commands/daemon/call.ts b/packages/wallet-cli/src/commands/daemon/call.ts index 9961c094ec..3a4a2a989c 100644 --- a/packages/wallet-cli/src/commands/daemon/call.ts +++ b/packages/wallet-cli/src/commands/daemon/call.ts @@ -4,7 +4,10 @@ import { Args, Command, Flags } from '@oclif/core'; import { sendCommand } from '../../daemon/daemon-client'; import { getDaemonPaths } from '../../daemon/paths'; -import { isErrorWithCode } from '../../daemon/utils'; +import { + formatJsonRpcError, + makeDaemonConnectionError, +} from '../../daemon/utils'; export default class DaemonCall extends Command { static override description = 'Call a messenger action on the wallet daemon'; @@ -18,7 +21,7 @@ export default class DaemonCall extends Command { static override args = { action: Args.string({ description: - 'The messenger action name (e.g. AccountsController:listAccounts)', + 'The messenger action name (e.g. KeyringController:getState)', required: true, }), params: Args.string({ @@ -70,19 +73,11 @@ export default class DaemonCall extends Command { ...(timeoutMs === undefined ? {} : { timeoutMs }), }); } catch (error) { - if ( - isErrorWithCode(error, 'ENOENT') || - isErrorWithCode(error, 'ECONNREFUSED') - ) { - this.error('Daemon is not running. Start it with `mm daemon start`.'); - } - this.error(error instanceof Error ? error.message : String(error)); + this.error(makeDaemonConnectionError(error)); } if (isJsonRpcFailure(response)) { - this.error( - `${response.error.message} (code ${String(response.error.code)})`, - ); + this.error(formatJsonRpcError(response.error)); } const isTTY = process.stdout.isTTY ?? false; diff --git a/packages/wallet-cli/src/commands/daemon/list.test.ts b/packages/wallet-cli/src/commands/daemon/list.test.ts new file mode 100644 index 0000000000..2a9843c0e2 --- /dev/null +++ b/packages/wallet-cli/src/commands/daemon/list.test.ts @@ -0,0 +1,223 @@ +import { sendCommand } from '../../daemon/daemon-client'; +import { runCommand } from '../../test/run-command'; +import DaemonList from './list'; + +jest.mock('../../daemon/daemon-client'); + +const mockSendCommand = jest.mocked(sendCommand); + +/** + * Force `process.stdout.isTTY` for the duration of `fn`, restoring it after. + * + * @param value - The value to assign to `process.stdout.isTTY`. + * @param fn - The callback to run while the override is in place. + */ +async function withTTY( + value: boolean | undefined, + fn: () => Promise, +): Promise { + const original = process.stdout.isTTY; + Object.defineProperty(process.stdout, 'isTTY', { + value, + configurable: true, + }); + try { + await fn(); + } finally { + Object.defineProperty(process.stdout, 'isTTY', { + value: original, + configurable: true, + }); + } +} + +describe('daemon list', () => { + beforeEach(() => { + mockSendCommand.mockResolvedValue({ + jsonrpc: '2.0', + id: '1', + result: ['NetworkController:getState', 'KeyringController:getState'], + }); + }); + + it('requests the listActions method', async () => { + await withTTY(true, async () => { + await runCommand(DaemonList); + }); + + expect(mockSendCommand).toHaveBeenCalledWith( + expect.objectContaining({ method: 'listActions' }), + ); + }); + + it('prints a sorted, indented action list with a count header to a TTY', async () => { + await withTTY(true, async () => { + const { stdout } = await runCommand(DaemonList); + + expect(stdout).toContain('2 callable actions'); + expect(stdout).toContain('mm daemon call '); + // Sorted: KeyringController before NetworkController. + expect(stdout.indexOf('KeyringController:getState')).toBeLessThan( + stdout.indexOf('NetworkController:getState'), + ); + expect(stdout).toContain(' KeyringController:getState'); + }); + }); + + it('singularizes the header for a single action', async () => { + mockSendCommand.mockResolvedValue({ + jsonrpc: '2.0', + id: '1', + result: ['KeyringController:getState'], + }); + + await withTTY(true, async () => { + const { stdout } = await runCommand(DaemonList); + + expect(stdout).toContain('1 callable action '); + }); + }); + + it('reports an empty registry to a TTY', async () => { + mockSendCommand.mockResolvedValue({ jsonrpc: '2.0', id: '1', result: [] }); + + await withTTY(true, async () => { + const { stdout } = await runCommand(DaemonList); + + expect(stdout).toContain('no callable actions'); + }); + }); + + it('writes a bare, sorted, newline-delimited list to a pipe (non-TTY)', async () => { + const writeSpy = jest + .spyOn(process.stdout, 'write') + .mockImplementation(() => true); + + await withTTY(false, async () => { + await runCommand(DaemonList); + }); + + expect(writeSpy).toHaveBeenCalledWith( + 'KeyringController:getState\nNetworkController:getState\n', + ); + writeSpy.mockRestore(); + }); + + it('sorts actions lexicographically, including within a shared namespace', async () => { + mockSendCommand.mockResolvedValue({ + jsonrpc: '2.0', + id: '1', + result: [ + 'KeyringController:getState', + 'AccountsController:listMultichainAccounts', + 'KeyringController:addNewAccount', + ], + }); + const writeSpy = jest + .spyOn(process.stdout, 'write') + .mockImplementation(() => true); + + await withTTY(false, async () => { + await runCommand(DaemonList); + }); + + expect(writeSpy).toHaveBeenCalledWith( + 'AccountsController:listMultichainAccounts\n' + + 'KeyringController:addNewAccount\n' + + 'KeyringController:getState\n', + ); + writeSpy.mockRestore(); + }); + + it('treats an undefined isTTY as non-TTY', async () => { + const writeSpy = jest + .spyOn(process.stdout, 'write') + .mockImplementation(() => true); + + await withTTY(undefined, async () => { + await runCommand(DaemonList); + }); + + expect(writeSpy).toHaveBeenCalledWith( + 'KeyringController:getState\nNetworkController:getState\n', + ); + writeSpy.mockRestore(); + }); + + it('writes nothing to a pipe when the registry is empty', async () => { + mockSendCommand.mockResolvedValue({ jsonrpc: '2.0', id: '1', result: [] }); + const writeSpy = jest + .spyOn(process.stdout, 'write') + .mockImplementation(() => true); + + await withTTY(false, async () => { + await runCommand(DaemonList); + }); + + expect(writeSpy).not.toHaveBeenCalled(); + writeSpy.mockRestore(); + }); + + it('returns a friendly hint when the daemon is not running (ENOENT)', async () => { + mockSendCommand.mockRejectedValue( + Object.assign(new Error('no such file'), { code: 'ENOENT' }), + ); + + const { error } = await runCommand(DaemonList); + + expect(error?.message).toContain('Daemon is not running'); + }); + + it('returns a friendly hint when the daemon refuses the connection', async () => { + mockSendCommand.mockRejectedValue( + Object.assign(new Error('refused'), { code: 'ECONNREFUSED' }), + ); + + const { error } = await runCommand(DaemonList); + + expect(error?.message).toContain('Daemon is not running'); + }); + + it('surfaces other socket errors with the raw message', async () => { + mockSendCommand.mockRejectedValue(new Error('Socket read timed out')); + + const { error } = await runCommand(DaemonList); + + expect(error?.message).toContain('Socket read timed out'); + }); + + it('handles non-Error throws from sendCommand', async () => { + mockSendCommand.mockImplementation(async () => + Promise.reject('string error' as unknown as Error), + ); + + const { error } = await runCommand(DaemonList); + + expect(error?.message).toContain('string error'); + }); + + it('errors when the daemon returns a JSON-RPC failure response', async () => { + mockSendCommand.mockResolvedValue({ + jsonrpc: '2.0', + id: '1', + error: { code: -32601, message: 'Method not found' }, + }); + + const { error } = await runCommand(DaemonList); + + expect(error?.message).toContain('Method not found'); + expect(error?.message).toContain('-32601'); + }); + + it('errors when the result is not an array of strings', async () => { + mockSendCommand.mockResolvedValue({ + jsonrpc: '2.0', + id: '1', + result: { not: 'an array' }, + }); + + const { error } = await runCommand(DaemonList); + + expect(error?.message).toContain('unexpected action list'); + }); +}); diff --git a/packages/wallet-cli/src/commands/daemon/list.ts b/packages/wallet-cli/src/commands/daemon/list.ts new file mode 100644 index 0000000000..a4d66bc645 --- /dev/null +++ b/packages/wallet-cli/src/commands/daemon/list.ts @@ -0,0 +1,61 @@ +import { isJsonRpcFailure } from '@metamask/utils'; +import { Command } from '@oclif/core'; + +import { sendCommand } from '../../daemon/daemon-client'; +import { getDaemonPaths } from '../../daemon/paths'; +import { + formatJsonRpcError, + isStringArray, + makeDaemonConnectionError, +} from '../../daemon/utils'; + +export default class DaemonList extends Command { + static override description = + 'List the messenger actions the running daemon can dispatch via `daemon call`'; + + static override examples = ['<%= config.bin %> daemon list']; + + public async run(): Promise { + await this.parse(DaemonList); + const { socketPath } = getDaemonPaths(this.config.dataDir); + + let response; + try { + response = await sendCommand({ socketPath, method: 'listActions' }); + } catch (error) { + this.error(makeDaemonConnectionError(error)); + } + + if (isJsonRpcFailure(response)) { + this.error(formatJsonRpcError(response.error)); + } + + if (!isStringArray(response.result)) { + this.error('Daemon returned an unexpected action list.'); + } + + const actions = [...response.result].sort(); + + const isTTY = process.stdout.isTTY ?? false; + if (!isTTY) { + // Bare output so it pipes cleanly into `grep`/`fzf`. + if (actions.length > 0) { + process.stdout.write(`${actions.join('\n')}\n`); + } + return; + } + + if (actions.length === 0) { + this.log('The daemon has no callable actions registered.'); + return; + } + + this.log( + `${actions.length} callable action${actions.length === 1 ? '' : 's'} ` + + '(dispatch with `mm daemon call `):', + ); + for (const action of actions) { + this.log(` ${action}`); + } + } +} diff --git a/packages/wallet-cli/src/daemon/daemon-entry.test.ts b/packages/wallet-cli/src/daemon/daemon-entry.test.ts index 0a670b28d3..ce32f142eb 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.test.ts @@ -62,7 +62,10 @@ function enoent(): NodeJS.ErrnoException { function createMockWallet(): MockCreateWalletResult { return { wallet: { - messenger: { call: jest.fn() }, + messenger: { + call: jest.fn(), + getRegisteredActionTypes: jest.fn().mockReturnValue([]), + }, state: {}, }, dispose: jest.fn().mockResolvedValue(undefined), @@ -495,6 +498,28 @@ describe('daemon-entry', () => { expect(typeof status.uptime).toBe('number'); }); + it('exposes listActions handler that returns the messenger action types', async () => { + const mock = createMockWallet(); + ( + mock.wallet.messenger.getRegisteredActionTypes as jest.Mock + ).mockReturnValue([ + 'NetworkController:getState', + 'KeyringController:getState', + ]); + mockCreateWallet.mockResolvedValue(mock); + mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); + + await importDaemonEntry(); + + const { handlers } = mockStartRpcSocketServer.mock.calls[0][0]; + const actions = await handlers.listActions(null); + + expect(actions).toStrictEqual([ + 'NetworkController:getState', + 'KeyringController:getState', + ]); + }); + it('logs to file via makeLogger', async () => { mockCreateWallet.mockResolvedValue(createMockWallet()); mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); diff --git a/packages/wallet-cli/src/daemon/daemon-entry.ts b/packages/wallet-cli/src/daemon/daemon-entry.ts index b39a5d4c90..a4b8f89d92 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.ts @@ -103,6 +103,11 @@ async function main(): Promise { pid: process.pid, uptime: Math.floor((Date.now() - startTime) / 1000), }), + // Exposes the callable surface for discovery: it grows silently as + // controllers are wired, so consumers need a way to see it without a + // hand-kept catalog that would rot. + listActions: async (): Promise => + constructedWallet.messenger.getRegisteredActionTypes(), // Arbitrary messenger dispatch is intentional: the CLI exposes the full // messenger surface over a Unix socket inside the per-user oclif data // directory. The dataDir is chmodded to 0o700 above and the socket to diff --git a/packages/wallet-cli/src/daemon/utils.test.ts b/packages/wallet-cli/src/daemon/utils.test.ts index 08efdc4f69..1c6a12695f 100644 --- a/packages/wallet-cli/src/daemon/utils.test.ts +++ b/packages/wallet-cli/src/daemon/utils.test.ts @@ -1,6 +1,9 @@ import { readFile } from 'node:fs/promises'; import { + formatJsonRpcError, + isStringArray, + makeDaemonConnectionError, isErrorWithCode, isProcessAlive, readPidFile, @@ -34,6 +37,69 @@ describe('isErrorWithCode', () => { }); }); +describe('makeDaemonConnectionError', () => { + it.each(['ENOENT', 'ECONNREFUSED'])( + 'reports a stopped daemon for %s', + (code) => { + const error = Object.assign(new Error('boom'), { code }); + expect(makeDaemonConnectionError(error)).toBe( + 'Daemon is not running. Start it with `mm daemon start`.', + ); + }, + ); + + it('reports a lost connection for ECONNRESET', () => { + const error = Object.assign(new Error('boom'), { code: 'ECONNRESET' }); + const message = makeDaemonConnectionError(error); + expect(message).toContain('Lost the connection to the daemon'); + expect(message).toContain('mm daemon status'); + }); + + it.each(['EACCES', 'EPERM'])( + 'reports a permission problem for %s', + (code) => { + const error = Object.assign(new Error('boom'), { code }); + expect(makeDaemonConnectionError(error)).toContain('permission denied'); + expect(makeDaemonConnectionError(error)).toContain('MM_DATA_DIR'); + }, + ); + + it('surfaces the raw message of an unrecognized Error', () => { + expect(makeDaemonConnectionError(new Error('Socket read timed out'))).toBe( + 'Socket read timed out', + ); + }); + + it('stringifies a non-Error throw', () => { + expect(makeDaemonConnectionError('kaboom')).toBe('kaboom'); + }); +}); + +describe('formatJsonRpcError', () => { + it('annotates the message with its numeric code', () => { + expect( + formatJsonRpcError({ code: -32601, message: 'Method not found' }), + ).toBe('Method not found (code -32601)'); + }); +}); + +describe('isStringArray', () => { + it('returns true for an array of strings', () => { + expect(isStringArray(['a', 'b'])).toBe(true); + expect(isStringArray([])).toBe(true); + }); + + it('returns false for an array with a non-string element', () => { + expect(isStringArray(['a', 42])).toBe(false); + }); + + it('returns false for non-array values', () => { + expect(isStringArray('a')).toBe(false); + expect(isStringArray({ 0: 'a' })).toBe(false); + expect(isStringArray(null)).toBe(false); + }); +}); + describe('readPidFile', () => { it('returns the PID number from a single-line file', async () => { mockReadFile.mockResolvedValue('12345'); diff --git a/packages/wallet-cli/src/daemon/utils.ts b/packages/wallet-cli/src/daemon/utils.ts index b3f5942f37..446d022488 100644 --- a/packages/wallet-cli/src/daemon/utils.ts +++ b/packages/wallet-cli/src/daemon/utils.ts @@ -1,3 +1,4 @@ +import type { JsonRpcError } from '@metamask/utils'; import { isErrorWithCode as hasErrorCode } from '@metamask/utils'; import { readFile } from 'node:fs/promises'; @@ -12,6 +13,65 @@ export function isErrorWithCode(error: unknown, code: string): boolean { return hasErrorCode(error) && error.code === code; } +/** + * Turn an error thrown while contacting the daemon socket into a user-facing + * message for `Command.error`, so every command reports the same failure the + * same way. Distinguishes a stopped daemon (`ENOENT`/`ECONNREFUSED`), a + * connection dropped mid-request (`ECONNRESET`), and a permission problem + * (`EACCES`/`EPERM`), falling back to the raw error message. + * + * @param error - The value thrown while contacting the daemon. + * @returns A human-readable explanation of the failure. + */ +export function makeDaemonConnectionError(error: unknown): string { + if ( + isErrorWithCode(error, 'ENOENT') || + isErrorWithCode(error, 'ECONNREFUSED') + ) { + return 'Daemon is not running. Start it with `mm daemon start`.'; + } + // A reset drops an already-established connection, so the daemon was running + // — most likely it crashed mid-request. Don't tell the user to start it. + if (isErrorWithCode(error, 'ECONNRESET')) { + return ( + 'Lost the connection to the daemon; it may have crashed while ' + + 'handling the request. Check `mm daemon status` and the daemon log.' + ); + } + if (isErrorWithCode(error, 'EACCES') || isErrorWithCode(error, 'EPERM')) { + return ( + 'Cannot connect to the daemon socket: permission denied. ' + + 'The socket may be owned by another user, or MM_DATA_DIR ' + + 'may point to a directory you cannot access.' + ); + } + return error instanceof Error ? error.message : String(error); +} + +/** + * Format the error of a JSON-RPC failure response into a user-facing message + * for `Command.error`, so every command reports RPC failures the same way. + * + * @param error - The `error` field of a JSON-RPC failure response. + * @returns The error message annotated with its numeric code. + */ +export function formatJsonRpcError(error: JsonRpcError): string { + return `${error.message} (code ${String(error.code)})`; +} + +/** + * Check whether a value is an array of strings. Used to validate untyped RPC + * results (e.g. the daemon's `listActions`) before treating them as `string[]`. + * + * @param value - The value to check. + * @returns True if the value is an array whose every element is a string. + */ +export function isStringArray(value: unknown): value is string[] { + return ( + Array.isArray(value) && value.every((item) => typeof item === 'string') + ); +} + /** * Read a PID from a file. The file may contain just the PID, or the PID on * the first line followed by additional metadata (e.g. start time written by