fix(daemon-server): await express listen() so RPC port is bound before startup completes#52
Conversation
…artup completes `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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR makes daemon startup await the HTTP server bind (listening event) so failures reject during startup; it adds a Vitest regression test that verifies startDaemonServer rejects when the RPC/WebSocket port is already bound. ChangesHTTP Server Startup Race Fix
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/webui/daemon-server.test.tsOops! Something went wrong! :( ESLint: 8.27.0 Error: ESLint configuration in --config » eslint-config-oclif is invalid:
Referenced from: /.eslintrc Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…acOS/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.
…atch the daemon 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.
1 participant
Summary
ECONNREFUSED 127.0.0.1:39138failure intest/cli/daemon.test.ts > bitsocial daemon webui > 5chan webui does not contain the hash redirect script(most recent occurrence: run 26333018289).src/webui/daemon-server.tswas callingwebuiExpressApp.listen(port)fire-and-forget — noawaiton'listening', no'error'handler.startDaemonServercould return (and the daemon could logCommunities in data path, which the test helper treats as readiness) before the OS had finished binding the RPC port. Worse, a bind failure surfaced asynchronously as an unhandled'error'event →uncaughtException→ daemon crashed after it had already advertised itself as ready.listen()in a promise that resolves on'listening'and rejects on'error'. NowstartDaemonServeronly returns once the port is bound, and bind failures propagate through the normal rejection path instead of killing the process.test/webui/daemon-server.test.tswith a deterministic reproducer that pre-binds the port and assertsstartDaemonServerrejects withEADDRINUSE. Fails on the old code (resolves successfully with broken server), passes after the fix in ~12ms.Test plan
test/webui/daemon-server.test.ts) fails onmaster, passes on this branch.test/cli/daemon.test.tssuite (22 tests, includes the flaky webui suite) passes locally.Fixes #42.
Summary by CodeRabbit
Bug Fixes
Tests