From 6a2f9daac778e993cc2e68ff2c23307f2303d171 Mon Sep 17 00:00:00 2001 From: Rinse Date: Sun, 24 May 2026 01:49:48 +0000 Subject: [PATCH 1/3] fix(daemon-server): await express listen() so port is bound before startup completes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `webuiExpressApp.listen(port)` was fire-and-forget — no await on 'listening', no 'error' handler. Under load `startDaemonServer` could return (and the daemon could log "Communities in data path") before the OS had finished binding the RPC port, and a bind failure showed up later as an uncaughtException that crashed the daemon *after* it had already advertised itself as ready. The webui suite's first fetch sometimes hit the open window and failed with ECONNREFUSED. Wrap listen() in a promise that resolves on 'listening' and rejects on 'error'. Adds a unit test that pre-binds the port and asserts the function rejects with EADDRINUSE. Fixes issue #42. --- src/webui/daemon-server.ts | 18 +++++++++++- test/webui/daemon-server.test.ts | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 test/webui/daemon-server.test.ts diff --git a/src/webui/daemon-server.ts b/src/webui/daemon-server.ts index 1fca51f..56e3511 100644 --- a/src/webui/daemon-server.ts +++ b/src/webui/daemon-server.ts @@ -46,7 +46,23 @@ export async function startDaemonServer(rpcUrl: URL, ipfsGatewayUrl: URL, pkcOpt // Start pkc-js RPC const log = PKCLogger("bitsocial-cli:daemon:startDaemonServer"); const webuiExpressApp = express(); - const httpServer = webuiExpressApp.listen(Number(rpcUrl.port)); + // Wait for bind to actually complete before returning. Calling express.listen() without + // awaiting 'listening' lets startup proceed before the port is accepting connections, + // and without an 'error' handler a bind failure becomes an uncaughtException that kills + // the daemon *after* it has already logged "Communities in data path" — see issue #42. + const httpServer = await new Promise((resolve, reject) => { + const server = webuiExpressApp.listen(Number(rpcUrl.port)); + const onListening = () => { + server.off("error", onError); + resolve(server); + }; + const onError = (err: Error) => { + server.off("listening", onListening); + reject(err); + }; + server.once("listening", onListening); + server.once("error", onError); + }); log("HTTP server is running on", "0.0.0.0" + ":" + rpcUrl.port); const rpcAuthKey = await _generateRpcAuthKeyIfNotExisting(pkcOptions.dataPath!); const PKCRpc = await import("@pkcprotocol/pkc-js/rpc"); diff --git a/test/webui/daemon-server.test.ts b/test/webui/daemon-server.test.ts new file mode 100644 index 0000000..1bf512e --- /dev/null +++ b/test/webui/daemon-server.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from "vitest"; +import net from "net"; +import { directory as randomDirectory } from "tempy"; +import { startDaemonServer } from "../../dist/webui/daemon-server.js"; + +// Regression test for issue #42: +// startDaemonServer used to call webuiExpressApp.listen(port) fire-and-forget — no +// await on 'listening', no 'error' handler. If bind failed, the promise still resolved +// and the bind error showed up later as an uncaughtException, crashing the daemon +// AFTER the test helper had already accepted "Communities in data path" as readiness. +// The contract we want: if the port can't be bound, startDaemonServer must reject. +describe("startDaemonServer bind contract", () => { + it("rejects when the RPC port is already taken (regression for issue #42)", async () => { + const blocker = net.createServer(); + const port = await new Promise((resolve, reject) => { + blocker.once("listening", () => { + const addr = blocker.address(); + if (addr && typeof addr === "object") resolve(addr.port); + else reject(new Error(`unexpected address ${JSON.stringify(addr)}`)); + }); + blocker.once("error", reject); + blocker.listen(0, "127.0.0.1"); + }); + + // Swallow any stray uncaughtException emitted by an unguarded server.listen() + // so the test process survives long enough to assert the actual behavior. + const stray: Error[] = []; + const uncaughtHandler = (err: Error) => stray.push(err); + process.on("uncaughtException", uncaughtHandler); + + try { + await expect( + startDaemonServer( + new URL(`ws://127.0.0.1:${port}`), + new URL("http://127.0.0.1:6754"), + { dataPath: randomDirectory() } + ) + ).rejects.toThrow(/EADDRINUSE|address already in use/i); + // And no stray uncaughtException either — the bind error must come back + // through the promise, not the process. + expect(stray).toEqual([]); + } finally { + process.off("uncaughtException", uncaughtHandler); + await new Promise((resolve) => blocker.close(() => resolve())); + } + }); +}); From 0d90f0fabfec8a6a531d10b62b28a1873e1d0f50 Mon Sep 17 00:00:00 2001 From: Rinse Date: Sun, 24 May 2026 05:43:31 +0000 Subject: [PATCH 2/3] test(daemon-server): bind blocker to 0.0.0.0 so EADDRINUSE fires on macOS/Windows Previous version bound the blocker to 127.0.0.1, but express.listen(port) binds to 0.0.0.0 (no host arg). On BSD-derived stacks (macOS, Windows) a 127.0.0.1 bind and a 0.0.0.0 bind on the same port can coexist, so the daemon's wildcard listen succeeded and the test asserted rejection but got a resolved promise. Linux is stricter and treats this as a conflict, which is why ubuntu passed but the other runners failed. --- test/webui/daemon-server.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/webui/daemon-server.test.ts b/test/webui/daemon-server.test.ts index 1bf512e..e838dfe 100644 --- a/test/webui/daemon-server.test.ts +++ b/test/webui/daemon-server.test.ts @@ -11,6 +11,11 @@ import { startDaemonServer } from "../../dist/webui/daemon-server.js"; // The contract we want: if the port can't be bound, startDaemonServer must reject. describe("startDaemonServer bind contract", () => { it("rejects when the RPC port is already taken (regression for issue #42)", async () => { + // Blocker must bind to 0.0.0.0 because express.listen(port) (no host arg) also + // binds to 0.0.0.0. On BSD-derived stacks (macOS, Windows) a wildcard bind and + // a 127.0.0.1 bind on the same port can coexist, so binding the blocker to + // 127.0.0.1 would let the daemon's wildcard bind succeed and the test wouldn't + // exercise the EADDRINUSE path. const blocker = net.createServer(); const port = await new Promise((resolve, reject) => { blocker.once("listening", () => { @@ -19,7 +24,7 @@ describe("startDaemonServer bind contract", () => { else reject(new Error(`unexpected address ${JSON.stringify(addr)}`)); }); blocker.once("error", reject); - blocker.listen(0, "127.0.0.1"); + blocker.listen(0, "0.0.0.0"); }); // Swallow any stray uncaughtException emitted by an unguarded server.listen() From 333ece28e97ee030a07004fe8857b37d6819c854 Mon Sep 17 00:00:00 2001 From: Rinse Date: Sun, 24 May 2026 06:51:31 +0000 Subject: [PATCH 3/3] test(daemon-server): blocker must use Node's default listen host to match the daemon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous attempt bound the blocker to 0.0.0.0, but Node's default when no host is passed to server.listen() is the IPv6 wildcard `::` with IPV6_V6ONLY=true. On macOS and Windows, `::` (IPv6-only) and `0.0.0.0` (IPv4) do not conflict, so the daemon's bind succeeded and the assertion failed. Drop the explicit host on the blocker too so it lands on the same default as express.listen(port) — `::` on dual-stack runners. --- test/webui/daemon-server.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/webui/daemon-server.test.ts b/test/webui/daemon-server.test.ts index e838dfe..3fa6eba 100644 --- a/test/webui/daemon-server.test.ts +++ b/test/webui/daemon-server.test.ts @@ -11,11 +11,11 @@ import { startDaemonServer } from "../../dist/webui/daemon-server.js"; // The contract we want: if the port can't be bound, startDaemonServer must reject. describe("startDaemonServer bind contract", () => { it("rejects when the RPC port is already taken (regression for issue #42)", async () => { - // Blocker must bind to 0.0.0.0 because express.listen(port) (no host arg) also - // binds to 0.0.0.0. On BSD-derived stacks (macOS, Windows) a wildcard bind and - // a 127.0.0.1 bind on the same port can coexist, so binding the blocker to - // 127.0.0.1 would let the daemon's wildcard bind succeed and the test wouldn't - // exercise the EADDRINUSE path. + // The blocker must bind the *same way* the daemon does — no host argument — + // so it lands on whatever default Node picks for this platform. On macOS and + // Windows that default is the IPv6 wildcard `::` with IPV6_V6ONLY=true, which + // does NOT conflict with a 0.0.0.0 bind. Binding the blocker to 127.0.0.1 or + // 0.0.0.0 would let the daemon's listen succeed there and never hit EADDRINUSE. const blocker = net.createServer(); const port = await new Promise((resolve, reject) => { blocker.once("listening", () => { @@ -24,7 +24,7 @@ describe("startDaemonServer bind contract", () => { else reject(new Error(`unexpected address ${JSON.stringify(addr)}`)); }); blocker.once("error", reject); - blocker.listen(0, "0.0.0.0"); + blocker.listen(0); }); // Swallow any stray uncaughtException emitted by an unguarded server.listen()