From c7a58f9b626df67e08cd797b34896d49d1d5de12 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Mon, 19 Oct 2026 16:47:02 +0100 Subject: [PATCH] fix(core): explain stale-peer and no-network errors instead of raw socket codes A peer that exits or restarts leaves a stale address behind, and the dials and sends to it reached the pi UI as bare ECONNREFUSED or ECONNRESET messages, one per attempt. Likewise a host with no network route raised ENETUNREACH on every first-contact beacon. Dial and send failures are now a PeerLinkError that names the peer and keeps the transport's errno. describeMeshError turns the known conditions into a sentence saying what happened, what it most likely means and whether anything needs doing, and the reporter writes each recognised condition once instead of once per attempt. The pi bridge uses the shared reporter. Refs #427 --- src/bridges/pi/extension.ts | 8 +- src/core/error-reporting.ts | 70 ++++++++- src/core/peer-link-error.ts | 41 ++++++ src/core/wire-mesh-transport.ts | 10 +- src/test/error-reporting.unit.test.ts | 141 +++++++++++++++++++ src/test/peer-link-error.integration.test.ts | 44 ++++++ 6 files changed, 301 insertions(+), 13 deletions(-) create mode 100644 src/core/peer-link-error.ts create mode 100644 src/test/peer-link-error.integration.test.ts diff --git a/src/bridges/pi/extension.ts b/src/bridges/pi/extension.ts index 08bb2c0f..a58e94fb 100644 --- a/src/bridges/pi/extension.ts +++ b/src/bridges/pi/extension.ts @@ -20,7 +20,7 @@ import { StringEnum } from "@earendil-works/pi-ai"; import { createBridgeMeshSync, - formatMeshError, + createMeshErrorReporter, buildAction, ensureProjectRoom, ensureRegistered, @@ -52,9 +52,9 @@ export default function (pi: ExtensionAPI) { tool.getWebUrlStatus = () => getWebUrlStatus(webHandle); let uiCtx: ExtensionUIContext | undefined; // The extension runs inside pi's own terminal UI, so mesh errors surface as pi notifications rather than raw stderr writes that would corrupt its display. - store.onError = (error) => { - uiCtx?.notify(formatMeshError(error).trimEnd(), "error"); - }; + store.onError = createMeshErrorReporter((line) => { + uiCtx?.notify(line.trimEnd(), "error"); + }); let projectRoom: string | undefined; const informationalBuffer: string[] = []; diff --git a/src/core/error-reporting.ts b/src/core/error-reporting.ts index 8d52744d..f1cb2d3d 100644 --- a/src/core/error-reporting.ts +++ b/src/core/error-reporting.ts @@ -2,18 +2,84 @@ * Default sink for a mesh store's error channel. MeshStore.onError carries every failure the mesh handles internally (a hub dial that was refused, a gossip send that failed, a default-front session that could not attach), and nothing shows them unless a bridge assigns a handler, so a bridge without a UI of its own routes them to stderr. */ +import { shortId } from "./display-name.js"; +import { errnoCode, PeerLinkError } from "./peer-link-error.js"; + +/** The causes of a refused or dropped connection to a peer, which all mean the process behind that address has gone. */ +const PEER_GONE_CODES: ReadonlySet = new Set([ + "ECONNREFUSED", + "ECONNRESET", +]); + +/** The causes of a first-contact broadcast that found no network to leave through. */ +const NO_NETWORK_CODES: ReadonlySet = new Set([ + "ENETUNREACH", + "EHOSTUNREACH", + "ENETDOWN", +]); + +/** A mesh error as a person should read it: `message` says what happened, what it most likely means and whether anything needs doing, and `key` identifies the condition so a repeat of it can be recognised. */ +export interface DescribedMeshError { + key: string; + message: string; +} + +function describePeerLinkError( + error: Readonly, +): DescribedMeshError { + const peer = shortId(error.peerId); + const code = errnoCode(error); + if (error.action === "send") { + return { + key: `peer-send:${error.peerId}`, + message: `The connection to peer ${peer} dropped while a message was being sent (${error.cause instanceof Error ? error.cause.message : String(error.cause)}). The peer most likely exited or restarted.`, + }; + } + if (code !== undefined && PEER_GONE_CODES.has(code)) { + return { + key: `peer-gone:${error.peerId}`, + message: `Could not connect to peer ${peer} on port ${String(error.port)} (${code}). Its agent most likely exited or restarted, leaving a stale address behind, and no action is needed. If a peer that should be running stays unreachable, check that nothing is blocking connections to localhost.`, + }; + } + return { + key: `peer-dial:${error.peerId}`, + message: `Could not connect to peer ${peer} on port ${String(error.port)}: ${error.cause instanceof Error ? error.cause.message : String(error.cause)}`, + }; +} + +/** Recognises the mesh failures that have a known meaning and a usual remedy, and returns them described for a person. Anything else is `undefined` and is reported as it came. */ +export function describeMeshError( + error: Readonly, +): DescribedMeshError | undefined { + if (error instanceof PeerLinkError) return describePeerLinkError(error); + const code = errnoCode(error); + if (code !== undefined && NO_NETWORK_CODES.has(code)) { + return { + key: `no-network:${code}`, + message: `Could not reach the local network to look for agents on other machines (${code}). Check that the network is connected; agents on this machine still find each other, and discovery of other machines resumes once a network is available.`, + }; + } + return undefined; +} + /** The single line a mesh error is reported as. */ export function formatMeshError(error: Readonly): string { - return `agent-comms: ${error.message}\n`; + return `agent-comms: ${describeMeshError(error)?.message ?? error.message}\n`; } -/** Builds the handler a bridge assigns to `store.onError`, writing each error to `write` as one line. `write` defaults to the process's stderr, which stdio bridges keep clear of protocol traffic. Returns the handler rather than assigning it, so the caller does the assignment itself, like wireDefaultCcPeerFront does for onCoordinatorRoleChanged. */ +/** Builds the handler a bridge assigns to `store.onError`, writing each error to `write` as one line. `write` defaults to the process's stderr, which stdio bridges keep clear of protocol traffic. A recognised condition is written once per reporter: a stale peer or a missing network repeats on every dial and beacon, and a second line says nothing the first did not. Errors with no known meaning are always written. Returns the handler rather than assigning it, so the caller does the assignment itself, like wireDefaultCcPeerFront does for onCoordinatorRoleChanged. */ export function createMeshErrorReporter( write: (line: string) => void = (line) => { process.stderr.write(line); }, ): (error: Error) => void { + const reported = new Set(); return (error) => { + const described = describeMeshError(error); + if (described !== undefined) { + if (reported.has(described.key)) return; + reported.add(described.key); + } write(formatMeshError(error)); }; } diff --git a/src/core/peer-link-error.ts b/src/core/peer-link-error.ts new file mode 100644 index 00000000..52ef4611 --- /dev/null +++ b/src/core/peer-link-error.ts @@ -0,0 +1,41 @@ +/** + * A failure on the data connection to one specific peer, carrying which peer and what the transport was doing so the error channel's consumers can say something actionable instead of repeating a raw socket message. + */ + +/** What the transport was doing on the connection to the peer when it failed. */ +export type PeerLinkAction = "dial" | "send"; + +export class PeerLinkError extends Error { + /** + * @param action - What the transport was doing when the connection failed. + * @param peerId - The full device-id of the peer the connection was to. + * @param port - The data port that was dialled; absent for a failure on an established connection, which no longer names one. + * @param cause - The transport's own error, kept intact so its errno `code` stays readable. + */ + constructor( + readonly action: PeerLinkAction, + readonly peerId: string, + readonly port: number | undefined, + cause: unknown, + ) { + const reason = cause instanceof Error ? cause.message : String(cause); + super( + action === "dial" + ? `connectToPeer(${peerId}, port ${String(port)}): ${reason}` + : `send(${peerId}) failed: ${reason}`, + { cause }, + ); + this.name = "PeerLinkError"; + } +} + +/** The errno code (`ECONNREFUSED`, `ENETUNREACH`, ...) of an error or the first error in its cause chain that has one. */ +export function errnoCode(error: unknown): string | undefined { + let current: unknown = error; + while (current instanceof Error) { + if ("code" in current && typeof current.code === "string") + return current.code; + current = current.cause; + } + return undefined; +} diff --git a/src/core/wire-mesh-transport.ts b/src/core/wire-mesh-transport.ts index dfd15a1f..9e17d881 100644 --- a/src/core/wire-mesh-transport.ts +++ b/src/core/wire-mesh-transport.ts @@ -92,6 +92,7 @@ import type { } from "./transport.js"; import type { PeerIdentity } from "./identity.js"; import { toIdentityPort } from "./wire-mesh-identity.js"; +import { PeerLinkError } from "./peer-link-error.js"; import { DATA_SERVER_ACCEPT, DEFAULT_LISTENER_ACCEPT, @@ -664,11 +665,8 @@ export class WireMeshTransport implements MeshTransport { const connection = await this.wireTransport .connect(`${host}:${String(peer.port)}`) .catch((error: unknown) => { - const reason = error instanceof Error ? error.message : String(error); this.events.onError?.( - new Error( - `connectToPeer(${peer.id}, port ${String(peer.port)}): ${reason}`, - ), + new PeerLinkError("dial", peer.id, peer.port, error), ); return undefined; }); @@ -720,9 +718,7 @@ export class WireMeshTransport implements MeshTransport { await this.sendFrame(session, buildCommand(message)).catch( (error: unknown) => { this.events.onError?.( - error instanceof Error - ? error - : new Error(`send(${handle.id}) failed: ${String(error)}`), + new PeerLinkError("send", handle.id, undefined, error), ); }, ); diff --git a/src/test/error-reporting.unit.test.ts b/src/test/error-reporting.unit.test.ts index 28857513..27714893 100644 --- a/src/test/error-reporting.unit.test.ts +++ b/src/test/error-reporting.unit.test.ts @@ -4,8 +4,24 @@ import { describe, expect, it, vi } from "vitest"; import { createMeshErrorReporter, + describeMeshError, formatMeshError, } from "../core/error-reporting.js"; +import { shortId } from "../core/display-name.js"; +import { PeerLinkError } from "../core/peer-link-error.js"; + +const PEER_ID = + "7d5bb622d5890adc2c4b63e8fd95fb27319be5f3876657b072f1a8a38f2cdbee"; +const OTHER_PEER_ID = "a".repeat(PEER_ID.length); +// An arbitrary data port, named so the assertions read as the dialled address. +const DATA_PORT = 41001; + +/** An error shaped like the ones Node's sockets raise, with the errno code on the error itself. */ +function errno(code: string, message: string): NodeJS.ErrnoException { + const error: NodeJS.ErrnoException = new Error(message); + error.code = code; + return error; +} describe("formatMeshError", () => { it("reports an error as one prefixed, newline-terminated line", () => { @@ -15,6 +31,89 @@ describe("formatMeshError", () => { }); }); +describe("describeMeshError", () => { + it("explains a refused dial as a stale peer address, naming the peer and port", () => { + const error = new PeerLinkError( + "dial", + PEER_ID, + DATA_PORT, + errno("ECONNREFUSED", "connect ECONNREFUSED 127.0.0.1:41001"), + ); + + const described = describeMeshError(error); + + expect(described?.message).toContain(shortId(PEER_ID)); + expect(described?.message).toContain(String(DATA_PORT)); + expect(described?.message).toContain("most likely exited or restarted"); + }); + + it("treats a reset dial like a refused one", () => { + const error = new PeerLinkError( + "dial", + PEER_ID, + DATA_PORT, + errno("ECONNRESET", "read ECONNRESET"), + ); + + expect(describeMeshError(error)?.message).toContain("stale address"); + }); + + it("keeps the transport's own reason for a dial failure it has no explanation for", () => { + const error = new PeerLinkError( + "dial", + PEER_ID, + DATA_PORT, + errno("ETIMEDOUT", "connect ETIMEDOUT 127.0.0.1:41001"), + ); + + const described = describeMeshError(error); + + expect(described?.message).toContain("connect ETIMEDOUT 127.0.0.1:41001"); + expect(described?.message).not.toContain("stale address"); + }); + + it("explains a send cut off by a disconnect, naming the peer", () => { + const error = new PeerLinkError( + "send", + PEER_ID, + undefined, + new Error("disconnected before a response arrived"), + ); + + const described = describeMeshError(error); + + expect(described?.message).toContain(shortId(PEER_ID)); + expect(described?.message).toContain( + "disconnected before a response arrived", + ); + }); + + it("explains a broadcast with nowhere to go as a missing network", () => { + const described = describeMeshError( + errno("ENETUNREACH", "send ENETUNREACH 255.255.255.255:19877"), + ); + + expect(described?.message).toContain("ENETUNREACH"); + expect(described?.message).toContain("network"); + }); + + it("recognises nothing in an error it has no meaning for", () => { + expect(describeMeshError(new Error("listen EADDRINUSE"))).toBeUndefined(); + }); + + it("keys a condition by the peer it is about", () => { + const refused = (peerId: string, port: number) => + describeMeshError( + new PeerLinkError("dial", peerId, port, errno("ECONNREFUSED", "x")), + )?.key; + + expect(refused(PEER_ID, DATA_PORT)).toBe(refused(PEER_ID, DATA_PORT + 1)); + expect(refused(PEER_ID, DATA_PORT)).not.toBe( + refused(OTHER_PEER_ID, DATA_PORT), + ); + }); +}); + describe("createMeshErrorReporter", () => { it("writes each error to the sink as one prefixed line", () => { const write = vi.fn<(line: string) => void>(); @@ -40,4 +139,46 @@ describe("createMeshErrorReporter", () => { stderr.mockRestore(); } }); + + it("writes a recognised condition once however often it repeats, even when the port changes", () => { + const write = vi.fn<(line: string) => void>(); + const report = createMeshErrorReporter(write); + const refused = (port: number) => + new PeerLinkError("dial", PEER_ID, port, errno("ECONNREFUSED", "x")); + + report(refused(DATA_PORT)); + report(refused(DATA_PORT + 1)); + report(refused(DATA_PORT + 2)); + + expect(write).toHaveBeenCalledTimes(1); + }); + + it("writes the same condition for a different peer", () => { + const write = vi.fn<(line: string) => void>(); + const report = createMeshErrorReporter(write); + + report( + new PeerLinkError("dial", PEER_ID, DATA_PORT, errno("ECONNREFUSED", "x")), + ); + report( + new PeerLinkError( + "dial", + OTHER_PEER_ID, + DATA_PORT, + errno("ECONNREFUSED", "x"), + ), + ); + + expect(write).toHaveBeenCalledTimes(2); + }); + + it("writes every error it has no meaning for", () => { + const write = vi.fn<(line: string) => void>(); + const report = createMeshErrorReporter(write); + + report(new Error("hub unreachable")); + report(new Error("hub unreachable")); + + expect(write).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/test/peer-link-error.integration.test.ts b/src/test/peer-link-error.integration.test.ts new file mode 100644 index 00000000..ee06fa63 --- /dev/null +++ b/src/test/peer-link-error.integration.test.ts @@ -0,0 +1,44 @@ +/** + * A dial to a peer whose process is gone reaches the error channel as a typed PeerLinkError whose errno survives the transport, which is what lets the reporter tell a stale peer address from any other dial failure. + */ +import { describe, expect, test } from "vitest"; +import { describeMeshError } from "../core/error-reporting.js"; +import { generateIdentity } from "../core/identity.js"; +import { errnoCode, PeerLinkError } from "../core/peer-link-error.js"; +import { WireMeshTransport } from "../core/wire-mesh-transport.js"; +import { freeLocalPort } from "./hub-helpers.js"; +import { noopTransportEvents, waitFor } from "./test-transport.js"; + +describe("WireMeshTransport dial failures", () => { + test("a refused dial is reported as a PeerLinkError carrying ECONNREFUSED", async () => { + const errors: Error[] = []; + const transport = new WireMeshTransport( + noopTransportEvents({ onError: (e) => void errors.push(e) }), + generateIdentity(), + ); + try { + const deadPort = await freeLocalPort(); + await transport.connectToPeer( + { + id: "unreachable-peer", + port: deadPort, + startedAt: new Date().toISOString(), + }, + "self", + ); + + await waitFor( + () => errors.length > 0, + "the transport reports the failed dial", + ); + const [error] = errors; + expect(error).toBeInstanceOf(PeerLinkError); + expect(errnoCode(error)).toBe("ECONNREFUSED"); + expect( + error === undefined ? undefined : describeMeshError(error)?.message, + ).toContain("stale address"); + } finally { + await transport.shutdown(); + } + }); +});