diff --git a/src/broker-amqp.ts b/src/broker-amqp.ts index 117d5c4..eee7b03 100644 --- a/src/broker-amqp.ts +++ b/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 { * 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 - * impostor does not become the broker by being asked again — and configuration errors already - * exit inside connectBroker before anything is thrown here. + * A failure that waiting cannot fix (see fatalBrokerReason) is thrown at once rather than retried — + * a permanent fault masquerading as "not reachable yet" is the silent non-progress this whole + * 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 { 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; - console.error(`mesh-tools: the broker is not reachable yet (${why}); retrying in ${delay / 1000}s`); - await new Promise((r) => setTimeout(r, delay)); + // A little jitter so every module that was up when the broker bounced does not retry in + // lockstep and stampede it as it recovers. + 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)); } } } diff --git a/test/patient-connect.test.ts b/test/patient-connect.test.ts new file mode 100644 index 0000000..ff8db7f --- /dev/null +++ b/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); +});