diff --git a/docs/nwc-timeouts.md b/docs/nwc-timeouts.md index 1d8421d..b4a59db 100644 --- a/docs/nwc-timeouts.md +++ b/docs/nwc-timeouts.md @@ -6,7 +6,7 @@ The wallet adapter adds an overall deadline: 15 seconds per read attempt and 45 Normal NIP-47 wallet errors do not rebuild the shared relay connection: a wallet error proves a response arrived, and rebuilding could interrupt unrelated payments. Transport failures and timeouts, including the library's own timeout, still trigger recovery. -CLI `/nwc/*` requests have a 120-second deadline covering headers and response-body consumption. Other daemon routes are not subject to this cap because mint payment operations may run longer and cannot be cancelled by aborting the CLI request. +Every CLI daemon request has a deadline covering headers and response-body consumption: 120 seconds by default, and 600 seconds for value-moving wallet routes (`/wallet/send/*`, `/wallet/receive/*`), whose mint operations can legitimately run longer. Aborting the CLI request never cancels the daemon-side operation; the longer bound only delays how soon the CLI reports the stall. A payment timeout is an **unknown outcome**, not proof that no payment occurred. Promise deadlines do not cancel the underlying operation. Check the mint quote, wallet transactions, and Cashu balance before creating and paying another invoice. diff --git a/src/daemon/wallet/index.nwc.test.ts b/src/daemon/wallet/index.nwc.test.ts index 7cb86a1..3bbf064 100644 --- a/src/daemon/wallet/index.nwc.test.ts +++ b/src/daemon/wallet/index.nwc.test.ts @@ -1,6 +1,6 @@ import { afterAll, beforeEach, describe, expect, it, mock } from "bun:test"; import { RestrictedError } from "applesauce-wallet-connect/helpers/error"; -import type { CocodClient } from "./cocod-client"; +import type { WalletAdapterOptions } from "./index"; /** * Regression tests for the NWC hang fixed in this change. @@ -107,13 +107,19 @@ mock.module("applesauce-relay", () => ({ RelayPool: MockRelayPool })); const { createWalletAdapter } = await import("./index"); -function makeClient(): CocodClient { +/** + * Type of the injected wallet client, derived from the adapter options so this + * test keeps compiling when the legacy `CocodClient` is replaced (see #118). + */ +type WalletClientOption = NonNullable; + +function makeClient(): WalletClientOption { return { getBalances: async () => ({ "https://mint.example": 0 }), getDefaultMint: async () => "https://mint.example", receiveBolt11: async () => ({ invoice: "lnbc-test-invoice" }), receiveCashu: async () => "ok", - } as unknown as CocodClient; + } as unknown as WalletClientOption; } function makeAdapter(timeoutMs = 25) { diff --git a/src/daemon/wallet/index.ts b/src/daemon/wallet/index.ts index 58ee991..4a7d2fe 100644 --- a/src/daemon/wallet/index.ts +++ b/src/daemon/wallet/index.ts @@ -182,7 +182,7 @@ export async function createWalletAdapter( logger.warn( `[nwc] ${label} failed (${(error as Error).message}); rebuilding NWC connection and retrying`, ); - rebuildNwcConnection("reconnected after timeout"); + rebuildNwcConnection("reconnected after stall"); const retry = wallet; if (!retry?.service) throw error; return await withTimeout( @@ -211,7 +211,7 @@ export async function createWalletAdapter( } catch (error) { // Include the library's own timeout, but not normal wallet error replies. if (!(error instanceof WalletBaseError)) { - rebuildNwcConnection("reconnected after payment timeout"); + rebuildNwcConnection("reconnected after payment stall"); } throw error; } diff --git a/src/utils/daemon-client.timeout.test.ts b/src/utils/daemon-client.timeout.test.ts index c7e1b85..43f829c 100644 --- a/src/utils/daemon-client.timeout.test.ts +++ b/src/utils/daemon-client.timeout.test.ts @@ -1,21 +1,32 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { callDaemonUrl, DAEMON_REQUEST_TIMEOUT_MS } from "./daemon-client"; +import { + callDaemonUrl, + DAEMON_LONG_REQUEST_TIMEOUT_MS, + DAEMON_REQUEST_TIMEOUT_MS, +} from "./daemon-client"; import { DEFAULT_CONFIG } from "./config"; const originalFetch = globalThis.fetch; -const originalSetTimeout = globalThis.setTimeout; -const originalClearTimeout = globalThis.clearTimeout; +const originalAbortTimeout = AbortSignal.timeout; + +/** Deadline (ms) passed to AbortSignal.timeout by the last request. */ +let requestedDeadlines: number[] = []; afterEach(() => { globalThis.fetch = originalFetch; - globalThis.setTimeout = originalSetTimeout; - globalThis.clearTimeout = originalClearTimeout; + AbortSignal.timeout = originalAbortTimeout; + requestedDeadlines = []; }); +/** + * Record the requested deadline and arm a fast one instead, so tests do not + * wait out the real 120s/600s bounds. + */ function shortenDeadline(): void { - globalThis.setTimeout = ((callback: (...args: unknown[]) => void, delay?: number, ...args: unknown[]) => - originalSetTimeout(callback, delay === DAEMON_REQUEST_TIMEOUT_MS ? 20 : delay, ...args) - ) as typeof setTimeout; + AbortSignal.timeout = ((ms: number) => { + requestedDeadlines.push(ms); + return originalAbortTimeout(20); + }) as typeof AbortSignal.timeout; } function stalledResponse(status = 200): void { @@ -33,7 +44,7 @@ function stalledResponse(status = 200): void { const request = (path = "/nwc/status") => callDaemonUrl("http://daemon.example", path, {}, DEFAULT_CONFIG); -describe("NWC daemon request deadline", () => { +describe("daemon request deadline", () => { test("bounds the wait for response headers", async () => { shortenDeadline(); globalThis.fetch = ((_input: string | URL | Request, init?: RequestInit) => new Promise((_resolve, reject) => { @@ -50,26 +61,27 @@ describe("NWC daemon request deadline", () => { }); } - test("does not impose the NWC deadline on mint payment routes", async () => { - let deadlines = 0; - globalThis.setTimeout = ((callback: (...args: unknown[]) => void, delay?: number, ...args: unknown[]) => { - if (delay === DAEMON_REQUEST_TIMEOUT_MS) deadlines++; - return originalSetTimeout(callback, delay, ...args); - }) as typeof setTimeout; - globalThis.fetch = (async () => Response.json({ output: "paid" })) as unknown as typeof fetch; - expect(await request("/wallet/send/bolt11")).toEqual({ output: "paid" }); - expect(deadlines).toBe(0); + test("applies the default deadline to ordinary routes", async () => { + shortenDeadline(); + globalThis.fetch = (async () => Response.json({ output: "ok" })) as unknown as typeof fetch; + expect(await request("/nwc/status")).toEqual({ output: "ok" }); + expect(requestedDeadlines).toEqual([DAEMON_REQUEST_TIMEOUT_MS]); }); - test("clears the deadline after consuming a successful response", async () => { - let cleared = 0; - globalThis.clearTimeout = ((timer) => { - cleared++; - originalClearTimeout(timer as ReturnType); - }) as typeof clearTimeout; - globalThis.fetch = (async () => Response.json({ output: "ok" })) as unknown as typeof fetch; - expect(await request()).toEqual({ output: "ok" }); - expect(cleared).toBe(1); + for (const path of ["/wallet/send/bolt11", "/wallet/receive/cashu"]) { + test(`uses the long deadline for value-moving route ${path}`, async () => { + shortenDeadline(); + globalThis.fetch = (async () => Response.json({ output: "paid" })) as unknown as typeof fetch; + expect(await request(path)).toEqual({ output: "paid" }); + expect(requestedDeadlines).toEqual([DAEMON_LONG_REQUEST_TIMEOUT_MS]); + }); + } + + test("bounds a stalled body on a long-running route too", async () => { + shortenDeadline(); + stalledResponse(); + await expect(request("/wallet/send/bolt11")).rejects.toThrow("payment outcome is unknown"); + expect(requestedDeadlines).toEqual([DAEMON_LONG_REQUEST_TIMEOUT_MS]); }); test("preserves HTTP errors rather than labeling them connection failures", async () => { diff --git a/src/utils/daemon-client.ts b/src/utils/daemon-client.ts index c6fed08..fe763f0 100644 --- a/src/utils/daemon-client.ts +++ b/src/utils/daemon-client.ts @@ -57,12 +57,31 @@ class DaemonConnectionError extends Error { } /** - * Upper bound for NWC requests, including response-body consumption. - * Other routes can perform long-running mint payments; aborting the client - * does not cancel those operations, so do not impose this cap on them. + * Upper bound for a single daemon request, including response-body + * consumption. The daemon bounds its own NWC operations, so this only guards + * against a wedged server; without it a hung request would block the CLI + * forever. */ export const DAEMON_REQUEST_TIMEOUT_MS = 120_000; +/** + * Upper bound for value-moving wallet routes. A cashu melt/swap can + * legitimately run longer than {@link DAEMON_REQUEST_TIMEOUT_MS} (the mint has + * no request timeout in routstrd), so these get a more generous bound that + * still prevents an indefinite CLI hang. + */ +export const DAEMON_LONG_REQUEST_TIMEOUT_MS = 600_000; + +/** Routes that may legitimately outlive the default request timeout. */ +const LONG_RUNNING_ROUTES = ["/wallet/send/", "/wallet/receive/"]; + +function requestTimeoutMs(path: string): number { + const pathname = path.split("?")[0] ?? path; + return LONG_RUNNING_ROUTES.some((route) => pathname.startsWith(route)) + ? DAEMON_LONG_REQUEST_TIMEOUT_MS + : DAEMON_REQUEST_TIMEOUT_MS; +} + export function getDaemonBaseUrl(config: RoutstrdConfig): string { if (config.daemonUrl) { return config.daemonUrl.replace(/\/$/, ""); @@ -106,40 +125,40 @@ export async function callDaemonUrl( if (authorization) headers.set("Authorization", authorization); if (bodyString) headers.set("Content-Type", "application/json"); - const controller = new AbortController(); - const timeoutId = path.startsWith("/nwc/") - ? setTimeout(() => controller.abort(), DAEMON_REQUEST_TIMEOUT_MS) - : undefined; - try { - let response: Response; - try { - response = await fetch(url, { - method, - headers, - body: bodyString, - signal: controller.signal, - }); - } catch (error) { - if (controller.signal.aborted) throw error; - // Only connection failures qualify for alternate-host retries. - throw new DaemonConnectionError(error); - } + const timeoutMs = requestTimeoutMs(path); + const timeoutError = () => + new Error( + `Daemon request timed out after ${timeoutMs / 1000}s; ` + + "any payment outcome is unknown — check before retrying", + ); + // The signal stays armed while the body is read, so a daemon that sends + // headers and then stalls the body cannot hang the CLI either. Aborting + // here never cancels the daemon's operation — see timeoutError's warning. + const signal = AbortSignal.timeout(timeoutMs); + let response: Response; + try { + response = await fetch(url, { + method, + headers, + body: bodyString, + signal, + }); + } catch (error) { + if (signal.aborted) throw timeoutError(); + // Only connection failures qualify for alternate-host retries. + throw new DaemonConnectionError(error); + } + + try { if (!response.ok) { const errorData = (await response.json()) as { error?: string }; throw new Error(errorData.error || `HTTP ${response.status}`); } - return await response.json() as CommandResponse; + return (await response.json()) as CommandResponse; } catch (error) { - if (controller.signal.aborted) { - throw new Error( - `Daemon request timed out after ${DAEMON_REQUEST_TIMEOUT_MS / 1000}s; ` + - "any payment outcome is unknown — check before retrying", - ); - } + if (signal.aborted) throw timeoutError(); throw error; - } finally { - if (timeoutId !== undefined) clearTimeout(timeoutId); } }