From c2603eaedbb22b1e9130b86ea9df465faffd1aff Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 22 Jul 2026 18:57:13 +0300 Subject: [PATCH 1/3] fix(wallet-cli): retry sendCommand only on ECONNREFUSED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daemon client's `sendCommand` retried a request once on both ECONNREFUSED and ECONNRESET. ECONNRESET can drop after the daemon has already received and acted on the request, so blindly re-sending a non-idempotent request (e.g. a transaction broadcast) could execute it twice. Restrict the retry to ECONNREFUSED — where the connection was never established, so the daemon provably never received the request — and surface ECONNRESET to the caller instead. This hardens the retry path before any mutating send/sign/transfer RPC becomes reachable over the daemon socket. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/CHANGELOG.md | 2 ++ .../src/daemon/daemon-client.test.ts | 36 +++++++++++++------ .../wallet-cli/src/daemon/daemon-client.ts | 24 ++++++++----- 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index 3ec8f37b592..55f79c71c3a 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- The daemon client (`sendCommand`) now retries a request only on `ECONNREFUSED`, no longer on `ECONNRESET` ([#9511](https://github.com/MetaMask/core/pull/9511)) + - `ECONNREFUSED` means the connection was never established, so the daemon provably never received the request and re-sending is safe. `ECONNRESET` can drop after the daemon has already received and acted on the request, so blindly re-sending could execute a non-idempotent action (e.g. a transaction broadcast) twice; `ECONNRESET` now surfaces to the caller instead of triggering a retry. This is a prerequisite for shipping any send/sign/transfer command. - `--password` / `MM_WALLET_PASSWORD` is now optional on `mm daemon start`; on subsequent runs, omitting it starts the daemon with a locked keyring, and the persisted vault is auto-unlocked when a password is supplied ([#8821](https://github.com/MetaMask/core/pull/8821)) - The daemon RPC server now validates `params` against each handler's superstruct before dispatch, returning a `-32602 invalidParams` error on mismatch instead of passing raw params to the handler ([#8846](https://github.com/MetaMask/core/pull/8846)) - Report daemon socket connection errors consistently across `mm daemon call` and `mm daemon list` ([#9339](https://github.com/MetaMask/core/pull/9339)) diff --git a/packages/wallet-cli/src/daemon/daemon-client.test.ts b/packages/wallet-cli/src/daemon/daemon-client.test.ts index 7aaea3c8a1c..bd15900c6c0 100644 --- a/packages/wallet-cli/src/daemon/daemon-client.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-client.test.ts @@ -165,21 +165,21 @@ describe('sendCommand', () => { expect(socket.destroy).toHaveBeenCalledTimes(2); }); - it('retries once on ECONNRESET', async () => { + it('does not retry on ECONNRESET, surfacing the error instead', async () => { + // ECONNRESET can drop after the daemon has already acted on the request, so + // re-sending a non-idempotent request could execute it twice. The error + // must surface to the caller rather than trigger a blind resend. setupMockSocket(); mockWriteLine.mockResolvedValue(undefined); - mockReadLine - .mockRejectedValueOnce( - Object.assign(new Error('reset'), { code: 'ECONNRESET' }), - ) - .mockImplementationOnce(respondWithMatchingId()); + mockReadLine.mockRejectedValue( + Object.assign(new Error('reset'), { code: 'ECONNRESET' }), + ); - const response = await sendCommand({ - socketPath: '/tmp/test.sock', - method: 'test', - }); + await expect( + sendCommand({ socketPath: '/tmp/test.sock', method: 'test' }), + ).rejects.toThrow('reset'); - expect(response).toHaveProperty('result'); + expect(mockReadLine).toHaveBeenCalledTimes(1); }); it('does not retry on other errors', async () => { @@ -286,6 +286,20 @@ describe('pingDaemon', () => { }); }); + it('returns unreachable with reason=refused on ECONNRESET without retrying', async () => { + // ECONNRESET is classified as refused but is not retried, so only a single + // connection attempt is made. + mockConnectionError('ECONNRESET'); + + const result = await pingDaemon('/tmp/test.sock'); + expect(result).toStrictEqual({ + status: 'unreachable', + reason: 'refused', + error: expect.any(Error), + }); + expect(mockCreateConnection).toHaveBeenCalledTimes(1); + }); + it('returns unreachable with reason=permission on EACCES', async () => { mockConnectionError('EACCES'); diff --git a/packages/wallet-cli/src/daemon/daemon-client.ts b/packages/wallet-cli/src/daemon/daemon-client.ts index a0017b8a875..3e3e9aa59eb 100644 --- a/packages/wallet-cli/src/daemon/daemon-client.ts +++ b/packages/wallet-cli/src/daemon/daemon-client.ts @@ -44,8 +44,12 @@ async function connectSocket(socketPath: string): Promise { * * Opens a connection, writes one JSON-RPC request line, reads one JSON-RPC * response line, then closes the connection. Retries once after a short delay - * on transient connection errors (ECONNREFUSED, ECONNRESET). Verifies that the - * response `id` matches the outgoing request `id`. + * only on `ECONNREFUSED` — the connection was never established, so the daemon + * provably never received the request and re-sending is safe. Does not retry + * on `ECONNRESET`, which can drop after the daemon has already received and + * acted on the request: blindly re-sending could execute a non-idempotent + * action (e.g. a transaction broadcast) twice, so it is surfaced to the caller + * instead. Verifies that the response `id` matches the outgoing request `id`. * * @param options - Command options. * @param options.socketPath - The Unix socket path. @@ -91,10 +95,13 @@ export async function sendCommand({ try { return await attempt(); } catch (error: unknown) { - if ( - !isErrorWithCode(error, 'ECONNREFUSED') && - !isErrorWithCode(error, 'ECONNRESET') - ) { + // Only retry on ECONNREFUSED: the connection was never established, so the + // daemon provably never received the request and re-sending is safe. + // ECONNRESET can drop *after* the daemon received and began (or finished) + // processing the request, so blindly re-sending a non-idempotent request + // (e.g. a transaction broadcast) could execute it twice. Surface it to the + // caller instead of retrying. + if (!isErrorWithCode(error, 'ECONNREFUSED')) { throw error; } await new Promise((resolve) => setTimeout(resolve, 100)); @@ -105,8 +112,9 @@ export async function sendCommand({ /** * Why an unreachable daemon cannot be queried. * - * - `'refused'`: connection refused after retry (`ECONNREFUSED` / `ECONNRESET`). - * Typical of a daemon that has crashed or is mid-restart. + * - `'refused'`: the connection could not be completed — `ECONNREFUSED` + * (retried once) or `ECONNRESET` (a mid-request drop, not retried). Typical + * of a daemon that has crashed or is mid-restart. * - `'timeout'`: the daemon accepted the connection but did not respond within * the read timeout — most likely wedged on a long-running operation. * - `'permission'`: the socket exists but cannot be opened (`EACCES` / `EPERM`). From 65401bb60575f6a862bc177dd7f3091865a26d5f Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 22 Jul 2026 18:58:42 +0300 Subject: [PATCH 2/3] docs(wallet-cli): point changelog entry at PR #9608 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index 55f79c71c3a..228f8827a4d 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- The daemon client (`sendCommand`) now retries a request only on `ECONNREFUSED`, no longer on `ECONNRESET` ([#9511](https://github.com/MetaMask/core/pull/9511)) +- The daemon client (`sendCommand`) now retries a request only on `ECONNREFUSED`, no longer on `ECONNRESET` ([#9608](https://github.com/MetaMask/core/pull/9608)) - `ECONNREFUSED` means the connection was never established, so the daemon provably never received the request and re-sending is safe. `ECONNRESET` can drop after the daemon has already received and acted on the request, so blindly re-sending could execute a non-idempotent action (e.g. a transaction broadcast) twice; `ECONNRESET` now surfaces to the caller instead of triggering a retry. This is a prerequisite for shipping any send/sign/transfer command. - `--password` / `MM_WALLET_PASSWORD` is now optional on `mm daemon start`; on subsequent runs, omitting it starts the daemon with a locked keyring, and the persisted vault is auto-unlocked when a password is supplied ([#8821](https://github.com/MetaMask/core/pull/8821)) - The daemon RPC server now validates `params` against each handler's superstruct before dispatch, returning a `-32602 invalidParams` error on mismatch instead of passing raw params to the handler ([#8846](https://github.com/MetaMask/core/pull/8846)) From 1fa26b868cdb5de2658ed70c1784d640129918ec Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 22 Jul 2026 19:16:45 +0300 Subject: [PATCH 3/3] docs(wallet-cli): shorten changelog entry Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index 228f8827a4d..e4075f6788c 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -21,8 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- The daemon client (`sendCommand`) now retries a request only on `ECONNREFUSED`, no longer on `ECONNRESET` ([#9608](https://github.com/MetaMask/core/pull/9608)) - - `ECONNREFUSED` means the connection was never established, so the daemon provably never received the request and re-sending is safe. `ECONNRESET` can drop after the daemon has already received and acted on the request, so blindly re-sending could execute a non-idempotent action (e.g. a transaction broadcast) twice; `ECONNRESET` now surfaces to the caller instead of triggering a retry. This is a prerequisite for shipping any send/sign/transfer command. +- The daemon client (`sendCommand`) now retries only on `ECONNREFUSED`, not `ECONNRESET`, since a reset can drop after the daemon has already acted on a request — re-sending could execute a non-idempotent action (e.g. a transaction broadcast) twice ([#9608](https://github.com/MetaMask/core/pull/9608)) - `--password` / `MM_WALLET_PASSWORD` is now optional on `mm daemon start`; on subsequent runs, omitting it starts the daemon with a locked keyring, and the persisted vault is auto-unlocked when a password is supplied ([#8821](https://github.com/MetaMask/core/pull/8821)) - The daemon RPC server now validates `params` against each handler's superstruct before dispatch, returning a `-32602 invalidParams` error on mismatch instead of passing raw params to the handler ([#8846](https://github.com/MetaMask/core/pull/8846)) - Report daemon socket connection errors consistently across `mm daemon call` and `mm daemon list` ([#9339](https://github.com/MetaMask/core/pull/9339))