Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/bridges/pi/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { StringEnum } from "@earendil-works/pi-ai";

import {
createBridgeMeshSync,
formatMeshError,
createMeshErrorReporter,
buildAction,
ensureProjectRoom,
ensureRegistered,
Expand Down Expand Up @@ -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[] = [];

Expand Down
70 changes: 68 additions & 2 deletions src/core/error-reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> = new Set([
"ECONNREFUSED",
"ECONNRESET",
]);

/** The causes of a first-contact broadcast that found no network to leave through. */
const NO_NETWORK_CODES: ReadonlySet<string> = 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<PeerLinkError>,
): 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<Error>,
): 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<Error>): 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<string>();
return (error) => {
const described = describeMeshError(error);
if (described !== undefined) {
if (reported.has(described.key)) return;
reported.add(described.key);
}
write(formatMeshError(error));
};
}
41 changes: 41 additions & 0 deletions src/core/peer-link-error.ts
Original file line number Diff line number Diff line change
@@ -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;
}
10 changes: 3 additions & 7 deletions src/core/wire-mesh-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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),
);
},
);
Expand Down
141 changes: 141 additions & 0 deletions src/test/error-reporting.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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>();
Expand All @@ -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);
});
});
44 changes: 44 additions & 0 deletions src/test/peer-link-error.integration.test.ts
Original file line number Diff line number Diff line change
@@ -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();
}
});
});
Loading