Skip to content

test(daemon-client): a restart probe that fails outright near the RPC deadline reports the daemon unavailable #3058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thymikee merged 2 commits into from
Sep 29, 2026
Merged

test(daemon-client): a restart probe that fails outright near the RPC deadline reports the daemon unavailable #3058

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6cd49a
test(daemon-client): a restart probe that fails outright near the RPC…
thymikee Sep 29, 2026
00833fb
test(daemon-client): document how the refused-probe case proves the p…
thymikee Sep 29, 2026
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
59 changes: 59 additions & 0 deletions src/daemon-client/__tests__/daemon-client-transport.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,65 @@ test('a restart health probe cut short by the RPC deadline reports the deadline
}
});

type RestartProbeFailure = 'http-503' | 'closed-connection' | 'refused-connection';

async function assertRestartProbeFailureReportsUnavailable(
t: Parameters<typeof skipWhenLoopbackUnavailable>[0],
failure: RestartProbeFailure,
) {
if (await skipWhenLoopbackUnavailable(t)) return;
let healthProbes = 0;
let rpcCount = 0;
const server = http.createServer((req, res) => {
if (req.url === '/health') {
healthProbes += 1;
if (failure === 'http-503') {
res.statusCode = 503;
res.end();
} else {
req.socket.destroy();
}
return;
}
rpcCount += 1;
res.statusCode = 409;
res.setHeader(DAEMON_HTTP_INSTANCE_MISMATCH_HEADER, 'true');
res.setHeader('connection', 'close');
res.end();
if (failure === 'refused-connection') server.close();
});
try {
const port = await listenOnLoopback(server);
await assert.rejects(sendWithStaleInstance(port, 150), (error: unknown) => {
assert.ok(error instanceof AppError);
assert.equal(error.message, 'Remote daemon is unavailable');
assert.equal(error.details?.daemonBaseUrl, `http://127.0.0.1:${port}`);
assert.notEqual(error.details?.reason, 'daemon_transport_timeout');
return true;
});
assert.equal(rpcCount, 1);
// A closed listener cannot count the probe; its attempt is proven by elimination: a skipped
// probe would exhaust the budget and report daemon_transport_timeout, which the error check rejects.
Comment on lines +296 to +297

@cubic-dev-ai cubic-dev-ai Bot Sep 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The elimination rationale in this comment doesn't match how the test actually rejects a skipped probe in the refused case. Once server.close() runs, a skipped probe isn't followed by budget exhaustion: sendRequestWithTransport retries the RPC, the closed listener refuses it immediately, and the request errors out as daemon_transport_failure ('Failed to communicate with daemon'). That error is rejected by assert.equal(error.message, 'Remote daemon is unavailable') and the notEqual(reason, ...) check, not by any budget-driven daemon_transport_timeout. The timeout path only applies to the still-open 503/closed-connection cases, where a skipped probe keeps looping on 409 until the 150 ms budget caps it. Suggest rewording the comment so the guard it describes is the one that actually fires.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/daemon-client/__tests__/daemon-client-transport.test.ts, line 296:

<comment>The elimination rationale in this comment doesn't match how the test actually rejects a skipped probe in the refused case. Once `server.close()` runs, a skipped probe isn't followed by budget exhaustion: `sendRequestWithTransport` retries the RPC, the closed listener refuses it immediately, and the request errors out as `daemon_transport_failure` ('Failed to communicate with daemon'). That error is rejected by `assert.equal(error.message, 'Remote daemon is unavailable')` and the `notEqual(reason, ...)` check, not by any budget-driven `daemon_transport_timeout`. The timeout path only applies to the still-open 503/closed-connection cases, where a skipped probe keeps looping on 409 until the 150 ms budget caps it. Suggest rewording the comment so the guard it describes is the one that actually fires.</comment>

<file context>
@@ -256,6 +256,65 @@ test('a restart health probe cut short by the RPC deadline reports the deadline
+      return true;
+    });
+    assert.equal(rpcCount, 1);
+    // A closed listener cannot count the probe; its attempt is proven by elimination: a skipped
+    // probe would exhaust the budget and report daemon_transport_timeout, which the error check rejects.
+    if (failure !== 'refused-connection') assert.equal(healthProbes, 1);
</file context>
Suggested change
// A closed listener cannot count the probe; its attempt is proven by elimination: a skipped
// probe would exhaust the budget and report daemon_transport_timeout, which the error check rejects.
// A closed listener may refuse the probe before the handler can count it, so the counter is
// not relied on here; a skipped probe still breaks the 'Remote daemon is unavailable' message check.
Fix with cubic

if (failure !== 'refused-connection') assert.equal(healthProbes, 1);
} finally {
await closeLoopbackServer(server);
}
}

// Catches a mutation that marks every unreachable restart probe timedOut: a probe that fails
// outright inside the capped budget would then surface as an RPC timeout.
test('a restart health probe answering 503 near the RPC deadline reports the daemon unavailable', async (t) => {
await assertRestartProbeFailureReportsUnavailable(t, 'http-503');
});

test('a restart health probe on a closed connection near the RPC deadline reports the daemon unavailable', async (t) => {
await assertRestartProbeFailureReportsUnavailable(t, 'closed-connection');
});

test('a restart health probe refused near the RPC deadline reports the daemon unavailable', async (t) => {
await assertRestartProbeFailureReportsUnavailable(t, 'refused-connection');
});

test('proxy forwards cached upstream identity and rejects a restarted upstream before dispatch', async (t) => {
if (await skipWhenLoopbackUnavailable(t)) return;
let upstreamInstance = 'upstream-one';
Expand Down
Loading