fix(cli): bound doctor health header probes (#2549)

rig doctor bounds the wait for /healthz response headers when the daemon port is held by a listener that accepts connections but never replies, and reports an unknown listener instead of waiting forever; the conditional cmux health probe uses the same bound. Reported in #2531. Contributed by @dajiaohuang.
This commit is contained in:
Wu Shuwen authored and GitHub committed 2026-11-05 03:37:15 -08:00
1 parent 80e568e9d9
commit f7113e5a5d
3 files changed
+87 -8

No files matched your search

+21 -5
View File
@@ -45,7 +45,7 @@ export interface DoctorDeps {
/** The caller's effective user id, compared with the one the port's daemon reports. Absent: this process's own;
* null: this platform has none. */
callerUid?: number | null;
/** How long the port's /healthz body may take to arrive (default 2000 ms); after that its owner is unknown. */
/** How long each part of the port's /healthz response may take (default 2000 ms). */
ownerReportTimeoutMs?: number;
/**
* Build B — path to a rig spec to check against the RUNNING rig of the same name (`--spec`).
@@ -301,6 +301,15 @@ export function runDoctorChecks(deps: DoctorDeps, providerChecks: DoctorCheck[]
const daemonPort = config.daemon.port ?? DEFAULT_PORT;
const daemonBase = `http://${formatDaemonHostForUrl(daemonHost)}:${daemonPort}`;
const fetchFn = deps.fetch ?? globalThis.fetch;
const healthHeaders = async (controller: AbortController) => {
let timer: ReturnType<typeof setTimeout> | undefined;
try {
return await Promise.race([
fetchFn(`${daemonBase}/healthz`, { signal: controller.signal }),
new Promise<null>(resolve => { timer = setTimeout(() => { resolve(null); controller.abort(); }, deps.ownerReportTimeoutMs ?? 2000); }),
]);
} finally { clearTimeout(timer); }
};
const portCheck = deps.checkPort(daemonPort, daemonHost).then(async (available): Promise<DoctorCheck> => {
if (available) {
return { name: "port", status: "pass", message: `Port ${daemonHost}:${daemonPort} available.` };
@@ -308,7 +317,13 @@ export function runDoctorChecks(deps: DoctorDeps, providerChecks: DoctorCheck[]
// Port in use — check if it's an OpenRig daemon via healthz, and whose.
const owner = new AbortController();
try {
const res = await fetchFn(`${daemonBase}/healthz`, { signal: owner.signal });
const res = await healthHeaders(owner);
if (!res) return {
name: "port", status: "warn",
message: `Port ${daemonHost}:${daemonPort} is in use by an unknown listener.`,
reason: "Its /healthz response headers did not arrive in time, so doctor cannot identify it.",
fix: `Check the listener on port ${daemonPort} or configure a free port for the daemon.`,
};
if (res.ok) return await listenerOwnerCheck(`${daemonHost}:${daemonPort}`, daemonBase, res, () => owner.abort(), deps);
} catch { /* not our daemon */ }
finally { owner.abort(); }
@@ -325,14 +340,15 @@ export function runDoctorChecks(deps: DoctorDeps, providerChecks: DoctorCheck[]
const asyncChecks: Promise<DoctorCheck>[] = [portCheck];
if (shellCmuxPassed) {
const daemonCmuxCheck = (async (): Promise<DoctorCheck> => {
const health = new AbortController();
try {
const healthRes = await fetchFn(`${daemonBase}/healthz`);
if (!healthRes.ok) {
const healthRes = await healthHeaders(health);
if (!healthRes?.ok) {
return { name: "cmux_daemon", status: "skipped", message: "Daemon healthz not ok. Skipping daemon cmux check." };
}
} catch {
return { name: "cmux_daemon", status: "skipped", message: "Daemon not reachable. Skipping daemon cmux check." };
}
} finally { health.abort(); }
try {
const cmuxRes = await fetchFn(`${daemonBase}/api/adapters/cmux/status`);
@@ -0,0 +1,42 @@
import { describe, expect, it } from "vitest";
import { spawn } from "node:child_process";
import { mkdtempSync, rmSync } from "node:fs";
import { createServer } from "node:http";
import { tmpdir } from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
describe.skipIf(process.platform === "win32")("doctor with a listener holding response headers", () => {
it("returns an unknown listener result through the built CLI", async () => {
const root = mkdtempSync(path.join(tmpdir(), "doctor-held-headers-"));
const requests: string[] = [];
const server = createServer(req => { requests.push(req.url ?? ""); });
await new Promise<void>(resolve => server.listen(0, "127.0.0.1", resolve));
try {
const port = (server.address() as { port: number }).port;
const cli = fileURLToPath(new URL("../dist/bin-wrapper.js", import.meta.url));
const output = await new Promise<string>((resolve, reject) => {
const child = spawn(process.execPath, [cli, "doctor", "--json"], {
env: { HOME: root, OPENRIG_HOME: path.join(root, "openrig"), OPENRIG_PORT: String(port), PATH: "/usr/bin:/bin", TMPDIR: root },
stdio: ["ignore", "pipe", "pipe"],
});
let stdout = "";
child.stdout.setEncoding("utf8");
child.stdout.on("data", chunk => { stdout += chunk; });
child.stderr.resume();
const timer = setTimeout(() => { child.kill(); reject(new Error("doctor did not finish")); }, 30_000);
child.on("error", error => { clearTimeout(timer); reject(error); });
child.on("close", () => { clearTimeout(timer); resolve(stdout); });
});
const result = JSON.parse(output) as { checks: Array<{ name: string; status: string; message: string }> };
const check = result.checks.find(item => item.name === "port");
expect(requests).toContain("/healthz");
expect(check?.status).toBe("warn");
expect(check?.message).toContain("unknown listener");
} finally {
server.closeAllConnections();
await new Promise<void>(resolve => server.close(() => resolve()));
rmSync(root, { recursive: true, force: true });
}
}, 40_000);
});
+24 -3
View File
@@ -565,7 +565,6 @@ describe("runDoctorChecks", () => {
// A listener that sends an OK header and never finishes its body must not hold doctor, or setup, before anything prints.
it("settles a /healthz body that never finishes as owner unknown within its bound, and releases the request", async () => {
// The port check's request carries a signal; doctor's other /healthz reads (the cmux check) don't.
const signals: AbortSignal[] = [];
const deps = makeDeps({ checkPort: async () => false, callerUid: 1002, ownerReportTimeoutMs: 50,
fetch: async (_url: string, init?: { signal?: AbortSignal }) => {
@@ -578,8 +577,8 @@ describe("runDoctorChecks", () => {
expect(result.status).toBe("warn");
expect(result.message).toContain("OS user is unknown");
expect(result.reason).toContain("didn't finish");
expect(signals).toHaveLength(1);
expect(signals[0]!.aborted).toBe(true);
expect(signals).toHaveLength(2);
expect(signals.every(signal => signal.aborted)).toBe(true);
});
it("settles a real listener's held /healthz body the same way, through the real fetch", async () => {
@@ -599,6 +598,28 @@ describe("runDoctorChecks", () => {
await new Promise(resolve => server.close(resolve));
}
});
it("bounds a real listener that never sends health response headers", async () => {
const { createServer } = await import("node:http");
const server = createServer(() => {});
await new Promise<void>(resolve => server.listen(0, "127.0.0.1", resolve));
const port = (server.address() as { port: number }).port;
try {
const deps = makeDeps({ checkPort: async () => false, ownerReportTimeoutMs: 100,
configStore: { resolve: () => ({ ...defaultConfig, daemon: { port, host: "127.0.0.1" } }) } as DoctorDeps["configStore"] });
delete deps.fetch;
const checks = runDoctorChecks(deps);
const result = await checks.portCheck;
expect(result.status).toBe("warn");
expect(result.message).toContain("unknown listener");
expect(result.reason).toContain("headers did not arrive");
const all = await Promise.all(checks.asyncChecks);
expect(all.find(check => check.name === "cmux_daemon")?.status).toBe("skipped");
} finally {
server.closeAllConnections();
await new Promise(resolve => server.close(resolve));
}
});
});
describe("rig doctor", () => {