Skip to content

Commit 8673e04

Browse files
ARHAEEMclaude
andcommitted
fix(daemon): fail clearly if listenAvoidingBlockedPorts exhausts all retries
The retry loop only exited via return (bound a usable port) or throw (non-fallback bind error). If it ever exhausted all 5 attempts it fell through returning undefined with the server closed/ unbound, and the caller (startDaemonServer → getBoundPort) then threw the misleading "Daemon server is not listening on a TCP port." Add an explicit, descriptive throw after the loop. Not reachable in practice — the first fallback sets port=0 and OS ephemeral ports (>=32768) are never in FETCH_BLOCKED_PORTS (all <=10080), so iteration 2 always binds — but the silent fall-through is fragile (a future change to the blocked set or retry count would make it a real crash with a confusing error). TDD test forces the exhaustion path and asserts the clear throw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c0a9010 commit 8673e04

2 files changed

Lines changed: 23 additions & 0 deletions

File tree

packages/mcp-server/src/daemon/server.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,12 @@ export async function listenAvoidingBlockedPorts(server, requestedPort, host) {
105105
await new Promise((resolve) => server.close(() => resolve()));
106106
port = 0; // blocked port → next attempt uses an ephemeral port
107107
}
108+
109+
// Unreachable in practice — the first fallback sets port=0 and OS ephemeral ports (>=32768) are
110+
// never in FETCH_BLOCKED_PORTS (all <=10080), so iteration 2 always binds and returns. But do NOT
111+
// fall through returning undefined with the server closed/unbound: the caller reads getBoundPort()
112+
// and would throw the misleading "Daemon server is not listening on a TCP port." Fail clearly.
113+
throw new Error(`Daemon could not bind a usable (non-browser-blocked) port after 5 attempts (requested port ${requestedPort}).`);
108114
}
109115

110116
function getBoundPort(server) {

packages/mcp-server/test/test-daemon-port-fallback.test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,20 @@ test('an unrelated fixed-port error is NOT swallowed as a fallback', async () =>
5252
const server = makeFakeServer((port) => (port === 8723 ? 'EPERM' : 'ok'));
5353
await assert.rejects(() => listenAvoidingBlockedPorts(server, 8723, '127.0.0.1'), /EPERM/);
5454
});
55+
56+
test('exhausting every retry on a browser-blocked port throws a clear error (not a silent unbound return)', async () => {
57+
// Force the (in-practice-unreachable) case where every bind — including ephemeral — lands on a
58+
// browser-blocked port. Ephemeral ports sit above the blocked range so this cannot really happen,
59+
// but the loop must not fall through returning undefined with the server unbound: the caller then
60+
// reads getBoundPort() and throws the misleading "not listening". An explicit throw fails clearly.
61+
const s = new EventEmitter();
62+
s._bound = null;
63+
s.listen = () => { queueMicrotask(() => { s._bound = 6666; s.emit('listening'); }); }; // always a blocked port
64+
s.address = () => (s._bound == null ? null : { port: s._bound, address: '127.0.0.1', family: 'IPv4' });
65+
s.close = (cb) => { s._bound = null; if (cb) queueMicrotask(cb); };
66+
67+
await assert.rejects(
68+
() => listenAvoidingBlockedPorts(s, 0, '127.0.0.1'),
69+
/could not bind a usable/i,
70+
);
71+
});

0 commit comments

Comments
 (0)