diff --git a/src/server/lib/audit/crawl-throttle.test.ts b/src/server/lib/audit/crawl-throttle.test.ts new file mode 100644 index 0000000..120ea39 --- /dev/null +++ b/src/server/lib/audit/crawl-throttle.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; +import { + backOff, + IDLE_THROTTLE, + parseRetryAfterMs, +} from "@/server/lib/audit/crawl-throttle"; + +describe("parseRetryAfterMs", () => { + it("reads delay-seconds and HTTP-dates, ignores garbage", () => { + expect(parseRetryAfterMs("5")).toBe(5_000); + const inTenSeconds = new Date(Date.now() + 10_000).toUTCString(); + const ms = parseRetryAfterMs(inTenSeconds); + expect(ms).toBeGreaterThan(8_000); + expect(ms).toBeLessThanOrEqual(10_000); + expect(parseRetryAfterMs("soon")).toBeUndefined(); + expect(parseRetryAfterMs(null)).toBeUndefined(); + }); +}); + +describe("backOff", () => { + it("pauses for Retry-After within bounds and starts spacing launches", () => { + expect(backOff(IDLE_THROTTLE, 5_000, 1_000)).toEqual({ + pausedUntil: 6_000, + launchGapMs: 250, + }); + expect(backOff(IDLE_THROTTLE, undefined, 1_000).pausedUntil).toBe(3_000); + expect(backOff(IDLE_THROTTLE, 0, 1_000).pausedUntil).toBe(2_000); + expect(backOff(IDLE_THROTTLE, 3_600_000, 1_000).pausedUntil).toBe(21_000); + }); + + it("treats a burst during one pause as one signal, then doubles per episode", () => { + const first = backOff(IDLE_THROTTLE, 2_000, 1_000); + const burst = backOff(first, 2_000, 1_500); + expect(burst).toEqual({ pausedUntil: 3_500, launchGapMs: 250 }); + + let throttle = burst; + for (const now of [10_000, 20_000, 30_000, 40_000, 50_000]) { + throttle = backOff(throttle, 2_000, now); + } + expect(throttle.launchGapMs).toBe(4_000); + }); +}); diff --git a/src/server/lib/audit/crawl-throttle.ts b/src/server/lib/audit/crawl-throttle.ts new file mode 100644 index 0000000..056cf75 --- /dev/null +++ b/src/server/lib/audit/crawl-throttle.ts @@ -0,0 +1,58 @@ +/** + * Site-wide backoff for rate limiting. A 429 means "too fast", not "go + * away": the whole crawl pauses for the server's Retry-After (or a default), + * every launch after that is spaced out, and the URL is retried a bounded + * number of times before it is recorded as blocked. + */ +export interface CrawlThrottle { + /** No fetch launches before this time (ms epoch). */ + pausedUntil: number; + /** Minimum spacing between launches; 0 until the site rate-limits us. */ + launchGapMs: number; +} + +/** Retries per URL within one chunk before a 429 is recorded as blocked. */ +export const MAX_RATE_LIMIT_RETRIES = 3; +const DEFAULT_PAUSE_MS = 2_000; +const MIN_PAUSE_MS = 1_000; +/** Longer Retry-After values are cut short: the chunk has a 90s deadline. */ +const MAX_PAUSE_MS = 20_000; +const MIN_LAUNCH_GAP_MS = 250; +const MAX_LAUNCH_GAP_MS = 4_000; + +export const IDLE_THROTTLE: CrawlThrottle = { pausedUntil: 0, launchGapMs: 0 }; + +/** `Retry-After` is either delay-seconds or an HTTP-date. */ +export function parseRetryAfterMs(header: string | null): number | undefined { + if (!header) return undefined; + const value = header.trim(); + if (/^\d+$/.test(value)) return Number(value) * 1000; + const at = Date.parse(value); + return Number.isNaN(at) ? undefined : Math.max(0, at - Date.now()); +} + +/** + * Register a 429. The concurrent fetches of one window tend to fail + * together, so a burst arriving while already paused counts as one signal: + * it can extend the pause but does not compound the launch gap. + */ +export function backOff( + throttle: CrawlThrottle, + retryAfterMs: number | undefined, + now = Date.now(), +): CrawlThrottle { + const pause = Math.min( + Math.max(retryAfterMs ?? DEFAULT_PAUSE_MS, MIN_PAUSE_MS), + MAX_PAUSE_MS, + ); + const alreadyPaused = now < throttle.pausedUntil; + return { + pausedUntil: Math.max(throttle.pausedUntil, now + pause), + launchGapMs: alreadyPaused + ? throttle.launchGapMs + : Math.min( + Math.max(throttle.launchGapMs * 2, MIN_LAUNCH_GAP_MS), + MAX_LAUNCH_GAP_MS, + ), + }; +} diff --git a/src/server/lib/audit/types.ts b/src/server/lib/audit/types.ts index 2fbe499..253617a 100644 --- a/src/server/lib/audit/types.ts +++ b/src/server/lib/audit/types.ts @@ -146,6 +146,11 @@ export interface CrawledPageResult { * response time is measured at headers and says nothing about body size. */ htmlBytes: number; + /** + * Server-requested wait before retrying, from a 429's Retry-After header. + * Only set on 429 responses. Transient — drives the crawl throttle. + */ + retryAfterMs?: number; imagesTotal: number; imagesMissingAlt: number; images: Array<{ src: string | null; alt: string | null }>; diff --git a/src/server/workflows/site-audit-workflow-helpers.ts b/src/server/workflows/site-audit-workflow-helpers.ts index a03463f..d18bcb9 100644 --- a/src/server/workflows/site-audit-workflow-helpers.ts +++ b/src/server/workflows/site-audit-workflow-helpers.ts @@ -4,6 +4,7 @@ import type { } from "@/server/lib/audit/types"; import { sha256Hex } from "@/server/lib/audit/ids"; import { normalizeUrl } from "@/server/lib/audit/url-utils"; +import { parseRetryAfterMs } from "@/server/lib/audit/crawl-throttle"; const CRAWL_USER_AGENT = "OpenSEO-Audit/1.0"; const MAX_HTML_BYTES = 1024 * 1024; @@ -127,6 +128,10 @@ export async function crawlPage( // The body was still fetched and buffered; report its size so the // crawl window's byte budget sees blocked/error pages too. htmlBytes: body.length, + retryAfterMs: + statusCode === 429 + ? parseRetryAfterMs(response.headers.get("retry-after")) + : undefined, }); } @@ -248,6 +253,7 @@ function emptyPageResult(input: { crawlDepth: number | null; inSitemap: boolean; htmlBytes?: number; + retryAfterMs?: number; }): CrawledPageResult { return { id: crypto.randomUUID(), @@ -275,6 +281,7 @@ function emptyPageResult(input: { contentHash: null, isHtml: false, htmlBytes: input.htmlBytes ?? 0, + retryAfterMs: input.retryAfterMs, imagesTotal: 0, imagesMissingAlt: 0, images: [], diff --git a/src/server/workflows/siteAuditWorkflowCrawl.ts b/src/server/workflows/siteAuditWorkflowCrawl.ts index b090955..ac16017 100644 --- a/src/server/workflows/siteAuditWorkflowCrawl.ts +++ b/src/server/workflows/siteAuditWorkflowCrawl.ts @@ -19,6 +19,12 @@ import { CRAWL_WINDOW, RETRY_CRAWL_WINDOW, } from "@/server/lib/audit/crawl-window"; +import { + backOff, + IDLE_THROTTLE, + MAX_RATE_LIMIT_RETRIES, + type CrawlThrottle, +} from "@/server/lib/audit/crawl-throttle"; import { crawlPage } from "@/server/workflows/site-audit-workflow-helpers"; import { pgStep } from "@/server/workflows/pgStep"; import { CRAWL_CHUNK_STEP } from "@/server/workflows/auditStepConfigs"; @@ -106,6 +112,9 @@ export async function runCrawlPhase( // the site's page weight the hard way — on heavy-page sites that meant an // exceededMemory death every ~200 pages. let windowHint = CRAWL_WINDOW.initial; + // Same for the launch gap a rate-limiting site taught us: relearning it + // per chunk would cost a fresh round of 429s every 200 pages. + let launchGapHint = IDLE_THROTTLE.launchGapMs; while (pending > 0 && attemptedTotal < params.maxPages) { chunkNo += 1; @@ -119,6 +128,7 @@ export async function runCrawlPhase( chunkNo, attemptedBefore: attemptedTotal, startWindow: windowHint, + startLaunchGapMs: launchGapHint, }), ); // Apply the chunk's counters even when it did no new work (a retried @@ -129,6 +139,7 @@ export async function runCrawlPhase( // `?? initial`: an instance in flight across a deploy replays cached // step results from before endWindow existed. windowHint = result.endWindow ?? CRAWL_WINDOW.initial; + launchGapHint = result.endLaunchGapMs ?? IDLE_THROTTLE.launchGapMs; // One zero-attempt chunk is normal (retry of a completed chunk number); // two in a row means the frontier is unservable — stop with what we // have instead of spinning forever. @@ -145,12 +156,14 @@ async function runCrawlChunk( chunkNo: number; attemptedBefore: number; startWindow: number; + startLaunchGapMs: number; }, ): Promise<{ attemptedInChunk: number; attempted: number; pending: number; endWindow: number; + endLaunchGapMs: number; }> { const { auditId, workflowInstanceId, origin, maxPages, robots, chunkNo } = input; @@ -171,6 +184,7 @@ async function runCrawlChunk( attempted: stats.attempted, pending: stats.pending, endWindow: input.startWindow, + endLaunchGapMs: input.startLaunchGapMs, }; } @@ -187,6 +201,15 @@ async function runCrawlChunk( let nextIndex = 0; let attemptedInChunk = 0; const inFlight = new Set>(); + // 429s are the site saying "slower", not "blocked": the URL goes back on + // the queue, the whole window pauses, and launches are spaced out. + let throttle: CrawlThrottle = { + ...IDLE_THROTTLE, + launchGapMs: input.startLaunchGapMs, + }; + let nextLaunchAt = 0; + const retryQueue: ClaimedUrl[] = []; + const retriesByUrl = new Map(); let persistThreshold = FIRST_PERSIST_BATCH_SIZE; let batch: CrawledPageResult[] = []; // Persistence runs concurrently with fetching (pipelined) but sequentially @@ -222,6 +245,15 @@ async function runCrawlChunk( const launch = (entry: ClaimedUrl) => { const promise = crawlPage(entry.url, entry.depth, entry.inSitemap) .then((page) => { + if (page.statusCode === 429) { + throttle = backOff(throttle, page.retryAfterMs); + const retries = retriesByUrl.get(entry.url) ?? 0; + if (retries < MAX_RATE_LIMIT_RETRIES) { + retriesByUrl.set(entry.url, retries + 1); + retryQueue.push(entry); + return; + } + } attemptedInChunk += 1; batch.push(page); if (batch.length >= persistThreshold) flush(); @@ -232,18 +264,26 @@ async function runCrawlChunk( inFlight.add(promise); }; + const hasNext = () => retryQueue.length > 0 || nextIndex < claimed.length; + while (true) { - while ( - inFlight.size < windowSize && - nextIndex < claimed.length && - Date.now() < deadlineAt - ) { - // queuedPersists changes when persistChain settles. Keep it out of the - // loop condition because the type-aware linter cannot see that async - // mutation and flags the otherwise valid backpressure check. - if (queuedPersists > MAX_QUEUED_PERSIST_BATCHES) break; - launch(claimed[nextIndex]); - nextIndex += 1; + const now = Date.now(); + const canLaunch = + hasNext() && + now < deadlineAt && + queuedPersists <= MAX_QUEUED_PERSIST_BATCHES; + if (canLaunch && inFlight.size < windowSize) { + const waitMs = Math.min( + Math.max(throttle.pausedUntil, nextLaunchAt) - now, + deadlineAt - now, + ); + if (waitMs > 0) { + await sleep(waitMs); + continue; + } + launch(retryQueue.shift() ?? claimed[nextIndex++]); + nextLaunchAt = Date.now() + throttle.launchGapMs; + continue; } if (inFlight.size > 0) { await Promise.race(inFlight); @@ -254,8 +294,8 @@ async function runCrawlChunk( // chunk is done (leases exhausted or soft deadline hit). if ( queuedPersists > MAX_QUEUED_PERSIST_BATCHES && - nextIndex < claimed.length && - Date.now() < deadlineAt + hasNext() && + now < deadlineAt ) { await persistChain; continue; @@ -265,8 +305,12 @@ async function runCrawlChunk( flush(); await persistChain; - // Leases we never launched (soft deadline) go back to the queue. - const unattempted = claimed.slice(nextIndex).map((entry) => entry.url); + // Leases we never launched or finished retrying (soft deadline) go back + // to the queue; the next chunk retries them with a fresh budget. + const unattempted = [ + ...retryQueue.map((entry) => entry.url), + ...claimed.slice(nextIndex).map((entry) => entry.url), + ]; if (unattempted.length > 0) { await scratchpad.releaseUrls(unattempted); } @@ -280,9 +324,14 @@ async function runCrawlChunk( attempted: stats.attempted, pending: stats.pending, endWindow: windowSize, + endLaunchGapMs: throttle.launchGapMs, }; } +function sleep(ms: number) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + async function persistCrawledPages(input: { auditId: string; workflowInstanceId: string; diff --git a/src/shared/audit-issues.ts b/src/shared/audit-issues.ts index baddfa7..8b306a6 100644 --- a/src/shared/audit-issues.ts +++ b/src/shared/audit-issues.ts @@ -20,7 +20,7 @@ export const AUDIT_ISSUE_TYPES = { severity: "critical", title: "Crawler was blocked", explanation: - "The site returned a bot challenge or access denial (e.g. a Cloudflare challenge, 403, or 429) instead of the page. We report this honestly rather than pretending the page is broken — but it means this page could not be audited, and other crawlers like search engines may face similar friction.", + "The site returned a bot challenge or access denial (e.g. a Cloudflare challenge, 403, or a 429 that persisted after we slowed down and retried) instead of the page. We report this honestly rather than pretending the page is broken — but it means this page could not be audited, and other crawlers like search engines may face similar friction.", howToFix: 'If you own this site, allowlist the "OpenSEO-Audit" user agent in your WAF/bot-protection settings (on Cloudflare: a WAF custom rule that skips bot protection when the user agent contains "OpenSEO-Audit"; on some free tiers you may need to relax bot protection). Then re-run the audit.', },