fix(cli): a finished capture exits even when a site's DNS never answers (#5983)

* fix(cli): a finished capture exits even when a site's DNS never answers

Capture resolved every page request with the system lookup and gave up after
10 s, but a system lookup cannot be cancelled: the abandoned one kept the
process alive until the OS resolver timed out, so a capture that had printed
its result could run past the app's 5-minute cap and lose its folder.
Resolve through c-ares instead, as hostAnswers does, and cancel it at the
same 10 s deadline.

* test(cli): a capture lookup that answers must exit right after too

Dropping the timer cleanup kept every successful lookup alive 10 s and no test noticed.

* fix(cli): capture falls back to the system lookup and exits once its result is out

c-ares answers first. When it has no answer, the system lookup (hosts file, OS-only DNS)
runs in a child process killed at the 10 s deadline, cached per host, because Node waits
for an in-process system lookup even in process.exit. Capture then asks the CLI to exit
100 ms after printing its result, as render does.

* fix(cli): capture reads the hosts file first and kills its lookup child on exit

The hosts file answers before DNS, as the system lookup does (overrides and 0.0.0.0
blocks keep working). A host whose system answer is cached skips c-ares, a caller that
joins a running lookup keeps its own deadline, and a pending lookup child is killed when
the CLI exits.

* fix(cli): capture's forced exit waits until a slow reader has the whole result

The root exit now waits for stdout and stderr to hand over what is already
written before it calls process.exit, so a large JSON result on a slow pipe
arrives complete. A hostname listed several times in the hosts file now has
every address checked, not only the first.

* fix(cli): keep process exit in the CLI root and let test fixtures exit

The output wait is now a helper that only waits; cli.ts still owns the
process.exit. The process-ownership check treats test fixtures like tests,
since each one runs as its own process.

* chore(cli): run the process-ownership check before every commit

A process.exit outside a process root used to pass the local hooks and fail
only in CI's Preflight.
This commit is contained in:
Miguel Ángel
2026-10-21 02:46:17 +00:00
committed by GitHub
parent bbb691001d
commit 1289734770
19 changed files with 553 additions and 127 deletions
+2
View File
@@ -62,6 +62,8 @@
// Warm-check client, spawned by path from warmCheck.process.test.ts.
"packages/cli/src/warmCheckClient.fixture.ts",
"packages/cli/src/browser/chromeOrphans.fixture.ts",
"packages/cli/src/capture/publicNetwork.fixture.ts",
"packages/cli/src/utils/commandResult.fixture.ts",
// Test fixture worker, spawned by path via the pools' workerEntryPath
// option from the crash-recovery tests; has no import-graph referrer.
"packages/producer/src/services/__fixtures__/crashOnMessageWorker.mjs",
+3
View File
@@ -33,6 +33,9 @@ pre-commit:
# fallow reuses one base-snapshot cache per checkout; `audit-cache prune` drops caches unused for a day,
# including those of removed checkouts. The commit gets the audit's own status.
run: env -u GIT_DIR -u GIT_INDEX_FILE -u GIT_WORK_TREE sh -c 'node_modules/.bin/fallow audit --base origin/main --fail-on-issues; status=$?; node_modules/.bin/fallow audit-cache prune --max-age-days 1 > /dev/null; exit $status'
cli-process-ownership:
glob: "packages/cli/src/**/*.ts"
run: bun run check:cli-process-ownership
largefiles:
# Rejects staged binaries over the limit that are not LFS pointers; see
# the script header. CI runs the same script with --range.
@@ -11,14 +11,36 @@ const fixture = vi.hoisted(() => ({
rebound: false,
dialed: [] as string[],
}));
vi.mock("node:dns/promises", () => ({
lookup: vi.fn(async (host: string) => {
if (host === "capture-fixture.example.com") {
return [{ address: fixture.rebound ? "127.0.0.1" : "93.184.216.34", family: 4 }];
}
throw new Error("Fixture has no such DNS name");
}),
}));
vi.mock("node:dns", async (importOriginal) => {
const real = await importOriginal<typeof import("node:dns")>();
return {
...real,
promises: {
...real.promises,
Resolver: class {
async resolve4(host: string) {
if (host === "capture-fixture.example.com") {
return [fixture.rebound ? "127.0.0.1" : "93.184.216.34"];
}
throw new Error("Fixture has no such DNS name");
}
cancel() {}
},
},
};
});
vi.mock("node:child_process", async (importOriginal) => {
const real = await importOriginal<typeof import("node:child_process")>();
return {
...real,
execFile: (...args: Parameters<typeof real.execFile>) => {
if (!(args[1] as string[] | undefined)?.includes("-e")) return real.execFile(...args);
const callback = args.at(-1) as (error: Error, stdout: string, stderr: string) => void;
callback(new Error("Fixture has no system DNS"), "", "ENOTFOUND");
return { once() {}, kill() {} } as unknown as ReturnType<typeof real.execFile>;
},
};
});
vi.mock("node:http", async (importOriginal) => {
const original = await importOriginal<typeof import("node:http")>();
return {
@@ -0,0 +1,49 @@
import { promises as dns } from "node:dns";
import { createRequire, syncBuiltinESMExports } from "node:module";
const server = process.env.HF_TEST_DNS ?? "";
if (!server) throw new Error("Missing DNS server");
const Resolver = dns.Resolver;
dns.Resolver = class extends Resolver {
constructor(options?: ConstructorParameters<typeof Resolver>[0]) {
super(options);
this.setServers([server]);
}
};
const requireBuiltin = createRequire(import.meta.url);
const fsPromises = requireBuiltin("node:fs/promises");
const readFile = fsPromises.readFile;
fsPromises.readFile = (path: string, ...rest: unknown[]) =>
/hosts$/.test(String(path))
? Promise.resolve(process.env.HF_TEST_HOSTS ?? "")
: readFile(path, ...rest);
// HF_TEST_SYSTEM: "stall" for a system lookup that never returns, or the address it answers.
const system = process.env.HF_TEST_SYSTEM ?? "stall";
const childProcess = requireBuiltin("node:child_process");
const execFile = childProcess.execFile;
let lookupPid: number | undefined;
childProcess.execFile = (file: string, _args: string[], ...rest: unknown[]) => {
const child = execFile(
file,
[
"-e",
system === "stall" ? "setInterval(() => {}, 1000)" : `process.stdout.write('["${system}"]')`,
],
...rest,
);
lookupPid = child.pid;
return child;
};
syncBuiltinESMExports();
const exitAfterMs = Number(process.env.HF_TEST_EXIT_AFTER_MS ?? 0);
if (exitAfterMs) {
setTimeout(() => {
process.stdout.write(`exit ${lookupPid}\n`);
process.exit(0);
}, exitAfterMs);
}
const { resolvePublicAddress } = await import("./publicNetwork.js");
const outcome = await resolvePublicAddress("assets.capture-fixture.com").catch(
(error: Error) => error.message,
);
process.stdout.write(`${outcome}\n`);
@@ -14,13 +14,25 @@ import type { TLSSocket } from "node:tls";
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
const transport = vi.hoisted(() => ({
lookup: vi.fn(),
resolve4: vi.fn(),
dials: [] as HttpsOptions[],
certificate: "",
trustCertificate: true,
}));
vi.mock("node:dns/promises", () => ({ lookup: transport.lookup }));
vi.mock("node:dns", async (importOriginal) => {
const real = await importOriginal<typeof import("node:dns")>();
return {
...real,
promises: {
...real.promises,
Resolver: class {
resolve4 = transport.resolve4;
cancel() {}
},
},
};
});
// Only the already-checked numeric peer is mapped to an isolated loopback server.
// The real Node HTTP/TLS transport, headers and certificate checks remain active.
@@ -172,9 +184,9 @@ describe("publicRequest controlled socket integration", () => {
receipts.length = 0;
transport.dials.length = 0;
transport.trustCertificate = true;
transport.lookup.mockReset();
transport.lookup.mockResolvedValue([{ address: "127.0.0.1", family: 4 }]);
transport.lookup.mockResolvedValueOnce([{ address: "93.184.216.34", family: 4 }]);
transport.resolve4.mockReset();
transport.resolve4.mockResolvedValue(["127.0.0.1"]);
transport.resolve4.mockResolvedValueOnce(["93.184.216.34"]);
});
afterAll(async () => {
@@ -193,12 +205,8 @@ describe("publicRequest controlled socket integration", () => {
it("pins the checked address without a second DNS lookup and preserves HTTP Host/path", async () => {
const response = await publicRequest(`http://assets.capture-fixture.com:${httpPort}/asset?q=1`);
expect(await response.text()).toBe("fixture response");
expect(transport.lookup).toHaveBeenCalledTimes(1);
expect(transport.lookup).toHaveBeenCalledWith("assets.capture-fixture.com", {
all: true,
family: 4,
verbatim: true,
});
expect(transport.resolve4).toHaveBeenCalledTimes(1);
expect(transport.resolve4).toHaveBeenCalledWith("assets.capture-fixture.com");
expect(transport.dials).toHaveLength(1);
expect(transport.dials[0]?.hostname).toBe("93.184.216.34");
expect(receipts).toEqual([
@@ -255,7 +263,7 @@ describe("publicRequest controlled socket integration", () => {
});
it("rejects private DNS answers before dialing or reaching a target", async () => {
transport.lookup.mockReset().mockResolvedValue([{ address: "127.0.0.1", family: 4 }]);
transport.resolve4.mockReset().mockResolvedValue(["127.0.0.1"]);
await expect(
publicRequest(`http://assets.capture-fixture.com:${httpPort}/asset`),
).rejects.toThrow(/non-public/);
@@ -0,0 +1,84 @@
import { spawn } from "node:child_process";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { expect, it, onTestFinished } from "vitest";
import { startTestDnsServer } from "../utils/dnsTestServer.js";
import { testProcessIsAlive } from "../utils/processTestUtils.js";
import { systemLookupInKillableChild } from "./publicNetwork.js";
const fixture = join(dirname(fileURLToPath(import.meta.url)), "publicNetwork.fixture.ts");
/** What the fixture printed, and how long its process lived after printing it. */
async function resolveInChild(
mode: "silent" | "answer",
system: string,
ipv4?: string,
env: Record<string, string> = {},
) {
const server = await startTestDnsServer(mode, ipv4);
onTestFinished(() => server.close());
const child = spawn(process.execPath, ["--import", "tsx", fixture], {
env: { ...process.env, HF_TEST_DNS: server.address, HF_TEST_SYSTEM: system, ...env },
stdio: ["ignore", "pipe", "inherit"],
});
onTestFinished(() => void child.kill("SIGKILL"));
let printed = "";
let printedAt = 0;
child.stdout.setEncoding("utf8");
child.stdout.on("data", (chunk: string) => {
printed += chunk;
if (printedAt) return;
printedAt = Date.now();
setTimeout(() => child.kill("SIGKILL"), 5_000).unref();
});
await new Promise((resolve) => child.once("exit", resolve));
return { printed: printed.trim(), livedOnMs: Date.now() - printedAt };
}
it("gives up when neither DNS nor the system lookup answers, and the process is gone right after", async () => {
const run = await resolveInChild("silent", "stall");
expect(run.printed).toBe("Capture DNS timed out");
expect(run.livedOnMs).toBeLessThan(1_000);
}, 30_000);
it("falls back to the system answer when DNS gives none", async () => {
const run = await resolveInChild("silent", "8.8.4.4");
expect(run.printed).toBe("8.8.4.4");
expect(run.livedOnMs).toBeLessThan(1_000);
}, 30_000);
it("uses the DNS answer without asking the system", async () => {
const run = await resolveInChild("answer", "8.8.4.4", "93.184.216.34");
expect(run.printed).toBe("93.184.216.34");
expect(run.livedOnMs).toBeLessThan(1_000);
}, 30_000);
it("takes the hosts file's answer over the DNS one, as the system lookup does", async () => {
const hosts = "# local override\n8.8.4.4 assets.capture-fixture.com\n";
const run = await resolveInChild("answer", "stall", "93.184.216.34", { HF_TEST_HOSTS: hosts });
expect(run.printed).toBe("8.8.4.4");
}, 30_000);
it("refuses a host the hosts file blocks", async () => {
const hosts = "0.0.0.0 assets.capture-fixture.com\n";
const run = await resolveInChild("answer", "stall", "93.184.216.34", { HF_TEST_HOSTS: hosts });
expect(run.printed).toBe("Capture DNS returned a non-public address");
}, 30_000);
it("refuses a host when any of its hosts file addresses is not public", async () => {
const hosts = "8.8.4.4 assets.capture-fixture.com\n0.0.0.0 assets.capture-fixture.com\n";
const run = await resolveInChild("answer", "stall", "93.184.216.34", { HF_TEST_HOSTS: hosts });
expect(run.printed).toBe("Capture DNS returned a non-public address");
}, 30_000);
it("leaves no system lookup running when the CLI exits mid-lookup", async () => {
const run = await resolveInChild("silent", "stall", undefined, { HF_TEST_EXIT_AFTER_MS: "6000" });
const pid = Number(/^exit (\d+)$/m.exec(run.printed)?.[1]);
onTestFinished(() => void (testProcessIsAlive(pid) && process.kill(pid, "SIGKILL")));
expect(pid).toBeGreaterThan(0);
expect(testProcessIsAlive(pid)).toBe(false);
}, 30_000);
it("runs the real system lookup in its child", async () => {
await expect(systemLookupInKillableChild("localhost", 10_000)).resolves.toContain("127.0.0.1");
}, 30_000);
+75 -34
View File
@@ -1,13 +1,33 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { EventEmitter } from "node:events";
import { PassThrough } from "node:stream";
const mocks = vi.hoisted(() => ({ lookup: vi.fn(), request: vi.fn() }));
vi.mock("node:dns/promises", () => ({ lookup: mocks.lookup }));
const mocks = vi.hoisted(() => ({ resolve4: vi.fn(), request: vi.fn(), execFile: vi.fn() }));
vi.mock("node:child_process", () => ({ execFile: mocks.execFile }));
vi.mock("node:fs/promises", () => ({ readFile: async () => "" }));
vi.mock("node:dns", () => ({
promises: {
Resolver: class {
resolve4 = mocks.resolve4;
cancel() {}
},
},
}));
vi.mock("node:http", () => ({ request: mocks.request }));
vi.mock("node:https", () => ({ request: mocks.request }));
import { isPublicIPv4, publicRequest, resolvePublicAddress } from "./publicNetwork.js";
beforeEach(() => vi.resetAllMocks());
/** The system lookup child prints `stdout`, or fails with `stderr`. */
function systemLookupPrints(stdout: string, stderr = "") {
mocks.execFile.mockImplementation((_file, _args, _options, callback) => {
callback(stdout ? null : new Error("Command failed"), stdout, stderr);
return new EventEmitter();
});
}
beforeEach(() => {
vi.resetAllMocks();
systemLookupPrints("", "ENOTFOUND");
});
describe("public address policy", () => {
it.each([
"127.0.0.1",
@@ -30,7 +50,7 @@ describe("public address policy", () => {
])("rejects %s", (ip) => expect(isPublicIPv4(ip)).toBe(false));
it("accepts public literals without DNS", async () => {
await expect(resolvePublicAddress("8.8.8.8")).resolves.toBe("8.8.8.8");
expect(mocks.lookup).not.toHaveBeenCalled();
expect(mocks.resolve4).not.toHaveBeenCalled();
});
it.each([
"localhost",
@@ -41,26 +61,61 @@ describe("public address policy", () => {
"[::1]",
])("blocks alias/literal %s before DNS", async (host) => {
await expect(resolvePublicAddress(host)).rejects.toThrow();
expect(mocks.lookup).not.toHaveBeenCalled();
expect(mocks.resolve4).not.toHaveBeenCalled();
});
it("rejects mixed DNS before opening a socket", async () => {
mocks.lookup.mockResolvedValue([
{ address: "8.8.8.8", family: 4 },
{ address: "127.0.0.1", family: 4 },
]);
mocks.resolve4.mockResolvedValue(["8.8.8.8", "127.0.0.1"]);
await expect(publicRequest("http://example.com/")).rejects.toThrow();
expect(mocks.request).not.toHaveBeenCalled();
});
it.each([{ answers: [] }, { answers: [{ address: "8.8.8.8", family: 6 }] }])(
it.each([{ answers: [] }, { answers: ["::ffff:8.8.8.8"] }])(
"rejects empty or malformed DNS",
async ({ answers }) => {
mocks.lookup.mockResolvedValue(answers);
mocks.resolve4.mockResolvedValue(answers);
await expect(resolvePublicAddress("example.com")).rejects.toThrow();
},
);
it("propagates DNS errors", async () => {
mocks.lookup.mockRejectedValue(new Error("DNS failed"));
await expect(resolvePublicAddress("example.com")).rejects.toThrow("DNS failed");
it("fails when neither DNS nor the system lookup answers", async () => {
mocks.resolve4.mockRejectedValue(new Error("queryA ENOTFOUND"));
await expect(resolvePublicAddress("example.com")).rejects.toThrow(
"Capture DNS failed: ENOTFOUND",
);
});
it("checks a system answer like a DNS one, and asks the system once per host", async () => {
mocks.resolve4.mockRejectedValue(new Error("queryA ENOTFOUND"));
systemLookupPrints('["127.0.0.1"]');
await expect(resolvePublicAddress("hosts-file.example.org")).rejects.toThrow(/non-public/);
systemLookupPrints('["8.8.8.8"]');
await expect(resolvePublicAddress("cached.example.org")).resolves.toBe("8.8.8.8");
await expect(resolvePublicAddress("cached.example.org")).resolves.toBe("8.8.8.8");
expect(mocks.execFile).toHaveBeenCalledTimes(2);
});
it("holds a later caller to its own deadline when it joins a system lookup already running", async () => {
vi.useFakeTimers();
try {
mocks.resolve4.mockRejectedValue(new Error("queryA ENOTFOUND"));
mocks.execFile.mockImplementation(() => new EventEmitter());
const first = resolvePublicAddress("slow.example.org").catch((error: Error) => error.message);
await vi.advanceTimersByTimeAsync(9_000);
const second = resolvePublicAddress("slow.example.org").catch(
(error: Error) => error.message,
);
await vi.advanceTimersByTimeAsync(1_000);
expect(await first).toBe("Capture DNS timed out");
await vi.advanceTimersByTimeAsync(8_000);
expect(await Promise.race([second, "still waiting"])).toBe("still waiting");
await vi.advanceTimersByTimeAsync(1_000);
expect(await second).toBe("Capture DNS timed out");
expect(mocks.execFile).toHaveBeenCalledOnce();
expect(mocks.resolve4).toHaveBeenCalledOnce();
} finally {
vi.useRealTimers();
}
});
it("never starts the system lookup when DNS answers", async () => {
mocks.resolve4.mockResolvedValue(["8.8.8.8"]);
await expect(resolvePublicAddress("example.org")).resolves.toBe("8.8.8.8");
expect(mocks.execFile).not.toHaveBeenCalled();
});
});
@@ -86,14 +141,12 @@ function fakeRequest() {
}
describe("pinned request", () => {
it("dials the checked numeric IP only, preserving TLS identity and manual redirects", async () => {
mocks.lookup
.mockResolvedValueOnce([{ address: "8.8.8.8", family: 4 }])
.mockResolvedValue([{ address: "127.0.0.1", family: 4 }]);
mocks.resolve4.mockResolvedValueOnce(["8.8.8.8"]).mockResolvedValue(["127.0.0.1"]);
fakeRequest();
const response = await publicRequest("https://example.com:8443/path?q=1", {
headers: { host: "evil.test", "accept-encoding": "gzip" },
});
expect(mocks.lookup).toHaveBeenCalledTimes(1);
expect(mocks.resolve4).toHaveBeenCalledTimes(1);
expect(mocks.request).toHaveBeenCalledTimes(1);
expect(mocks.request.mock.calls[0]?.[0]).toMatchObject({
hostname: "8.8.8.8",
@@ -116,9 +169,9 @@ describe("pinned request", () => {
);
it("does not dial after abort during DNS", async () => {
const controller = new AbortController();
mocks.lookup.mockImplementation(async () => {
mocks.resolve4.mockImplementation(async () => {
controller.abort();
return [{ address: "8.8.8.8", family: 4 }];
return ["8.8.8.8"];
});
await expect(
publicRequest("https://example.com", { signal: controller.signal }),
@@ -128,20 +181,8 @@ describe("pinned request", () => {
});
describe("transport lifecycle", () => {
it("bounds stalled DNS without dialing", async () => {
vi.useFakeTimers();
try {
mocks.lookup.mockImplementation(() => new Promise(() => {}));
const result = expect(publicRequest("https://example.com")).rejects.toThrow("DNS timed out");
await vi.advanceTimersByTimeAsync(10_000);
await result;
expect(mocks.request).not.toHaveBeenCalled();
} finally {
vi.useRealTimers();
}
});
it("cancels the upstream stream and socket when the body is cancelled", async () => {
mocks.lookup.mockResolvedValue([{ address: "8.8.8.8", family: 4 }]);
mocks.resolve4.mockResolvedValue(["8.8.8.8"]);
const incoming = Object.assign(new PassThrough(), { statusCode: 200, headers: {} });
const req = Object.assign(new EventEmitter(), { end: vi.fn(), destroy: vi.fn() });
mocks.request.mockImplementation((_options, callback) => {
@@ -161,7 +202,7 @@ describe("transport lifecycle", () => {
}).not.toThrow();
});
it("fails unsupported compressed responses instead of returning corrupted asset bytes", async () => {
mocks.lookup.mockResolvedValue([{ address: "8.8.8.8", family: 4 }]);
mocks.resolve4.mockResolvedValue(["8.8.8.8"]);
const incoming = Object.assign(new PassThrough(), {
statusCode: 200,
headers: { "content-encoding": "gzip" },
+106 -18
View File
@@ -1,8 +1,13 @@
import { lookup } from "node:dns/promises";
import { execFile, type ChildProcess } from "node:child_process";
import { promises as dns } from "node:dns";
import { readFile } from "node:fs/promises";
import { isIP } from "node:net";
import { request as httpRequest, type IncomingMessage, type ClientRequest } from "node:http";
import { request as httpsRequest } from "node:https";
const DNS_TIMEOUT_MS = 10_000;
const CARES_TIMEOUT_MS = 5_000;
function ipv4Number(address: string): number {
return address.split(".").reduce((value, octet) => value * 256 + Number(octet), 0);
}
@@ -50,29 +55,112 @@ export async function resolvePublicAddress(hostname: string): Promise<string> {
) {
throw new Error("Capture requires a public hostname");
}
let timer: ReturnType<typeof setTimeout> | undefined;
try {
const answers = await Promise.race([
lookup(host, { all: true, family: 4, verbatim: true }),
new Promise<never>((_resolve, reject) => {
timer = setTimeout(() => reject(new Error("Capture DNS timed out")), 10_000);
timer.unref();
}),
]);
if (
!answers[0] ||
answers.some((answer) => answer.family !== 4 || !isPublicIPv4(answer.address))
) {
throw new Error("Capture DNS returned a non-public address");
const answers = await resolveIPv4(host);
const first = answers[0];
if (!first || !answers.every(isPublicIPv4)) {
throw new Error("Capture DNS returned a non-public address");
}
return first;
}
async function resolveIPv4(host: string): Promise<string[]> {
const fromHostsFile = (await hostsFileAnswers()).get(host);
if (fromHostsFile) return fromHostsFile;
const deadline = Date.now() + DNS_TIMEOUT_MS;
const fromSystem = systemAnswers.get(host);
if (fromSystem) return beforeDeadline(fromSystem, deadline);
return resolveWithCares(host).catch(() =>
beforeDeadline(systemLookupInKillableChild(host, deadline - Date.now()), deadline),
);
}
let hostsFile: Promise<Map<string, string[]>> | undefined;
function hostsFileAnswers(): Promise<Map<string, string[]>> {
const path =
process.platform === "win32"
? `${process.env.SystemRoot ?? "C:\\Windows"}\\System32\\drivers\\etc\\hosts`
: "/etc/hosts";
hostsFile ??= readFile(path, "utf8").then(parseHostsFile, () => new Map());
return hostsFile;
}
function parseHostsFile(text: string): Map<string, string[]> {
const answers = new Map<string, string[]>();
for (const line of text.split(/\r?\n/)) {
const [address, ...names] = line.replace(/#.*/, "").trim().split(/\s+/);
if (!address || isIP(address) !== 4) continue;
for (const name of names) {
const key = name.toLowerCase().replace(/\.$/, "");
answers.set(key, [...(answers.get(key) ?? []), address]);
}
const first = answers[0];
if (!first) throw new Error("Capture DNS returned no address");
return first.address;
}
return answers;
}
function beforeDeadline<T>(work: Promise<T>, deadline: number): Promise<T> {
let timer: ReturnType<typeof setTimeout> | undefined;
const late = new Promise<never>((_resolve, reject) => {
timer = setTimeout(
() => reject(new Error("Capture DNS timed out")),
Math.max(0, deadline - Date.now()),
);
});
return Promise.race([work, late]).finally(() => clearTimeout(timer));
}
async function resolveWithCares(host: string): Promise<string[]> {
const cancellableResolver = new dns.Resolver();
const timer = setTimeout(() => cancellableResolver.cancel(), CARES_TIMEOUT_MS);
try {
return await cancellableResolver.resolve4(host);
} finally {
clearTimeout(timer);
}
}
const systemAnswers = new Map<string, Promise<string[]>>();
const SYSTEM_LOOKUP = `require("node:dns").lookup(process.argv[1], { all: true, family: 4 }, (error, answers) =>
error ? process.stderr.write(String(error.code)) : process.stdout.write(JSON.stringify(answers.map((a) => a.address))))`;
const liveLookups = new Set<ChildProcess>();
export function systemLookupInKillableChild(host: string, timeoutMs: number): Promise<string[]> {
if (!process.listeners("exit").includes(killLiveLookups)) process.on("exit", killLiveLookups);
const answers = new Promise<string[]>((resolve, reject) => {
const child = execFile(
process.execPath,
["-e", SYSTEM_LOOKUP, host],
{
timeout: Math.max(1, timeoutMs),
killSignal: "SIGKILL",
windowsHide: true,
env: { ...process.env, NODE_OPTIONS: "" },
},
(error, stdout, stderr) => {
if (error?.killed) reject(new Error("Capture DNS timed out"));
else if (!stdout) reject(new Error(`Capture DNS failed: ${stderr || error?.message}`));
else {
try {
resolve(JSON.parse(stdout) as string[]);
} catch (parseError) {
reject(parseError);
}
}
},
);
liveLookups.add(child);
child.once("exit", () => liveLookups.delete(child));
});
systemAnswers.set(host, answers);
answers.catch(() => systemAnswers.delete(host));
return answers;
}
function killLiveLookups(): void {
for (const child of liveLookups) child.kill("SIGKILL");
}
/** Pinned asset transport; redirects are returned for caller validation. */
export async function publicRequest(url: string | URL, init: RequestInit = {}): Promise<Response> {
const target = new URL(url);
+5 -1
View File
@@ -98,6 +98,7 @@ import {
CliRuntimeError,
CliUsageError,
consumeCommandResult,
outputWritten,
registerRootExitCodeSanitizer,
registerRootExitRequester,
type CommandResult,
@@ -352,7 +353,10 @@ registerRootExitRequester((exitCode) => {
exitCode,
kind: exitCode === 0 ? "success" : "runtime_error",
presented: true,
}).finally(() => process.exit(exitCode));
}).finally(async () => {
await outputWritten();
process.exit(exitCode);
});
});
registerRootExitCodeSanitizer(() => {
+53 -1
View File
@@ -10,7 +10,7 @@ import {
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { CliRuntimeError } from "../utils/commandResult.js";
import { CliRuntimeError, registerRootExitRequester } from "../utils/commandResult.js";
import { CAPTURE_PHASE_SCHEMA } from "../capture/types.js";
const { captureWebsiteMock } = vi.hoisted(() => ({
@@ -205,6 +205,58 @@ describe("capture command — vision control", () => {
);
});
it.each([
[true, 0],
[false, 1],
])("asks to end the process once its result is out (ok %s)", async (ok, exitCode) => {
vi.useFakeTimers();
const exit = vi.fn();
registerRootExitRequester(exit);
try {
vi.spyOn(console, "log").mockImplementation(() => {});
vi.spyOn(console, "error").mockImplementation(() => {});
captureWebsiteMock.mockResolvedValueOnce({
ok,
projectDir: "/tmp/capture",
url: "https://example.com",
title: "Example",
extracted: {},
screenshots: [],
tokens: { sections: [], fonts: [] },
assets: [],
warnings: [],
});
await captureCommand.run!({
args: { url: "https://example.com", output: "/tmp/hf-exit", json: true },
} as never).catch(() => {});
expect(exit).not.toHaveBeenCalled();
vi.advanceTimersByTime(100);
expect(exit).toHaveBeenCalledWith(exitCode);
} finally {
registerRootExitRequester(() => {});
vi.useRealTimers();
}
});
it("asks to end the process with 1 after a capture that threw", async () => {
vi.useFakeTimers();
const exit = vi.fn();
registerRootExitRequester(exit);
try {
vi.spyOn(console, "log").mockImplementation(() => {});
vi.spyOn(console, "error").mockImplementation(() => {});
captureWebsiteMock.mockRejectedValueOnce(new Error("browser crashed"));
await captureCommand.run!({
args: { url: "https://example.com", output: "/tmp/hf-exit-threw", json: true },
} as never).catch(() => {});
vi.advanceTimersByTime(100);
expect(exit).toHaveBeenCalledWith(1);
} finally {
registerRootExitRequester(() => {});
vi.useRealTimers();
}
});
it("returns a nonzero command result after presenting failed capture JSON once", async () => {
const log = vi.spyOn(console, "log").mockImplementation(() => {});
vi.spyOn(console, "error").mockImplementation(() => {});
+3 -1
View File
@@ -1,5 +1,5 @@
import { DEFAULT_MAX_SCREENSHOTS } from "../capture/types.js";
import { failCommand } from "../utils/commandResult.js";
import { failCommand, scheduleCliExit } from "../utils/commandResult.js";
import { defineCommand } from "citty";
import { resolve } from "node:path";
import type { Example } from "./_examples.js";
@@ -303,8 +303,10 @@ export default defineCommand({
} else {
console.error(`\n ✗ Capture failed: ${errMsg}\n`);
}
scheduleCliExit(1);
failCommand();
}
scheduleCliExit(captureFailed ? 1 : 0);
if (captureFailed) failCommand();
},
});
+3 -23
View File
@@ -1,6 +1,6 @@
import { captureArtifactRevision } from "../utils/artifactCapture.js";
import { recordRenderSuccess, registerRenderOutput } from "@hyperframes/studio-server/history";
import { failCommand, requestCliExit } from "../utils/commandResult.js";
import { failCommand, requestCliExit, scheduleCliExit } from "../utils/commandResult.js";
import { defineCommand } from "citty";
import type { Example } from "./_examples.js";
import { mkdtempSync, readdirSync, readFileSync, statSync, writeFileSync, rmSync } from "node:fs";
@@ -921,7 +921,7 @@ async function renderDocker(
runPostRenderStep("warnIfWebmAlphaDropped", () =>
warnIfWebmAlphaDropped(outputPath, options.format, options.quiet),
);
if (options.exitAfterComplete) scheduleRenderProcessExit();
if (options.exitAfterComplete) scheduleCliExit();
return { renderTimeMs: elapsed };
}
@@ -1205,7 +1205,7 @@ async function executeLocalRender(
}),
);
}
if (options.exitAfterComplete) scheduleRenderProcessExit();
if (options.exitAfterComplete) scheduleCliExit();
const durationMs = job.perfSummary
? Math.round(job.perfSummary.compositionDurationSeconds * 1000)
: undefined;
@@ -1219,26 +1219,6 @@ async function executeLocalRender(
};
}
type UnrefableTimer = {
unref: () => void;
};
function isUnrefableTimer(
timer: ReturnType<typeof setTimeout>,
): timer is ReturnType<typeof setTimeout> & UnrefableTimer {
return (
typeof timer === "object" &&
timer !== null &&
"unref" in timer &&
typeof timer.unref === "function"
);
}
function scheduleRenderProcessExit(): void {
const timer = setTimeout(() => requestCliExit(0), 100);
if (isUnrefableTimer(timer)) timer.unref();
}
function getMemorySnapshot() {
return {
peakMemoryMb: bytesToMb(process.memoryUsage.rss()),
@@ -0,0 +1,6 @@
import { outputWritten, registerRootExitRequester, scheduleCliExit } from "./commandResult.js";
registerRootExitRequester((exitCode) => void outputWritten().then(() => process.exit(exitCode)));
setInterval(function stalledLookup() {}, 1_000);
console.log(JSON.stringify({ title: "x".repeat(1024 * 1024) }));
scheduleCliExit(0);
@@ -0,0 +1,23 @@
import { spawn } from "node:child_process";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { expect, it, onTestFinished } from "vitest";
const fixture = join(dirname(fileURLToPath(import.meta.url)), "commandResult.fixture.ts");
it("a forced exit still hands a slow reader the whole result", async () => {
const child = spawn(process.execPath, ["--import", "tsx", fixture], {
stdio: ["ignore", "pipe", "inherit"],
});
onTestFinished(() => void child.kill("SIGKILL"));
let printed = "";
child.stdout.setEncoding("utf8");
child.stdout.on("data", (chunk: string) => {
printed += chunk;
child.stdout.pause();
setTimeout(() => child.stdout.resume(), 100);
});
const exitCode = await new Promise((resolve) => child.once("close", resolve));
expect(exitCode).toBe(0);
expect((JSON.parse(printed) as { title: string }).title).toHaveLength(1024 * 1024);
}, 60_000);
+27
View File
@@ -108,6 +108,12 @@ export function sanitizeSuccessfulExitCode(): void {
rootExitCodeSanitizer?.();
}
export async function outputWritten(): Promise<void> {
const written = (stream: NodeJS.WriteStream) =>
new Promise<void>((resolve) => stream.write("", () => resolve()));
await Promise.all([written(process.stdout), written(process.stderr)]);
}
/** Ask cli.ts to finalize telemetry/output and then terminate the process. */
export function requestCliExit(exitCode = 0): void {
if (!rootExitRequester) {
@@ -120,3 +126,24 @@ export function requestCliExit(exitCode = 0): void {
}
rootExitRequester(exitCode);
}
type UnrefableTimer = {
unref: () => void;
};
function isUnrefableTimer(
timer: ReturnType<typeof setTimeout>,
): timer is ReturnType<typeof setTimeout> & UnrefableTimer {
return (
typeof timer === "object" &&
timer !== null &&
"unref" in timer &&
typeof timer.unref === "function"
);
}
/** For a command whose result is out: ends the process if anything it started is still holding it. */
export function scheduleCliExit(exitCode = 0): void {
const timer = setTimeout(() => requestCliExit(exitCode), 100);
if (isUnrefableTimer(timer)) timer.unref();
}
+37
View File
@@ -0,0 +1,37 @@
import { createSocket } from "node:dgram";
export interface TestDnsServer {
/** `host:port`, for a Resolver's setServers. */
address: string;
close(): void;
}
/** A DNS server: "silent" never replies, "answer" answers A with `ipv4`, "no-aaaa" answers A and drops AAAA. */
export async function startTestDnsServer(
mode: "silent" | "answer" | "no-aaaa",
ipv4 = "127.0.0.1",
): Promise<TestDnsServer> {
const socket = createSocket("udp4");
socket.on("message", (msg, peer) => {
if (mode === "silent") return;
let end = 12;
while (msg[end] !== 0) end += (msg[end] ?? 0) + 1;
end += 5;
const isA = msg.readUInt16BE(end - 4) === 1;
if (mode === "no-aaaa" && !isA) return;
const header = Buffer.alloc(12);
msg.copy(header, 0, 0, 2);
header.writeUInt16BE(0x8180, 2);
header.writeUInt16BE(1, 4);
header.writeUInt16BE(isA ? 1 : 0, 6);
const answer = Buffer.from([0xc0, 0x0c, 0, 1, 0, 1, 0, 0, 0, 60, 0, 4]);
const record = Buffer.concat([answer, Buffer.from(ipv4.split(".").map(Number))]);
socket.send(
Buffer.concat([header, msg.subarray(12, end), ...(isA ? [record] : [])]),
peer.port,
peer.address,
);
});
await new Promise<void>((resolve) => socket.bind(0, "127.0.0.1", resolve));
return { address: `127.0.0.1:${socket.address().port}`, close: () => socket.close() };
}
+6 -27
View File
@@ -1,5 +1,5 @@
import { createSocket, type Socket } from "node:dgram";
import { afterEach, describe, expect, it, vi } from "vitest";
import { startTestDnsServer, type TestDnsServer } from "./dnsTestServer.js";
// Point every Resolver hostAnswers creates at a local UDP server this test controls.
const dnsServer = vi.hoisted(() => ({ address: "" }));
@@ -14,38 +14,17 @@ vi.mock("node:dns", async (importOriginal) => {
return { ...real, promises: { ...real.promises, Resolver } };
});
let socket: Socket | undefined;
let server: TestDnsServer | undefined;
/** A DNS server: "silent" never replies, "answer" answers A with 127.0.0.1, "no-aaaa" answers A and drops AAAA. */
async function startServer(mode: "silent" | "answer" | "no-aaaa"): Promise<void> {
socket = createSocket("udp4");
socket.on("message", (msg, peer) => {
if (mode === "silent") return;
let end = 12;
while (msg[end] !== 0) end += (msg[end] ?? 0) + 1;
end += 5;
const isA = msg.readUInt16BE(end - 4) === 1;
if (mode === "no-aaaa" && !isA) return;
const header = Buffer.alloc(12);
msg.copy(header, 0, 0, 2);
header.writeUInt16BE(0x8180, 2);
header.writeUInt16BE(1, 4);
header.writeUInt16BE(isA ? 1 : 0, 6);
const record = Buffer.from([0xc0, 0x0c, 0, 1, 0, 1, 0, 0, 0, 60, 0, 4, 127, 0, 0, 1]);
socket?.send(
Buffer.concat([header, msg.subarray(12, end), ...(isA ? [record] : [])]),
peer.port,
peer.address,
);
});
await new Promise<void>((resolve) => socket?.bind(0, "127.0.0.1", resolve));
dnsServer.address = `127.0.0.1:${socket.address().port}`;
server = await startTestDnsServer(mode);
dnsServer.address = server.address;
}
describe("hostAnswers", () => {
afterEach(() => {
socket?.close();
socket = undefined;
server?.close();
server = undefined;
});
it("says no, within its short timeout, when DNS never replies", async () => {
+3 -1
View File
@@ -39,7 +39,9 @@ function listTypeScriptFiles(directory, roots) {
return readdirSync(directory).flatMap((name) => {
const path = join(directory, name);
if (statSync(path).isDirectory()) return listTypeScriptFiles(path, roots);
if (!path.endsWith(".ts") || path.endsWith(".test.ts") || roots.includes(path)) return [];
if (!path.endsWith(".ts") || /\.(test|fixture)\.ts$/.test(path) || roots.includes(path)) {
return [];
}
return [path];
});
}
+18 -1
View File
@@ -1,6 +1,12 @@
import assert from "node:assert/strict";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { describe, it } from "node:test";
import { listDirectProcessTermination } from "./check-cli-process-ownership.mjs";
import {
checkCliProcessOwnership,
listDirectProcessTermination,
} from "./check-cli-process-ownership.mjs";
describe("CLI process ownership checker", () => {
it("reports direct process exit and exitCode writes", () => {
@@ -25,4 +31,15 @@ describe("CLI process ownership checker", () => {
[],
);
});
it("lets a test fixture, spawned as its own process, exit", (t) => {
const root = mkdtempSync(join(tmpdir(), "hf-ownership-"));
t.after(() => rmSync(root, { recursive: true, force: true }));
mkdirSync(join(root, "packages/cli/src/utils"), { recursive: true });
writeFileSync(join(root, "packages/cli/src/utils/a.fixture.ts"), "process.exit(0);\n");
writeFileSync(join(root, "packages/cli/src/utils/a.ts"), "process.exit(0);\n");
assert.deepEqual(checkCliProcessOwnership(root), [
"packages/cli/src/utils/a.ts:1:1 uses process.exit",
]);
});
});