Merge pull request #128 from Routstr/fix/cli-remote-url-loopback

fix(cli): normalize remote URLs and retry the other loopback family
2026-10-05 12:28:23 +00:00 · 2026-10-04 13:37:54 +00:00
parent 8d41ae8e3f b1a346c6b6
commit 64257e7d38
4 changed files with 287 additions and 36 deletions
@@ -10,6 +10,7 @@ import {
  loadConfig,
  getDaemonBaseUrl,
  getUserNpub,
+  normalizeBaseUrl,
  type CommandResponse,
 } from "./utils/daemon-client";
 import {
@@ -601,7 +602,7 @@ program
    }

    try {
-      new URL(url);
+      new URL(normalizeBaseUrl(url));
    } catch {
      console.error(`Invalid URL: ${url}`);
      process.exit(1);
@@ -609,7 +610,7 @@ program

    if (options.authUrl) {
      try {
-        new URL(options.authUrl);
+        new URL(normalizeBaseUrl(options.authUrl));
      } catch {
        console.error(`Invalid auth URL: ${options.authUrl}`);
        process.exit(1);
@@ -620,9 +621,10 @@ program
      mkdirSync(CONFIG_DIR, { recursive: true });
    }

-    const updates: Partial<RoutstrdConfig> = { daemonUrl: url };
+    const normalizedUrl = normalizeBaseUrl(url);
+    const updates: Partial<RoutstrdConfig> = { daemonUrl: normalizedUrl };
    if (options.authUrl) {
-      updates.authUrl = options.authUrl;
+      updates.authUrl = normalizeBaseUrl(options.authUrl);
    }
    let generatedNpub: string | undefined;

@@ -641,9 +643,9 @@ program

    saveDaemonConfig(updatedConfig);

-    console.log(`Remote daemon URL set to: ${url}`);
+    console.log(`Remote daemon URL set to: ${normalizedUrl}`);
    if (options.authUrl) {
-      console.log(`Auth proxy URL set to: ${options.authUrl}`);
+      console.log(`Auth proxy URL set to: ${normalizeBaseUrl(options.authUrl)}`);
    }
    if (generatedNpub) {
      console.log(
@@ -0,0 +1,103 @@
+import { describe, expect, test } from "bun:test";
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+// Each case loads config in an isolated process, without touching user config
+// or sharing fetch mocks with other test files.
+async function runCase(config: object, action: string, responses: number[]) {
+  const dir = mkdtempSync(join(tmpdir(), "routstrd-retry-"));
+  try {
+    writeFileSync(join(dir, "config.json"), JSON.stringify(config));
+    const script = `
+      const { callDaemon, callAuth, isDaemonRunning } = await import(${JSON.stringify(join(import.meta.dir, "daemon-client.ts"))});
+      const responses = ${JSON.stringify(responses)};
+      const requests = [];
+      globalThis.fetch = async (url, init) => {
+        requests.push({ url: String(url), method: init?.method ?? "GET" });
+        const status = responses.shift();
+        if (status === 0) throw new TypeError("Connection reset");
+        if (status === undefined) throw new Error("Unexpected request");
+        return Response.json(status >= 400 ? {error: "denied"} : {output: "ok"}, {status});
+      };
+      let result, error;
+      try { result = await (${action}); } catch (e) { error = e.message; }
+      console.log(JSON.stringify({ requests, result, error }));
+    `;
+    const proc = Bun.spawn([process.execPath, "--eval", script], {
+      env: { ...process.env, ROUTSTRD_DIR: dir }, stdout: "pipe", stderr: "pipe",
+    });
+    const output = await new Response(proc.stdout).text();
+    const stderr = await new Response(proc.stderr).text();
+    expect(await proc.exited, stderr).toBe(0);
+    return JSON.parse(output) as {
+      requests: { url: string; method: string }[];
+      result?: unknown;
+      error?: string;
+    };
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+}
+
+const remote = { daemonUrl: "http://localhost:18008" };
+
+describe("daemon loopback retries", () => {
+  test("GET retries a connection failure", async () => {
+    const r = await runCase(remote, 'callDaemon("/status")', [0, 200]);
+    expect(r.result).toEqual({ output: "ok" });
+    expect(r.requests.map(r => r.url)).toEqual([
+      "http://localhost:18008/status", "http://127.0.0.1:18008/status",
+    ]);
+  });
+
+  for (const method of ["POST", "PATCH", "DELETE"]) {
+    for (const [name, config, client] of [
+      ["remote", remote, "callDaemon"],
+      ["local", { host: "0.0.0.0", port: 18008 }, "callDaemon"],
+      ["auth", { authUrl: "http://localhost:18008" }, "callAuth"],
+    ] as const) {
+      test(`${name} ${method} is never replayed after a reset`, async () => {
+        const r = await runCase(config, `${client}("/wallet/send/cashu", {method: "${method}", body: {amount: 10}})`, [200, 0, 200]);
+        expect(r.error).toContain("outcome is unknown");
+        expect(r.requests.map(r => r.method)).toEqual(["GET", method]);
+      });
+    }
+  }
+
+  test("selects IPv4 with health probes before sending POST once", async () => {
+    const r = await runCase(remote, 'callDaemon("/wallet/send/cashu", {method: "POST"})', [0, 200, 200]);
+    expect(r.result).toEqual({ output: "ok" });
+    expect(r.requests).toEqual([
+      { url: "http://localhost:18008/health", method: "GET" },
+      { url: "http://127.0.0.1:18008/health", method: "GET" },
+      { url: "http://127.0.0.1:18008/wallet/send/cashu", method: "POST" },
+    ]);
+  });
+
+  test("a single-address write also reports an unknown outcome", async () => {
+    const r = await runCase({daemonUrl: "http://127.0.0.1:18008"}, 'callDaemon("/wallet/send/cashu", {method: "POST"})', [0]);
+    expect(r.error).toContain("outcome is unknown");
+    expect(r.requests).toHaveLength(1);
+  });
+
+  test("write address selection stops on HTTP errors", async () => {
+    const r = await runCase(remote, 'callDaemon("/wallet/send/cashu", {method: "POST"})', [403, 200]);
+    expect(r.error).toBe("denied");
+    expect(r.requests).toHaveLength(1);
+  });
+
+  for (const config of [remote, {host: "0.0.0.0", port: 18008}]) {
+    test(`health HTTP errors stop fallback (${JSON.stringify(config)})`, async () => {
+      const r = await runCase(config, "isDaemonRunning()", [403, 200]);
+      expect(r.result).toBe(false);
+      expect(r.requests).toHaveLength(1);
+    });
+  }
+
+  test("health connection failures still allow fallback", async () => {
+    const r = await runCase(remote, "isDaemonRunning()", [0, 200]);
+    expect(r.result).toBe(true);
+    expect(r.requests).toHaveLength(2);
+  });
+});
@@ -49,6 +49,48 @@ function localDaemonBaseUrls(config: RoutstrdConfig): string[] {
  );
 }

+/**
+ * Normalize a user-supplied daemon/auth URL.
+ *
+ * `new URL("localhost:8008")` does not throw -- it parses `localhost:` as the
+ * scheme -- so a scheme-less value silently becomes an unusable base URL.
+ * Add `http://` when no scheme is present and strip trailing slashes.
+ */
+export function normalizeBaseUrl(raw: string): string {
+  const trimmed = raw.trim().replace(/\/+$/, "");
+  if (!/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed)) {
+    return `http://${trimmed}`;
+  }
+  return trimmed;
+}
+
+/**
+ * Candidate base URLs for a configured remote endpoint.
+ *
+ * When the host is `localhost`, prefer the other loopback family as well:
+ * some environments (notably rootless podman) accept a connection on one
+ * family and immediately reset it, so a single `localhost` URL can fail even
+ * though the server is reachable on `127.0.0.1` or `[::1]`.
+ */
+export function baseUrlCandidates(baseUrl: string): string[] {
+  const normalized = normalizeBaseUrl(baseUrl);
+  let url: URL;
+  try {
+    url = new URL(normalized);
+  } catch {
+    return [normalized];
+  }
+  if (url.hostname !== "localhost") return [normalized];
+
+  const candidates: string[] = [];
+  for (const host of ["localhost", "127.0.0.1", "[::1]"]) {
+    const candidate = new URL(normalized);
+    candidate.hostname = host;
+    candidates.push(candidate.toString().replace(/\/+$/, ""));
+  }
+  return [...new Set(candidates)];
+}
+
 class DaemonConnectionError extends Error {
  constructor(cause: unknown) {
    super("Failed to connect to daemon", { cause });
@@ -84,14 +126,14 @@ function requestTimeoutMs(path: string): number {

 export function getDaemonBaseUrl(config: RoutstrdConfig): string {
  if (config.daemonUrl) {
-    return config.daemonUrl.replace(/\/$/, "");
+    return normalizeBaseUrl(config.daemonUrl);
  }
  return `http://${urlHost(config.host)}:${config.port}`;
 }

 export function getAuthBaseUrl(config: RoutstrdConfig): string {
  if (config.authUrl) {
-    return config.authUrl.replace(/\/$/, "");
+    return normalizeBaseUrl(config.authUrl);
  }
  return getDaemonBaseUrl(config);
 }
@@ -162,22 +204,59 @@ export async function callDaemonUrl(
  }
 }

+/** Resolve loopback addresses with safe reads, never by replaying a write. */
+async function callDaemonCandidates(
+  candidates: string[],
+  path: string,
+  options: { method?: "GET" | "POST" | "PATCH" | "DELETE"; body?: object },
+  config: RoutstrdConfig,
+): Promise<CommandResponse> {
+  const isRead = (options.method ?? "GET") === "GET";
+  let connectionError: DaemonConnectionError | undefined;
+  for (const baseUrl of candidates) {
+    if (!isRead && candidates.length > 1) {
+      try {
+        // Select an address before dispatching a potentially money-moving
+        // request. An HTTP error is authoritative, not a reason to fall back.
+        await callDaemonUrl(baseUrl, "/health", { method: "GET" }, config);
+      } catch (error) {
+        if (!(error instanceof DaemonConnectionError)) throw error;
+        connectionError = error;
+        continue;
+      }
+    }
+    try {
+      return await callDaemonUrl(baseUrl, path, options, config);
+    } catch (error) {
+      if (!(error instanceof DaemonConnectionError)) throw error;
+      if (!isRead) {
+        // fetch can reject after the server has committed the operation.
+        throw new Error(
+          "Connection lost; operation outcome is unknown — check its status before retrying",
+          { cause: error },
+        );
+      }
+      connectionError = error;
+    }
+  }
+  throw connectionError ?? new Error("No daemon host candidates available");
+}
+
 async function callLocalDaemon(
  path: string,
  options: { method?: "GET" | "POST" | "PATCH" | "DELETE"; body?: object },
  config: RoutstrdConfig,
 ): Promise<CommandResponse> {
-  // Retry only connection failures; HTTP errors prove that a server answered.
-  let connectionError: DaemonConnectionError | undefined;
-  for (const baseUrl of localDaemonBaseUrls(config)) {
-    try {
-      return await callDaemonUrl(baseUrl, path, options, config);
-    } catch (error) {
-      if (!(error instanceof DaemonConnectionError)) throw error;
-      connectionError = error;
-    }
-  }
-  throw connectionError ?? new Error("No daemon host candidates available");
+  return callDaemonCandidates(localDaemonBaseUrls(config), path, options, config);
+}
+
+async function callRemoteDaemon(
+  baseUrl: string,
+  path: string,
+  options: { method?: "GET" | "POST" | "PATCH" | "DELETE"; body?: object },
+  config: RoutstrdConfig,
+): Promise<CommandResponse> {
+  return callDaemonCandidates(baseUrlCandidates(baseUrl), path, options, config);
 }

 export async function callDaemon(
@@ -186,7 +265,7 @@ export async function callDaemon(
 ): Promise<CommandResponse> {
  const config = await loadConfig();
  if (config.daemonUrl) {
-    return callDaemonUrl(getDaemonBaseUrl(config), path, options, config);
+    return callRemoteDaemon(getDaemonBaseUrl(config), path, options, config);
  }
  return callLocalDaemon(path, options, config);
 }
@@ -201,7 +280,7 @@ export async function callAuth(
  if (!config.authUrl && !config.daemonUrl) {
    return callLocalDaemon(path, options, config);
  }
-  return callDaemonUrl(getAuthBaseUrl(config), path, options, config);
+  return callRemoteDaemon(getAuthBaseUrl(config), path, options, config);
 }

 export async function isDaemonRunning(): Promise<boolean> {
@@ -209,21 +288,23 @@ export async function isDaemonRunning(): Promise<boolean> {
    const config = await loadConfig();

    if (config.daemonUrl) {
-      const baseUrl = config.daemonUrl.replace(/\/$/, "");
-      const url = `${baseUrl}/health`;
-      let authorization: string | undefined;
-      if (config.nsec) {
-        const secretKey = parseSecretKey(config.nsec);
-        authorization = await createNIP98Authorization(secretKey, url, "GET");
-      }
-      try {
-        const response = await fetch(url, {
-          headers: authorization ? { Authorization: authorization } : {},
-        });
-        return response.ok;
-      } catch {
-        return false;
+      for (const baseUrl of baseUrlCandidates(config.daemonUrl)) {
+        const url = `${baseUrl}/health`;
+        let authorization: string | undefined;
+        if (config.nsec) {
+          const secretKey = parseSecretKey(config.nsec);
+          authorization = await createNIP98Authorization(secretKey, url, "GET");
+        }
+        try {
+          const response = await fetch(url, {
+            headers: authorization ? { Authorization: authorization } : {},
+          });
+          return response.ok;
+        } catch {
+          // Try the next candidate host.
+        }
      }
+      return false;
    }

    // A wildcard bind may be listening on either loopback family.
@@ -234,7 +315,7 @@ export async function isDaemonRunning(): Promise<boolean> {
        const response = await fetch(`${baseUrl}/health`, {
          signal: controller.signal,
        });
-        if (response.ok) return true;
+        return response.ok;
      } catch {
        // Try the next candidate host.
      } finally {
@@ -0,0 +1,65 @@
+import { describe, expect, test } from "bun:test";
+import {
+  baseUrlCandidates,
+  getAuthBaseUrl,
+  getDaemonBaseUrl,
+  normalizeBaseUrl,
+} from "./daemon-client";
+import { DEFAULT_CONFIG } from "./config";
+
+describe("normalizeBaseUrl", () => {
+  test("adds http:// when the scheme is omitted", () => {
+    // `new URL("localhost:18008")` parses "localhost:" as the scheme, so this
+    // must be repaired before it reaches fetch.
+    expect(normalizeBaseUrl("localhost:18008")).toBe("http://localhost:18008");
+    expect(normalizeBaseUrl("127.0.0.1:8008")).toBe("http://127.0.0.1:8008");
+    expect(normalizeBaseUrl("daemon.example")).toBe("http://daemon.example");
+  });
+
+  test("adds http:// for a bracketed IPv6 host without a scheme", () => {
+    expect(normalizeBaseUrl("[::1]:8008")).toBe("http://[::1]:8008");
+  });
+
+  test("preserves an explicit scheme", () => {
+    expect(normalizeBaseUrl("https://daemon.example")).toBe("https://daemon.example");
+    expect(normalizeBaseUrl("http://127.0.0.1:18008")).toBe("http://127.0.0.1:18008");
+  });
+
+  test("trims whitespace and trailing slashes", () => {
+    expect(normalizeBaseUrl("  http://localhost:18008/  ")).toBe("http://localhost:18008");
+    expect(normalizeBaseUrl("localhost:18008///")).toBe("http://localhost:18008");
+  });
+});
+
+describe("baseUrlCandidates", () => {
+  test("offers both loopback families for localhost", () => {
+    expect(baseUrlCandidates("localhost:18008")).toEqual([
+      "http://localhost:18008",
+      "http://127.0.0.1:18008",
+      "http://[::1]:18008",
+    ]);
+  });
+
+  test("leaves non-loopback hosts alone", () => {
+    expect(baseUrlCandidates("http://daemon.example:8008")).toEqual([
+      "http://daemon.example:8008",
+    ]);
+  });
+});
+
+describe("remote base URL resolution", () => {
+  test("normalizes a legacy scheme-less daemonUrl", () => {
+    expect(getDaemonBaseUrl({ ...DEFAULT_CONFIG, daemonUrl: "localhost:18008" })).toBe(
+      "http://localhost:18008",
+    );
+  });
+
+  test("normalizes authUrl and falls back to daemonUrl", () => {
+    expect(
+      getAuthBaseUrl({ ...DEFAULT_CONFIG, daemonUrl: "localhost:18008", authUrl: "localhost:9000" }),
+    ).toBe("http://localhost:9000");
+    expect(getAuthBaseUrl({ ...DEFAULT_CONFIG, daemonUrl: "localhost:18008" })).toBe(
+      "http://localhost:18008",
+    );
+  });
+});