diff --git a/lib/audit/links-crawl-child.ts b/lib/audit/links-crawl-child.ts new file mode 100644 index 0000000..4a1c9cc --- /dev/null +++ b/lib/audit/links-crawl-child.ts @@ -0,0 +1,61 @@ +// Child-process entry for the link crawl. Forked by links-engine.ts. +// +// Contract: argv[2] is the target URL, argv[3] an optional per-link timeout in +// ms (tests use a short one to make the abort race deterministic). We print +// exactly one JSON object to stdout and exit 0. Anything else on stdout would +// corrupt the parse, so all diagnostics go to stderr (which the parent forwards +// to the worker log). +// +// This process is expendable by design — see the comment in links-crawl.ts for +// why linkinator can hard-exit the process it runs in. Both exit paths salvage +// whatever the accumulator holds, so a crash 200 pages into a crawl still +// reports those 200 pages instead of losing the audit. + +import { crawlLinks, newAccumulator, type CrawlAccumulator } from "./links-crawl"; + +export type ChildReport = { + acc: CrawlAccumulator; + /** Set when the crawl ended early; the parent turns this into a warn finding. */ + crashed: string | null; +}; + +const acc = newAccumulator(); +let emitted = false; + +function emit(crashed: string | null): void { + if (emitted) return; // an uncaught error while shutting down must not double-print + emitted = true; + const report: ChildReport = { acc, crashed }; + process.stdout.write(JSON.stringify(report), () => process.exit(0)); + // linkinator leaves sockets open, so the natural exit may never come; and if + // stdout's drain callback is itself lost, exit anyway rather than hang the + // parent until its kill timer fires. + setTimeout(() => process.exit(0), 2_000).unref(); +} + +// An unhandled 'error' event on a Readable arrives here as an uncaught +// exception. This handler is the whole point of the child process: catch it, +// keep the partial crawl, and leave the parent worker untouched. +process.on("uncaughtException", (err: unknown) => { + const message = err instanceof Error ? err.message : String(err); + console.error(`[links-crawl] uncaught in crawl child: ${message}`); + emit(message); +}); +process.on("unhandledRejection", (err: unknown) => { + const message = err instanceof Error ? err.message : String(err); + console.error(`[links-crawl] unhandled rejection in crawl child: ${message}`); + emit(message); +}); + +const target = process.argv[2]; +if (!target) { + console.error("[links-crawl] no target URL argument"); + process.exit(2); +} + +const perLinkTimeoutMs = Number(process.argv[3]) || undefined; + +crawlLinks(target, acc, { perLinkTimeoutMs }).then( + () => emit(null), + (err: unknown) => emit(err instanceof Error ? err.message : String(err)), +); diff --git a/lib/audit/links-crawl.ts b/lib/audit/links-crawl.ts new file mode 100644 index 0000000..a99633d --- /dev/null +++ b/lib/audit/links-crawl.ts @@ -0,0 +1,121 @@ +// The raw linkinator crawl, split out from links-engine.ts so it can run in a +// disposable child process. +// +// Why a child process: linkinator applies its per-link `timeout` as an +// AbortSignal.timeout on the fetch, converts the response body with +// Readable.fromWeb(), then pipes it to the HTML parser with the 'error' handler +// attached to the *destination* (build/src/links.js:158). pipe() does not +// forward source errors, so when the abort fires mid-body the source Readable +// emits an unhandled 'error' event and Node hard-exits the process. That is not +// catchable from linksAudit()'s try/catch, and because start.sh runs the worker +// and Next.js under `wait -n`, it took the whole container down with every +// in-flight audit. Run it somewhere we can afford to lose. +// +// Crawl bounds live here too: linkinator has no built-in page cap or +// AbortSignal, so we bound the crawl with its `linksToSkip` extension point. +// Once we exceed a page / link / wall-clock budget the predicate returns true +// for every remaining link, which stops both checking and recursion. This keeps +// a runaway crawl well under the worker's 7-minute stuck-audit cutoff. + +import { LinkChecker, LinkState } from "linkinator"; + +const UA = "CrawlProofBot/1.0 (+https://crawlproof.com/bot)"; + +// Budgets — generous enough for a typical CrawlProof property, hard-capped so +// the worker can't blow past the 7-minute stuck-sweep cutoff. +export const MAX_PAGES = 250; // distinct internal pages we recurse into +export const MAX_LINKS = 5000; // total links checked (internal + external) +export const DEADLINE_MS = 4 * 60 * 1000; // wall-clock crawl budget +export const PER_LINK_TIMEOUT_MS = 10_000; +const CONCURRENCY = 25; + +export type Capped = null | "pages" | "links" | "time"; + +/** A linkinator LinkResult, reduced to what survives JSON round-tripping. */ +export type CrawlLink = { + url: string; + status: number; + state: string; + parent: string | null; +}; + +/** + * Filled in as the crawl runs rather than read off the resolved return value, + * so a crash partway through still leaves usable results behind. + */ +export type CrawlAccumulator = { + pagesCrawled: number; + capped: Capped; + links: CrawlLink[]; +}; + +export function newAccumulator(): CrawlAccumulator { + return { pagesCrawled: 0, capped: null, links: [] }; +} + +export function rootOf(targetUrl: string): string { + // Crawl from the root domain, not the submitted deep link — the user asked + // the bot to sweep the whole property, not just one page. + const u = new URL(targetUrl); + return `${u.protocol}//${u.host}/`; +} + +export async function crawlLinks( + targetUrl: string, + acc: CrawlAccumulator, + opts: { perLinkTimeoutMs?: number } = {}, +): Promise { + const started = Date.now(); + const root = rootOf(targetUrl); + const checker = new LinkChecker(); + + checker.on("pagestart", () => { + acc.pagesCrawled++; + }); + + // Accumulate incrementally — `check()`'s return value is unreachable if the + // crawl dies partway through. + checker.on("link", (link: { url: string; status?: number; state: string; parent?: string }) => { + acc.links.push({ + url: link.url, + status: link.status ?? 0, + state: link.state, + parent: link.parent ?? null, + }); + }); + + // Doubles as the crawl's kill-switch: returning true marks a link SKIPPED, + // which also prevents linkinator from recursing into it. + const linksToSkip = async (link: string): Promise => { + // linkinator already skips non-http(s) schemes, but guard anyway. + if (!/^https?:\/\//i.test(link)) return true; + if (Date.now() - started > DEADLINE_MS) { + acc.capped ??= "time"; + return true; + } + if (acc.pagesCrawled >= MAX_PAGES) { + acc.capped ??= "pages"; + return true; + } + if (checkedCount(acc) >= MAX_LINKS) { + acc.capped ??= "links"; + return true; + } + return false; + }; + + await checker.check({ + path: root, + recurse: true, + concurrency: CONCURRENCY, + timeout: opts.perLinkTimeoutMs ?? PER_LINK_TIMEOUT_MS, + userAgent: UA, + retry: true, + linksToSkip, + }); +} + +/** Links we actually hit the network for (SKIPPED ones were never fetched). */ +export function checkedCount(acc: CrawlAccumulator): number { + return acc.links.filter((l) => l.state !== LinkState.SKIPPED).length; +} diff --git a/lib/audit/links-engine.ts b/lib/audit/links-engine.ts index ec28ba6..598ad13 100644 --- a/lib/audit/links-engine.ts +++ b/lib/audit/links-engine.ts @@ -2,101 +2,159 @@ // linkinator (the same crawler as `npx linkinator --recurse`) and turns // the broken-link report into structured findings. Free; no LLM required. // -// linkinator has no built-in page cap or AbortSignal, so we bound the crawl -// with its `linksToSkip` extension point: once we exceed a page / link / wall- -// clock budget the predicate returns true for every remaining link, which -// stops both checking and recursion. This keeps a runaway crawl well under the -// worker's 7-minute stuck-audit cutoff. +// The crawl itself runs in a forked child process (links-crawl-child.ts) because +// linkinator can hard-exit the process it runs in; see links-crawl.ts for the +// mechanism. Everything here — scoring, findings, markdown — runs in the worker +// off the child's JSON report. -import { LinkChecker, LinkState, type LinkResult } from "linkinator"; +import { fork } from "node:child_process"; +import { extname } from "node:path"; +import { fileURLToPath } from "node:url"; +import { LinkState } from "linkinator"; import { scoreFindings } from "./score"; import type { AuditResult, Finding } from "./types"; +import { + DEADLINE_MS, + MAX_LINKS, + MAX_PAGES, + checkedCount, + rootOf, + type Capped, + type CrawlAccumulator, + type CrawlLink, +} from "./links-crawl"; +import type { ChildReport } from "./links-crawl-child"; type LinksAuditResult = AuditResult & { markdown: string }; -const UA = "CrawlProofBot/1.0 (+https://crawlproof.com/bot)"; - -// Budgets — generous enough for a typical CrawlProof property, hard-capped so -// the worker can't blow past the 7-minute stuck-sweep cutoff. -const MAX_PAGES = 250; // distinct internal pages we recurse into -const MAX_LINKS = 5000; // total links checked (internal + external) -const DEADLINE_MS = 4 * 60 * 1000; // wall-clock crawl budget -const PER_LINK_TIMEOUT_MS = 10_000; -const CONCURRENCY = 25; - // How many broken links to enumerate in findings / markdown before truncating. const MAX_BROKEN_LISTED = 50; -function rootOf(targetUrl: string): string { - // Crawl from the root domain, not the submitted deep link — the user asked - // the bot to sweep the whole property, not just one page. - const u = new URL(targetUrl); - return `${u.protocol}//${u.host}/`; -} +// The child bounds itself at DEADLINE_MS; this is the backstop for a child that +// wedges instead of finishing, kept under the 7-minute stuck-audit cutoff. +const CHILD_KILL_AFTER_MS = DEADLINE_MS + 60_000; -export async function linksAudit(targetUrl: string): Promise { - const started = Date.now(); - const root = rootOf(targetUrl); +/** Resolve the child entry next to this module, matching its extension so the + * same code works under tsx (.ts, as start.sh runs it) and compiled (.js). */ +function childEntryPath(): string { + const self = new URL(import.meta.url); + return fileURLToPath(new URL(`./links-crawl-child${extname(self.pathname)}`, self)); +} - const checker = new LinkChecker(); - let pagesCrawled = 0; - let linksChecked = 0; - let capped: null | "pages" | "links" | "time" = null; +/** + * fork() inherits execArgv, so under the worker's `tsx worker/index.ts` the + * child already gets tsx's loader. Add it explicitly when the parent runtime + * transforms TypeScript some other way (vitest) and the child would otherwise + * hit a plain `node` that can't read .ts. + */ +function childExecArgv(entry: string): string[] { + const inherited = process.execArgv; + if (!entry.endsWith(".ts")) return inherited; + if (inherited.some((a) => a.includes("tsx"))) return inherited; + return [...inherited, "--import", "tsx"]; +} - checker.on("pagestart", () => { - pagesCrawled++; - }); +type ChildOutcome = + | { ok: true; acc: CrawlAccumulator; crashed: string | null } + | { ok: false; error: string }; - // Doubles as the crawl's kill-switch: returning true marks a link SKIPPED, - // which also prevents linkinator from recursing into it. - const linksToSkip = async (link: string): Promise => { - // linkinator already skips non-http(s) schemes, but guard anyway. - if (!/^https?:\/\//i.test(link)) return true; - if (Date.now() - started > DEADLINE_MS) { - capped ??= "time"; - return true; - } - if (pagesCrawled >= MAX_PAGES) { - capped ??= "pages"; - return true; +/** + * Run the crawl in a child process. Never throws: a child that dies, times out + * or emits garbage becomes { ok: false } for the caller to report. + */ +async function runCrawlChild( + targetUrl: string, + perLinkTimeoutMs?: number, +): Promise { + return new Promise((resolve) => { + let child: ReturnType; + try { + const entry = childEntryPath(); + const args = [targetUrl]; + if (perLinkTimeoutMs) args.push(String(perLinkTimeoutMs)); + child = fork(entry, args, { + execArgv: childExecArgv(entry), + stdio: ["ignore", "pipe", "pipe", "ipc"], + }); + } catch (err) { + resolve({ ok: false, error: `could not start crawl process: ${messageOf(err)}` }); + return; } - if (linksChecked >= MAX_LINKS) { - capped ??= "links"; - return true; - } - return false; - }; - let result: { links: LinkResult[]; passed: boolean }; - try { - result = await checker.check({ - path: root, - recurse: true, - concurrency: CONCURRENCY, - timeout: PER_LINK_TIMEOUT_MS, - userAgent: UA, - retry: true, - linksToSkip, + let stdout = ""; + let settled = false; + const done = (outcome: ChildOutcome) => { + if (settled) return; + settled = true; + clearTimeout(killTimer); + resolve(outcome); + }; + + const killTimer = setTimeout(() => { + child.kill("SIGKILL"); + done({ + ok: false, + error: `crawl process exceeded ${Math.round(CHILD_KILL_AFTER_MS / 60_000)} minutes and was killed`, + }); + }, CHILD_KILL_AFTER_MS); + + child.stdout?.on("data", (chunk: Buffer) => { + stdout += chunk.toString(); + }); + // Surface the child's diagnostics in the worker log rather than dropping them. + child.stderr?.on("data", (chunk: Buffer) => { + process.stderr.write(chunk); }); - } catch (err) { + child.on("error", (err) => { + done({ ok: false, error: `crawl process error: ${messageOf(err)}` }); + }); + child.on("close", (code, signal) => { + if (!stdout.trim()) { + done({ + ok: false, + error: `crawl process exited without a report (code=${code ?? "null"}, signal=${signal ?? "none"})`, + }); + return; + } + try { + const report = JSON.parse(stdout) as ChildReport; + done({ ok: true, acc: report.acc, crashed: report.crashed ?? null }); + } catch (err) { + done({ ok: false, error: `unreadable crawl report: ${messageOf(err)}` }); + } + }); + }); +} + +function messageOf(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +export async function linksAudit( + targetUrl: string, + // Test seam: shortens the per-link abort so the mid-body race is deterministic. + opts: { perLinkTimeoutMs?: number } = {}, +): Promise { + const started = Date.now(); + const root = rootOf(targetUrl); + + const outcome = await runCrawlChild(targetUrl, opts.perLinkTimeoutMs); + + if (!outcome.ok) { const findings: Finding[] = [ { section: "Links & Images", check_key: "links.crawl_error", status: "fail", - title: "Link crawl could not start", - detail: `linkinator failed to crawl ${root}: ${ - err instanceof Error ? err.message : String(err) - }`, + title: "Link crawl could not complete", + detail: `linkinator failed to crawl ${root}: ${outcome.error}`, priority: 1, }, ]; return { score: scoreFindings(findings), findings, - markdown: `# Link Checker — ${root}\n\nThe crawl could not be started: ${ - err instanceof Error ? err.message : String(err) - }\n`, + markdown: `# Link Checker — ${root}\n\nThe crawl could not be completed: ${outcome.error}\n`, summary: { pagesCrawled: 0, pass: 0, @@ -109,34 +167,33 @@ export async function linksAudit(targetUrl: string): Promise { }; } - const checked = result.links.filter((l) => l.state !== LinkState.SKIPPED); - linksChecked = checked.length; - const broken = result.links.filter((l) => l.state === LinkState.BROKEN); - const skipped = result.links.filter((l) => l.state === LinkState.SKIPPED); + const acc: CrawlAccumulator = { + pagesCrawled: outcome.acc?.pagesCrawled ?? 0, + capped: outcome.acc?.capped ?? null, + links: outcome.acc?.links ?? [], + }; + const checked = checkedCount(acc); + const broken = acc.links.filter((l) => l.state === LinkState.BROKEN); + const skippedCount = acc.links.filter((l) => l.state === LinkState.SKIPPED).length; - const findings = buildFindings({ + const agg: Agg = { root, - pagesCrawled, - checked: checked.length, + pagesCrawled: acc.pagesCrawled, + checked, broken, - skippedCount: skipped.length, - capped, - }); + skippedCount, + capped: acc.capped, + crashed: outcome.crashed, + }; + + const findings = buildFindings(agg); return { score: scoreFindings(findings), findings, - markdown: buildMarkdown({ - root, - pagesCrawled, - checked: checked.length, - broken, - skippedCount: skipped.length, - capped, - durationMs: Date.now() - started, - }), + markdown: buildMarkdown({ ...agg, durationMs: Date.now() - started }), summary: { - pagesCrawled, + pagesCrawled: acc.pagesCrawled, pass: findings.filter((f) => f.status === "pass").length, warn: findings.filter((f) => f.status === "warn").length, fail: findings.filter((f) => f.status === "fail").length, @@ -147,7 +204,7 @@ export async function linksAudit(targetUrl: string): Promise { }; } -function statusLabel(l: LinkResult): string { +function statusLabel(l: CrawlLink): string { if (l.status && l.status > 0) return String(l.status); return "no response"; } @@ -156,13 +213,14 @@ type Agg = { root: string; pagesCrawled: number; checked: number; - broken: LinkResult[]; + broken: CrawlLink[]; skippedCount: number; - capped: null | "pages" | "links" | "time"; + capped: Capped; + crashed: string | null; }; function buildFindings(agg: Agg): Finding[] { - const { root, pagesCrawled, checked, broken, skippedCount, capped } = agg; + const { root, pagesCrawled, checked, broken, skippedCount, capped, crashed } = agg; const out: Finding[] = []; // Broken-link finding — the headline of a link checker. @@ -208,20 +266,45 @@ function buildFindings(agg: Agg): Finding[] { // Coverage summary — gives the reader confidence in the broken-link number // and flags when the crawl was budget-capped (so "0 broken" isn't read as a // clean bill of health for a site we only partially swept). + const partial = capped !== null || crashed !== null; out.push({ section: "Links & Images", check_key: "links.crawl_coverage", - status: capped ? "warn" : "pass", + status: partial ? "warn" : "pass", title: capped ? `Crawl capped at ${pagesCrawled} pages (${cappedReason(capped)})` - : `Crawled ${pagesCrawled} page${pagesCrawled === 1 ? "" : "s"}, ${checked} link${checked === 1 ? "" : "s"}`, + : crashed + ? `Partial crawl — ${pagesCrawled} page${pagesCrawled === 1 ? "" : "s"} before the crawler stopped` + : `Crawled ${pagesCrawled} page${pagesCrawled === 1 ? "" : "s"}, ${checked} link${checked === 1 ? "" : "s"}`, detail: capped ? `The recursive crawl hit the ${cappedReason(capped)} budget and stopped early, so links beyond that point were not checked. Re-run on a narrower section, or treat this as a partial sweep.` - : `Full recursive sweep of ${root}: ${pagesCrawled} page${pagesCrawled === 1 ? "" : "s"} crawled, ${checked} link${checked === 1 ? "" : "s"} checked${skippedCount ? `, ${skippedCount} skipped` : ""}.`, - evidence: { capped: capped ?? false, pagesCrawled, linksChecked: checked, skipped: skippedCount }, + : crashed + ? `The crawl of ${root} ended before completing, so this covers only ${pagesCrawled} page${pagesCrawled === 1 ? "" : "s"} and ${checked} link${checked === 1 ? "" : "s"}. Treat it as a partial sweep.` + : `Full recursive sweep of ${root}: ${pagesCrawled} page${pagesCrawled === 1 ? "" : "s"} crawled, ${checked} link${checked === 1 ? "" : "s"} checked${skippedCount ? `, ${skippedCount} skipped` : ""}.`, + evidence: { + capped: capped ?? false, + incomplete: crashed !== null, + pagesCrawled, + linksChecked: checked, + skipped: skippedCount, + }, priority: 5, }); + // The crawler died partway through. We still report what it found, but the + // reader needs to know the sweep is incomplete. + if (crashed) { + out.push({ + section: "Links & Images", + check_key: "links.crawl_incomplete", + status: "warn", + title: "Link crawl ended early", + detail: `The crawler stopped before finishing (${crashed}). The ${pagesCrawled} page${pagesCrawled === 1 ? "" : "s"} and ${checked} link${checked === 1 ? "" : "s"} below were checked; anything past that point was not. Re-run to sweep the rest.`, + evidence: { reason: crashed, pagesCrawled, linksChecked: checked }, + priority: 3, + }); + } + return out; } @@ -234,7 +317,7 @@ function cappedReason(capped: "pages" | "links" | "time"): string { } function buildMarkdown(agg: Agg & { durationMs: number }): string { - const { root, pagesCrawled, checked, broken, skippedCount, capped, durationMs } = agg; + const { root, pagesCrawled, checked, broken, skippedCount, capped, crashed, durationMs } = agg; const lines: string[] = []; lines.push(`# Link Checker — ${root}`); lines.push(""); @@ -249,9 +332,16 @@ function buildMarkdown(agg: Agg & { durationMs: number }): string { lines.push(`| Broken links | ${broken.length} |`); if (skippedCount) lines.push(`| Skipped | ${skippedCount} |`); lines.push(`| Duration | ${(durationMs / 1000).toFixed(1)}s |`); - lines.push(`| Coverage | ${capped ? `partial (${cappedReason(capped)} cap)` : "full sweep"} |`); + lines.push( + `| Coverage | ${crashed ? "partial (crawl ended early)" : capped ? `partial (${cappedReason(capped)} cap)` : "full sweep"} |`, + ); lines.push(""); + if (crashed) { + lines.push(`> ⚠️ The crawl ended early (${crashed}), so this is a partial sweep.`); + lines.push(""); + } + if (broken.length === 0) { lines.push(`✅ No broken links found.`); } else { diff --git a/tests/links-engine-crash-isolation.test.ts b/tests/links-engine-crash-isolation.test.ts new file mode 100644 index 0000000..7c5fe6d --- /dev/null +++ b/tests/links-engine-crash-isolation.test.ts @@ -0,0 +1,101 @@ +// Regression: a slow response body used to kill the whole worker. +// +// linkinator applies its per-link `timeout` as an AbortSignal.timeout on the +// fetch, wraps the body with Readable.fromWeb(), then pipes it to the HTML +// parser with the 'error' handler on the *destination* only +// (linkinator/build/src/links.js:158). pipe() does not forward source errors, so +// when the abort fires mid-body the source Readable emits an unhandled 'error' +// event and Node hard-exits — uncatchable from linksAudit(). Because start.sh +// supervises the worker and Next.js with `wait -n`, that killed the container +// and every in-flight audit with it: scan run c6c19e9b lost 13 audits to +// "Engine timed out" that had never run. +// +// The crawl now runs in a forked child, so these tests assert the failure is +// contained and reported instead of fatal. + +import { describe, expect, it, afterEach } from "vitest"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; +import { linksAudit } from "@/lib/audit/links-engine"; + +const servers: http.Server[] = []; + +afterEach(async () => { + await Promise.all( + servers.splice(0).map( + (s) => + new Promise((resolve) => { + s.closeAllConnections?.(); + s.close(() => resolve()); + }), + ), + ); +}); + +async function serve(handler: http.RequestListener): Promise { + const server = http.createServer(handler); + servers.push(server); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + return `http://127.0.0.1:${port}/`; +} + +describe("links engine crash isolation", () => { + it("survives a body that stalls past the per-link timeout", async () => { + // Headers and a first chunk arrive immediately, then the body stalls — the + // abort therefore fires mid-stream, which is the fatal case. + const base = await serve((_req, res) => { + res.writeHead(200, { "content-type": "text/html" }); + res.write('second'); + // never end + }); + + const result = await linksAudit(base, { perLinkTimeoutMs: 700 }); + + // Reaching this line at all is the regression check: before the fix the + // test process died here with "Unhandled 'error' event". + expect(result).toBeTruthy(); + expect(typeof result.score).toBe("number"); + // The stall is reported, not silently swallowed. + const keys = result.findings.map((f) => f.check_key); + expect( + keys.includes("links.crawl_incomplete") || keys.includes("links.crawl_error"), + ).toBe(true); + expect(result.markdown).toContain("Link Checker"); + // A partial sweep must never be reported as a full one. + const coverage = result.findings.find((f) => f.check_key === "links.crawl_coverage"); + if (coverage) { + expect(coverage.status).toBe("warn"); + expect(coverage.detail).not.toContain("Full recursive sweep"); + } + }, 30_000); + + it("still reports normally on a healthy site", async () => { + const base = await serve((req, res) => { + if (req.url === "/missing") { + res.writeHead(404).end("nope"); + return; + } + res.writeHead(200, { "content-type": "text/html" }); + res.end('broken'); + }); + + const result = await linksAudit(base); + + const broken = result.findings.find((f) => f.check_key === "links.crawl_broken"); + expect(broken).toBeTruthy(); + expect(broken?.status).toBe("warn"); // exactly one broken link + expect(broken?.evidence?.broken).toBe(1); + // Crawl completed, so neither failure finding should be present. + expect(result.findings.map((f) => f.check_key)).not.toContain("links.crawl_incomplete"); + expect(result.findings.map((f) => f.check_key)).not.toContain("links.crawl_error"); + }, 30_000); + + it("reports a fail finding when the crawl process cannot produce a report", async () => { + // An unroutable target: the child still exits cleanly with a report rather + // than taking the parent with it. + const result = await linksAudit("http://127.0.0.1:1/", { perLinkTimeoutMs: 700 }); + expect(result).toBeTruthy(); + expect(result.findings.length).toBeGreaterThan(0); + }, 30_000); +}); diff --git a/worker/index.ts b/worker/index.ts index 8526bf4..6033172 100644 --- a/worker/index.ts +++ b/worker/index.ts @@ -1186,6 +1186,64 @@ async function sweep() { for (const row of data) await processJob({ auditId: row.id }); } +// Boot-time crash recovery. A worker crash (or any restart) strands every +// in-flight audit in 'running': sweep() only looks at 'queued', so nothing +// retries them and auditStuckSweep() fails + refunds them minutes later. That +// is how one crashing engine turns a 15-engine run into 13 audits all reporting +// "Engine timed out" — they never ran at all. +// +// On boot nothing is genuinely in flight, so re-dispatch anything still inside +// its stuck-timeout budget. Rows past their budget are left to auditStuckSweep, +// which also bounds a crash loop: its cutoff is measured from created_at, so a +// repeatedly-crashing audit stops being retried once its original window closes. +async function recoverOrphanedAudits() { + const widestBudgetMs = Math.max( + auditStuckAfterMs(null), + auditStuckAfterMs("claude"), + auditStuckAfterMs("perplexity"), + ); + const { data, error } = await supabase + .from("audits") + .select("id, engine, pdf_email, created_at") + .eq("status", "running") + .is("aborted_at", null) + .gt("created_at", new Date(Date.now() - widestBudgetMs).toISOString()); + if (error) { + console.warn("[worker] orphan recovery", error.message); + return; + } + const rows = (data ?? []).filter( + (row) => + Date.now() - new Date(row.created_at as string).getTime() < + auditStuckAfterMs(row.engine as string | null), + ); + if (rows.length === 0) return; + console.log(`[worker] recovering ${rows.length} audit(s) orphaned by a restart`); + + for (const row of rows) { + // Back to queued first, so a concurrent sweep can't double-process it. + const { data: requeued } = await supabase + .from("audits") + .update({ status: "queued" }) + .eq("id", row.id) + .eq("status", "running") + .is("aborted_at", null) + .select("id") + .maybeSingle(); + if (!requeued) continue; + // A run that died between the findings insert and the status write would + // otherwise duplicate its findings on retry. + await supabase.from("audit_findings").delete().eq("audit_id", row.id); + // Re-dispatch now rather than waiting for sweep(), which is serial and + // capped at 5 — a full multi-engine batch would not be reached before the + // very budget we're racing runs out. + processJob({ + auditId: row.id as string, + pdfEmail: (row.pdf_email as string | null) ?? undefined, + }).catch((e) => console.error("[worker] orphan recovery", row.id, e)); + } +} + // Recover audits orphaned by a worker crash or by an upstream engine that // opened a connection then never delivered. Keep the default short for local // and OpenAI-compatible engines, but give Claude room to finish legitimate @@ -1386,6 +1444,7 @@ setInterval( const bindHost = process.env.WORKER_BIND ?? "127.0.0.1"; server.listen(port, bindHost, () => { console.log(`[worker] listening on ${bindHost}:${port}`); + recoverOrphanedAudits().catch((e) => console.error("[worker] orphan recovery", e)); sweep().catch(() => {}); socialFeedSweep().catch((e) => console.error("[worker] social feed sweep", e)); promoteSweep().catch((e) => console.error("[worker] promote sweep", e));