fix(audit): isolate the link crawl so linkinator can't kill the worker - #198
Merged
Merged
Conversation
A slow response body took down the whole container and every in-flight audit with it. 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 attached to 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. That is not catchable from linksAudit()'s try/catch, and because start.sh supervises the worker and Next.js with `wait -n`, the container restarted. Scan run c6c19e9b lost 13 of 15 audits this way: only spec (0.6s) and dns (0.7s) finished before the crash, and the remaining 13 sat in 'running' until auditStuckSweep failed and refunded them, reporting "Engine timed out (no response in 7 minutes)" for engines that never made a request. Every recent multi-engine run shows the same signature — links failed means the batch died; links completed means the batch was fine. - lib/audit/links-crawl.ts: the raw crawl plus its budgets, filling an accumulator as it goes so a crash partway through keeps its results. - lib/audit/links-crawl-child.ts: forked entry that catches the uncaught stream error, emits the partial crawl as JSON and exits 0. - lib/audit/links-engine.ts: forks the child and builds findings from its report; a child that dies, wedges or emits garbage becomes a finding instead of a dead worker. Reports a partial sweep honestly rather than claiming full coverage. - worker/index.ts: recover audits orphaned by a restart on boot. sweep() only looks at 'queued', so a crash previously guaranteed their loss. Re-dispatch is bounded by the existing created_at stuck cutoff, so a crash loop cannot retry forever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ThreatCrush Security Scan35 finding(s) HIGH/CRITICAL: 3 | MEDIUM: 23 | LOW: 9
Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Scan run
c6c19e9breported 13 of 15 audits asEngine timed out (no response in 7 minutes). None of them timed out — none of them ran. The worker process was hard-killed about one second in.From the Railway logs for
crawlproof.com, right afterspec(0.6s) anddns(0.7s) completed:The
linksengine is the source. linkinator applies its per-linktimeoutas anAbortSignal.timeouton the fetch, wraps the body withReadable.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 10s abort fires mid-body the source Readable emits an unhandled'error'event and Node hard-exits. Not catchable fromlinksAudit()'stry/catch. And becausestart.shsupervises the worker and Next.js withwait -n, the crash took the container down along with every in-flight audit.Those 13 then sat in
running—sweep()only picks upqueued, so nothing retried them — untilauditStuckSweep()failed and refunded them minutes later. The message was the reaper's guess, not an engine report.The correlation holds across every recent multi-engine run:
linksIt's a race on body timing, which is why the same 15-engine fan-out succeeded on Aug 16 00:00.
What changed
lib/audit/links-crawl.ts— the raw crawl and its budgets, extracted. Fills an accumulator from linkinator'slink/pagestartevents instead of readingcheck()'s return value, so a crash partway through still leaves usable results.lib/audit/links-crawl-child.ts— forked entry. An unhandled'error'event arrives here as an uncaught exception, where it is genuinely recoverable: salvage the partial crawl, print it as JSON, exit 0.lib/audit/links-engine.ts— forks the child and builds findings from its report. A child that dies, wedges past its budget, or emits garbage becomes a finding rather than a dead worker. A partial sweep is now reported as partial instead of claiming full coverage.worker/index.ts—recoverOrphanedAudits()on boot. Any crash previously guaranteed the loss of every in-flight audit; now they are re-dispatched if still inside their stuck-timeout budget. Bounded by the existingcreated_atcutoff, so a crash loop cannot retry forever.fork()inheritsexecArgv, so tsx's loader carries into the child understart.sh;childExecArgv()adds it explicitly for runtimes that transform TypeScript another way (vitest).Verification
tests/links-engine-crash-isolation.test.tsserves a body that stalls mid-stream — the exact fatal case. Reaching the assertions at all is the regression check.exit 1and the identicalUnhandled 'error' eventstack.npx tsx, asstart.shruns it: worker survives, audit completes with score 60 and alinks.crawl_incomplete=warnfinding.npm run lintis broken at HEAD —next lintwas removed in Next 16 — and CI does not run it.)Follow-ups, not in this PR
source.on('error')is worth sending upstream to linkinator.start.sh'swait -nmeans any worker crash also restarts Next.js. Supervising them independently would shrink the blast radius further.claudeengine 400s until 2026-09-01 on the Anthropic org spend cap, anddns-enginelogsAI analysis failed; using baseline.🤖 Generated with Claude Code