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
36 changes: 35 additions & 1 deletion src/daemon-client/__tests__/daemon-client-transport.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import assert from 'node:assert/strict';
import http from 'node:http';
import { test } from 'vitest';
import { test, vi } from 'vitest';
import { AppError } from '@agent-device/kernel/errors';
import {
DAEMON_HTTP_INSTANCE_HEADER,
Expand Down Expand Up @@ -222,6 +222,40 @@ test('a delayed restart health probe stops at the RPC deadline without retrying'
}
});

test('a restart health probe cut short by the RPC deadline reports the deadline on a lagging clock', async (t) => {
if (await skipWhenLoopbackUnavailable(t)) return;
let rpcCount = 0;
let healthProbes = 0;
const server = http.createServer((req, res) => {
if (req.url === '/health') {
healthProbes += 1;
const delayedResponse = setTimeout(() => res.end('{}'), 1000);
res.on('close', () => clearTimeout(delayedResponse));
return;
}
rpcCount += 1;
res.statusCode = 409;
res.setHeader(DAEMON_HTTP_INSTANCE_MISMATCH_HEADER, 'true');
res.end();
});
// Timers start from the event loop's cached clock, so the probe's timer can fire while
// performance.now() is still short of the deadline. A frozen clock makes that gap certain.
const now = vi.spyOn(performance, 'now').mockReturnValue(performance.now());
try {
const port = await listenOnLoopback(server);
await assert.rejects(
sendWithStaleInstance(port, 150),
(error: unknown) =>
error instanceof AppError && error.details?.reason === 'daemon_transport_timeout',
Comment thread
okwasniewski marked this conversation as resolved.
);
assert.equal(rpcCount, 1);
assert.equal(healthProbes, 1);
} finally {
now.mockRestore();
await closeLoopbackServer(server);
}
});

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
32 changes: 26 additions & 6 deletions src/daemon-client/daemon-client-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export type RemoteDaemonHealth = {
instanceId?: string;
/** The daemon behind a proxy, as the proxy's health reported it. */
upstream?: RemoteDaemonHealthLink;
/** The probe ran out of its time budget before an answer, rather than failing outright. */
timedOut?: true;
};

type RemoteDaemonHealthLink = Pick<
Expand Down Expand Up @@ -137,9 +139,12 @@ async function readDaemonHttpHealth(
info.baseUrl ? REMOTE_DAEMON_HEALTHCHECK_TIMEOUT_MS : LOCAL_DAEMON_HEALTHCHECK_TIMEOUT_MS,
probeTimeoutMs ?? Number.POSITIVE_INFINITY,
);
if (timeoutMs <= 0) return { reachable: false };
if (timeoutMs <= 0) return { reachable: false, timedOut: true };
const signal = AbortSignal.timeout(Math.ceil(timeoutMs));
return await new Promise((resolve) => {
const headers = info.baseUrl ? buildDaemonHttpAuthHeaders(info.token) : {};
const unreachable = (): RemoteDaemonHealth =>
signal.aborted ? { reachable: false, timedOut: true } : { reachable: false };
const req = transport.request(
{
protocol: url.protocol,
Expand All @@ -148,7 +153,7 @@ async function readDaemonHttpHealth(
path: url.pathname + url.search,
method: 'GET',
timeout: timeoutMs,
signal: AbortSignal.timeout(Math.ceil(timeoutMs)),
signal,
headers,
},
(res) => {
Expand All @@ -165,16 +170,16 @@ async function readDaemonHttpHealth(
...readHealthPayload(body),
});
});
res.on('error', () => resolve({ reachable: false }));
res.on('aborted', () => resolve({ reachable: false }));
res.on('error', () => resolve(unreachable()));
res.on('aborted', () => resolve(unreachable()));
},
);
req.on('timeout', () => {
req.destroy();
resolve({ reachable: false });
resolve({ reachable: false, timedOut: true });
});
req.on('error', () => {
resolve({ reachable: false });
resolve(unreachable());
});
req.end();
});
Expand Down Expand Up @@ -255,6 +260,21 @@ async function retryAfterRemoteInstanceMismatch(
deadline,
);
const health = await readRemoteDaemonHealth(info, probeTimeoutMs);
// The probe's timer starts from the event loop's cached clock, so it can expire while
// performance.now() is still short of the deadline: a probe the RPC deadline capped that ran out
// of time is the RPC timing out.
if (
health.timedOut &&
timeoutMs !== undefined &&
probeTimeoutMs !== undefined &&
probeTimeoutMs <= REMOTE_DAEMON_HEALTHCHECK_TIMEOUT_MS
) {
throw handleRequestTimeout({
info,
statePaths,
...timeoutRequestContext(req, true, timeoutMs),
});
}
const remainingMs = remainingRemoteRequestTimeoutMs(info, req, statePaths, timeoutMs, deadline);
if (!health.reachable) {
throw new AppError('COMMAND_FAILED', 'Remote daemon is unavailable', {
Expand Down
24 changes: 12 additions & 12 deletions test/wire-compat/ledger.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@
"src/daemon-client/daemon-client-rpc.ts#rejectDaemonHttpRpcError": "sha256:83b0312fe88bc3b3497de799e23d8d14455f665b7cf880f4617cd62b181351cd",
"src/daemon-client/daemon-client-rpc.ts#resolveDaemonHttpResult": "sha256:296f9d376ce67c8cb20209bdf1c59c2423ffcfa35b5565a99e70271263149048",
"src/daemon-client/daemon-client-rpc.ts#toDaemonHttpRpcError": "sha256:888246763c48670e7da893054d025744654f8715c3b4906312617a2b5028316b",
"src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealth": "sha256:df6b92e343a451a03b194e1e4aacac0c89b48a632723af24e087b377e73e05e3",
"src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealth": "sha256:a3380ef1b8d85808111eb0a00533430cf64ea76d9d0c35596080077795380475",
"src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealthLink": "sha256:464809c9bb14b44d098781722e9e9e2ff044c45464e3c7c4cb5690886e857fdf",
"src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:6b64ae9b8e461a4b7dc1afb88465fcbaef908c31cc0be24499b2f440616e1cb5",
"src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth": "sha256:eef0e153eb6f0bc02175e464dd6d98559b500b65ea8c74ba2203cfef09ec09a0",
"src/daemon-client/daemon-client-transport.ts#readHealthLink": "sha256:d7a84687d1c9b089460151a0532870e92a932d45f9567d248cea7accf8908593",
"src/daemon-client/daemon-client-transport.ts#readHealthPayload": "sha256:4e85ffc3e35e02379c393e9312344757e003cf1f0ad9eb8d1f77d90c81c861f1",
"src/daemon-client/daemon-client-transport.ts#readRemoteDaemonHealth": "sha256:bcefa89fbb7fcbd6fee1b5ecb217955b9eee8d6fd2ad175fb653011edbd199d9",
Expand Down Expand Up @@ -267,11 +267,6 @@
"digest": "sha256:4e85ffc3e35e02379c393e9312344757e003cf1f0ad9eb8d1f77d90c81c861f1",
"rationale": "#2198 slice B teaches the client to read the `upstream` link a proxy's /health already nests. The field is optional and additive: a daemon or proxy that does not send it parses exactly as before, and no request shape changes. readHealthPayload reads the same top-level fields through readHealthLink and additionally the nested `upstream` object when present; a payload without it yields the previous shape."
},
{
"declaration": "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth",
"digest": "sha256:6b64ae9b8e461a4b7dc1afb88465fcbaef908c31cc0be24499b2f440616e1cb5",
"rationale": "#2650 gives every HTTP health probe a total-time cap (3s remote or 500ms local), including response-body reads. A restart retry also uses the RPC's remaining deadline. The health request and accepted payload fields are unchanged; stalled probes now end sooner."
},
{
"declaration": "src/daemon-client/daemon-client-transport.ts#readRemoteDaemonHealth",
"digest": "sha256:bcefa89fbb7fcbd6fee1b5ecb217955b9eee8d6fd2ad175fb653011edbd199d9",
Expand Down Expand Up @@ -357,11 +352,6 @@
"digest": "sha256:4398710f0faac4dbe76ca448c1dd87ff6d062016c1f4e52f610f7a8729453688",
"rationale": "#2650 adds an optional daemon instance ID to health metadata or reads it when present. Older peers ignore the added field, and new clients keep probing legacy peers without an instance ID; existing fields and RPC envelopes are unchanged."
},
{
"declaration": "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealth",
"digest": "sha256:df6b92e343a451a03b194e1e4aacac0c89b48a632723af24e087b377e73e05e3",
"rationale": "#2650 adds an optional daemon instance ID to health metadata or reads it when present. Older peers ignore the added field, and new clients keep probing legacy peers without an instance ID; existing fields and RPC envelopes are unchanged."
},
{
"declaration": "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealthLink",
"digest": "sha256:464809c9bb14b44d098781722e9e9e2ff044c45464e3c7c4cb5690886e857fdf",
Expand Down Expand Up @@ -426,6 +416,16 @@
"declaration": "src/daemon-client/daemon-client-transport.ts#isRemoteInstanceMismatch",
"digest": "sha256:6d4cfdd36bd44686d2a48990491faf63ba9117fb760f982fd72922c99be8daee",
"rationale": "#2650 adds conditional RPC refusal and retry only for peers that advertise complete instance IDs. Released peers advertise none, so new clients keep per-command health probes and send their unchanged RPC; older clients omit the new request precondition and parse existing responses as before."
},
{
"declaration": "src/daemon-client/daemon-client-transport.ts#RemoteDaemonHealth",
"digest": "sha256:a3380ef1b8d85808111eb0a00533430cf64ea76d9d0c35596080077795380475",
"rationale": "A restart retry must tell a probe that ran out of time from one that failed, so the client can report the RPC deadline instead of 'Remote daemon is unavailable'. `timedOut` is client-local: the prober sets it on its own timeout and abort paths and never reads it from a /health payload. The health request and the accepted payload fields are unchanged, so a released daemon or proxy is probed and parsed exactly as before."
},
{
"declaration": "src/daemon-client/daemon-client-transport.ts#readDaemonHttpHealth",
"digest": "sha256:eef0e153eb6f0bc02175e464dd6d98559b500b65ea8c74ba2203cfef09ec09a0",
"rationale": "A restart retry must tell a probe that ran out of time from one that failed, so the client can report the RPC deadline instead of 'Remote daemon is unavailable'. `timedOut` is client-local: the prober sets it on its own timeout and abort paths and never reads it from a /health payload. The health request and the accepted payload fields are unchanged, so a released daemon or proxy is probed and parsed exactly as before."
}
]
}
Loading