src/broker-amqp.ts

@@ -24,6 +24,41 @@ interface Reply {
   error?: string;
 }
 
+/** The broker presented a certificate whose fingerprint is not the one the mesh pinned. A distinct
+ *  type rather than a message to grep, so a caller deciding "wait or refuse" (serve mode's patient
+ *  reconnect, novox/hq issue 058) tells this apart from an absent broker by `instanceof`, not by a
+ *  prose string that a later reword would silently turn back into an infinite retry against an
+ *  impostor. */
+export class PinMismatchError extends Error {}
+
+/**
+ * Why a broker connection failed in a way no amount of waiting will fix — or null when it is worth
+ * retrying. Serve mode's patient reconnect (novox/hq issue 058) uses this to tell a permanent
+ * fault from a broker that is merely not up yet. Three failures are permanent:
+ *
+ *   - the certificate does not match the pin — an impostor does not become the broker by being
+ *     asked again (typed, so a reworded message cannot silently turn this back into a retry);
+ *   - the broker URL is not a URL — a malformed address never parses on the next try;
+ *   - the broker answered and refused the login — a wrong or revoked credential, not an absent
+ *     broker, and it will refuse the next attempt identically.
+ *
+ * Everything else — connection refused, timeout, DNS not resolving yet — is the overlay still
+ * coming up, and is retried.
+ */
+export function fatalBrokerReason(err: unknown): string | null {
+  if (err instanceof PinMismatchError) return "the broker's certificate does not match the pin";
+  const e = err as { code?: unknown; message?: unknown };
+  const code = typeof e?.code === "string" ? e.code : "";
+  const message = typeof e?.message === "string" ? e.message : String(err);
+  if (code === "ERR_INVALID_URL" || /invalid url/i.test(message)) {
+    return `the broker URL is not a URL (${message})`;
+  }
+  if (/access[-_ ]?refused|login was refused|handshake terminated|\b403\b/i.test(message)) {
+    return `the broker refused the login (${message})`;
+  }
+  return null;
+}
+
 /** A broker credential as the mesh delivers it (novox/hq ADR 0043): an amqps URL, the fingerprint
  *  of the certificate the broker must present, and the node and module the account is scoped to (so
  *  the runtime names its queue as the mesh did). A plain string is a bootstrap URL. */
@@ -299,7 +334,7 @@ async function pinnedOptions(rawUrl: string, fingerprint: string): Promise<tls.C
 
   const seen = createHash("sha256").update(certificate.raw).digest("hex");
   if (seen !== normalizeFingerprint(fingerprint)) {
-    throw new Error(
+    throw new PinMismatchError(
       `the broker's certificate (sha256:${seen}) does not match the pinned ${fingerprint} — refusing`,
     );
   }

src/main.ts

@@ -22,7 +22,7 @@
 
 import { readFileSync } from "node:fs";
 import { pathToFileURL } from "node:url";
-import { connectAmqp } from "./broker-amqp.js";
+import { connectAmqp, fatalBrokerReason } from "./broker-amqp.js";
 import type { Credential } from "./broker-amqp.js";
 import { runTools } from "./runtime.js";
 import { invokeTool } from "@novox/mesh-sdk/tools";
@@ -73,19 +73,27 @@ async function connectBroker(): Promise<Broker> {
  * runtime, which read as a crash-loop to every restart-counting health check and every person
  * watching. Retried indefinitely, aloud: the dependency appears or somebody reads why not.
  *
- * Only reachability retries. A pinned-certificate mismatch is a refusal, not a wait — an
+ * A failure that waiting cannot fix (see fatalBrokerReason) is thrown at once rather than retried —
- * impostor does not become the broker by being asked again — and configuration errors already
+ * a permanent fault masquerading as "not reachable yet" is the silent non-progress this whole
- * exit inside connectBroker before anything is thrown here.
+ * change exists to remove. Missing-file and empty-URL configuration errors exit inside
+ * connectBroker before they reach here; a malformed URL and a refused login are caught here.
  */
 async function connectBrokerPatiently(): Promise<Broker> {
   for (let delay = 2_000; ; delay = Math.min(delay * 2, 30_000)) {
     try {
       return await connectBroker();
     } catch (err) {
+      const fatal = fatalBrokerReason(err);
+      if (fatal !== null) {
+        console.error(`mesh-tools: ${fatal} — waiting will not fix this; giving up`);
+        throw err;
+      }
       const why = err instanceof Error ? err.message : String(err);
-      if (why.includes("does not match the pinned")) throw err;
+      // A little jitter so every module that was up when the broker bounced does not retry in
-      console.error(`mesh-tools: the broker is not reachable yet (${why}); retrying in ${delay / 1000}s`);
+      // lockstep and stampede it as it recovers.
-      await new Promise((r) => setTimeout(r, delay));
+      const wait = delay + Math.floor(Math.random() * 1_000);
+      console.error(`mesh-tools: the broker is not reachable yet (${why}); retrying in ${Math.round(wait / 1000)}s`);
+      await new Promise((r) => setTimeout(r, wait));
     }
   }
 }

test/patient-connect.test.ts

@@ -0,0 +1,46 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { fatalBrokerReason, PinMismatchError } from "../src/broker-amqp.ts";
+
+// novox/hq issue 058 (and its review): serve mode retries a broker that is not up yet, but must
+// give up at once on a failure waiting cannot fix — otherwise a permanent fault loops for ever
+// disguised as "not reachable". This is the classifier that draws the line; the bed cannot test it
+// (it starts the consumer only after the broker is up), so it is proven here.
+
+test("a broker that is not up yet is retryable, not fatal", () => {
+  for (const err of [
+    Object.assign(new Error("connect ECONNREFUSED 10.42.0.1:5671"), { code: "ECONNREFUSED" }),
+    Object.assign(new Error("connect ETIMEDOUT"), { code: "ETIMEDOUT" }),
+    Object.assign(new Error("getaddrinfo EAI_AGAIN anchor.internal"), { code: "EAI_AGAIN" }),
+    new Error("timed out fetching the broker's certificate"),
+  ]) {
+    assert.equal(fatalBrokerReason(err), null, `should retry: ${(err as Error).message}`);
+  }
+});
+
+test("a certificate that does not match the pin is fatal, by type not by message", () => {
+  // Typed, so rewording the message cannot turn an impostor back into an infinite retry.
+  assert.notEqual(fatalBrokerReason(new PinMismatchError("anything at all")), null);
+  // A plain Error with pin-ish words is NOT treated as the pin case — only the type is.
+  assert.equal(fatalBrokerReason(new Error("the pinned value was fine")), null);
+});
+
+test("a malformed broker URL is fatal — it never parses on the next try", () => {
+  assert.notEqual(fatalBrokerReason(Object.assign(new Error("Invalid URL"), { code: "ERR_INVALID_URL" })), null);
+  assert.notEqual(fatalBrokerReason(new Error("Invalid URL: not-a-url")), null);
+});
+
+test("a refused login is fatal — a wrong or revoked credential, not an absent broker", () => {
+  for (const msg of [
+    "Handshake terminated by server: 403 (ACCESS-REFUSED) with message \"ACCESS_REFUSED - Login was refused\"",
+    "Login was refused using authentication mechanism PLAIN",
+    "ACCESS_REFUSED",
+  ]) {
+    assert.notEqual(fatalBrokerReason(new Error(msg)), null, `should be fatal: ${msg}`);
+  }
+});
+
+test("a non-Error value does not crash the classifier", () => {
+  assert.equal(fatalBrokerReason("just a string"), null);
+  assert.equal(fatalBrokerReason(undefined), null);
+});