From 0a1035c4226da797beb30f521f291febcf095f29 Mon Sep 17 00:00:00 2001 From: harryautomazione Date: Tue, 16 Jun 2026 12:24:50 +0200 Subject: [PATCH 01/32] feat: implement per-domain request throttling (ThrottlingRequestManager) (cherry picked from commit 5422b402311c166d3c0b41de666ae8f407a919e7) --- .../src/internals/basic-crawler.ts | 34 ++ .../src/internals/browser-crawler.ts | 30 + packages/core/src/storages/index.ts | 1 + .../src/storages/request_manager_tandem.ts | 21 + .../storages/throttling_request_manager.ts | 526 ++++++++++++++++++ .../src/internals/http-crawler.ts | 18 +- packages/utils/src/internals/robots.ts | 13 +- test/core/crawlers/http_crawler.test.ts | 45 +- .../throttling_request_manager.test.ts | 128 +++++ 9 files changed, 813 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/storages/throttling_request_manager.ts create mode 100644 test/core/storages/throttling_request_manager.test.ts diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index e59f4c910551..88959ca9f351 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -59,6 +59,7 @@ import { RequestHandlerError, RequestManagerTandem, RequestQueue, + ThrottlingRequestManager, RequestState, RetryRequestError, Router, @@ -795,6 +796,7 @@ export class BasicCrawler< requestList: ow.optional.object.validate(validators.requestList), requestQueue: ow.optional.object.validate(validators.requestQueue), + requestManager: ow.optional.object, // Subclasses override this function instead of passing it // in constructor, so this validation needs to apply only // if the user creates an instance of BasicCrawler directly. @@ -2039,6 +2041,25 @@ export class BasicCrawler< }); await this.getRequestManager(); + + if (this.respectRobotsTxtFile) { + let isThrottling = false; + let currentManager = this.requestManager; + if (currentManager instanceof RequestManagerTandem) { + currentManager = (currentManager as any).resolvedRequestManager; + } + if (currentManager instanceof ThrottlingRequestManager) { + isThrottling = true; + } + + if (!isThrottling) { + this.log.warning( + 'The `respectRobotsTxtFile` option is enabled, but the crawler is not using a `ThrottlingRequestManager`. ' + + 'Crawl delays defined in robots.txt will NOT be respected. ' + + 'To respect crawl delays, wrap your request queue in a `ThrottlingRequestManager`.', + ); + } + } } /** @@ -2123,6 +2144,19 @@ export class BasicCrawler< const robotsTxtFile = await this.getRobotsTxtFileForUrl(url); const userAgent = typeof this.respectRobotsTxtFile === 'object' ? this.respectRobotsTxtFile?.userAgent : '*'; + if (robotsTxtFile) { + if ( + this.requestManager && + 'setCrawlDelay' in this.requestManager && + typeof (this.requestManager as any).setCrawlDelay === 'function' + ) { + const crawlDelay = robotsTxtFile.getCrawlDelay(userAgent); + if (crawlDelay !== undefined) { + (this.requestManager as any).setCrawlDelay(url, crawlDelay); + } + } + } + return !robotsTxtFile || robotsTxtFile.isAllowed(url, userAgent); } diff --git a/packages/browser-crawler/src/internals/browser-crawler.ts b/packages/browser-crawler/src/internals/browser-crawler.ts index dc42b20ddd53..2c126a263f85 100644 --- a/packages/browser-crawler/src/internals/browser-crawler.ts +++ b/packages/browser-crawler/src/internals/browser-crawler.ts @@ -28,6 +28,7 @@ import { toughCookieToBrowserPoolCookie, tryAbsoluteURL, validators, + parseRetryAfterHeader, } from '@crawlee/basic'; import type { BrowserController, @@ -844,6 +845,35 @@ export abstract class BrowserCrawler< throw new Error(`${status} - Internal Server Error`); } + + if (status === 429) { + const headers = typeof (response as any).headers === 'function' ? (response as any).headers() : {}; + let retryAfterHeader: string | undefined; + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === 'retry-after') { + retryAfterHeader = headers[key]; + break; + } + } + const retryAfterStr = Array.isArray(retryAfterHeader) ? retryAfterHeader[0] : retryAfterHeader; + const retryAfterMs = parseRetryAfterHeader(retryAfterStr); + const requestManager = this.requestManager; + if ( + requestManager && + 'recordDomainDelay' in requestManager && + typeof (requestManager as any).recordDomainDelay === 'function' + ) { + const recorded = (requestManager as any).recordDomainDelay( + crawlingContext.request.url, + retryAfterMs, + ); + if (recorded) { + throw new Error( + `Request to ${crawlingContext.request.url} failed with 429. Domain is throttled.`, + ); + } + } + } } if (this.sessionPool && response && session) { diff --git a/packages/core/src/storages/index.ts b/packages/core/src/storages/index.ts index 7d596e062a39..f9c3d3f7a567 100644 --- a/packages/core/src/storages/index.ts +++ b/packages/core/src/storages/index.ts @@ -11,3 +11,4 @@ export * from './utils.js'; export * from './access_checking.js'; export * from './sitemap_request_loader.js'; export * from './request_manager_tandem.js'; +export * from './throttling_request_manager.js'; diff --git a/packages/core/src/storages/request_manager_tandem.ts b/packages/core/src/storages/request_manager_tandem.ts index 9ba885368d36..6b26822fd5fb 100644 --- a/packages/core/src/storages/request_manager_tandem.ts +++ b/packages/core/src/storages/request_manager_tandem.ts @@ -244,4 +244,25 @@ export class RequestManagerTandem implements IRequestManager { this.expectedRequestProcessingSecs = secs; await this.resolvedRequestManager?.setExpectedRequestProcessingTimeSecs?.(secs); } + + setCrawlDelay(url: string, delaySeconds: number): void { + if ( + this.resolvedRequestManager && + 'setCrawlDelay' in this.resolvedRequestManager && + typeof (this.resolvedRequestManager as any).setCrawlDelay === 'function' + ) { + (this.resolvedRequestManager as any).setCrawlDelay(url, delaySeconds); + } + } + + recordDomainDelay(url: string, retryAfterMs?: number | null): boolean { + if ( + this.resolvedRequestManager && + 'recordDomainDelay' in this.resolvedRequestManager && + typeof (this.resolvedRequestManager as any).recordDomainDelay === 'function' + ) { + return (this.resolvedRequestManager as any).recordDomainDelay(url, retryAfterMs); + } + return false; + } } diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts new file mode 100644 index 000000000000..b6605f8441d5 --- /dev/null +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -0,0 +1,526 @@ +import { URL } from 'node:url'; +import { setTimeout as sleep } from 'node:timers/promises'; +import type { Dictionary, ProcessedRequest, BatchAddRequestsResult } from '@crawlee/types'; +import ow from 'ow'; + +import type { Configuration } from '../configuration.js'; +import type { CrawleeLogger } from '../log.js'; +import type { Request, Source } from '../request.js'; +import { serviceLocator } from '../service_locator.js'; +import type { IRequestManager, RequestsLike } from './request_manager.js'; +import type { + AddRequestsBatchedOptions, + AddRequestsBatchedResult, + RequestQueueOperationInfo, + RequestQueueOperationOptions, +} from './request_queue.js'; +import { RequestQueue } from './request_queue.js'; +import type { StorageIdentifier } from './storage_instance_manager.js'; +import type { StorageOpenOptions } from './utils.js'; + +export type RequestManagerOpener = ( + identifier: string | StorageIdentifier, + options?: StorageOpenOptions, +) => Promise; + +export interface ThrottlingRequestManagerOptions { + inner: T; + domains: string[]; + requestManagerOpener?: RequestManagerOpener; + baseDelayMs?: number; + maxDelayMs?: number; +} + +interface DomainState { + domain: string; + throttledUntil: number; // Date.now() timestamp in ms + consecutive429Count: number; + crawlDelayMs: number | null; +} + +export function parseRetryAfterHeader(value?: string | null): number | null { + if (!value) { + return null; + } + + const seconds = parseInt(value, 10); + if (!isNaN(seconds) && String(seconds) === value.trim()) { + return seconds * 1000; + } + + try { + const date = Date.parse(value); + if (!isNaN(date)) { + const delayMs = date - Date.now(); + return delayMs > 0 ? delayMs : null; + } + } catch { + // Ignore + } + + return null; +} + +export class ThrottlingRequestManager implements IRequestManager { + private readonly inner: T; + private readonly domains: string[]; + private readonly requestManagerOpener: RequestManagerOpener; + private readonly baseDelayMs: number; + private readonly maxDelayMs: number; + + private readonly domainStates = new Map(); + private readonly subManagers = new Map(); + private readonly log: CrawleeLogger; + + private newWorkSignaled = false; + private resolveNewWork: (() => void) | null = null; + + constructor( + options: ThrottlingRequestManagerOptions, + protected readonly config: Configuration = serviceLocator.getConfiguration(), + ) { + ow( + options, + ow.object.exactShape({ + inner: ow.object, + domains: ow.array.ofType(ow.string), + requestManagerOpener: ow.optional.function, + baseDelayMs: ow.optional.number, + maxDelayMs: ow.optional.number, + }), + ); + + this.inner = options.inner; + this.domains = options.domains; + this.requestManagerOpener = + options.requestManagerOpener ?? + ((idOrAlias, opts) => { + return RequestQueue.open(idOrAlias, opts) as unknown as Promise; + }); + this.baseDelayMs = options.baseDelayMs ?? 2000; + this.maxDelayMs = options.maxDelayMs ?? 60000; + this.log = serviceLocator.getLogger().child({ prefix: 'ThrottlingRequestManager' }); + + for (const domain of this.domains) { + if (domain) { + const lowerDomain = domain.toLowerCase(); + this.domainStates.set(lowerDomain, { + domain: lowerDomain, + throttledUntil: 0, + consecutive429Count: 0, + crawlDelayMs: null, + }); + } + } + } + + private getUrlFromRequest(requestLike: Source | string): string { + if (typeof requestLike === 'string') { + return requestLike; + } + return requestLike.url ?? ''; + } + + private extractDomain(url: string): string { + try { + const parsed = new URL(url); + return parsed.hostname.toLowerCase(); + } catch { + return ''; + } + } + + private getDomainState(url: string): DomainState | null { + const domain = this.extractDomain(url); + return this.domainStates.get(domain) ?? null; + } + + private selectManager(url: string): T { + const domain = this.extractDomain(url); + return this.subManagers.get(domain) ?? this.inner; + } + + private async getOrCreateSubManager(domain: string): Promise { + let sm = this.subManagers.get(domain); + if (!sm) { + sm = await this.requestManagerOpener({ alias: `throttled-${domain}` }, { configuration: this.config }); + this.subManagers.set(domain, sm); + } + return sm; + } + + private signalNewWork(): void { + this.newWorkSignaled = true; + if (this.resolveNewWork) { + this.resolveNewWork(); + this.resolveNewWork = null; + } + } + + private clearNewWork(): void { + this.newWorkSignaled = false; + } + + private async waitForNewWorkOrTimeout(timeoutMs: number): Promise { + if (this.newWorkSignaled) { + return; + } + + let timeoutId: NodeJS.Timeout | undefined; + const timeoutPromise = new Promise((resolve) => { + timeoutId = setTimeout(resolve, timeoutMs); + }); + + const workPromise = new Promise((resolve) => { + this.resolveNewWork = resolve; + }); + + await Promise.race([workPromise, timeoutPromise]); + + if (timeoutId) { + clearTimeout(timeoutId); + } + this.resolveNewWork = null; + } + + private markDomainDispatched(domain: string): void { + const state = this.domainStates.get(domain); + if (state && state.crawlDelayMs !== null) { + state.throttledUntil = Date.now() + state.crawlDelayMs; + } + } + + private getEarliestAvailableTime(now: number): number { + let earliest = now + this.maxDelayMs; + for (const state of this.domainStates.values()) { + if (now < state.throttledUntil && state.throttledUntil < earliest) { + earliest = state.throttledUntil; + } + } + return earliest; + } + + recordDomainDelay(url: string, retryAfterMs?: number | null): boolean { + const state = this.getDomainState(url); + if (!state) { + return false; + } + + state.consecutive429Count += 1; + let delayMs = + retryAfterMs !== undefined && retryAfterMs !== null + ? retryAfterMs + : this.baseDelayMs * Math.pow(2, state.consecutive429Count - 1); + + if (delayMs > this.maxDelayMs) { + const source = + retryAfterMs !== undefined && retryAfterMs !== null ? 'Retry-After header' : 'exponential backoff'; + this.log.warning( + `Capping ${source} delay of ${(delayMs / 1000).toFixed(1)}s for domain "${state.domain}" ` + + `to maxDelayMs (${(this.maxDelayMs / 1000).toFixed(1)}s); the domain may continue to rate-limit. ` + + `Consider increasing maxDelayMs if this recurs.`, + ); + delayMs = this.maxDelayMs; + } + + state.throttledUntil = Date.now() + delayMs; + + this.log.info( + `Rate limit (429) detected for domain "${state.domain}" ` + + `(consecutive: ${state.consecutive429Count}, delay: ${(delayMs / 1000).toFixed(1)}s)`, + ); + + this.signalNewWork(); + return true; + } + + recordSuccess(url: string): void { + const state = this.getDomainState(url); + if (state && state.consecutive429Count > 0) { + this.log.debug(`Resetting rate limit state for domain "${state.domain}" after successful request`); + state.consecutive429Count = 0; + } + } + + setCrawlDelay(url: string, delaySeconds: number): void { + const state = this.getDomainState(url); + if (state?.crawlDelayMs !== null) { + return; + } + state.crawlDelayMs = delaySeconds * 1000; + this.log.debug(`Set crawl-delay for domain "${state.domain}" to ${delaySeconds}s`); + } + + // --- IRequestManager Implementation --- + + async addRequest(requestLike: Source, options?: RequestQueueOperationOptions): Promise { + const url = this.getUrlFromRequest(requestLike); + const domain = this.extractDomain(url); + + let result: RequestQueueOperationInfo; + if (this.domainStates.has(domain)) { + const sm = await this.getOrCreateSubManager(domain); + result = await sm.addRequest(requestLike, options); + } else { + result = await this.inner.addRequest(requestLike, options); + } + + this.signalNewWork(); + return result; + } + + async addRequests( + requestsLike: RequestsLike, + options: RequestQueueOperationOptions = {}, + ): Promise { + const innerRequests: (Source | string)[] = []; + const domainRequests = new Map(); + + for await (const request of requestsLike) { + const url = this.getUrlFromRequest(request); + const domain = this.extractDomain(url); + + if (this.domainStates.has(domain)) { + if (!domainRequests.has(domain)) { + domainRequests.set(domain, []); + } + domainRequests.get(domain)!.push(request); + } else { + innerRequests.push(request); + } + } + + const results: BatchAddRequestsResult = { + processedRequests: [], + unprocessedRequests: [], + }; + + if (innerRequests.length > 0) { + if ('addRequests' in this.inner && typeof (this.inner as any).addRequests === 'function') { + const res = await (this.inner as any).addRequests(innerRequests, options); + results.processedRequests.push(...res.processedRequests); + results.unprocessedRequests.push(...res.unprocessedRequests); + } else { + for (const req of innerRequests) { + const res = await this.inner.addRequest(typeof req === 'string' ? { url: req } : req, options); + results.processedRequests.push(res); + } + } + } + + for (const [domain, reqs] of domainRequests.entries()) { + const sm = await this.getOrCreateSubManager(domain); + if ('addRequests' in sm && typeof (sm as any).addRequests === 'function') { + const res = await (sm as any).addRequests(reqs, options); + results.processedRequests.push(...res.processedRequests); + results.unprocessedRequests.push(...res.unprocessedRequests); + } else { + for (const req of reqs) { + const res = await sm.addRequest(typeof req === 'string' ? { url: req } : req, options); + results.processedRequests.push(res); + } + } + } + + if (innerRequests.length > 0 || domainRequests.size > 0) { + this.signalNewWork(); + } + + return results; + } + + async addRequestsBatched( + requests: RequestsLike, + options: AddRequestsBatchedOptions = {}, + ): Promise { + const allRequests: (Source | string)[] = []; + for await (const req of requests) { + allRequests.push(req); + } + + const { batchSize = 1000, waitBetweenBatchesMillis = 1000, forefront } = options; + const operationOptions: RequestQueueOperationOptions = { forefront }; + + const initialBatch = allRequests.slice(0, batchSize); + const remainingBatches = allRequests.slice(batchSize); + + const addedRequests = (await this.addRequests(initialBatch, operationOptions)).processedRequests; + + let promise: Promise; + if (remainingBatches.length > 0) { + promise = (async () => { + const finalAddedRequests: ProcessedRequest[] = []; + for (let i = 0; i < remainingBatches.length; i += batchSize) { + const chunk = remainingBatches.slice(i, i + batchSize); + const res = await this.addRequests(chunk, { ...operationOptions, cache: false }); + finalAddedRequests.push(...res.processedRequests); + await sleep(waitBetweenBatchesMillis); + } + return finalAddedRequests; + })(); + + if (options.waitForAllRequestsToBeAdded) { + addedRequests.push(...(await promise)); + } + } else { + promise = Promise.resolve([]); + } + + return { + addedRequests, + waitForAllRequestsToBeAdded: promise, + }; + } + + async reclaimRequest( + request: Request, + options?: RequestQueueOperationOptions, + ): Promise { + const manager = this.selectManager(request.url); + const result = await manager.reclaimRequest(request, options); + this.signalNewWork(); + return result; + } + + async markRequestAsHandled(request: Request): Promise { + const manager = this.selectManager(request.url); + const result = await manager.markRequestAsHandled(request); + const isSuccess = request.errorMessages.length <= request.retryCount; + if (isSuccess) { + this.recordSuccess(request.url); + } + return result; + } + + async getTotalCount(): Promise { + const counts = await Promise.all([ + this.inner.getTotalCount(), + ...Array.from(this.subManagers.values()).map((sm) => sm.getTotalCount()), + ]); + return counts.reduce((a, b) => a + b, 0); + } + + async getPendingCount(): Promise { + const counts = await Promise.all([ + this.inner.getPendingCount(), + ...Array.from(this.subManagers.values()).map((sm) => sm.getPendingCount()), + ]); + return counts.reduce((a, b) => a + b, 0); + } + + async getHandledCount(): Promise { + const counts = await Promise.all([ + this.inner.getHandledCount(), + ...Array.from(this.subManagers.values()).map((sm) => sm.getHandledCount()), + ]); + return counts.reduce((a, b) => a + b, 0); + } + + async isEmpty(): Promise { + const empties = await Promise.all([ + this.inner.isEmpty(), + ...Array.from(this.subManagers.values()).map((sm) => sm.isEmpty()), + ]); + return empties.every(Boolean); + } + + async isFinished(): Promise { + const finished = await Promise.all([ + this.inner.isFinished(), + ...Array.from(this.subManagers.values()).map((sm) => sm.isFinished()), + ]); + return finished.every(Boolean); + } + + async purge(): Promise { + await Promise.all([this.inner.purge?.(), ...Array.from(this.subManagers.values(), async (sm) => sm.purge?.())]); + for (const state of this.domainStates.values()) { + state.consecutive429Count = 0; + state.throttledUntil = 0; + } + } + + async setExpectedRequestProcessingTimeSecs(secs: number): Promise { + await Promise.all([ + this.inner.setExpectedRequestProcessingTimeSecs?.(secs), + ...Array.from(this.subManagers.values(), (sm) => sm.setExpectedRequestProcessingTimeSecs?.(secs)), + ]); + } + + async fetchNextRequest(): Promise | null> { + while (true) { + this.clearNewWork(); + + const now = Date.now(); + const availableDomains: string[] = []; + for (const [domain, state] of this.domainStates.entries()) { + if (this.subManagers.has(domain) && now >= state.throttledUntil) { + availableDomains.push(domain); + } + } + + availableDomains.sort((a, b) => { + const stateA = this.domainStates.get(a)!; + const stateB = this.domainStates.get(b)!; + return stateA.throttledUntil - stateB.throttledUntil; + }); + + for (const domain of availableDomains) { + const sm = this.subManagers.get(domain)!; + const req = await sm.fetchNextRequest(); + if (req) { + this.markDomainDispatched(domain); + return req; + } + } + + const request = await this.inner.fetchNextRequest(); + if (request) { + return request; + } + + if (this.subManagers.size === 0) { + return null; + } + + const subManagersEmpty = await Promise.all(Array.from(this.subManagers.values()).map((sm) => sm.isEmpty())); + if (subManagersEmpty.every(Boolean)) { + return null; + } + + const earliest = this.getEarliestAvailableTime(now); + const sleepDurationMs = Math.max(earliest - now, 100); + + this.log.debug( + `All configured domains are throttled and inner manager is empty. ` + + `Waiting up to ${(sleepDurationMs / 1000).toFixed(1)}s for earliest domain to become available or new work.`, + ); + + await this.waitForNewWorkOrTimeout(sleepDurationMs); + } + } + + async *[Symbol.asyncIterator]() { + while (true) { + const req = await this.fetchNextRequest(); + if (!req) break; + yield req; + } + } + + async persistState(): Promise { + await Promise.all([ + this.inner.persistState?.(), + ...Array.from(this.subManagers.values(), async (sm) => sm.persistState?.()), + ]); + } + + async drop(): Promise { + const drops = [ + (this.inner as any).drop?.(), + ...Array.from(this.subManagers.values()).map((sm) => (sm as any).drop?.()), + ]; + await Promise.all(drops); + this.subManagers.clear(); + } +} diff --git a/packages/http-crawler/src/internals/http-crawler.ts b/packages/http-crawler/src/internals/http-crawler.ts index eed6d49b2b21..6dab7d457a35 100644 --- a/packages/http-crawler/src/internals/http-crawler.ts +++ b/packages/http-crawler/src/internals/http-crawler.ts @@ -26,7 +26,7 @@ import { Router, SessionError, } from '@crawlee/basic'; -import { type LoadedRequest, getCookiesFromResponse } from '@crawlee/core'; +import { type LoadedRequest, getCookiesFromResponse, parseRetryAfterHeader } from '@crawlee/core'; import { ResponseWithUrl } from '@crawlee/http-client'; import type { Awaitable, Dictionary, ISession } from '@crawlee/types'; import { type CheerioRoot, RETRY_CSS_SELECTORS } from '@crawlee/utils'; @@ -604,6 +604,22 @@ export class HttpCrawler< return $; }; + if (response.status === 429) { + const retryAfterHeader = response.headers.get('retry-after'); + const retryAfterMs = parseRetryAfterHeader(retryAfterHeader); + const requestManager = this.requestManager; + if ( + requestManager && + 'recordDomainDelay' in requestManager && + typeof (requestManager as any).recordDomainDelay === 'function' + ) { + const recorded = (requestManager as any).recordDomainDelay(crawlingContext.request.url, retryAfterMs); + if (recorded) { + throw new Error(`Request to ${crawlingContext.request.url} failed with 429. Domain is throttled.`); + } + } + } + this._throwOnBlockedRequest(response.status); if (this.saveResponseCookies) { diff --git a/packages/utils/src/internals/robots.ts b/packages/utils/src/internals/robots.ts index 43d01be4a8e2..d613fd61eb17 100644 --- a/packages/utils/src/internals/robots.ts +++ b/packages/utils/src/internals/robots.ts @@ -25,7 +25,7 @@ import { Sitemap } from './sitemap.js'; */ export class RobotsTxtFile { private constructor( - private robots: Pick, + private robots: Pick, private proxyUrl?: string, private logger?: CrawleeLogger, ) {} @@ -97,6 +97,9 @@ export class RobotsTxtFile { getSitemaps() { return []; }, + getCrawlDelay() { + return undefined; + }, }, proxyUrl, logger, @@ -107,6 +110,14 @@ export class RobotsTxtFile { return new RobotsTxtFile(robotsParser(url.toString(), await response.text()), proxyUrl, logger); } + /** + * Get crawl delay for a given user agent. + * @param [userAgent] relevant user agent, default to `*` + */ + getCrawlDelay(userAgent = '*'): number | undefined { + return this.robots.getCrawlDelay(userAgent); + } + /** * Check if a URL should be crawled by robots. * @param url the URL to check against the rules in robots.txt diff --git a/test/core/crawlers/http_crawler.test.ts b/test/core/crawlers/http_crawler.test.ts index 351143b04afb..1cfb8db9bc5e 100644 --- a/test/core/crawlers/http_crawler.test.ts +++ b/test/core/crawlers/http_crawler.test.ts @@ -4,7 +4,7 @@ import { Readable } from 'node:stream'; import type { ConcurrencySystemOptions } from '@crawlee/core'; import { MemoryStorageBackend, serviceLocator } from '@crawlee/core'; -import { ConcurrencySystem, HttpCrawler, SessionPool } from '@crawlee/http'; +import { ConcurrencySystem, HttpCrawler, RequestQueue, SessionPool, ThrottlingRequestManager } from '@crawlee/http'; import { ResponseWithUrl } from '@crawlee/http-client'; import iconv from 'iconv-lite'; @@ -562,3 +562,46 @@ test('works with a custom HttpClient', async () => { expect(results[0].includes('Schmexample Domain')).toBeTruthy(); expect(results[1].includes('Schmexample Domain')).toBeTruthy(); }); + +test('429 on throttled domain records delay and keeps session', async () => { + router.set('/429', (req, res) => { + res.statusCode = 429; + res.setHeader('retry-after', '2'); // 2 seconds + res.end(); + }); + + const innerQueue = await RequestQueue.open(); + const throttlingManager = new ThrottlingRequestManager({ + inner: innerQueue, + domains: ['127.0.0.1'], + }); + + const sessionPool = new SessionPool(); + let sessionRetired = false; + + const crawler = new HttpCrawler({ + requestManager: throttlingManager, + sessionPool, + maxRequestRetries: 1, + preNavigationHooks: [ + async ({ session }) => { + vitest.spyOn(session, 'retire').mockImplementation(() => { + sessionRetired = true; + }); + }, + ], + requestHandler: async () => {}, + }); + + const targetUrl = `${url}/429`; + await crawler.run([targetUrl]); + + // The request should have been retried and eventually fail, but the session should NOT be retired. + expect(sessionRetired).toBe(false); + + // Throttling delay should be registered in the manager. + const state = (throttlingManager as any).domainStates.get('127.0.0.1'); + expect(state).toBeDefined(); + expect(state.consecutive429Count).toBeGreaterThan(0); + expect(state.throttledUntil).toBeGreaterThan(Date.now()); +}); diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts new file mode 100644 index 000000000000..96bf25f482bc --- /dev/null +++ b/test/core/storages/throttling_request_manager.test.ts @@ -0,0 +1,128 @@ +import { MemoryStorageBackend, RequestQueue, serviceLocator } from '@crawlee/core'; +import { + ThrottlingRequestManager, + parseRetryAfterHeader, +} from '../../../packages/core/src/storages/throttling_request_manager.js'; + +describe('ThrottlingRequestManager', () => { + beforeEach(() => { + serviceLocator.setStorageBackend(new MemoryStorageBackend()); + }); + + async function createQueue(name = 'inner-queue') { + return RequestQueue.open({ name }); + } + + test('parseRetryAfterHeader parsing seconds and date', () => { + expect(parseRetryAfterHeader('120')).toBe(120_000); + expect(parseRetryAfterHeader(' 5 ')).toBe(5000); + + // date format + const futureDate = new Date(Date.now() + 5000).toUTCString(); + const delay = parseRetryAfterHeader(futureDate); + expect(delay).toBeGreaterThan(0); + expect(delay).toBeLessThanOrEqual(5500); + + expect(parseRetryAfterHeader(null)).toBeNull(); + expect(parseRetryAfterHeader('invalid')).toBeNull(); + }); + + test('Routing: requests to configured domains route to sub-managers, others to inner queue', async () => { + const inner = await createQueue(); + const manager = new ThrottlingRequestManager({ + inner, + domains: ['example.com'], + }); + + // Add request to inner domain + await manager.addRequest({ url: 'https://other.com/a' }); + // Add request to throttled domain + await manager.addRequest({ url: 'https://example.com/a' }); + + expect(await inner.getTotalCount()).toBe(1); + expect(await manager.getTotalCount()).toBe(2); + + // Fetching next request should yield them + const req1 = await manager.fetchNextRequest(); + expect(req1!.url).toBe('https://example.com/a'); // Throttled domains checked first + + const req2 = await manager.fetchNextRequest(); + expect(req2!.url).toBe('https://other.com/a'); + + expect(await manager.fetchNextRequest()).toBeNull(); + }); + + test('addRequests routing', async () => { + const inner = await createQueue(); + const manager = new ThrottlingRequestManager({ + inner, + domains: ['example.com', 'foo.com'], + }); + + await manager.addRequests([ + { url: 'https://example.com/1' }, + { url: 'https://other.com/1' }, + { url: 'https://foo.com/1' }, + ]); + + expect(await inner.getTotalCount()).toBe(1); + expect(await manager.getTotalCount()).toBe(3); + }); + + test('recordDomainDelay enforces throttling and fair scheduling', async () => { + const inner = await createQueue(); + const manager = new ThrottlingRequestManager({ + inner, + domains: ['example.com', 'foo.com'], + baseDelayMs: 100, + }); + + await manager.addRequest({ url: 'https://example.com/1' }); + await manager.addRequest({ url: 'https://foo.com/1' }); + + // Record a 500ms delay on example.com + const recorded = manager.recordDomainDelay('https://example.com/1', 500); + expect(recorded).toBe(true); + + // Record success reset check (does not reset delay, but resets consecutive count) + manager.recordSuccess('https://example.com/1'); + + // Fetch next request - should fetch foo.com since example.com is throttled + const req1 = await manager.fetchNextRequest(); + expect(req1!.url).toBe('https://foo.com/1'); + + // Since example.com is still throttled, and inner is empty, calling fetchNextRequest + // should wait and then return the request once the throttle expires. + const start = Date.now(); + const req2 = await manager.fetchNextRequest(); + const elapsed = Date.now() - start; + + expect(elapsed).toBeGreaterThanOrEqual(400); + expect(req2!.url).toBe('https://example.com/1'); + }); + + test('setCrawlDelay sets crawl-delay successfully', async () => { + const inner = await createQueue(); + const manager = new ThrottlingRequestManager({ + inner, + domains: ['example.com'], + }); + + manager.setCrawlDelay('https://example.com/1', 0.2); // 0.2 seconds = 200ms + + await manager.addRequest({ url: 'https://example.com/1' }); + await manager.addRequest({ url: 'https://example.com/2' }); + + const req1 = await manager.fetchNextRequest(); + expect(req1!.url).toBe('https://example.com/1'); + + // After fetching req1, the crawl-delay of 200ms is applied to example.com. + // So fetching next request immediately should sleep/wait. + const start = Date.now(); + const req2 = await manager.fetchNextRequest(); + const elapsed = Date.now() - start; + + expect(elapsed).toBeGreaterThanOrEqual(150); + expect(req2!.url).toBe('https://example.com/2'); + }); +}); From f88413a7e1c9c8f3eb48d6bb8d8ef127530626e2 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:33:16 +0200 Subject: [PATCH 02/32] test(basic-crawler): cover the `requestManager` constructor option `requestManager` was missing from `optionsShape`, so `ow.object.exactShape` rejected the documented option outright; nothing covered it. Adds the regression test the drive-by fix in this branch never got. --- test/core/crawlers/basic_crawler.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/core/crawlers/basic_crawler.test.ts b/test/core/crawlers/basic_crawler.test.ts index df894f97e6f3..1cd0a858bddf 100644 --- a/test/core/crawlers/basic_crawler.test.ts +++ b/test/core/crawlers/basic_crawler.test.ts @@ -174,6 +174,23 @@ describe('BasicCrawler', () => { expect(await requestList.isEmpty()).toBe(true); }); + test('accepts a `requestManager` and crawls from it', async () => { + const requestManager = await RequestQueue.open(); + await requestManager.addRequest({ url: 'https://example.com/from-request-manager' }); + + const processed: string[] = []; + const crawler = new BasicCrawler({ + requestManager, + requestHandler: async ({ request }) => { + processed.push(request.url); + }, + }); + + await crawler.run(); + + expect(processed).toEqual(['https://example.com/from-request-manager']); + }); + test('folds a supplied concurrencySystem into its pool and never tears the system down', async () => { const sources = [...Array(20).keys()].map((index) => ({ url: `https://example.com/${index}` })); const requestList = await RequestList.open(null, sources); From 7c84d147deedee7e8acff34d3d25b35fc71bda89 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:36:58 +0200 Subject: [PATCH 03/32] fix(core): reopen per-domain sub-queues instead of discovering them lazily Sub-queues live under a stable `throttled-` alias and outlive the process, but the map tracking them was only filled on insert - so a restart saw none, reported the crawl finished, and stranded everything a previous run had left throttled. Also fixes `purge` and `persistState` skipping them. --- .../storages/throttling_request_manager.ts | 125 +++++++++--------- .../throttling_request_manager.test.ts | 29 ++++ 2 files changed, 92 insertions(+), 62 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index b6605f8441d5..64ad06e9f04c 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -72,6 +72,13 @@ export class ThrottlingRequestManager(); private readonly log: CrawleeLogger; + /** + * Sub-managers are keyed by a stable alias, so they outlive the process. They must therefore be reopened + * for every configured domain rather than created on first insert - otherwise a restart sees an empty map, + * reports the crawl finished, and strands whatever the previous run left in them. + */ + private subManagersReady?: Promise; + private newWorkSignaled = false; private resolveNewWork: (() => void) | null = null; @@ -135,18 +142,31 @@ export class ThrottlingRequestManager { + await this.ensureSubManagers(); const domain = this.extractDomain(url); return this.subManagers.get(domain) ?? this.inner; } - private async getOrCreateSubManager(domain: string): Promise { - let sm = this.subManagers.get(domain); - if (!sm) { - sm = await this.requestManagerOpener({ alias: `throttled-${domain}` }, { configuration: this.config }); - this.subManagers.set(domain, sm); - } - return sm; + private async ensureSubManagers(): Promise { + this.subManagersReady ??= (async () => { + await Promise.all( + Array.from(this.domainStates.keys(), async (domain) => { + const subManager = await this.requestManagerOpener( + { alias: `throttled-${domain}` }, + { configuration: this.config }, + ); + this.subManagers.set(domain, subManager); + }), + ); + })(); + + await this.subManagersReady; + } + + private async getSubManagers(): Promise { + await this.ensureSubManagers(); + return Array.from(this.subManagers.values()); } private signalNewWork(): void { @@ -254,16 +274,8 @@ export class ThrottlingRequestManager { - const url = this.getUrlFromRequest(requestLike); - const domain = this.extractDomain(url); - - let result: RequestQueueOperationInfo; - if (this.domainStates.has(domain)) { - const sm = await this.getOrCreateSubManager(domain); - result = await sm.addRequest(requestLike, options); - } else { - result = await this.inner.addRequest(requestLike, options); - } + const manager = await this.selectManager(this.getUrlFromRequest(requestLike)); + const result = await manager.addRequest(requestLike, options); this.signalNewWork(); return result; @@ -308,8 +320,10 @@ export class ThrottlingRequestManager { - const manager = this.selectManager(request.url); + const manager = await this.selectManager(request.url); const result = await manager.reclaimRequest(request, options); this.signalNewWork(); return result; } async markRequestAsHandled(request: Request): Promise { - const manager = this.selectManager(request.url); + const manager = await this.selectManager(request.url); const result = await manager.markRequestAsHandled(request); const isSuccess = request.errorMessages.length <= request.retryCount; if (isSuccess) { @@ -393,47 +407,27 @@ export class ThrottlingRequestManager { - const counts = await Promise.all([ - this.inner.getTotalCount(), - ...Array.from(this.subManagers.values()).map((sm) => sm.getTotalCount()), - ]); - return counts.reduce((a, b) => a + b, 0); + return this.sumOverManagers((manager) => manager.getTotalCount()); } async getPendingCount(): Promise { - const counts = await Promise.all([ - this.inner.getPendingCount(), - ...Array.from(this.subManagers.values()).map((sm) => sm.getPendingCount()), - ]); - return counts.reduce((a, b) => a + b, 0); + return this.sumOverManagers((manager) => manager.getPendingCount()); } async getHandledCount(): Promise { - const counts = await Promise.all([ - this.inner.getHandledCount(), - ...Array.from(this.subManagers.values()).map((sm) => sm.getHandledCount()), - ]); - return counts.reduce((a, b) => a + b, 0); + return this.sumOverManagers((manager) => manager.getHandledCount()); } async isEmpty(): Promise { - const empties = await Promise.all([ - this.inner.isEmpty(), - ...Array.from(this.subManagers.values()).map((sm) => sm.isEmpty()), - ]); - return empties.every(Boolean); + return this.everyManager((manager) => manager.isEmpty()); } async isFinished(): Promise { - const finished = await Promise.all([ - this.inner.isFinished(), - ...Array.from(this.subManagers.values()).map((sm) => sm.isFinished()), - ]); - return finished.every(Boolean); + return this.everyManager((manager) => manager.isFinished()); } async purge(): Promise { - await Promise.all([this.inner.purge?.(), ...Array.from(this.subManagers.values(), async (sm) => sm.purge?.())]); + await this.forEachManager((manager) => manager.purge?.()); for (const state of this.domainStates.values()) { state.consecutive429Count = 0; state.throttledUntil = 0; @@ -441,20 +435,33 @@ export class ThrottlingRequestManager { - await Promise.all([ - this.inner.setExpectedRequestProcessingTimeSecs?.(secs), - ...Array.from(this.subManagers.values(), (sm) => sm.setExpectedRequestProcessingTimeSecs?.(secs)), - ]); + await this.forEachManager((manager) => manager.setExpectedRequestProcessingTimeSecs?.(secs)); + } + + private async forEachManager(fn: (manager: T) => Promise | undefined): Promise { + await Promise.all([this.inner, ...(await this.getSubManagers())].map(fn)); + } + + private async sumOverManagers(fn: (manager: T) => Promise): Promise { + const counts = await Promise.all([this.inner, ...(await this.getSubManagers())].map(fn)); + return counts.reduce((a, b) => a + b, 0); + } + + private async everyManager(fn: (manager: T) => Promise): Promise { + const results = await Promise.all([this.inner, ...(await this.getSubManagers())].map(fn)); + return results.every(Boolean); } async fetchNextRequest(): Promise | null> { + await this.ensureSubManagers(); + while (true) { this.clearNewWork(); const now = Date.now(); const availableDomains: string[] = []; for (const [domain, state] of this.domainStates.entries()) { - if (this.subManagers.has(domain) && now >= state.throttledUntil) { + if (now >= state.throttledUntil) { availableDomains.push(domain); } } @@ -483,7 +490,7 @@ export class ThrottlingRequestManager sm.isEmpty())); + const subManagersEmpty = await Promise.all(Array.from(this.subManagers.values(), (sm) => sm.isEmpty())); if (subManagersEmpty.every(Boolean)) { return null; } @@ -509,18 +516,12 @@ export class ThrottlingRequestManager { - await Promise.all([ - this.inner.persistState?.(), - ...Array.from(this.subManagers.values(), async (sm) => sm.persistState?.()), - ]); + await this.forEachManager((manager) => manager.persistState?.()); } async drop(): Promise { - const drops = [ - (this.inner as any).drop?.(), - ...Array.from(this.subManagers.values()).map((sm) => (sm as any).drop?.()), - ]; - await Promise.all(drops); + await this.forEachManager((manager) => (manager as { drop?(): Promise }).drop?.()); this.subManagers.clear(); + this.subManagersReady = undefined; } } diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 96bf25f482bc..350a57267a01 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -101,6 +101,35 @@ describe('ThrottlingRequestManager', () => { expect(req2!.url).toBe('https://example.com/1'); }); + test('picks up requests left in per-domain sub-queues by a previous run', async () => { + const domains = ['example.com']; + + const firstRun = new ThrottlingRequestManager({ inner: await createQueue(), domains }); + await firstRun.addRequest({ url: 'https://example.com/left-behind' }); + expect(await firstRun.getPendingCount()).toBe(1); + + // A restart builds a brand new manager over the same storage backend. + const secondRun = new ThrottlingRequestManager({ inner: await createQueue(), domains }); + + expect(await secondRun.isEmpty()).toBe(false); + expect(await secondRun.isFinished()).toBe(false); + expect(await secondRun.getPendingCount()).toBe(1); + expect((await secondRun.fetchNextRequest())!.url).toBe('https://example.com/left-behind'); + }); + + test('purge empties per-domain sub-queues it has not touched yet', async () => { + const domains = ['example.com']; + + const firstRun = new ThrottlingRequestManager({ inner: await createQueue(), domains }); + await firstRun.addRequest({ url: 'https://example.com/stale' }); + + const secondRun = new ThrottlingRequestManager({ inner: await createQueue(), domains }); + await secondRun.purge(); + + const subQueue = await RequestQueue.open({ alias: 'throttled-example.com' }); + expect(await subQueue.getPendingCount()).toBe(0); + }); + test('setCrawlDelay sets crawl-delay successfully', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ From acf8aabe56faecb6e8777aa8df53245c22db0191 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:41:35 +0200 Subject: [PATCH 04/32] fix(core): don't park a concurrency slot waiting out a throttled domain `fetchNextRequest` slept until the backoff expired (up to `maxDelayMs`), holding a task slot the autoscaler reads as spare capacity - so it scaled up toward `maxConcurrency` and released the lot at once, and `stop()` could not complete. It now returns `null` and `isEmpty()` reports throttled requests as unavailable, so the task loop idles instead. Drops the single-waiter wake primitive, which could only ever wake one of several concurrent callers. --- .../storages/throttling_request_manager.ts | 136 +++++------------- .../throttling_request_manager.test.ts | 56 ++++++-- 2 files changed, 79 insertions(+), 113 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 64ad06e9f04c..73bb02652c36 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -79,9 +79,6 @@ export class ThrottlingRequestManager; - private newWorkSignaled = false; - private resolveNewWork: (() => void) | null = null; - constructor( options: ThrottlingRequestManagerOptions, protected readonly config: Configuration = serviceLocator.getConfiguration(), @@ -169,40 +166,6 @@ export class ThrottlingRequestManager { - if (this.newWorkSignaled) { - return; - } - - let timeoutId: NodeJS.Timeout | undefined; - const timeoutPromise = new Promise((resolve) => { - timeoutId = setTimeout(resolve, timeoutMs); - }); - - const workPromise = new Promise((resolve) => { - this.resolveNewWork = resolve; - }); - - await Promise.race([workPromise, timeoutPromise]); - - if (timeoutId) { - clearTimeout(timeoutId); - } - this.resolveNewWork = null; - } - private markDomainDispatched(domain: string): void { const state = this.domainStates.get(domain); if (state && state.crawlDelayMs !== null) { @@ -210,14 +173,13 @@ export class ThrottlingRequestManager now >= state.throttledUntil) + .sort((a, b) => a.throttledUntil - b.throttledUntil) + .map((state) => state.domain); } recordDomainDelay(url: string, retryAfterMs?: number | null): boolean { @@ -250,7 +212,6 @@ export class ThrottlingRequestManager { const manager = await this.selectManager(this.getUrlFromRequest(requestLike)); - const result = await manager.addRequest(requestLike, options); - - this.signalNewWork(); - return result; + return manager.addRequest(requestLike, options); } async addRequests( @@ -337,7 +295,6 @@ export class ThrottlingRequestManager 0 || domainRequests.size > 0) { - this.signalNewWork(); } return results; @@ -391,9 +348,7 @@ export class ThrottlingRequestManager { const manager = await this.selectManager(request.url); - const result = await manager.reclaimRequest(request, options); - this.signalNewWork(); - return result; + return manager.reclaimRequest(request, options); } async markRequestAsHandled(request: Request): Promise { @@ -418,10 +373,22 @@ export class ThrottlingRequestManager manager.getHandledCount()); } + /** + * Whether the next {@apilink ThrottlingRequestManager.fetchNextRequest} would return `null`. + * + * Requests waiting on a throttled domain count as unavailable, so a crawler whose task loop is gated on + * this idles for the backoff instead of spinning on a fetch that cannot succeed yet. + */ async isEmpty(): Promise { - return this.everyManager((manager) => manager.isEmpty()); + await this.ensureSubManagers(); + + const fetchable = [this.inner, ...this.fetchableDomains().map((domain) => this.subManagers.get(domain)!)]; + const results = await Promise.all(fetchable.map((manager) => manager.isEmpty())); + + return results.every(Boolean); } + /** Unlike {@apilink ThrottlingRequestManager.isEmpty}, throttled requests still count as outstanding work. */ async isFinished(): Promise { return this.everyManager((manager) => manager.isFinished()); } @@ -452,59 +419,26 @@ export class ThrottlingRequestManager(): Promise | null> { await this.ensureSubManagers(); - while (true) { - this.clearNewWork(); - - const now = Date.now(); - const availableDomains: string[] = []; - for (const [domain, state] of this.domainStates.entries()) { - if (now >= state.throttledUntil) { - availableDomains.push(domain); - } - } - - availableDomains.sort((a, b) => { - const stateA = this.domainStates.get(a)!; - const stateB = this.domainStates.get(b)!; - return stateA.throttledUntil - stateB.throttledUntil; - }); - - for (const domain of availableDomains) { - const sm = this.subManagers.get(domain)!; - const req = await sm.fetchNextRequest(); - if (req) { - this.markDomainDispatched(domain); - return req; - } - } - - const request = await this.inner.fetchNextRequest(); + for (const domain of this.fetchableDomains()) { + const request = await this.subManagers.get(domain)!.fetchNextRequest(); if (request) { + this.markDomainDispatched(domain); return request; } - - if (this.subManagers.size === 0) { - return null; - } - - const subManagersEmpty = await Promise.all(Array.from(this.subManagers.values(), (sm) => sm.isEmpty())); - if (subManagersEmpty.every(Boolean)) { - return null; - } - - const earliest = this.getEarliestAvailableTime(now); - const sleepDurationMs = Math.max(earliest - now, 100); - - this.log.debug( - `All configured domains are throttled and inner manager is empty. ` + - `Waiting up to ${(sleepDurationMs / 1000).toFixed(1)}s for earliest domain to become available or new work.`, - ); - - await this.waitForNewWorkOrTimeout(sleepDurationMs); } + + return this.inner.fetchNextRequest(); } async *[Symbol.asyncIterator]() { diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 350a57267a01..8bc92f8ea12d 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -3,6 +3,7 @@ import { ThrottlingRequestManager, parseRetryAfterHeader, } from '../../../packages/core/src/storages/throttling_request_manager.js'; +import { sleep } from '@crawlee/utils'; describe('ThrottlingRequestManager', () => { beforeEach(() => { @@ -13,6 +14,19 @@ describe('ThrottlingRequestManager', () => { return RequestQueue.open({ name }); } + /** Models the crawler's task loop: poll, and idle while the manager reports itself empty. */ + async function pollForNextRequest(manager: ThrottlingRequestManager, timeoutMs = 5000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const request = await manager.fetchNextRequest(); + if (request) { + return request; + } + await sleep(20); + } + throw new Error('Timed out waiting for a request to become available'); + } + test('parseRetryAfterHeader parsing seconds and date', () => { expect(parseRetryAfterHeader('120')).toBe(120_000); expect(parseRetryAfterHeader(' 5 ')).toBe(5000); @@ -91,14 +105,34 @@ describe('ThrottlingRequestManager', () => { const req1 = await manager.fetchNextRequest(); expect(req1!.url).toBe('https://foo.com/1'); - // Since example.com is still throttled, and inner is empty, calling fetchNextRequest - // should wait and then return the request once the throttle expires. + // example.com is still throttled and inner is empty, so there is nothing to fetch right now - + // and the manager must say so rather than block the caller. + expect(await manager.fetchNextRequest()).toBeNull(); + expect(await manager.isEmpty()).toBe(true); + // ...while still reporting the throttled request as outstanding work. + expect(await manager.isFinished()).toBe(false); + const start = Date.now(); - const req2 = await manager.fetchNextRequest(); - const elapsed = Date.now() - start; + const req2 = await pollForNextRequest(manager); - expect(elapsed).toBeGreaterThanOrEqual(400); - expect(req2!.url).toBe('https://example.com/1'); + expect(Date.now() - start).toBeGreaterThanOrEqual(400); + expect(req2.url).toBe('https://example.com/1'); + }); + + test('fetchNextRequest does not block while a domain is throttled', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + maxDelayMs: 60_000, + }); + + await manager.addRequest({ url: 'https://example.com/1' }); + manager.recordDomainDelay('https://example.com/1', 60_000); + + const start = Date.now(); + expect(await manager.fetchNextRequest()).toBeNull(); + + expect(Date.now() - start).toBeLessThan(1000); }); test('picks up requests left in per-domain sub-queues by a previous run', async () => { @@ -145,13 +179,11 @@ describe('ThrottlingRequestManager', () => { const req1 = await manager.fetchNextRequest(); expect(req1!.url).toBe('https://example.com/1'); - // After fetching req1, the crawl-delay of 200ms is applied to example.com. - // So fetching next request immediately should sleep/wait. + // Dispatching req1 pushes example.com's next slot 200ms out. const start = Date.now(); - const req2 = await manager.fetchNextRequest(); - const elapsed = Date.now() - start; + const req2 = await pollForNextRequest(manager); - expect(elapsed).toBeGreaterThanOrEqual(150); - expect(req2!.url).toBe('https://example.com/2'); + expect(Date.now() - start).toBeGreaterThanOrEqual(150); + expect(req2.url).toBe('https://example.com/2'); }); }); From c176431feb547df6cfa9e25821318f32ce04b0b1 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:47:30 +0200 Subject: [PATCH 05/32] fix(core): delegate batched adds instead of reimplementing them The hand-rolled `addRequestsBatched` drained its whole input up front, ignored `maxNewRequests`, never reported `requestsOverLimit`, dropped the in-flight batch count `isFinished()` depends on, and left its background promise unhandled so a backend error killed the process. It now chunks lazily, routes each chunk by domain, and hands the slices to the target managers' own `addRequestsBatched`. Drops the public `addRequests`, which only existed to support the duck-typed fallback and is not part of `IRequestManager`. --- .../storages/throttling_request_manager.ts | 193 ++++++++++-------- .../throttling_request_manager.test.ts | 76 ++++++- 2 files changed, 183 insertions(+), 86 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 73bb02652c36..c543ad7234c7 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -1,9 +1,10 @@ import { URL } from 'node:url'; import { setTimeout as sleep } from 'node:timers/promises'; -import type { Dictionary, ProcessedRequest, BatchAddRequestsResult } from '@crawlee/types'; +import type { Dictionary, ProcessedRequest } from '@crawlee/types'; import ow from 'ow'; import type { Configuration } from '../configuration.js'; +import { asyncifyIterable, chunkedAsyncIterable, peekableAsyncIterable } from '../iterables.js'; import type { CrawleeLogger } from '../log.js'; import type { Request, Source } from '../request.js'; import { serviceLocator } from '../service_locator.js'; @@ -79,6 +80,9 @@ export class ThrottlingRequestManager; + /** Batches still being added in the background; keeps {@apilink ThrottlingRequestManager.isFinished} honest. */ + private inProgressBatchCount = 0; + constructor( options: ThrottlingRequestManagerOptions, protected readonly config: Configuration = serviceLocator.getConfiguration(), @@ -141,8 +145,12 @@ export class ThrottlingRequestManager { await this.ensureSubManagers(); - const domain = this.extractDomain(url); - return this.subManagers.get(domain) ?? this.inner; + return this.managerForUrl(url); + } + + /** Only valid once {@apilink ThrottlingRequestManager.ensureSubManagers} has resolved. */ + private managerForUrl(url: string): T { + return this.subManagers.get(this.extractDomain(url)) ?? this.inner; } private async ensureSubManagers(): Promise { @@ -239,108 +247,121 @@ export class ThrottlingRequestManager { - const innerRequests: (Source | string)[] = []; - const domainRequests = new Map(); + /** + * Adds requests in batches, routing each one to the manager that owns its domain. + * + * Batching, validation, deduplication and `Retry-After`-free bookkeeping are all delegated to the target + * managers - this only decides where each request goes, one batch at a time, so a lazy or unbounded input + * iterable is never fully materialized. + */ + async addRequestsBatched( + requests: RequestsLike, + options: AddRequestsBatchedOptions = {}, + ): Promise { + await this.ensureSubManagers(); + + const { batchSize = 1000, waitBetweenBatchesMillis = 1000, forefront, maxNewRequests } = options; + + let remainingBudget = maxNewRequests ?? Infinity; + const requestsOverLimit: Source[] = []; - for await (const request of requestsLike) { - const url = this.getUrlFromRequest(request); - const domain = this.extractDomain(url); + // Never hand a target more than the budget allows, so an over-large final batch cannot overshoot. + const effectiveChunkSize = + maxNewRequests !== undefined ? () => Math.min(batchSize, remainingBudget) : batchSize; + + // An async generator is both the iterator `chunkedAsyncIterable` consumes and an iterable we can drain + // leftovers from later - the same object, so only unconsumed requests end up over the limit. + async function* iterateRequests(): AsyncGenerator { + yield* asyncifyIterable(requests); + } - if (this.domainStates.has(domain)) { - if (!domainRequests.has(domain)) { - domainRequests.set(domain, []); + const requestIterator = iterateRequests(); + const chunks = peekableAsyncIterable(chunkedAsyncIterable(requestIterator, effectiveChunkSize)); + const chunksIterator = chunks[Symbol.asyncIterator](); + + const processChunk = async (chunk: (Source | string)[]): Promise => { + const byManager = new Map(); + for (const request of chunk) { + const manager = this.managerForUrl(this.getUrlFromRequest(request)); + const bucket = byManager.get(manager); + if (bucket) { + bucket.push(request); + } else { + byManager.set(manager, [request]); } - domainRequests.get(domain)!.push(request); - } else { - innerRequests.push(request); } - } - const results: BatchAddRequestsResult = { - processedRequests: [], - unprocessedRequests: [], - }; + const results = await Promise.all( + Array.from(byManager, ([manager, slice]) => + manager.addRequestsBatched(slice, { + forefront, + // The slice is already one batch, and we need its results before releasing the next one. + batchSize: slice.length, + waitForAllRequestsToBeAdded: true, + }), + ), + ); - if (innerRequests.length > 0) { - if ('addRequests' in this.inner && typeof (this.inner as any).addRequests === 'function') { - const res = await (this.inner as any).addRequests(innerRequests, options); - results.processedRequests.push(...res.processedRequests); - results.unprocessedRequests.push(...res.unprocessedRequests); - } else { - for (const req of innerRequests) { - const res = await this.inner.addRequest(typeof req === 'string' ? { url: req } : req, options); - results.processedRequests.push(res); - } + const processedRequests = results.flatMap((result) => result.addedRequests); + if (maxNewRequests !== undefined) { + remainingBudget -= processedRequests.filter((request) => !request.wasAlreadyPresent).length; } - } - await this.ensureSubManagers(); + return processedRequests; + }; - for (const [domain, reqs] of domainRequests.entries()) { - const sm = this.subManagers.get(domain)!; - if ('addRequests' in sm && typeof (sm as any).addRequests === 'function') { - const res = await (sm as any).addRequests(reqs, options); - results.processedRequests.push(...res.processedRequests); - results.unprocessedRequests.push(...res.unprocessedRequests); - } else { - for (const req of reqs) { - const res = await sm.addRequest(typeof req === 'string' ? { url: req } : req, options); - results.processedRequests.push(res); + const buildResult = async ( + addedRequests: ProcessedRequest[], + waitForAllRequestsToBeAdded: Promise, + ): Promise => { + if (maxNewRequests !== undefined) { + // `chunkedAsyncIterable` stops pulling once the budget-derived chunk size hits zero, so whatever + // is left is still sitting in the source iterator. + for await (const request of requestIterator) { + requestsOverLimit.push(typeof request === 'string' ? { url: request } : request); } } - } - if (innerRequests.length > 0 || domainRequests.size > 0) { - } - - return results; - } + return { addedRequests, waitForAllRequestsToBeAdded, requestsOverLimit }; + }; - async addRequestsBatched( - requests: RequestsLike, - options: AddRequestsBatchedOptions = {}, - ): Promise { - const allRequests: (Source | string)[] = []; - for await (const req of requests) { - allRequests.push(req); + const initialChunk = await chunksIterator.peek(); + if (initialChunk === undefined) { + return buildResult([], Promise.resolve([])); } - const { batchSize = 1000, waitBetweenBatchesMillis = 1000, forefront } = options; - const operationOptions: RequestQueueOperationOptions = { forefront }; + const addedRequests = await processChunk(initialChunk); + await chunksIterator.next(); - const initialBatch = allRequests.slice(0, batchSize); - const remainingBatches = allRequests.slice(batchSize); + if ((await chunksIterator.peek()) === undefined) { + return buildResult(addedRequests, Promise.resolve([])); + } - const addedRequests = (await this.addRequests(initialBatch, operationOptions)).processedRequests; + const remainder = (async () => { + const added: ProcessedRequest[] = []; + for await (const chunk of chunks) { + added.push(...(await processChunk(chunk))); + await sleep(waitBetweenBatchesMillis); + } + return added; + })(); - let promise: Promise; - if (remainingBatches.length > 0) { - promise = (async () => { - const finalAddedRequests: ProcessedRequest[] = []; - for (let i = 0; i < remainingBatches.length; i += batchSize) { - const chunk = remainingBatches.slice(i, i + batchSize); - const res = await this.addRequests(chunk, { ...operationOptions, cache: false }); - finalAddedRequests.push(...res.processedRequests); - await sleep(waitBetweenBatchesMillis); - } - return finalAddedRequests; - })(); + // Keep the crawler from concluding it is finished while batches are still landing. The caller is not + // obliged to await `remainder`, so every derived promise needs its own handler - an unhandled rejection + // here would take the process down. + this.inProgressBatchCount += 1; + void remainder + .catch(() => {}) + .finally(() => { + this.inProgressBatchCount -= 1; + }); - if (options.waitForAllRequestsToBeAdded) { - addedRequests.push(...(await promise)); - } - } else { - promise = Promise.resolve([]); + // With a budget we must drain everything before we can report what went over it. + if (options.waitForAllRequestsToBeAdded || maxNewRequests !== undefined) { + addedRequests.push(...(await remainder)); } - return { - addedRequests, - waitForAllRequestsToBeAdded: promise, - }; + return buildResult(addedRequests, remainder); } async reclaimRequest( @@ -390,6 +411,10 @@ export class ThrottlingRequestManager { + if (this.inProgressBatchCount > 0) { + return false; + } + return this.everyManager((manager) => manager.isFinished()); } diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 8bc92f8ea12d..5b754a9e12be 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -66,14 +66,14 @@ describe('ThrottlingRequestManager', () => { expect(await manager.fetchNextRequest()).toBeNull(); }); - test('addRequests routing', async () => { + test('addRequestsBatched routing', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ inner, domains: ['example.com', 'foo.com'], }); - await manager.addRequests([ + await manager.addRequestsBatched([ { url: 'https://example.com/1' }, { url: 'https://other.com/1' }, { url: 'https://foo.com/1' }, @@ -83,6 +83,78 @@ describe('ThrottlingRequestManager', () => { expect(await manager.getTotalCount()).toBe(3); }); + test('addRequestsBatched consumes the input lazily and honours maxNewRequests', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + }); + + let produced = 0; + async function* urls() { + for (let i = 0; i < 10; i++) { + produced++; + yield { url: `https://example.com/${i}` }; + } + } + + const result = await manager.addRequestsBatched(urls(), { batchSize: 2, maxNewRequests: 4 }); + + expect(await manager.getTotalCount()).toBe(4); + expect(result.requestsOverLimit).toHaveLength(6); + // Everything was pulled to report the leftovers, but only via the budget-capped chunks. + expect(produced).toBe(10); + }); + + test('addRequestsBatched keeps isFinished false while background batches are landing', async () => { + const inner = await createQueue(); + // Reports itself done the moment each batch lands, so only our own batch bookkeeping can hold the crawl open. + const eagerlyFinished = { + ...inner, + addRequestsBatched: inner.addRequestsBatched.bind(inner), + getTotalCount: inner.getTotalCount.bind(inner), + isEmpty: async () => true, + isFinished: async () => true, + } as unknown as RequestQueue; + + const manager = new ThrottlingRequestManager({ inner: eagerlyFinished, domains: [] }); + + const result = await manager.addRequestsBatched( + [{ url: 'https://other.com/1' }, { url: 'https://other.com/2' }], + { batchSize: 1, waitBetweenBatchesMillis: 0 }, + ); + + // The inner manager reports itself finished as soon as its own batch lands, but ours must not - + // there is still a batch in flight behind it. + expect(await manager.isFinished()).toBe(false); + + await result.waitForAllRequestsToBeAdded; + expect(await manager.getTotalCount()).toBe(2); + }); + + test('addRequestsBatched surfaces a background failure without an unhandled rejection', async () => { + const inner = await createQueue(); + let batches = 0; + const flaky = { + ...inner, + addRequestsBatched: async (...args: Parameters) => { + if (++batches > 1) { + throw new Error('backend exploded'); + } + return inner.addRequestsBatched(...args); + }, + } as unknown as RequestQueue; + + const manager = new ThrottlingRequestManager({ inner: flaky, domains: [] }); + + const result = await manager.addRequestsBatched( + [{ url: 'https://other.com/1' }, { url: 'https://other.com/2' }], + { batchSize: 1, waitBetweenBatchesMillis: 0 }, + ); + + // Nobody is obliged to await this - but if it rejects unhandled, Node kills the process. + await expect(result.waitForAllRequestsToBeAdded).rejects.toThrow('backend exploded'); + }); + test('recordDomainDelay enforces throttling and fair scheduling', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ From cad6eaa3fcf36b8e1846127dd099d21926c48cbe Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:49:13 +0200 Subject: [PATCH 06/32] fix(core): warn that `requestsFromUrl` sources are not domain-routed A URL list's contents are unknown until the owning manager expands it, so those requests always land in the inner manager and silently escape throttling. Say so once instead of pretending they were routed. --- .../storages/throttling_request_manager.ts | 26 +++++++++++++++++++ .../throttling_request_manager.test.ts | 16 ++++++++++++ 2 files changed, 42 insertions(+) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index c543ad7234c7..bca0c599496f 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -83,6 +83,12 @@ export class ThrottlingRequestManager(); + + private get hasThrottledDomains(): boolean { + return this.domainStates.size > 0; + } + constructor( options: ThrottlingRequestManagerOptions, protected readonly config: Configuration = serviceLocator.getConfiguration(), @@ -126,9 +132,29 @@ export class ThrottlingRequestManager { await expect(result.waitForAllRequestsToBeAdded).rejects.toThrow('backend exploded'); }); + test('warns that requestsFromUrl sources cannot be domain-routed', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + }); + const warning = vitest.spyOn((manager as any).log, 'warning').mockImplementation(() => {}); + + await manager.addRequestsBatched([ + { requestsFromUrl: 'https://example.com/urls.txt' }, + { requestsFromUrl: 'https://example.com/more.txt' }, + ]); + + expect(warning).toHaveBeenCalledTimes(1); + expect(warning.mock.calls[0][0]).toMatch(/requestsFromUrl/); + }); + test('recordDomainDelay enforces throttling and fair scheduling', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ From d7bbd410ee17f93a5ceb3062534deebf1d3ad540 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:51:42 +0200 Subject: [PATCH 07/32] fix(core): advance the 429 backoff per rate-limit event, not per response Every already-in-flight request comes back 429, so counting each one made the exponent track concurrency (8 parallel requests jumped straight to 2^7). The counter now advances once per event and decays on its own after a quiet window, which also removes the `errorMessages`/`retryCount` success guess - that read as success for skipped requests and silently flattened the curve. --- .../storages/throttling_request_manager.ts | 59 ++++++++++++------- .../throttling_request_manager.test.ts | 54 ++++++++++++++++- 2 files changed, 88 insertions(+), 25 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index bca0c599496f..c62bf0dd876c 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -34,8 +34,12 @@ export interface ThrottlingRequestManagerOptions state.domain); } + /** + * Records a 429 response and puts the URL's domain into backoff. + * + * @returns `false` if the domain is not configured for throttling, in which case this is a no-op. + */ recordDomainDelay(url: string, retryAfterMs?: number | null): boolean { const state = this.getDomainState(url); if (!state) { return false; } + const now = Date.now(); + + // Requests already in flight when the limit was hit all come back 429. They describe one rate-limit + // event, so only the first advances the backoff - otherwise concurrency alone drives the exponent. + if (now < state.throttledUntil) { + return true; + } + + // A domain that has served us for a full extra backoff window is no longer rate-limiting; start over + // rather than carrying the old exponent into an unrelated burst. + if (now >= state.backoffDecaysAt) { + state.consecutive429Count = 0; + } + state.consecutive429Count += 1; - let delayMs = - retryAfterMs !== undefined && retryAfterMs !== null - ? retryAfterMs - : this.baseDelayMs * Math.pow(2, state.consecutive429Count - 1); + + const retryAfterGiven = retryAfterMs !== undefined && retryAfterMs !== null; + let delayMs = retryAfterGiven ? retryAfterMs : this.baseDelayMs * Math.pow(2, state.consecutive429Count - 1); if (delayMs > this.maxDelayMs) { - const source = - retryAfterMs !== undefined && retryAfterMs !== null ? 'Retry-After header' : 'exponential backoff'; + const source = retryAfterGiven ? 'Retry-After header' : 'exponential backoff'; this.log.warning( `Capping ${source} delay of ${(delayMs / 1000).toFixed(1)}s for domain "${state.domain}" ` + `to maxDelayMs (${(this.maxDelayMs / 1000).toFixed(1)}s); the domain may continue to rate-limit. ` + @@ -239,7 +261,8 @@ export class ThrottlingRequestManager 0) { - this.log.debug(`Resetting rate limit state for domain "${state.domain}" after successful request`); - state.consecutive429Count = 0; - } - } - setCrawlDelay(url: string, delaySeconds: number): void { const state = this.getDomainState(url); if (state?.crawlDelayMs !== null) { @@ -400,12 +415,7 @@ export class ThrottlingRequestManager { const manager = await this.selectManager(request.url); - const result = await manager.markRequestAsHandled(request); - const isSuccess = request.errorMessages.length <= request.retryCount; - if (isSuccess) { - this.recordSuccess(request.url); - } - return result; + return manager.markRequestAsHandled(request); } async getTotalCount(): Promise { @@ -444,11 +454,16 @@ export class ThrottlingRequestManager manager.isFinished()); } + /** + * Empties every manager and clears the accumulated backoff. A robots.txt `Crawl-delay` is a property of the + * site rather than of the run, so it survives. + */ async purge(): Promise { await this.forEachManager((manager) => manager.purge?.()); for (const state of this.domainStates.values()) { state.consecutive429Count = 0; state.throttledUntil = 0; + state.backoffDecaysAt = 0; } } diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 4a673de057b5..2cb9422c875a 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -14,6 +14,10 @@ describe('ThrottlingRequestManager', () => { return RequestQueue.open({ name }); } + function domainState(manager: ThrottlingRequestManager, domain: string) { + return (manager as any).domainStates.get(domain); + } + /** Models the crawler's task loop: poll, and idle while the manager reports itself empty. */ async function pollForNextRequest(manager: ThrottlingRequestManager, timeoutMs = 5000) { const deadline = Date.now() + timeoutMs; @@ -186,9 +190,6 @@ describe('ThrottlingRequestManager', () => { const recorded = manager.recordDomainDelay('https://example.com/1', 500); expect(recorded).toBe(true); - // Record success reset check (does not reset delay, but resets consecutive count) - manager.recordSuccess('https://example.com/1'); - // Fetch next request - should fetch foo.com since example.com is throttled const req1 = await manager.fetchNextRequest(); expect(req1!.url).toBe('https://foo.com/1'); @@ -207,6 +208,53 @@ describe('ThrottlingRequestManager', () => { expect(req2.url).toBe('https://example.com/1'); }); + test('a burst of concurrent 429s advances the backoff only once', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + baseDelayMs: 50, + maxDelayMs: 60_000, + }); + + // Eight requests were already in flight when the limit was hit; they all come back 429. + for (let i = 0; i < 8; i++) { + expect(manager.recordDomainDelay('https://example.com/1')).toBe(true); + } + + expect(domainState(manager, 'example.com').consecutive429Count).toBe(1); + }); + + test('the backoff decays once the domain has stopped rate-limiting', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + baseDelayMs: 20, + maxDelayMs: 60_000, + }); + + manager.recordDomainDelay('https://example.com/1'); + await sleep(30); + manager.recordDomainDelay('https://example.com/1'); + expect(domainState(manager, 'example.com').consecutive429Count).toBe(2); + + // Wait out the delay plus a full extra window with no further 429. + await sleep(120); + manager.recordDomainDelay('https://example.com/1'); + expect(domainState(manager, 'example.com').consecutive429Count).toBe(1); + }); + + test('caps the delay at maxDelayMs', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + maxDelayMs: 1000, + }); + + manager.recordDomainDelay('https://example.com/1', 3_600_000); + + expect(domainState(manager, 'example.com').throttledUntil).toBeLessThanOrEqual(Date.now() + 1000); + }); + test('fetchNextRequest does not block while a domain is throttled', async () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), From 0785300ebc2c7f7d6668ec4bb3eb48b58b29dc58 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:54:42 +0200 Subject: [PATCH 08/32] fix(crawlers): don't spend a retry or the session on a throttled 429 A plain `Error` charged the 429 to `maxRequestRetries` and to the session's error score, so a healthy but rate-limited domain still failed its requests and still burned proxies - the two things the throttling manager exists to avoid. Throwing `RequestThrottledError` instead reclaims the request without recording a failure, and the manager paces the retry behind the backoff. --- .../src/internals/basic-crawler.ts | 24 +++++++- .../src/internals/browser-crawler.ts | 7 +-- packages/core/src/errors.ts | 13 +++++ .../src/internals/http-crawler.ts | 9 ++- test/core/crawlers/http_crawler.test.ts | 57 +++++++++++-------- 5 files changed, 79 insertions(+), 31 deletions(-) diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index 88959ca9f351..f7e65c171763 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -57,6 +57,7 @@ import { OwnedOrInjected, purgeDefaultStorages, RequestHandlerError, + RequestThrottledError, RequestManagerTandem, RequestQueue, ThrottlingRequestManager, @@ -1111,7 +1112,7 @@ export class BasicCrawler< ); // SessionError already retired the session in `requestFunctionErrorHandler`; // skip `markBad` to avoid double-counting usage/error score. - if (!(unwrappedError instanceof SessionError)) { + if (!this.errorAbsolvesSession(unwrappedError)) { crawlingContext.session?.markBad(); } return; @@ -2337,7 +2338,7 @@ export class BasicCrawler< } // decrease the session score if the request fails (but the error handler did not throw); // skip when the error is a SessionError, which already retired the session - if (!(err instanceof SessionError)) { + if (!this.errorAbsolvesSession(err)) { crawlingContext.session.markBad(); } } finally { @@ -2497,6 +2498,17 @@ export class BasicCrawler< request: Request, source: IRequestManager, ): Promise { + if (error instanceof RequestThrottledError) { + // The domain told us to come back later, so the request was never really attempted. Put it back + // without recording a failure - it costs neither a retry nor session reputation. + this.log.debug(`Deferring request because its domain is rate-limiting us. ${error.message}`, { + id: request.id, + url: request.url, + }); + await source.reclaimRequest(request, { forefront: request.userData?.__crawlee?.forefront }); + return; + } + request.pushErrorMessage(error); if (error instanceof CriticalError) { @@ -2597,6 +2609,14 @@ export class BasicCrawler< : [error.message || error, userLine].join('\n'); } + /** + * Whether the session should be spared for this error - either because it was already retired, or because the + * failure says nothing about the session (a rate limit is a property of the domain). + */ + private errorAbsolvesSession(error: Error): boolean { + return error instanceof SessionError || error instanceof RequestThrottledError; + } + private canRequestBeRetried(request: Request, error: Error) { // Request should never be retried, or the error encountered makes it not able to be retried. if (request.noRetry || error instanceof NonRetryableError) { diff --git a/packages/browser-crawler/src/internals/browser-crawler.ts b/packages/browser-crawler/src/internals/browser-crawler.ts index 2c126a263f85..4f47d76c3523 100644 --- a/packages/browser-crawler/src/internals/browser-crawler.ts +++ b/packages/browser-crawler/src/internals/browser-crawler.ts @@ -21,14 +21,15 @@ import { enqueueLinks, NavigationSkippedError, OwnedOrInjected, + parseRetryAfterHeader, remainingNavigationWindowMillis, RequestState, + RequestThrottledError, resolveBaseUrlForEnqueueLinksFiltering, SessionError, toughCookieToBrowserPoolCookie, tryAbsoluteURL, validators, - parseRetryAfterHeader, } from '@crawlee/basic'; import type { BrowserController, @@ -868,9 +869,7 @@ export abstract class BrowserCrawler< retryAfterMs, ); if (recorded) { - throw new Error( - `Request to ${crawlingContext.request.url} failed with 429. Domain is throttled.`, - ); + throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); } } } diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 13a1c79b3ee4..78fea2eb0676 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -53,6 +53,19 @@ export class RetryRequestError extends Error { } } +/** + * Thrown when a domain has rate-limited us and the request should simply be attempted again later. + * + * The request is reclaimed without recording a failure: it costs neither a retry nor session reputation, because + * nothing about the request or the session was at fault. A {@apilink ThrottlingRequestManager} holds it back until + * the domain's backoff expires, so retries are paced rather than immediate. + */ +export class RequestThrottledError extends RetryRequestError { + constructor(message?: string) { + super(message ?? 'Request is being retried later because its domain is rate-limiting us'); + } +} + /** * Errors of `SessionError` type retire the session associated with the request and trigger a regular retry. * diff --git a/packages/http-crawler/src/internals/http-crawler.ts b/packages/http-crawler/src/internals/http-crawler.ts index 6dab7d457a35..1215f1389f9f 100644 --- a/packages/http-crawler/src/internals/http-crawler.ts +++ b/packages/http-crawler/src/internals/http-crawler.ts @@ -26,7 +26,12 @@ import { Router, SessionError, } from '@crawlee/basic'; -import { type LoadedRequest, getCookiesFromResponse, parseRetryAfterHeader } from '@crawlee/core'; +import { + type LoadedRequest, + RequestThrottledError, + getCookiesFromResponse, + parseRetryAfterHeader, +} from '@crawlee/core'; import { ResponseWithUrl } from '@crawlee/http-client'; import type { Awaitable, Dictionary, ISession } from '@crawlee/types'; import { type CheerioRoot, RETRY_CSS_SELECTORS } from '@crawlee/utils'; @@ -615,7 +620,7 @@ export class HttpCrawler< ) { const recorded = (requestManager as any).recordDomainDelay(crawlingContext.request.url, retryAfterMs); if (recorded) { - throw new Error(`Request to ${crawlingContext.request.url} failed with 429. Domain is throttled.`); + throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); } } } diff --git a/test/core/crawlers/http_crawler.test.ts b/test/core/crawlers/http_crawler.test.ts index 1cfb8db9bc5e..b7b92986cb95 100644 --- a/test/core/crawlers/http_crawler.test.ts +++ b/test/core/crawlers/http_crawler.test.ts @@ -563,45 +563,56 @@ test('works with a custom HttpClient', async () => { expect(results[1].includes('Schmexample Domain')).toBeTruthy(); }); -test('429 on throttled domain records delay and keeps session', async () => { - router.set('/429', (req, res) => { - res.statusCode = 429; - res.setHeader('retry-after', '2'); // 2 seconds - res.end(); +test('a 429 on a throttled domain paces the retry without spending it or the session', async () => { + const hits: number[] = []; + router.set('/429-then-ok', (req, res) => { + hits.push(Date.now()); + if (hits.length === 1) { + res.statusCode = 429; + res.setHeader('retry-after', '1'); + res.end(); + return; + } + res.setHeader('content-type', 'text/html'); + res.end('ok'); }); - const innerQueue = await RequestQueue.open(); const throttlingManager = new ThrottlingRequestManager({ - inner: innerQueue, + inner: await RequestQueue.open(), domains: ['127.0.0.1'], }); const sessionPool = new SessionPool(); - let sessionRetired = false; + const retiredSessions: string[] = []; + const markedBad: string[] = []; + const handled: string[] = []; const crawler = new HttpCrawler({ requestManager: throttlingManager, sessionPool, - maxRequestRetries: 1, + maxRequestRetries: 0, preNavigationHooks: [ async ({ session }) => { - vitest.spyOn(session, 'retire').mockImplementation(() => { - sessionRetired = true; - }); + vitest.spyOn(session!, 'retire').mockImplementation(() => retiredSessions.push(session!.id)); + vitest.spyOn(session!, 'markBad').mockImplementation(() => markedBad.push(session!.id)); }, ], - requestHandler: async () => {}, + requestHandler: async ({ request }) => { + handled.push(request.url); + }, }); - const targetUrl = `${url}/429`; - await crawler.run([targetUrl]); + const stats = await crawler.run([`${url}/429-then-ok`]); - // The request should have been retried and eventually fail, but the session should NOT be retired. - expect(sessionRetired).toBe(false); + // `maxRequestRetries: 0` would have failed the request outright had the 429 been charged as a retry. + expect(handled).toHaveLength(1); + expect(stats.requestsFailed).toBe(0); - // Throttling delay should be registered in the manager. - const state = (throttlingManager as any).domainStates.get('127.0.0.1'); - expect(state).toBeDefined(); - expect(state.consecutive429Count).toBeGreaterThan(0); - expect(state.throttledUntil).toBeGreaterThan(Date.now()); -}); + // The domain's `Retry-After` was honoured between the two attempts... + expect(hits).toHaveLength(2); + expect(hits[1] - hits[0]).toBeGreaterThanOrEqual(1000); + + // ...and the session came out untouched - a rate limit says nothing about it. + expect(retiredSessions).toEqual([]); + expect(markedBad).toEqual([]); +}, 30_000); From a1269e582b09cb8e905c8e27cb100689cffb71bc Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 21:59:46 +0200 Subject: [PATCH 09/32] fix(crawlers): warn only when a robots.txt crawl-delay is actually dropped The startup warning read a tandem's unresolved inner manager, so it fired for correctly configured crawlers whenever `run()` got no URLs - while staying silent when a `ThrottlingRequestManager` was configured without the crawled domain in its `domains` list. `setCrawlDelay` now reports whether it took effect and the warning follows that, naming the domain at fault. --- .../src/internals/basic-crawler.ts | 63 ++++++++++--------- .../src/storages/request_manager_tandem.ts | 31 +++++---- .../storages/throttling_request_manager.ts | 22 +++++-- test/core/crawlers/basic_crawler.test.ts | 59 ++++++++++++++++- 4 files changed, 123 insertions(+), 52 deletions(-) diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index f7e65c171763..0ec19362f448 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -31,6 +31,7 @@ import type { StorageIdentifier, TaskLoopPredicates, TypedRequestsLike, + ThrottlingRequestManager, } from '@crawlee/core'; import { AutoscaledPool, @@ -60,7 +61,6 @@ import { RequestThrottledError, RequestManagerTandem, RequestQueue, - ThrottlingRequestManager, RequestState, RetryRequestError, Router, @@ -1831,6 +1831,13 @@ export class BasicCrawler< } } + private warnOncePerRun(key: string, message: string): void { + if (!this.loggedPerRun.has(key)) { + this.log.warning(message); + this.loggedPerRun.add(key); + } + } + /** * Adds requests to the queue in batches. By default, it will resolve after the initial batch is added, and continue * adding the rest in background. You can configure the batch size via `batchSize` option and the sleep time in between @@ -2042,25 +2049,6 @@ export class BasicCrawler< }); await this.getRequestManager(); - - if (this.respectRobotsTxtFile) { - let isThrottling = false; - let currentManager = this.requestManager; - if (currentManager instanceof RequestManagerTandem) { - currentManager = (currentManager as any).resolvedRequestManager; - } - if (currentManager instanceof ThrottlingRequestManager) { - isThrottling = true; - } - - if (!isThrottling) { - this.log.warning( - 'The `respectRobotsTxtFile` option is enabled, but the crawler is not using a `ThrottlingRequestManager`. ' + - 'Crawl delays defined in robots.txt will NOT be respected. ' + - 'To respect crawl delays, wrap your request queue in a `ThrottlingRequestManager`.', - ); - } - } } /** @@ -2146,21 +2134,38 @@ export class BasicCrawler< const userAgent = typeof this.respectRobotsTxtFile === 'object' ? this.respectRobotsTxtFile?.userAgent : '*'; if (robotsTxtFile) { - if ( - this.requestManager && - 'setCrawlDelay' in this.requestManager && - typeof (this.requestManager as any).setCrawlDelay === 'function' - ) { - const crawlDelay = robotsTxtFile.getCrawlDelay(userAgent); - if (crawlDelay !== undefined) { - (this.requestManager as any).setCrawlDelay(url, crawlDelay); - } + const crawlDelay = robotsTxtFile.getCrawlDelay(userAgent); + if (crawlDelay !== undefined) { + this.applyCrawlDelay(url, crawlDelay); } } return !robotsTxtFile || robotsTxtFile.isAllowed(url, userAgent); } + /** + * Hands a robots.txt `Crawl-delay` to the request manager, warning if nothing is able to honour it. + * + * Only a {@apilink ThrottlingRequestManager} can pace dispatch per domain, and only for the domains it was + * configured with - so the warning is driven by whether the delay was actually accepted, not by the type of + * the manager. Both are easy to get wrong: a correctly wrapped manager still drops the delay for a domain + * missing from its `domains` list. + */ + private applyCrawlDelay(url: string, delaySeconds: number): void { + const manager = this.requestManager as Partial | undefined; + if (manager?.setCrawlDelay?.(url, delaySeconds)) { + return; + } + + const domain = URL.canParse(url) ? new URL(url).hostname : url; + this.warnOncePerRun( + `crawlDelayIgnored:${domain}`, + `robots.txt for "${domain}" defines a crawl-delay of ${delaySeconds}s, but nothing is set up to honour it, ` + + 'so requests to that domain will not be paced. Pass a `ThrottlingRequestManager` as `requestManager` ' + + `and include "${domain}" in its \`domains\` option to enforce the delay.`, + ); + } + protected async getRobotsTxtFileForUrl(url: string): Promise { if (!this.respectRobotsTxtFile) { return undefined; diff --git a/packages/core/src/storages/request_manager_tandem.ts b/packages/core/src/storages/request_manager_tandem.ts index 6b26822fd5fb..78d904cc4a17 100644 --- a/packages/core/src/storages/request_manager_tandem.ts +++ b/packages/core/src/storages/request_manager_tandem.ts @@ -11,6 +11,7 @@ import type { RequestQueueOperationInfo, RequestQueueOperationOptions, } from './request_queue.js'; +import { ThrottlingRequestManager } from './throttling_request_manager.js'; /** * A request manager that combines a {@apilink IRequestLoader} (such as a `RequestList`) with a writable @@ -245,24 +246,22 @@ export class RequestManagerTandem implements IRequestManager { await this.resolvedRequestManager?.setExpectedRequestProcessingTimeSecs?.(secs); } - setCrawlDelay(url: string, delaySeconds: number): void { - if ( - this.resolvedRequestManager && - 'setCrawlDelay' in this.resolvedRequestManager && - typeof (this.resolvedRequestManager as any).setCrawlDelay === 'function' - ) { - (this.resolvedRequestManager as any).setCrawlDelay(url, delaySeconds); - } + /** + * Forwards to the wrapped manager if it throttles. Reports `false` when it does not, or when it has not been + * resolved yet - a tandem resolves on first use, so the answer is only meaningful once crawling has started. + */ + setCrawlDelay(url: string, delaySeconds: number): boolean { + return this.throttlingRequestManager?.setCrawlDelay(url, delaySeconds) ?? false; } + /** @see {@apilink RequestManagerTandem.setCrawlDelay} */ recordDomainDelay(url: string, retryAfterMs?: number | null): boolean { - if ( - this.resolvedRequestManager && - 'recordDomainDelay' in this.resolvedRequestManager && - typeof (this.resolvedRequestManager as any).recordDomainDelay === 'function' - ) { - return (this.resolvedRequestManager as any).recordDomainDelay(url, retryAfterMs); - } - return false; + return this.throttlingRequestManager?.recordDomainDelay(url, retryAfterMs) ?? false; + } + + private get throttlingRequestManager(): ThrottlingRequestManager | undefined { + return this.resolvedRequestManager instanceof ThrottlingRequestManager + ? this.resolvedRequestManager + : undefined; } } diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index c62bf0dd876c..9ac9d208bf66 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -272,13 +272,25 @@ export class ThrottlingRequestManager { expect(addRequestsBatchedSpy).toHaveBeenCalledOnce(); }); + describe('robots.txt crawl-delay', () => { + const crawlerWithCrawlDelay = (options: Partial) => + new (class MockedRobotsTxtCrawler extends BasicCrawler { + override async getRobotsTxtFileForUrl(_: string) { + return RobotsTxtFile.from('http://example.com/robots.txt', 'User-agent: *\nCrawl-delay: 5\n'); + } + })({ respectRobotsTxtFile: true, requestHandler: async () => {}, ...options } as BasicCrawlerOptions); + + test('is applied when a ThrottlingRequestManager covers the domain', async () => { + const requestManager = new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['example.com'], + }); + const crawler = crawlerWithCrawlDelay({ requestManager }); + const warning = vitest.spyOn(crawler.log, 'warning').mockImplementation(() => {}); + + await crawler.addRequests(['http://example.com/1']); + + expect(warning).not.toHaveBeenCalled(); + expect(requestManager.setCrawlDelay('http://example.com/1', 999)).toBe(true); + }); + + test('warns when the request manager cannot honour it', async () => { + const crawler = crawlerWithCrawlDelay({ requestQueue: await RequestQueue.open() }); + const warning = vitest.spyOn(crawler.log, 'warning').mockImplementation(() => {}); + + await crawler.addRequests(['http://example.com/1', 'http://example.com/2']); + + expect(warning).toHaveBeenCalledTimes(1); + expect(warning.mock.calls[0][0]).toMatch(/crawl-delay of 5s/); + }); + + test('warns when the domain is missing from the manager `domains` list', async () => { + const requestManager = new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['some-other-domain.com'], + }); + const crawler = crawlerWithCrawlDelay({ requestManager }); + const warning = vitest.spyOn(crawler.log, 'warning').mockImplementation(() => {}); + + await crawler.addRequests(['http://example.com/1']); + + expect(warning).toHaveBeenCalledTimes(1); + expect(warning.mock.calls[0][0]).toMatch(/example\.com/); + }); + }); + test('enqueueLinks should respect custom user-agent robots.txt rules', async () => { const requestQueue = await RequestQueue.open(); const visitedUrls: string[] = []; @@ -2419,7 +2474,7 @@ describe('BasicCrawler', () => { const crawler = new (class MockedRobotsTxtCrawler extends BasicCrawler { override async getRobotsTxtFileForUrl(_: string) { - return { isAllowed: isAllowedSpy } as unknown as RobotsTxtFile; + return { isAllowed: isAllowedSpy, getCrawlDelay: () => undefined } as unknown as RobotsTxtFile; } })({ requestQueue, From 61e81353c9ea84c066d520e54a6f521b43bd0814 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 22:12:04 +0200 Subject: [PATCH 10/32] refactor(crawlers): fold the 429 interception into one typed helper The `'x' in y && typeof (y as any).x === 'function'` dance was repeated at both call sites, and the browser one hand-rolled a case-insensitive header scan plus an array unwrap that neither Playwright nor Puppeteer can produce. Also records the rate limit before the error-status throw, so opting 429 into `additionalHttpErrorStatusCodes` no longer skips the backoff. --- .../src/internals/basic-crawler.ts | 33 +++++++++++++++- .../src/internals/browser-crawler.ts | 38 +++++-------------- .../src/internals/http-crawler.ts | 25 +++++------- 3 files changed, 51 insertions(+), 45 deletions(-) diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index 0ec19362f448..960c9046e7cb 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -58,6 +58,7 @@ import { OwnedOrInjected, purgeDefaultStorages, RequestHandlerError, + parseRetryAfterHeader, RequestThrottledError, RequestManagerTandem, RequestQueue, @@ -2143,6 +2144,31 @@ export class BasicCrawler< return !robotsTxtFile || robotsTxtFile.isAllowed(url, userAgent); } + /** + * Records an HTTP 429 against the URL's domain so the request manager can pace the retry. + * + * @param retryAfterHeader The raw `Retry-After` response header, if the server sent one. + * @returns `true` if a manager took responsibility for the delay, in which case the caller should throw + * {@apilink RequestThrottledError} rather than treating the response as a blocked session. + */ + protected recordDomainRateLimit(url: string, retryAfterHeader?: string | null): boolean { + const manager = this.requestManager as Partial | undefined; + if (manager?.recordDomainDelay?.(url, parseRetryAfterHeader(retryAfterHeader)) === true) { + return true; + } + + const domain = hostnameOrUrl(url); + this.warnOncePerRun( + `rateLimitNotThrottled:${domain}`, + `"${domain}" responded with HTTP 429 (Too Many Requests), but nothing is set up to back off from it, ` + + 'so the request will be retried without a per-domain delay and its session will be retired. ' + + `Pass a \`ThrottlingRequestManager\` as \`requestManager\` and include "${domain}" in its \`domains\` ` + + 'option to honour `Retry-After` and apply exponential backoff instead.', + ); + + return false; + } + /** * Hands a robots.txt `Crawl-delay` to the request manager, warning if nothing is able to honour it. * @@ -2157,7 +2183,7 @@ export class BasicCrawler< return; } - const domain = URL.canParse(url) ? new URL(url).hostname : url; + const domain = hostnameOrUrl(url); this.warnOncePerRun( `crawlDelayIgnored:${domain}`, `robots.txt for "${domain}" defines a crawl-delay of ${delaySeconds}s, but nothing is set up to honour it, ` + @@ -2752,6 +2778,11 @@ export interface CrawlerRunOptions extends CrawlerAddRequestsOptions { purgeRequestQueue?: boolean; } +/** The hostname of `url`, falling back to the whole string when it is not parseable - for log messages only. */ +function hostnameOrUrl(url: string): string { + return URL.canParse(url) ? new URL(url).hostname : url; +} + /** * Creates new {@apilink Router} instance that works based on request labels. * This instance can then serve as a {@apilink BasicCrawlerOptions.requestHandler|`requestHandler`} of our {@apilink BasicCrawler}. diff --git a/packages/browser-crawler/src/internals/browser-crawler.ts b/packages/browser-crawler/src/internals/browser-crawler.ts index 4f47d76c3523..bc4b4b9553e6 100644 --- a/packages/browser-crawler/src/internals/browser-crawler.ts +++ b/packages/browser-crawler/src/internals/browser-crawler.ts @@ -21,7 +21,6 @@ import { enqueueLinks, NavigationSkippedError, OwnedOrInjected, - parseRetryAfterHeader, remainingNavigationWindowMillis, RequestState, RequestThrottledError, @@ -839,6 +838,16 @@ export abstract class BrowserCrawler< this.stats.registerStatusCode(status); + // Ahead of the error-status throw below: a 429 the user opted into treating as an error is still a + // rate limit the domain should back off from. + if (status === 429) { + // Both drivers lower-case header names and join duplicates, so a plain lookup is enough. + const retryAfter = (response as { headers?(): Record }).headers?.()['retry-after']; + if (this.recordDomainRateLimit(crawlingContext.request.url, retryAfter)) { + throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); + } + } + if (this.isErrorStatusCode(status)) { if (this.additionalHttpErrorStatusCodes.has(status)) { throw new Error(`${status} - Error status code was set by user.`); @@ -846,33 +855,6 @@ export abstract class BrowserCrawler< throw new Error(`${status} - Internal Server Error`); } - - if (status === 429) { - const headers = typeof (response as any).headers === 'function' ? (response as any).headers() : {}; - let retryAfterHeader: string | undefined; - for (const key of Object.keys(headers)) { - if (key.toLowerCase() === 'retry-after') { - retryAfterHeader = headers[key]; - break; - } - } - const retryAfterStr = Array.isArray(retryAfterHeader) ? retryAfterHeader[0] : retryAfterHeader; - const retryAfterMs = parseRetryAfterHeader(retryAfterStr); - const requestManager = this.requestManager; - if ( - requestManager && - 'recordDomainDelay' in requestManager && - typeof (requestManager as any).recordDomainDelay === 'function' - ) { - const recorded = (requestManager as any).recordDomainDelay( - crawlingContext.request.url, - retryAfterMs, - ); - if (recorded) { - throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); - } - } - } } if (this.sessionPool && response && session) { diff --git a/packages/http-crawler/src/internals/http-crawler.ts b/packages/http-crawler/src/internals/http-crawler.ts index 1215f1389f9f..803c3c28e00f 100644 --- a/packages/http-crawler/src/internals/http-crawler.ts +++ b/packages/http-crawler/src/internals/http-crawler.ts @@ -574,6 +574,15 @@ export class HttpCrawler< tryCancel(); + // Before `parseResponse`, which throws for error status codes - a 429 the user opted into treating as an + // error is still a rate limit the domain should back off from. + if (crawlingContext.response.status === 429) { + const retryAfter = crawlingContext.response.headers.get('retry-after'); + if (this.recordDomainRateLimit(crawlingContext.request.url, retryAfter)) { + throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); + } + } + // Reading the body is still part of the navigation, so it draws from the same shared window: on a server // that streams the body slowly the request completes (headers arrive) but the body read would otherwise // run unbounded. `extendTimeout` from a post-navigation hook has already pushed this deadline out if asked. @@ -609,22 +618,6 @@ export class HttpCrawler< return $; }; - if (response.status === 429) { - const retryAfterHeader = response.headers.get('retry-after'); - const retryAfterMs = parseRetryAfterHeader(retryAfterHeader); - const requestManager = this.requestManager; - if ( - requestManager && - 'recordDomainDelay' in requestManager && - typeof (requestManager as any).recordDomainDelay === 'function' - ) { - const recorded = (requestManager as any).recordDomainDelay(crawlingContext.request.url, retryAfterMs); - if (recorded) { - throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); - } - } - } - this._throwOnBlockedRequest(response.status); if (this.saveResponseCookies) { From 260b9075089f01455ad8115157d4185697fbd6e4 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 22:12:58 +0200 Subject: [PATCH 11/32] fix(core): tighten `Retry-After` parsing `String(parseInt(v)) === v.trim()` rejected valid zero-padded values like `05` and accepted `-5`, which set a delay in the past - reporting the domain as throttled while applying no backoff at all. Also drops a try/catch around `Date.parse`, which does not throw. --- .../storages/throttling_request_manager.ts | 28 +++++++++++-------- .../throttling_request_manager.test.ts | 11 ++++++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 9ac9d208bf66..5a082102063a 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -43,24 +43,30 @@ interface DomainState { crawlDelayMs: number | null; } +/** + * Parses a `Retry-After` response header into a delay in milliseconds. + * + * The header holds either a non-negative number of seconds or an HTTP-date. + * See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Retry-After). + * + * @returns The delay in milliseconds, or `null` if the header is absent, unparseable, or already elapsed. + */ export function parseRetryAfterHeader(value?: string | null): number | null { if (!value) { return null; } - const seconds = parseInt(value, 10); - if (!isNaN(seconds) && String(seconds) === value.trim()) { - return seconds * 1000; + const trimmed = value.trim(); + + // Per the spec this is a `delay-seconds`: digits only, so a negative or fractional value is not one. + if (/^\d+$/.test(trimmed)) { + return Number(trimmed) * 1000; } - try { - const date = Date.parse(value); - if (!isNaN(date)) { - const delayMs = date - Date.now(); - return delayMs > 0 ? delayMs : null; - } - } catch { - // Ignore + const date = Date.parse(trimmed); + if (!Number.isNaN(date)) { + const delayMs = date - Date.now(); + return delayMs > 0 ? delayMs : null; } return null; diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 2cb9422c875a..d1d9d9fdbb92 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -34,6 +34,9 @@ describe('ThrottlingRequestManager', () => { test('parseRetryAfterHeader parsing seconds and date', () => { expect(parseRetryAfterHeader('120')).toBe(120_000); expect(parseRetryAfterHeader(' 5 ')).toBe(5000); + expect(parseRetryAfterHeader('0')).toBe(0); + // Zero-padded values are valid `delay-seconds`. + expect(parseRetryAfterHeader('05')).toBe(5000); // date format const futureDate = new Date(Date.now() + 5000).toUTCString(); @@ -41,8 +44,16 @@ describe('ThrottlingRequestManager', () => { expect(delay).toBeGreaterThan(0); expect(delay).toBeLessThanOrEqual(5500); + // A date in the past means "no delay", not a negative one. + expect(parseRetryAfterHeader(new Date(Date.now() - 5000).toUTCString())).toBeNull(); + expect(parseRetryAfterHeader(null)).toBeNull(); + expect(parseRetryAfterHeader(undefined)).toBeNull(); + expect(parseRetryAfterHeader('')).toBeNull(); expect(parseRetryAfterHeader('invalid')).toBeNull(); + // Not `delay-seconds`; a negative delay would have suppressed the backoff entirely. + expect(parseRetryAfterHeader('-5')).toBeNull(); + expect(parseRetryAfterHeader('1.5')).toBeNull(); }); test('Routing: requests to configured domains route to sub-managers, others to inner queue', async () => { From 6388defbe3c32767bdc601214db4a3552f259ab8 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 22:16:20 +0200 Subject: [PATCH 12/32] docs(core): document ThrottlingRequestManager and drop leftovers A 526-line public class shipped with no doc comment at all, so its defaults, its opt-in nature and the meaning of `recordDomainDelay`'s return value were undiscoverable. Also removes a field only the constructor read. --- .../storages/throttling_request_manager.ts | 87 ++++++++++++++++--- 1 file changed, 75 insertions(+), 12 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 5a082102063a..c82f0486ab69 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -19,16 +19,48 @@ import { RequestQueue } from './request_queue.js'; import type { StorageIdentifier } from './storage_instance_manager.js'; import type { StorageOpenOptions } from './utils.js'; +/** + * Opens a request manager, matching the shape of storage `open` methods such as + * {@apilink RequestQueue.open|`RequestQueue.open`}. + * + * {@apilink ThrottlingRequestManager} calls this once per configured domain, so every per-domain queue shares the + * concrete type and storage backend of the manager being wrapped. + */ export type RequestManagerOpener = ( identifier: string | StorageIdentifier, options?: StorageOpenOptions, ) => Promise; export interface ThrottlingRequestManagerOptions { + /** + * The request manager to wrap, usually a {@apilink RequestQueue}. Requests for domains that are not throttled + * are stored here. + */ inner: T; + + /** + * Hostnames to throttle. Matching is case-insensitive and exact - wildcards such as `*.example.com` are not + * supported, so list each subdomain you care about. Requests for any other domain bypass throttling entirely. + */ domains: string[]; + + /** + * Opens the per-domain queues, one per entry in `domains`, each under the alias `throttled-`. + * @default RequestQueue.open + */ requestManagerOpener?: RequestManagerOpener; + + /** + * The delay applied after a domain's first HTTP 429, doubled on each subsequent one. + * @default 2000 + */ baseDelayMs?: number; + + /** + * Upper bound on the delay between requests to a rate-limited domain, applied to both the exponential + * backoff and a `Retry-After` value. + * @default 60000 + */ maxDelayMs?: number; } @@ -72,9 +104,42 @@ export function parseRetryAfterHeader(value?: string | null): number | null { return null; } +/** + * A request manager that wraps another one and paces requests per domain. + * + * Requests for the configured {@apilink ThrottlingRequestManagerOptions.domains|`domains`} are routed into their own + * queue when they are added, so each request lives in exactly one place and deduplication keeps working. Everything + * else goes to the wrapped manager untouched. + * + * {@apilink ThrottlingRequestManager.fetchNextRequest|`fetchNextRequest()`} serves the domain that has been waiting + * longest and skips any that are backing off, falling back to the wrapped manager. It never blocks: while every + * remaining request belongs to a throttled domain it returns `null` and {@apilink ThrottlingRequestManager.isEmpty} + * reports `true`, so the crawler idles instead of holding a concurrency slot open. + * + * Delays come from two places: + * - HTTP 429 responses, honouring `Retry-After` and otherwise backing off exponentially. The crawlers report these + * automatically; a request that is throttled is retried later without counting against `maxRequestRetries` and + * without penalising its session. + * - robots.txt `Crawl-delay` directives, when `respectRobotsTxtFile` is enabled. + * + * This is opt-in: throttling only happens for a domain you list explicitly. + * + * **Example usage:** + * + * ```ts + * const crawler = new CheerioCrawler({ + * requestManager: new ThrottlingRequestManager({ + * inner: await RequestQueue.open(), + * domains: ['api.example.com', 'slow-site.org'], + * }), + * requestHandler: async ({ request }) => { ... }, + * }); + * ``` + * + * @category Sources + */ export class ThrottlingRequestManager implements IRequestManager { private readonly inner: T; - private readonly domains: string[]; private readonly requestManagerOpener: RequestManagerOpener; private readonly baseDelayMs: number; private readonly maxDelayMs: number; @@ -115,21 +180,18 @@ export class ThrottlingRequestManager { - return RequestQueue.open(idOrAlias, opts) as unknown as Promise; - }); - this.baseDelayMs = options.baseDelayMs ?? 2000; - this.maxDelayMs = options.maxDelayMs ?? 60000; + ((idOrAlias, opts) => RequestQueue.open(idOrAlias, opts) as unknown as Promise); + this.baseDelayMs = options.baseDelayMs ?? 2_000; + this.maxDelayMs = options.maxDelayMs ?? 60_000; this.log = serviceLocator.getLogger().child({ prefix: 'ThrottlingRequestManager' }); - for (const domain of this.domains) { + for (const domain of options.domains) { if (domain) { - const lowerDomain = domain.toLowerCase(); - this.domainStates.set(lowerDomain, { - domain: lowerDomain, + const hostname = domain.toLowerCase(); + this.domainStates.set(hostname, { + domain: hostname, throttledUntil: 0, backoffDecaysAt: 0, consecutive429Count: 0, @@ -490,7 +552,8 @@ export class ThrottlingRequestManager Promise | undefined): Promise { - await Promise.all([this.inner, ...(await this.getSubManagers())].map(fn)); + // `fn` targets optional members, so it may return nothing - the wrapper normalizes that for `Promise.all`. + await Promise.all([this.inner, ...(await this.getSubManagers())].map(async (manager) => fn(manager))); } private async sumOverManagers(fn: (manager: T) => Promise): Promise { From b3d26b4887656c27da78f162deedd05eb0090809 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 22:17:57 +0200 Subject: [PATCH 13/32] docs: cover per-domain throttling in the guides and upgrading notes A new public request manager was absent from the request-loaders guide that indexes them, and it silently changes what a 429 does - which until now the session-management guide was the only place to describe. --- docs/guides/request_loaders.mdx | 39 +++++++++++++++++++++++++++++- docs/guides/session_management.mdx | 6 +++++ docs/upgrading/upgrading_v4.md | 16 ++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/docs/guides/request_loaders.mdx b/docs/guides/request_loaders.mdx index d2be9bd6c3e6..31e764d3bbc3 100644 --- a/docs/guides/request_loaders.mdx +++ b/docs/guides/request_loaders.mdx @@ -26,6 +26,7 @@ The request loader abstractions are built around two interfaces and a couple of - `IRequestLoader`: The base interface for reading requests in a crawl. - `IRequestManager`: Extends `IRequestLoader` with write capabilities (adding and reclaiming requests). - `RequestManagerTandem`: Combines a read-only `IRequestLoader` with a writable `IRequestManager`. +- `ThrottlingRequestManager`: Wraps a writable `IRequestManager` and paces requests per domain. And the concrete request loader implementations: @@ -79,6 +80,8 @@ class SitemapRequestLoader class RequestManagerTandem +class ThrottlingRequestManager + %% ======================== %% Inheritance arrows %% ======================== @@ -88,6 +91,7 @@ IRequestLoader <|.. RequestList IRequestLoader <|.. SitemapRequestLoader IRequestManager <|.. RequestQueue IRequestManager <|.. RequestManagerTandem +IRequestManager <|.. ThrottlingRequestManager ``` :::info Crawler usage @@ -130,6 +134,39 @@ The loader supports filtering URLs using glob patterns and regular expressions, The `IRequestManager` interface extends `IRequestLoader` with **write** capabilities. In addition to reading requests, a request manager can add new requests and reclaim failed ones. This is essential for dynamic crawling, where new URLs emerge during the crawl, or when requests fail and need to be retried. The `RequestQueue` is the primary built-in request manager — see the [Request storage](./request-storage) guide for details. +## Per-domain throttling + +Some sites answer bursts of traffic with HTTP 429 (Too Many Requests) rather than an outright block. By default a 429 is treated as a blocked session: the session is retired and the request is retried straight away on a fresh one, which churns through proxies without actually slowing down. + +The `ThrottlingRequestManager` handles it at the scheduling layer instead. Wrap your request manager in it and list the domains you want paced: + +```ts +import { CheerioCrawler, RequestQueue, ThrottlingRequestManager } from 'crawlee'; + +const crawler = new CheerioCrawler({ + requestManager: new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['api.example.com'], + // optional, these are the defaults + baseDelayMs: 2_000, + maxDelayMs: 60_000, + }), + requestHandler: async ({ request }) => { + // ... + }, +}); +``` + +Requests for a listed domain are routed into their own queue as they are added. When one of those domains answers with a 429, the crawler honours its `Retry-After` header — or backs off exponentially from `baseDelayMs` up to `maxDelayMs` if there is none — and holds that domain's requests back for the duration. Requests for every other domain keep flowing at full speed, the throttled request is retried later without counting against `maxRequestRetries`, and its session is left alone, because a rate limit says nothing about the session. + +This is opt-in and exact: only the domains you list are throttled, and matching is case-insensitive with no wildcard support, so list each subdomain you care about. + +:::note robots.txt crawl-delay + +`ThrottlingRequestManager` is also what enforces `Crawl-delay` directives when `respectRobotsTxtFile` is enabled. Without it, or for a domain missing from `domains`, the directive is ignored and the crawler warns you about it. + +::: + ## Request manager tandem The `RequestManagerTandem` class combines the read-only capabilities of an `IRequestLoader` (like `RequestList`) with the read-write capabilities of an `IRequestManager` (like `RequestQueue`). This is useful when you need to load initial requests from a static source (such as a file, sitemap, or database) and also dynamically add or retry requests during the crawl. @@ -174,6 +211,6 @@ Similarly, you can combine a `Site ## Conclusion -This guide introduced the request loader abstractions: the read-only `IRequestLoader`, the writable `IRequestManager`, and the `RequestManagerTandem` that combines them, along with the `RequestList` and `SitemapRequestLoader` implementations. You also saw how to pair a loader with a queue using the `toTandem()` helper to handle both static and dynamically discovered requests. +This guide introduced the request loader abstractions: the read-only `IRequestLoader`, the writable `IRequestManager`, and the `RequestManagerTandem` that combines them, along with the `RequestList` and `SitemapRequestLoader` implementations. You also saw how to pair a loader with a queue using the `toTandem()` helper to handle both static and dynamically discovered requests, and how `ThrottlingRequestManager` paces requests to individual domains. If you have questions or need assistance, feel free to reach out on our [GitHub](https://github.com/apify/crawlee) or join our [Discord community](https://discord.com/invite/jyEM2PRvMU). Happy scraping! diff --git a/docs/guides/session_management.mdx b/docs/guides/session_management.mdx index b5a1ba558f55..25bd7feb27f4 100644 --- a/docs/guides/session_management.mdx +++ b/docs/guides/session_management.mdx @@ -213,6 +213,12 @@ const crawler = new CheerioCrawler({ For sites that respond with a `200` page that is actually a bot wall (Cloudflare challenges, Google's rate-limit page), set `retryOnBlocked: true` to have the crawler detect those by content and retry as well. For deeper anti-blocking measures see the [avoid blocking guide](./avoid-blocking). +:::tip A 429 is a rate limit, not a block + +Retiring a session on HTTP 429 burns proxies without slowing anything down — the site is asking you to wait, not telling you the session is unwelcome. Wrap your request manager in a `ThrottlingRequestManager` to back off per domain instead of rotating; see [per-domain throttling](./request-loaders#per-domain-throttling). Sessions are left untouched for the domains it covers. + +::: + ## Sharing a session pool between crawlers A `SessionPool` instance can be shared across multiple crawlers by passing the same object to each crawler's `sessionPool` option. This is useful in multi-stage scrapers — for example a fast `CheerioCrawler` that discovers links and a `PlaywrightCrawler` that renders detail pages — where you want both stages to reuse the same proven, non-blocked identities and their cookies instead of each warming up its own pool from scratch. diff --git a/docs/upgrading/upgrading_v4.md b/docs/upgrading/upgrading_v4.md index 94632286ae33..95ee38765a16 100644 --- a/docs/upgrading/upgrading_v4.md +++ b/docs/upgrading/upgrading_v4.md @@ -1212,6 +1212,22 @@ const crawler = new CheerioCrawler({ requestManager: await requestList.toTandem( A lone `requestList` now runs through a tandem over an auto-opened queue (rather than a read-only adapter). This means retries and `maxRequestsPerCrawl` accounting for that path now follow queue semantics. +### HTTP 429 can now back off per domain instead of retiring the session + +`blockedStatusCodes` still defaults to `[401, 403, 429]`, so out of the box a 429 retires the session and retries immediately, as in v3. New in v4 is the opt-in `ThrottlingRequestManager`, which handles rate limits at the scheduling layer instead: + +```typescript +const crawler = new CheerioCrawler({ + requestManager: new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['api.example.com'], + }), + requestHandler, +}); +``` + +For the domains you list, a 429 honours `Retry-After` (or backs off exponentially), holds only that domain's requests back, and leaves both the session and the request's retry budget untouched. It is also what enforces robots.txt `Crawl-delay` directives — with `respectRobotsTxtFile` enabled and no throttling manager covering the domain, the directive is ignored and the crawler warns about it. See the [request loaders guide](../guides/request-loaders#per-domain-throttling). + ### `BasicCrawler.requestList` and `BasicCrawler.requestQueue` fields removed The public `requestList` and `requestQueue` instance fields are gone. The crawler exposes a single `protected requestManager?: IRequestManager` instead. Access the active manager via the new async `getRequestManager()` method. From 157682fa7de6db856598f48370bd6dc350b0ec8d Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Wed, 5 Aug 2026 22:24:03 +0200 Subject: [PATCH 14/32] chore: regenerate public API snapshots `pnpm api:check` runs in CI and the new exports were never recorded, so the check failed deterministically. Also narrows the constructor's `config` parameter to private - it was leaking into the public surface unintentionally. --- docs/public-api/crawlee-basic.api.md | 2 + docs/public-api/crawlee-browser.api.md | 1 + docs/public-api/crawlee-core.api.md | 54 +++++++++++++++++++ docs/public-api/crawlee-http.api.md | 1 + docs/public-api/crawlee-jsdom.api.md | 1 + docs/public-api/crawlee-playwright.api.md | 1 + docs/public-api/crawlee-puppeteer.api.md | 1 + docs/public-api/crawlee-stagehand.api.md | 1 + docs/public-api/crawlee-utils.api.md | 1 + .../src/storages/request_manager_tandem.ts | 5 +- .../storages/throttling_request_manager.ts | 3 +- .../src/internals/http-crawler.ts | 7 +-- 12 files changed, 70 insertions(+), 8 deletions(-) diff --git a/docs/public-api/crawlee-basic.api.md b/docs/public-api/crawlee-basic.api.md index 697e1cc7efff..137134539817 100644 --- a/docs/public-api/crawlee-basic.api.md +++ b/docs/public-api/crawlee-basic.api.md @@ -123,6 +123,7 @@ export class BasicCrawler & BasePredicate; requestList: ObjectPredicate & BasePredicate; requestQueue: ObjectPredicate & BasePredicate; + requestManager: ObjectPredicate & BasePredicate; requestHandler: Predicate & BasePredicate; requestHandlerTimeoutSecs: NumberPredicate & BasePredicate; errorHandler: Predicate & BasePredicate; @@ -158,6 +159,7 @@ export class BasicCrawler; readonly proxyConfiguration?: IProxyConfiguration; pushData(data: Parameters[0], datasetIdentifier?: string | StorageIdentifier): Promise; + protected recordDomainRateLimit(url: string, retryAfterHeader?: string | null): boolean; // (undocumented) protected readonly requestHandler: RequestHandler; protected requestManager?: IRequestManager; diff --git a/docs/public-api/crawlee-browser.api.md b/docs/public-api/crawlee-browser.api.md index 3ebd1b34b5f5..2092a85a3382 100644 --- a/docs/public-api/crawlee-browser.api.md +++ b/docs/public-api/crawlee-browser.api.md @@ -78,6 +78,7 @@ export abstract class BrowserCrawler & BasePredicate; requestList: ObjectPredicate & BasePredicate; requestQueue: ObjectPredicate & BasePredicate; + requestManager: ObjectPredicate & BasePredicate; requestHandler: Predicate & BasePredicate; requestHandlerTimeoutSecs: NumberPredicate & BasePredicate; errorHandler: Predicate & BasePredicate; diff --git a/docs/public-api/crawlee-core.api.md b/docs/public-api/crawlee-core.api.md index 8daf0892b080..80edabe711d0 100644 --- a/docs/public-api/crawlee-core.api.md +++ b/docs/public-api/crawlee-core.api.md @@ -1083,6 +1083,9 @@ interface NewUrlOptions { export class NonRetryableError extends Error { } +// @public +export function parseRetryAfterHeader(value?: string | null): number | null; + // @public export function parseValue(body: Buffer | ArrayBuffer | string, contentTypeHeader: string | null): string | Buffer | ArrayBuffer | Record; @@ -1303,6 +1306,9 @@ export interface RequestListState { nextUniqueKey: string | null; } +// @public +export type RequestManagerOpener = (identifier: string | StorageIdentifier, options?: StorageOpenOptions) => Promise; + // @public export class RequestManagerTandem implements IRequestManager { // (undocumented) @@ -1329,6 +1335,8 @@ export class RequestManagerTandem implements IRequestManager { purge(): Promise; // (undocumented) reclaimRequest(request: Request_2, options?: RequestQueueOperationOptions): Promise; + recordDomainDelay(url: string, retryAfterMs?: number | null): boolean; + setCrawlDelay(url: string, delaySeconds: number): boolean; setExpectedRequestProcessingTimeSecs(secs: number): Promise; } @@ -1487,6 +1495,11 @@ export enum RequestState { UNPROCESSED = 0 } +// @public +export class RequestThrottledError extends RetryRequestError { + constructor(message?: string); +} + // @public export interface RequestTransform { // (undocumented) @@ -2010,6 +2023,47 @@ export interface TaskLoopPredicates { isTaskReadyFunction?: () => Promise; } +// @public +export class ThrottlingRequestManager implements IRequestManager { + // (undocumented) + [Symbol.asyncIterator](): AsyncGenerator, void, unknown>; + constructor(options: ThrottlingRequestManagerOptions, config?: Configuration); + // (undocumented) + addRequest(requestLike: Source, options?: RequestQueueOperationOptions): Promise; + addRequestsBatched(requests: RequestsLike, options?: AddRequestsBatchedOptions): Promise; + // (undocumented) + drop(): Promise; + fetchNextRequest(): Promise | null>; + // (undocumented) + getHandledCount(): Promise; + // (undocumented) + getPendingCount(): Promise; + // (undocumented) + getTotalCount(): Promise; + isEmpty(): Promise; + isFinished(): Promise; + // (undocumented) + markRequestAsHandled(request: Request_2): Promise; + // (undocumented) + persistState(): Promise; + purge(): Promise; + // (undocumented) + reclaimRequest(request: Request_2, options?: RequestQueueOperationOptions): Promise; + recordDomainDelay(url: string, retryAfterMs?: number | null): boolean; + setCrawlDelay(url: string, delaySeconds: number): boolean; + // (undocumented) + setExpectedRequestProcessingTimeSecs(secs: number): Promise; +} + +// @public +export interface ThrottlingRequestManagerOptions { + baseDelayMs?: number; + domains: string[]; + inner: T; + maxDelayMs?: number; + requestManagerOpener?: RequestManagerOpener; +} + export { tryAbsoluteURL } // @public diff --git a/docs/public-api/crawlee-http.api.md b/docs/public-api/crawlee-http.api.md index 78d28252db28..bd67aff8faf6 100644 --- a/docs/public-api/crawlee-http.api.md +++ b/docs/public-api/crawlee-http.api.md @@ -127,6 +127,7 @@ export class HttpCrawler = extendContext: Predicate & BasePredicate; requestList: ObjectPredicate & BasePredicate; requestQueue: ObjectPredicate & BasePredicate; + requestManager: ObjectPredicate & BasePredicate; requestHandler: Predicate & BasePredicate; requestHandlerTimeoutSecs: NumberPredicate & BasePredicate; errorHandler: Predicate & BasePredicate; diff --git a/docs/public-api/crawlee-jsdom.api.md b/docs/public-api/crawlee-jsdom.api.md index 39d82600fe43..667fc945384b 100644 --- a/docs/public-api/crawlee-jsdom.api.md +++ b/docs/public-api/crawlee-jsdom.api.md @@ -71,6 +71,7 @@ export class JSDOMCrawler, ExtendedContext extendContext: Predicate & BasePredicate; requestList: ObjectPredicate & BasePredicate; requestQueue: ObjectPredicate & BasePredicate; + requestManager: ObjectPredicate & BasePredicate; requestHandler: Predicate & BasePredicate; requestHandlerTimeoutSecs: NumberPredicate & BasePredicate; errorHandler: Predicate & BasePredicate; diff --git a/docs/public-api/crawlee-playwright.api.md b/docs/public-api/crawlee-playwright.api.md index d6dd5f707761..81c7771f7f43 100644 --- a/docs/public-api/crawlee-playwright.api.md +++ b/docs/public-api/crawlee-playwright.api.md @@ -362,6 +362,7 @@ export class PlaywrightCrawler, ExtendedCon extendContext: Predicate & BasePredicate; requestList: ObjectPredicate & BasePredicate; requestQueue: ObjectPredicate & BasePredicate; + requestManager: ObjectPredicate & BasePredicate; requestHandler: Predicate & BasePredicate; requestHandlerTimeoutSecs: NumberPredicate & BasePredicate; errorHandler: Predicate & BasePredicate; diff --git a/docs/public-api/crawlee-puppeteer.api.md b/docs/public-api/crawlee-puppeteer.api.md index 5fb1fee847bf..e2f074b29b2c 100644 --- a/docs/public-api/crawlee-puppeteer.api.md +++ b/docs/public-api/crawlee-puppeteer.api.md @@ -214,6 +214,7 @@ export class PuppeteerCrawler, ExtendedCont extendContext: Predicate & BasePredicate; requestList: ObjectPredicate & BasePredicate; requestQueue: ObjectPredicate & BasePredicate; + requestManager: ObjectPredicate & BasePredicate; requestHandler: Predicate & BasePredicate; requestHandlerTimeoutSecs: NumberPredicate & BasePredicate; errorHandler: Predicate & BasePredicate; diff --git a/docs/public-api/crawlee-stagehand.api.md b/docs/public-api/crawlee-stagehand.api.md index a90c9583878e..3c23018f092e 100644 --- a/docs/public-api/crawlee-stagehand.api.md +++ b/docs/public-api/crawlee-stagehand.api.md @@ -102,6 +102,7 @@ export class StagehandCrawler, ExtendedCont extendContext: Predicate & BasePredicate; requestList: ObjectPredicate & BasePredicate; requestQueue: ObjectPredicate & BasePredicate; + requestManager: ObjectPredicate & BasePredicate; requestHandler: Predicate & BasePredicate; requestHandlerTimeoutSecs: NumberPredicate & BasePredicate; errorHandler: Predicate & BasePredicate; diff --git a/docs/public-api/crawlee-utils.api.md b/docs/public-api/crawlee-utils.api.md index 083fab3ff0f3..f863874131e2 100644 --- a/docs/public-api/crawlee-utils.api.md +++ b/docs/public-api/crawlee-utils.api.md @@ -171,6 +171,7 @@ export class RobotsTxtFile { logger?: CrawleeLogger; }): Promise; static from(url: string, content: string, proxyUrl?: string): RobotsTxtFile; + getCrawlDelay(userAgent?: string): number | undefined; getSitemaps(): string[]; isAllowed(url: string, userAgent?: string): boolean; parseSitemaps(): Promise; diff --git a/packages/core/src/storages/request_manager_tandem.ts b/packages/core/src/storages/request_manager_tandem.ts index 78d904cc4a17..80f6fe0a04e6 100644 --- a/packages/core/src/storages/request_manager_tandem.ts +++ b/packages/core/src/storages/request_manager_tandem.ts @@ -254,7 +254,10 @@ export class RequestManagerTandem implements IRequestManager { return this.throttlingRequestManager?.setCrawlDelay(url, delaySeconds) ?? false; } - /** @see {@apilink RequestManagerTandem.setCrawlDelay} */ + /** + * Forwards to the wrapped manager if it throttles, otherwise reports `false`. + * @see {@apilink RequestManagerTandem.setCrawlDelay} + */ recordDomainDelay(url: string, retryAfterMs?: number | null): boolean { return this.throttlingRequestManager?.recordDomainDelay(url, retryAfterMs) ?? false; } diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index c82f0486ab69..d531e3c0e680 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -31,6 +31,7 @@ export type RequestManagerOpener = options?: StorageOpenOptions, ) => Promise; +/** Options for {@apilink ThrottlingRequestManager}. */ export interface ThrottlingRequestManagerOptions { /** * The request manager to wrap, usually a {@apilink RequestQueue}. Requests for domains that are not throttled @@ -166,7 +167,7 @@ export class ThrottlingRequestManager, - protected readonly config: Configuration = serviceLocator.getConfiguration(), + private readonly config: Configuration = serviceLocator.getConfiguration(), ) { ow( options, diff --git a/packages/http-crawler/src/internals/http-crawler.ts b/packages/http-crawler/src/internals/http-crawler.ts index 803c3c28e00f..ea260e126447 100644 --- a/packages/http-crawler/src/internals/http-crawler.ts +++ b/packages/http-crawler/src/internals/http-crawler.ts @@ -26,12 +26,7 @@ import { Router, SessionError, } from '@crawlee/basic'; -import { - type LoadedRequest, - RequestThrottledError, - getCookiesFromResponse, - parseRetryAfterHeader, -} from '@crawlee/core'; +import { type LoadedRequest, RequestThrottledError, getCookiesFromResponse } from '@crawlee/core'; import { ResponseWithUrl } from '@crawlee/http-client'; import type { Awaitable, Dictionary, ISession } from '@crawlee/types'; import { type CheerioRoot, RETRY_CSS_SELECTORS } from '@crawlee/utils'; From aeca8f8484da06ff1e67613f40ffe86860f566f9 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Thu, 6 Aug 2026 21:09:02 +0200 Subject: [PATCH 15/32] fix(core): treat `Retry-After: 0` as no deadline rather than no delay --- packages/core/src/storages/throttling_request_manager.ts | 6 +++++- test/core/storages/throttling_request_manager.test.ts | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index d531e3c0e680..eae2e7e5d414 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -93,7 +93,11 @@ export function parseRetryAfterHeader(value?: string | null): number | null { // Per the spec this is a `delay-seconds`: digits only, so a negative or fractional value is not one. if (/^\d+$/.test(trimmed)) { - return Number(trimmed) * 1000; + // `Retry-After: 0` names no future deadline, same as an HTTP-date that has already passed. Reporting it + // as a zero delay would leave the domain unthrottled while still counting as a rate-limit event, so the + // caller would defer the request for free and re-send it immediately. + const delayMs = Number(trimmed) * 1000; + return delayMs > 0 ? delayMs : null; } const date = Date.parse(trimmed); diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index d1d9d9fdbb92..7ae18192a727 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -34,10 +34,13 @@ describe('ThrottlingRequestManager', () => { test('parseRetryAfterHeader parsing seconds and date', () => { expect(parseRetryAfterHeader('120')).toBe(120_000); expect(parseRetryAfterHeader(' 5 ')).toBe(5000); - expect(parseRetryAfterHeader('0')).toBe(0); // Zero-padded values are valid `delay-seconds`. expect(parseRetryAfterHeader('05')).toBe(5000); + // A zero delay names no deadline; reporting it as one would leave the domain unthrottled and busy-loop. + expect(parseRetryAfterHeader('0')).toBeNull(); + expect(parseRetryAfterHeader('00')).toBeNull(); + // date format const futureDate = new Date(Date.now() + 5000).toUTCString(); const delay = parseRetryAfterHeader(futureDate); From 89a8dc2313d17bf53c4f91310747f707da0e63db Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Thu, 6 Aug 2026 23:24:27 +0200 Subject: [PATCH 16/32] fix(core): track the crawl-delay and the 429 backoff on separate clocks --- .../storages/throttling_request_manager.ts | 35 +++++++++++++------ .../throttling_request_manager.test.ts | 24 ++++++++++++- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index eae2e7e5d414..466c75148d5a 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -67,8 +67,14 @@ export interface ThrottlingRequestManagerOptions now >= state.throttledUntil) - .sort((a, b) => a.throttledUntil - b.throttledUntil) + .filter((state) => now >= throttledUntil(state)) + .sort((a, b) => throttledUntil(a) - throttledUntil(b)) .map((state) => state.domain); } @@ -309,7 +321,9 @@ export class ThrottlingRequestManager manager.purge?.()); for (const state of this.domainStates.values()) { state.consecutive429Count = 0; - state.throttledUntil = 0; + state.backoffUntil = 0; + state.crawlDelayUntil = 0; state.backoffDecaysAt = 0; } } diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 7ae18192a727..dfafbf89aff5 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -266,7 +266,7 @@ describe('ThrottlingRequestManager', () => { manager.recordDomainDelay('https://example.com/1', 3_600_000); - expect(domainState(manager, 'example.com').throttledUntil).toBeLessThanOrEqual(Date.now() + 1000); + expect(domainState(manager, 'example.com').backoffUntil).toBeLessThanOrEqual(Date.now() + 1000); }); test('fetchNextRequest does not block while a domain is throttled', async () => { @@ -314,6 +314,28 @@ describe('ThrottlingRequestManager', () => { expect(await subQueue.getPendingCount()).toBe(0); }); + test('a crawl-delay does not swallow the 429 backoff', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + }); + + manager.setCrawlDelay('https://example.com/1', 5); + await manager.addRequest({ url: 'https://example.com/1' }); + await manager.addRequest({ url: 'https://example.com/2' }); + + // Dispatching arms the crawl-delay, which used to read as an already-active backoff. + await manager.fetchNextRequest(); + expect(manager.recordDomainDelay('https://example.com/1', 30_000)).toBe(true); + + const state = domainState(manager, 'example.com'); + expect(state.consecutive429Count).toBe(1); + expect(state.backoffUntil).toBeGreaterThan(Date.now() + 25_000); + + // The longer of the two clocks wins, so the domain stays parked. + expect(await manager.fetchNextRequest()).toBeNull(); + }); + test('setCrawlDelay sets crawl-delay successfully', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ From 6c479a1c3a353457ed39780fc66d60a3198dc31b Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sat, 8 Aug 2026 15:30:41 +0200 Subject: [PATCH 17/32] feat(core): give up on a domain that rate-limits us indefinitely A throttled 429 costs no retry, so a domain that never recovered kept the crawl alive forever - `run()` simply never resolved, while the statistics reported zero requests throughout. Each domain now carries a stall clock, reset whenever one of its requests is handled, and the crawler aborts with a `PersistentRateLimitError` once a domain that still has queued work has been rate-limiting for `maxDomainStallSecs`. Those requests are deliberately left in their queue, so re-running without purging storages resumes them if the limit lifts. Also renames the manager's delay options to seconds, matching the rest of the crawler options, and replaces the `Partial` casts in `BasicCrawler` with a `SupportsDomainThrottling` guard. That drops the tandem's two forwarded methods: a tandem-wrapped throttler no longer throttles, but the existing warnings say so rather than leaving it silent. --- docs/guides/request_loaders.mdx | 9 +- docs/public-api/crawlee-core.api.md | 27 +++- .../src/internals/basic-crawler.ts | 23 ++-- packages/core/src/errors.ts | 10 ++ .../src/storages/request_manager_tandem.ts | 23 ---- .../storages/throttling_request_manager.ts | 120 ++++++++++++++++-- test/core/crawlers/http_crawler.test.ts | 38 +++++- .../throttling_request_manager.test.ts | 72 +++++++++-- 8 files changed, 261 insertions(+), 61 deletions(-) diff --git a/docs/guides/request_loaders.mdx b/docs/guides/request_loaders.mdx index 31e764d3bbc3..a270fd619e52 100644 --- a/docs/guides/request_loaders.mdx +++ b/docs/guides/request_loaders.mdx @@ -148,8 +148,9 @@ const crawler = new CheerioCrawler({ inner: await RequestQueue.open(), domains: ['api.example.com'], // optional, these are the defaults - baseDelayMs: 2_000, - maxDelayMs: 60_000, + baseDelaySecs: 2, + maxDelaySecs: 60, + maxDomainStallSecs: 900, }), requestHandler: async ({ request }) => { // ... @@ -157,7 +158,9 @@ const crawler = new CheerioCrawler({ }); ``` -Requests for a listed domain are routed into their own queue as they are added. When one of those domains answers with a 429, the crawler honours its `Retry-After` header — or backs off exponentially from `baseDelayMs` up to `maxDelayMs` if there is none — and holds that domain's requests back for the duration. Requests for every other domain keep flowing at full speed, the throttled request is retried later without counting against `maxRequestRetries`, and its session is left alone, because a rate limit says nothing about the session. +Requests for a listed domain are routed into their own queue as they are added. When one of those domains answers with a 429, the crawler honours its `Retry-After` header — or backs off exponentially from `baseDelaySecs` up to `maxDelaySecs` if there is none — and holds that domain's requests back for the duration. Requests for every other domain keep flowing at full speed, the throttled request is retried later without counting against `maxRequestRetries`, and its session is left alone, because a rate limit says nothing about the session. + +Because a throttled request costs no retries, a domain that never stops rate-limiting would otherwise keep the crawl alive forever. If one goes `maxDomainStallSecs` without letting a single request through, the crawl shuts down with a `PersistentRateLimitError` — at that point the concurrency is too high for that domain, or it has blocked you outright, and waiting longer will not help. Its requests are left in their queue on purpose, so re-running the crawl without purging storages resumes them if the rate limit lifts. This is opt-in and exact: only the domains you list are throttled, and matching is case-insensitive with no wildcard support, so list each subdomain you care about. diff --git a/docs/public-api/crawlee-core.api.md b/docs/public-api/crawlee-core.api.md index 80edabe711d0..cb676fe2e7c8 100644 --- a/docs/public-api/crawlee-core.api.md +++ b/docs/public-api/crawlee-core.api.md @@ -1097,6 +1097,10 @@ export interface PersistenceOptions { enable?: boolean; } +// @public +export class PersistentRateLimitError extends CriticalError { +} + // @public export class ProxyConfiguration implements IProxyConfiguration { constructor(options?: ProxyConfigurationOptions); @@ -1335,8 +1339,6 @@ export class RequestManagerTandem implements IRequestManager { purge(): Promise; // (undocumented) reclaimRequest(request: Request_2, options?: RequestQueueOperationOptions): Promise; - recordDomainDelay(url: string, retryAfterMs?: number | null): boolean; - setCrawlDelay(url: string, delaySeconds: number): boolean; setExpectedRequestProcessingTimeSecs(secs: number): Promise; } @@ -1999,6 +2001,19 @@ export class StorageStatsTracker> { get current(): T; } +// @public +export interface SupportsDomainThrottling { + // (undocumented) + assertNoStalledDomains(): Promise; + // (undocumented) + recordDomainDelay(url: string, retryAfterMs?: number | null): boolean; + // (undocumented) + setCrawlDelay(url: string, delaySeconds: number): boolean; +} + +// @public +export function supportsDomainThrottling(manager: unknown): manager is SupportsDomainThrottling; + // @public export interface SystemInfo { // (undocumented) @@ -2024,13 +2039,14 @@ export interface TaskLoopPredicates { } // @public -export class ThrottlingRequestManager implements IRequestManager { +export class ThrottlingRequestManager implements IRequestManager, SupportsDomainThrottling { // (undocumented) [Symbol.asyncIterator](): AsyncGenerator, void, unknown>; constructor(options: ThrottlingRequestManagerOptions, config?: Configuration); // (undocumented) addRequest(requestLike: Source, options?: RequestQueueOperationOptions): Promise; addRequestsBatched(requests: RequestsLike, options?: AddRequestsBatchedOptions): Promise; + assertNoStalledDomains(): Promise; // (undocumented) drop(): Promise; fetchNextRequest(): Promise | null>; @@ -2057,10 +2073,11 @@ export class ThrottlingRequestManager { - baseDelayMs?: number; + baseDelaySecs?: number; domains: string[]; inner: T; - maxDelayMs?: number; + maxDelaySecs?: number; + maxDomainStallSecs?: number; requestManagerOpener?: RequestManagerOpener; } diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index 960c9046e7cb..f6d7c3a77153 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -31,7 +31,6 @@ import type { StorageIdentifier, TaskLoopPredicates, TypedRequestsLike, - ThrottlingRequestManager, } from '@crawlee/core'; import { AutoscaledPool, @@ -64,6 +63,7 @@ import { RequestQueue, RequestState, RetryRequestError, + supportsDomainThrottling, Router, ServiceLocator, serviceLocator, @@ -1172,6 +1172,12 @@ export class BasicCrawler< return true; } + // Checked here because this runs only once nothing is in flight, which is exactly when a + // crawl that cannot progress looks indistinguishable from one that is merely waiting. + if (supportsDomainThrottling(this.requestManager)) { + await this.requestManager.assertNoStalledDomains(); + } + const isFinished = isFinishedFunction ? await isFinishedFunction() : await this.defaultIsFinishedFunction(); @@ -2152,8 +2158,10 @@ export class BasicCrawler< * {@apilink RequestThrottledError} rather than treating the response as a blocked session. */ protected recordDomainRateLimit(url: string, retryAfterHeader?: string | null): boolean { - const manager = this.requestManager as Partial | undefined; - if (manager?.recordDomainDelay?.(url, parseRetryAfterHeader(retryAfterHeader)) === true) { + if ( + supportsDomainThrottling(this.requestManager) && + this.requestManager.recordDomainDelay(url, parseRetryAfterHeader(retryAfterHeader)) + ) { return true; } @@ -2172,14 +2180,11 @@ export class BasicCrawler< /** * Hands a robots.txt `Crawl-delay` to the request manager, warning if nothing is able to honour it. * - * Only a {@apilink ThrottlingRequestManager} can pace dispatch per domain, and only for the domains it was - * configured with - so the warning is driven by whether the delay was actually accepted, not by the type of - * the manager. Both are easy to get wrong: a correctly wrapped manager still drops the delay for a domain - * missing from its `domains` list. + * The warning is driven by whether the delay was actually accepted rather than by the type of the manager, + * because a manager that does throttle still drops the delay for a domain missing from its `domains` list. */ private applyCrawlDelay(url: string, delaySeconds: number): void { - const manager = this.requestManager as Partial | undefined; - if (manager?.setCrawlDelay?.(url, delaySeconds)) { + if (supportsDomainThrottling(this.requestManager) && this.requestManager.setCrawlDelay(url, delaySeconds)) { return; } diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 78fea2eb0676..a5ed96dfcb5c 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -66,6 +66,16 @@ export class RequestThrottledError extends RetryRequestError { } } +/** + * Thrown when a domain has rate-limited us for so long that no request has got through, and the crawl is + * abandoned rather than kept waiting. + * + * Waiting longer will not help: at this point the concurrency is too high for the domain, or it has blocked us. + * The affected requests are deliberately left in their queue, so re-running the crawl without purging storages + * resumes them once the domain recovers. + */ +export class PersistentRateLimitError extends CriticalError {} + /** * Errors of `SessionError` type retire the session associated with the request and trigger a regular retry. * diff --git a/packages/core/src/storages/request_manager_tandem.ts b/packages/core/src/storages/request_manager_tandem.ts index 80f6fe0a04e6..9ba885368d36 100644 --- a/packages/core/src/storages/request_manager_tandem.ts +++ b/packages/core/src/storages/request_manager_tandem.ts @@ -11,7 +11,6 @@ import type { RequestQueueOperationInfo, RequestQueueOperationOptions, } from './request_queue.js'; -import { ThrottlingRequestManager } from './throttling_request_manager.js'; /** * A request manager that combines a {@apilink IRequestLoader} (such as a `RequestList`) with a writable @@ -245,26 +244,4 @@ export class RequestManagerTandem implements IRequestManager { this.expectedRequestProcessingSecs = secs; await this.resolvedRequestManager?.setExpectedRequestProcessingTimeSecs?.(secs); } - - /** - * Forwards to the wrapped manager if it throttles. Reports `false` when it does not, or when it has not been - * resolved yet - a tandem resolves on first use, so the answer is only meaningful once crawling has started. - */ - setCrawlDelay(url: string, delaySeconds: number): boolean { - return this.throttlingRequestManager?.setCrawlDelay(url, delaySeconds) ?? false; - } - - /** - * Forwards to the wrapped manager if it throttles, otherwise reports `false`. - * @see {@apilink RequestManagerTandem.setCrawlDelay} - */ - recordDomainDelay(url: string, retryAfterMs?: number | null): boolean { - return this.throttlingRequestManager?.recordDomainDelay(url, retryAfterMs) ?? false; - } - - private get throttlingRequestManager(): ThrottlingRequestManager | undefined { - return this.resolvedRequestManager instanceof ThrottlingRequestManager - ? this.resolvedRequestManager - : undefined; - } } diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 466c75148d5a..a3a33fcf6e39 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -4,6 +4,7 @@ import type { Dictionary, ProcessedRequest } from '@crawlee/types'; import ow from 'ow'; import type { Configuration } from '../configuration.js'; +import { PersistentRateLimitError } from '../errors.js'; import { asyncifyIterable, chunkedAsyncIterable, peekableAsyncIterable } from '../iterables.js'; import type { CrawleeLogger } from '../log.js'; import type { Request, Source } from '../request.js'; @@ -31,6 +32,32 @@ export type RequestManagerOpener = options?: StorageOpenOptions, ) => Promise; +/** + * A request manager that can pace requests per domain, as {@apilink ThrottlingRequestManager} does. + * + * The crawlers detect this structurally rather than by type, so a wrapper can opt in by forwarding these three + * methods without {@apilink IRequestManager} having to know that throttling exists. + */ +export interface SupportsDomainThrottling { + /** @see {@apilink ThrottlingRequestManager.recordDomainDelay} */ + recordDomainDelay(url: string, retryAfterMs?: number | null): boolean; + /** @see {@apilink ThrottlingRequestManager.setCrawlDelay} */ + setCrawlDelay(url: string, delaySeconds: number): boolean; + /** @see {@apilink ThrottlingRequestManager.assertNoStalledDomains} */ + assertNoStalledDomains(): Promise; +} + +/** Whether `manager` can pace requests per domain. */ +export function supportsDomainThrottling(manager: unknown): manager is SupportsDomainThrottling { + const candidate = manager as Partial | null | undefined; + + return ( + typeof candidate?.recordDomainDelay === 'function' && + typeof candidate.setCrawlDelay === 'function' && + typeof candidate.assertNoStalledDomains === 'function' + ); +} + /** Options for {@apilink ThrottlingRequestManager}. */ export interface ThrottlingRequestManagerOptions { /** @@ -53,16 +80,27 @@ export interface ThrottlingRequestManagerOptions implements IRequestManager { +export class ThrottlingRequestManager + implements IRequestManager, SupportsDomainThrottling +{ private readonly inner: T; private readonly requestManagerOpener: RequestManagerOpener; private readonly baseDelayMs: number; private readonly maxDelayMs: number; + private readonly maxDomainStallMs: number; private readonly domainStates = new Map(); private readonly subManagers = new Map(); @@ -190,8 +233,9 @@ export class ThrottlingRequestManager RequestQueue.open(idOrAlias, opts) as unknown as Promise); - this.baseDelayMs = options.baseDelayMs ?? 2_000; - this.maxDelayMs = options.maxDelayMs ?? 60_000; + this.baseDelayMs = (options.baseDelaySecs ?? 2) * 1000; + this.maxDelayMs = (options.maxDelaySecs ?? 60) * 1000; + this.maxDomainStallMs = (options.maxDomainStallSecs ?? 900) * 1000; this.log = serviceLocator.getLogger().child({ prefix: 'ThrottlingRequestManager' }); + const now = Date.now(); + for (const domain of options.domains) { if (domain) { const hostname = domain.toLowerCase(); @@ -213,6 +260,7 @@ export class ThrottlingRequestManager { + await this.ensureSubManagers(); + + const now = Date.now(); + const candidates = Array.from(this.domainStates.values()).filter( + (state) => state.consecutive429Count > 0 && now - state.lastProgressAt > this.maxDomainStallMs, + ); + + const stalled = ( + await Promise.all( + candidates.map(async (state) => ((await this.subManagers.get(state.domain)!.isEmpty()) ? null : state)), + ) + ).filter((state) => state !== null); + + if (stalled.length === 0) { + return; + } + + const summary = stalled + .map((state) => `"${state.domain}" (${((now - state.lastProgressAt) / 1000).toFixed(0)}s)`) + .join(', '); + + throw new PersistentRateLimitError( + `Giving up: ${summary} rate-limited every request for longer than maxDomainStallSecs ` + + `(${(this.maxDomainStallMs / 1000).toFixed(0)}s). Waiting longer will not help - lower the ` + + `crawler's concurrency, or drop these domains. Their requests are still queued, so re-running ` + + `without purging storages will resume them if the rate limit lifts.`, + ); + } + + /** Records that a domain let a request through, which is what stall detection watches for. */ + private recordProgress(url: string): void { + const state = this.getDomainState(url); + if (state) { + state.lastProgressAt = Date.now(); + } + } + // --- IRequestManager Implementation --- async addRequest(requestLike: Source, options?: RequestQueueOperationOptions): Promise { @@ -514,6 +608,8 @@ export class ThrottlingRequestManager { const manager = await this.selectManager(request.url); + // Reached whether the request succeeded or ran out of retries; either way the domain answered us. + this.recordProgress(request.url); return manager.markRequestAsHandled(request); } @@ -559,11 +655,13 @@ export class ThrottlingRequestManager { await this.forEachManager((manager) => manager.purge?.()); + const now = Date.now(); for (const state of this.domainStates.values()) { state.consecutive429Count = 0; state.backoffUntil = 0; state.crawlDelayUntil = 0; state.backoffDecaysAt = 0; + state.lastProgressAt = now; } } diff --git a/test/core/crawlers/http_crawler.test.ts b/test/core/crawlers/http_crawler.test.ts index b7b92986cb95..e25d87a1768c 100644 --- a/test/core/crawlers/http_crawler.test.ts +++ b/test/core/crawlers/http_crawler.test.ts @@ -4,7 +4,14 @@ import { Readable } from 'node:stream'; import type { ConcurrencySystemOptions } from '@crawlee/core'; import { MemoryStorageBackend, serviceLocator } from '@crawlee/core'; -import { ConcurrencySystem, HttpCrawler, RequestQueue, SessionPool, ThrottlingRequestManager } from '@crawlee/http'; +import { + ConcurrencySystem, + HttpCrawler, + PersistentRateLimitError, + RequestQueue, + SessionPool, + ThrottlingRequestManager, +} from '@crawlee/http'; import { ResponseWithUrl } from '@crawlee/http-client'; import iconv from 'iconv-lite'; @@ -616,3 +623,32 @@ test('a 429 on a throttled domain paces the retry without spending it or the ses expect(retiredSessions).toEqual([]); expect(markedBad).toEqual([]); }, 30_000); + +test('a domain that never stops rate-limiting shuts the crawl down instead of hanging', async () => { + let hits = 0; + router.set('/always-429', (req, res) => { + hits++; + res.statusCode = 429; + res.end(); + }); + + const crawler = new HttpCrawler({ + requestManager: new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['127.0.0.1'], + baseDelaySecs: 0.05, + maxDelaySecs: 0.1, + maxDomainStallSecs: 2, + }), + maxRequestRetries: 0, + requestHandler: async () => {}, + }); + + // Without the stall detector this never resolves - a throttled request costs no retries. + await expect(crawler.run([`${url}/always-429`])).rejects.toThrow(PersistentRateLimitError); + + expect(hits).toBeGreaterThan(1); + + // The request is deliberately left queued, so a later run can pick it up if the rate limit lifts. + expect(await crawler.getRequestManager().then((manager) => manager.getPendingCount())).toBe(1); +}, 30_000); diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index dfafbf89aff5..eb66c3a52613 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -1,4 +1,4 @@ -import { MemoryStorageBackend, RequestQueue, serviceLocator } from '@crawlee/core'; +import { MemoryStorageBackend, PersistentRateLimitError, RequestQueue, serviceLocator } from '@crawlee/core'; import { ThrottlingRequestManager, parseRetryAfterHeader, @@ -194,7 +194,7 @@ describe('ThrottlingRequestManager', () => { const manager = new ThrottlingRequestManager({ inner, domains: ['example.com', 'foo.com'], - baseDelayMs: 100, + baseDelaySecs: 0.1, }); await manager.addRequest({ url: 'https://example.com/1' }); @@ -226,8 +226,8 @@ describe('ThrottlingRequestManager', () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: ['example.com'], - baseDelayMs: 50, - maxDelayMs: 60_000, + baseDelaySecs: 0.05, + maxDelaySecs: 60, }); // Eight requests were already in flight when the limit was hit; they all come back 429. @@ -242,8 +242,8 @@ describe('ThrottlingRequestManager', () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: ['example.com'], - baseDelayMs: 20, - maxDelayMs: 60_000, + baseDelaySecs: 0.02, + maxDelaySecs: 60, }); manager.recordDomainDelay('https://example.com/1'); @@ -257,11 +257,11 @@ describe('ThrottlingRequestManager', () => { expect(domainState(manager, 'example.com').consecutive429Count).toBe(1); }); - test('caps the delay at maxDelayMs', async () => { + test('caps the delay at maxDelaySecs', async () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: ['example.com'], - maxDelayMs: 1000, + maxDelaySecs: 1, }); manager.recordDomainDelay('https://example.com/1', 3_600_000); @@ -273,7 +273,7 @@ describe('ThrottlingRequestManager', () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: ['example.com'], - maxDelayMs: 60_000, + maxDelaySecs: 60, }); await manager.addRequest({ url: 'https://example.com/1' }); @@ -314,6 +314,60 @@ describe('ThrottlingRequestManager', () => { expect(await subQueue.getPendingCount()).toBe(0); }); + describe('stall detection', () => { + const stallingManager = async () => + new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + baseDelaySecs: 0.01, + maxDomainStallSecs: 30, + }); + + /** Ages the domain past its stall threshold. Backdating beats sleeping - a loaded CI box cannot race it. */ + const stallFor = (manager: ThrottlingRequestManager, domain: string) => { + domainState(manager, domain).lastProgressAt -= 60_000; + }; + + test('gives up on a domain that never lets a request through', async () => { + const manager = await stallingManager(); + await manager.addRequest({ url: 'https://example.com/1' }); + manager.recordDomainDelay('https://example.com/1'); + + await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + + stallFor(manager, 'example.com'); + await expect(manager.assertNoStalledDomains()).rejects.toThrow(PersistentRateLimitError); + }); + + test('a handled request resets the clock', async () => { + const manager = await stallingManager(); + await manager.addRequest({ url: 'https://example.com/1' }); + await manager.addRequest({ url: 'https://example.com/2' }); + manager.recordDomainDelay('https://example.com/1'); + stallFor(manager, 'example.com'); + + await manager.markRequestAsHandled((await pollForNextRequest(manager))!); + + await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + }); + + test('a domain that has run out of work is finished, not stalled', async () => { + const manager = await stallingManager(); + manager.recordDomainDelay('https://example.com/1'); + stallFor(manager, 'example.com'); + + await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + }); + + test('a domain that was never rate-limited is never stalled', async () => { + const manager = await stallingManager(); + await manager.addRequest({ url: 'https://example.com/1' }); + stallFor(manager, 'example.com'); + + await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + }); + }); + test('a crawl-delay does not swallow the 429 backoff', async () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), From 0ddc4527a1c4a92676d70303270989c3e1983f62 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sat, 8 Aug 2026 16:35:24 +0200 Subject: [PATCH 18/32] refactor(core): share one batched-add loop instead of two copies `ThrottlingRequestManager.addRequestsBatched` was a near-verbatim copy of `RequestQueue`'s: same budget-derived chunk size, same peek/next dance, same over-limit drain. The copy had already fallen behind - the transaction handling `RequestQueue` gained never reached it - so the loop now lives in `drainRequestBatches` and both callers supply only what actually differs, namely how a chunk is added and how the input is normalized. Fixes a latent bug in the process: `RequestQueue` ran its background batches in an `async` promise executor, which swallows throws. A failing background batch therefore left `waitForAllRequestsToBeAdded` pending forever and never decremented `inProgressRequestBatchCount`, so the queue reported itself unfinished indefinitely - now it rejects and settles. --- packages/core/src/storages/batched_adds.ts | 138 +++++++++++++++ packages/core/src/storages/request_queue.ts | 165 +++++------------- .../storages/throttling_request_manager.ts | 150 ++++++---------- test/core/storages/request_queue.test.ts | 29 +++ 4 files changed, 259 insertions(+), 223 deletions(-) create mode 100644 packages/core/src/storages/batched_adds.ts diff --git a/packages/core/src/storages/batched_adds.ts b/packages/core/src/storages/batched_adds.ts new file mode 100644 index 000000000000..4372d0904d67 --- /dev/null +++ b/packages/core/src/storages/batched_adds.ts @@ -0,0 +1,138 @@ +import { setTimeout as sleep } from 'node:timers/promises'; +import type { ProcessedRequest } from '@crawlee/types'; + +import { chunkedAsyncIterable, peekableAsyncIterable } from '../iterables.js'; +import type { Source } from '../request.js'; +import type { AddRequestsBatchedResult } from './request_queue.js'; + +export interface DrainRequestBatchesOptions { + /** + * The requests to add, already normalized by the caller. Consumed lazily: an unbounded or expensive + * iterable is only pulled from as far as the batching (and any `maxNewRequests` budget) requires. + */ + items: AsyncGenerator; + + batchSize: number; + waitBetweenBatchesMillis: number; + waitForAllRequestsToBeAdded: boolean; + maxNewRequests?: number; + + /** + * Adds a single chunk and reports what it processed. + * + * @param isInitial Whether this is the first chunk, which is added before this function returns. Later + * chunks land in the background, which is why some callers cache only the first. + */ + processChunk: (chunk: TItem[], isInitial: boolean) => Promise; + + /** + * Called with the promise covering every chunk after the first, so the caller can keep its own + * `isFinished` honest while batches are still landing. + */ + trackBackgroundBatches?: (batches: Promise) => void; + + /** + * Await the remaining chunks before returning even when the caller did not ask to. Set when the chunks + * carry state that must not outlive the current scope, regardless of what the caller wanted. + */ + alwaysAwaitRemainder?: boolean; + + /** + * Wraps the remaining chunks when they are *not* awaited, i.e. when they will outlive the caller's + * scope. Defaults to running them as-is. + */ + runDetachedRemainder?: (run: () => Promise) => Promise; +} + +/** + * Drives the chunk-by-chunk half of `addRequestsBatched`: the first chunk is added before returning and the + * rest continue in the background, paced by `waitBetweenBatchesMillis`. + * + * Callers differ only in how a chunk is added and how the input is normalized, so that is all + * {@apilink DrainRequestBatchesOptions} asks for - the budget arithmetic, the lazy chunking and the + * over-limit reporting are identical for everyone and live here. + */ +export async function drainRequestBatches( + options: DrainRequestBatchesOptions, +): Promise { + const { + items, + batchSize, + waitBetweenBatchesMillis, + waitForAllRequestsToBeAdded, + maxNewRequests, + processChunk, + trackBackgroundBatches, + alwaysAwaitRemainder = false, + runDetachedRemainder = async (run) => run(), + } = options; + + let remainingBudget = maxNewRequests ?? Infinity; + const requestsOverLimit: Source[] = []; + + // Never hand a chunk more than the budget allows, so an over-large final batch cannot overshoot. + const effectiveChunkSize = maxNewRequests !== undefined ? () => Math.min(batchSize, remainingBudget) : batchSize; + + const chunks = peekableAsyncIterable(chunkedAsyncIterable(items, effectiveChunkSize)); + const chunksIterator = chunks[Symbol.asyncIterator](); + + const addChunk = async (chunk: TItem[], isInitial: boolean) => { + const processedRequests = await processChunk(chunk, isInitial); + + if (maxNewRequests !== undefined) { + remainingBudget -= processedRequests.filter((request) => !request.wasAlreadyPresent).length; + } + + return processedRequests; + }; + + const buildResult = async ( + addedRequests: ProcessedRequest[], + waitForAll: Promise, + ): Promise => { + if (maxNewRequests !== undefined) { + // `chunkedAsyncIterable` stops pulling once the budget-derived chunk size hits zero, so whatever + // is left is still sitting in `items` rather than in a chunk we have seen. + for await (const item of items) { + requestsOverLimit.push(item); + } + } + + return { addedRequests, waitForAllRequestsToBeAdded: waitForAll, requestsOverLimit }; + }; + + const initialChunk = await chunksIterator.peek(); + if (initialChunk === undefined) { + return buildResult([], Promise.resolve([])); + } + + const addedRequests = await addChunk(initialChunk, true); + await chunksIterator.next(); + + if ((await chunksIterator.peek()) === undefined) { + return buildResult(addedRequests, Promise.resolve([])); + } + + const processRemainingChunks = async () => { + const added: ProcessedRequest[] = []; + for await (const chunk of chunks) { + added.push(...(await addChunk(chunk, false))); + await sleep(waitBetweenBatchesMillis); + } + return added; + }; + + // With a budget we must drain everything before we can report what went over it. + const awaitsRemainder = waitForAllRequestsToBeAdded || maxNewRequests !== undefined || alwaysAwaitRemainder; + const remainder = awaitsRemainder ? processRemainingChunks() : runDetachedRemainder(processRemainingChunks); + + // The caller is not obliged to await `remainder`, so give it a handler of its own - an unhandled + // rejection here would otherwise take the process down. + trackBackgroundBatches?.(remainder.catch(() => {})); + + if (awaitsRemainder) { + addedRequests.push(...(await remainder)); + } + + return buildResult(addedRequests, remainder); +} diff --git a/packages/core/src/storages/request_queue.ts b/packages/core/src/storages/request_queue.ts index fbf0f3a3002b..3d4fcaf6b8f6 100644 --- a/packages/core/src/storages/request_queue.ts +++ b/packages/core/src/storages/request_queue.ts @@ -11,7 +11,7 @@ import type { RequestQueueInfo, } from '@crawlee/types'; import { isAsyncIterable, isIterable } from '@crawlee/utils/internal'; -import { downloadListOfUrls, sleep } from '@crawlee/utils'; +import { downloadListOfUrls } from '@crawlee/utils'; import ow from 'ow'; import type { ReadonlyDeep } from 'type-fest'; @@ -22,7 +22,6 @@ import { Configuration } from '../configuration.js'; import { getObjectType } from '../debug.js'; import type { EventManager } from '../events/event_manager.js'; import { EventType } from '../events/event_manager.js'; -import { chunkedAsyncIterable, peekableAsyncIterable } from '../iterables.js'; import type { CrawleeLogger } from '../log.js'; import type { IProxyConfiguration } from '../proxy_configuration.js'; import type { InternalSource, RequestOptions, Source } from '../request.js'; @@ -30,6 +29,7 @@ import { Request } from '../request.js'; import { serviceLocator } from '../service_locator.js'; import type { JournalEntry, StorageTransaction } from './transaction.js'; import { activeStorageTransaction, rejectOperationInTransaction, withDirectStorageAccess } from './transaction.js'; +import { drainRequestBatches } from './batched_adds.js'; import type { IRequestManager, RequestsLike } from './request_manager.js'; import type { RequestQueueStats } from './storage_stats.js'; import { StorageStatsTracker } from './storage_stats.js'; @@ -630,133 +630,54 @@ export class RequestQueue implements IStorage, IRequestManager { } } - const { batchSize = 1000, maxNewRequests = undefined } = options; - // Under `deferred` no chunk performs backend I/O, so pacing them would only stall the handler. - const waitBetweenBatchesMillis = deferred ? 0 : (options.waitBetweenBatchesMillis ?? 1000); - - let remainingBudget = maxNewRequests ?? Infinity; - const requestsOverLimit: Source[] = []; - - // If there's a limit on the number of added requests, do not send batches bigger than the limit - const effectiveChunkSize = - maxNewRequests !== undefined ? () => Math.min(batchSize, remainingBudget) : batchSize; - - // Hold onto the underlying iterator so we can drain leftovers from it in buildResult - const requestIterator = generateRequests(); - - const chunks = peekableAsyncIterable( - chunkedAsyncIterable(requestIterator, effectiveChunkSize) as AsyncIterable, - ); - const chunksIterator = chunks[Symbol.asyncIterator](); - - /** - * Process a chunk: send it to the queue, then update the remaining budget if maxNewRequests is active. - * - * Requests the backend reports as unprocessed are warned about and skipped rather than retried: - * `unprocessedRequests` is what remains after the backend's own transient-error handling - a - * semantic rejection (e.g. a malformed `userData` shape) that re-sending would only re-poke. - * Retrying transient failures is the storage backend's job, not the frontend's. - */ - const processChunk = async (chunk: Source[], cache = true) => { - const { processedRequests, unprocessedRequests } = await this.addRequests(chunk, { - forefront: options.forefront, - cache, - }); - - if (unprocessedRequests.length > 0) { - this.log.warning( - 'Some requests were rejected by the request queue and will be skipped. ' + - "This usually means the request data is malformed (e.g. an invalid 'userData' shape).", - { unprocessedRequests }, - ); - } - - if (maxNewRequests !== undefined) { - remainingBudget -= processedRequests.filter((r) => !r.wasAlreadyPresent).length; - } - - return processedRequests; - }; + return drainRequestBatches({ + items: generateRequests(), + batchSize: options.batchSize ?? 1000, + // Under `deferred` no chunk performs backend I/O, so pacing them would only stall the handler. + waitBetweenBatchesMillis: deferred ? 0 : (options.waitBetweenBatchesMillis ?? 1000), + waitForAllRequestsToBeAdded: options.waitForAllRequestsToBeAdded ?? false, + maxNewRequests: options.maxNewRequests, + + /** + * Requests the backend reports as unprocessed are warned about and skipped rather than retried: + * `unprocessedRequests` is what remains after the backend's own transient-error handling - a + * semantic rejection (e.g. a malformed `userData` shape) that re-sending would only re-poke. + * Retrying transient failures is the storage backend's job, not the frontend's. + */ + processChunk: async (chunk, isInitial) => { + const { processedRequests, unprocessedRequests } = await this.addRequests(chunk, { + forefront: options.forefront, + cache: isInitial, + }); - /** - * Build the final result. When maxNewRequests is set, drains any remaining items - * from the underlying request iterator into requestsOverLimit. - * - * We accept the iterator explicitly (rather than closing over it) to make it obvious - * that this is the *same* iterator that `chunkedAsyncIterable` has been consuming — - * so only unconsumed items are drained. We drain `requestIterator` (not `chunks`) - * because `chunkedAsyncIterable` stops yielding when the budget-based chunk size - * drops to 0, leaving unconsumed items in the underlying iterator. - */ - const buildResult = async ( - addedRequests: ProcessedRequest[], - waitForAllRequestsToBeAdded: Promise, - unconsumedIterator: AsyncGenerator, - ): Promise => { - if (maxNewRequests !== undefined) { - for await (const request of unconsumedIterator) { - requestsOverLimit.push(request); + if (unprocessedRequests.length > 0) { + this.log.warning( + 'Some requests were rejected by the request queue and will be skipped. ' + + "This usually means the request data is malformed (e.g. an invalid 'userData' shape).", + { unprocessedRequests }, + ); } - } - - return { addedRequests, waitForAllRequestsToBeAdded, requestsOverLimit }; - }; - - // Add initial batch to process right away - const initialChunk = await chunksIterator.peek(); - if (initialChunk === undefined) { - return buildResult([], Promise.resolve([]), requestIterator); - } - - const addedRequests = await processChunk(initialChunk); - await chunksIterator.next(); - - // If we have no more requests to add (either exhausted or budget hit), return immediately - if ((await chunksIterator.peek()) === undefined) { - return buildResult(addedRequests, Promise.resolve([]), requestIterator); - } - - const processRemainingChunks = async () => { - const finalAddedRequests: ProcessedRequest[] = []; - for await (const requestChunk of chunks) { - finalAddedRequests.push(...(await processChunk(requestChunk, false))); - await sleep(waitBetweenBatchesMillis); - } + return processedRequests; + }, - return finalAddedRequests; - }; + // `deferred` must await the remainder - a writer that finishes after commit would have nowhere + // to put its journal entries. + alwaysAwaitRemainder: deferred, - // maxNewRequests needs all batches to report skipped requests accurately; `deferred` needs them - // too - a writer that finishes after commit would have nowhere to put its journal entries. - const awaitsRemainingChunks = options.waitForAllRequestsToBeAdded || maxNewRequests !== undefined || deferred; - - // eslint-disable-next-line no-async-promise-executor - const promise = new Promise(async (resolve) => { - if (awaitsRemainingChunks) { - // Awaited below, i.e. still within the caller's transaction scope, so the additions are - // journaled like the initial chunk - introspection must not depend on where the chunk - // boundary happened to fall. - resolve(await processRemainingChunks()); - } else { - // Nobody awaits this writer, so it outlives the transaction scope it inherits and must - // not record into a transaction that may already be closed. It writes directly - its - // write-through additions were never going to be rolled back anyway - which means the - // requests it adds are not journaled. See `StorageTransactionView.enqueuedUrls`. - resolve(await withDirectStorageAccess(processRemainingChunks)); - } - }); + // An un-awaited writer outlives the transaction scope it inherits, so it must not record into a + // transaction that may already be closed. It writes directly - its write-through additions were + // never going to be rolled back anyway - which means the requests it adds are not journaled. + // See `StorageTransactionView.enqueuedUrls`. + runDetachedRemainder: withDirectStorageAccess, - this.inProgressRequestBatchCount += 1; - void promise.finally(() => { - this.inProgressRequestBatchCount -= 1; + trackBackgroundBatches: (batches) => { + this.inProgressRequestBatchCount += 1; + void batches.finally(() => { + this.inProgressRequestBatchCount -= 1; + }); + }, }); - - if (awaitsRemainingChunks) { - addedRequests.push(...(await promise)); - } - - return buildResult(addedRequests, promise, requestIterator); } /** diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index a3a33fcf6e39..f6ab4e5cc4ea 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -1,14 +1,14 @@ import { URL } from 'node:url'; -import { setTimeout as sleep } from 'node:timers/promises'; -import type { Dictionary, ProcessedRequest } from '@crawlee/types'; +import type { Dictionary } from '@crawlee/types'; import ow from 'ow'; import type { Configuration } from '../configuration.js'; import { PersistentRateLimitError } from '../errors.js'; -import { asyncifyIterable, chunkedAsyncIterable, peekableAsyncIterable } from '../iterables.js'; +import { asyncifyIterable } from '../iterables.js'; import type { CrawleeLogger } from '../log.js'; import type { Request, Source } from '../request.js'; import { serviceLocator } from '../service_locator.js'; +import { drainRequestBatches } from './batched_adds.js'; import type { IRequestManager, RequestsLike } from './request_manager.js'; import type { AddRequestsBatchedOptions, @@ -494,108 +494,56 @@ export class ThrottlingRequestManager { await this.ensureSubManagers(); - const { batchSize = 1000, waitBetweenBatchesMillis = 1000, forefront, maxNewRequests } = options; - - let remainingBudget = maxNewRequests ?? Infinity; - const requestsOverLimit: Source[] = []; - - // Never hand a target more than the budget allows, so an over-large final batch cannot overshoot. - const effectiveChunkSize = - maxNewRequests !== undefined ? () => Math.min(batchSize, remainingBudget) : batchSize; - - // An async generator is both the iterator `chunkedAsyncIterable` consumes and an iterable we can drain - // leftovers from later - the same object, so only unconsumed requests end up over the limit. - async function* iterateRequests(): AsyncGenerator { - yield* asyncifyIterable(requests); - } - - const requestIterator = iterateRequests(); - const chunks = peekableAsyncIterable(chunkedAsyncIterable(requestIterator, effectiveChunkSize)); - const chunksIterator = chunks[Symbol.asyncIterator](); - - const processChunk = async (chunk: (Source | string)[]): Promise => { - const byManager = new Map(); - for (const request of chunk) { - const manager = this.managerForUrl(this.getUrlFromRequest(request)); - const bucket = byManager.get(manager); - if (bucket) { - bucket.push(request); - } else { - byManager.set(manager, [request]); - } - } - - const results = await Promise.all( - Array.from(byManager, ([manager, slice]) => - manager.addRequestsBatched(slice, { - forefront, - // The slice is already one batch, and we need its results before releasing the next one. - batchSize: slice.length, - waitForAllRequestsToBeAdded: true, - }), - ), - ); - - const processedRequests = results.flatMap((result) => result.addedRequests); - if (maxNewRequests !== undefined) { - remainingBudget -= processedRequests.filter((request) => !request.wasAlreadyPresent).length; - } - - return processedRequests; - }; - - const buildResult = async ( - addedRequests: ProcessedRequest[], - waitForAllRequestsToBeAdded: Promise, - ): Promise => { - if (maxNewRequests !== undefined) { - // `chunkedAsyncIterable` stops pulling once the budget-derived chunk size hits zero, so whatever - // is left is still sitting in the source iterator. - for await (const request of requestIterator) { - requestsOverLimit.push(typeof request === 'string' ? { url: request } : request); - } + // Normalized up front so the shared batching helper - and `requestsOverLimit` - only ever see `Source`. + async function* iterateRequests(): AsyncGenerator { + for await (const request of asyncifyIterable(requests)) { + yield typeof request === 'string' ? { url: request } : request; } - - return { addedRequests, waitForAllRequestsToBeAdded, requestsOverLimit }; - }; - - const initialChunk = await chunksIterator.peek(); - if (initialChunk === undefined) { - return buildResult([], Promise.resolve([])); - } - - const addedRequests = await processChunk(initialChunk); - await chunksIterator.next(); - - if ((await chunksIterator.peek()) === undefined) { - return buildResult(addedRequests, Promise.resolve([])); } - const remainder = (async () => { - const added: ProcessedRequest[] = []; - for await (const chunk of chunks) { - added.push(...(await processChunk(chunk))); - await sleep(waitBetweenBatchesMillis); - } - return added; - })(); - - // Keep the crawler from concluding it is finished while batches are still landing. The caller is not - // obliged to await `remainder`, so every derived promise needs its own handler - an unhandled rejection - // here would take the process down. - this.inProgressBatchCount += 1; - void remainder - .catch(() => {}) - .finally(() => { - this.inProgressBatchCount -= 1; - }); - - // With a budget we must drain everything before we can report what went over it. - if (options.waitForAllRequestsToBeAdded || maxNewRequests !== undefined) { - addedRequests.push(...(await remainder)); - } + return drainRequestBatches({ + items: iterateRequests(), + batchSize: options.batchSize ?? 1000, + waitBetweenBatchesMillis: options.waitBetweenBatchesMillis ?? 1000, + waitForAllRequestsToBeAdded: options.waitForAllRequestsToBeAdded ?? false, + maxNewRequests: options.maxNewRequests, + + // Routing is the only thing this manager adds; the targets do the batching, validation and + // deduplication themselves. + processChunk: async (chunk) => { + const byManager = new Map(); + for (const request of chunk) { + const manager = this.managerForUrl(this.getUrlFromRequest(request)); + const bucket = byManager.get(manager); + if (bucket) { + bucket.push(request); + } else { + byManager.set(manager, [request]); + } + } - return buildResult(addedRequests, remainder); + const results = await Promise.all( + Array.from(byManager, ([manager, slice]) => + manager.addRequestsBatched(slice, { + forefront: options.forefront, + // The slice is already one batch, and we need its results before releasing the next one. + batchSize: slice.length, + waitForAllRequestsToBeAdded: true, + }), + ), + ); + + return results.flatMap((result) => result.addedRequests); + }, + + // Keeps the crawler from concluding it is finished while batches are still landing. + trackBackgroundBatches: (batches) => { + this.inProgressBatchCount += 1; + void batches.finally(() => { + this.inProgressBatchCount -= 1; + }); + }, + }); } async reclaimRequest( diff --git a/test/core/storages/request_queue.test.ts b/test/core/storages/request_queue.test.ts index afdfa4ac901a..36977f30d9a5 100644 --- a/test/core/storages/request_queue.test.ts +++ b/test/core/storages/request_queue.test.ts @@ -654,3 +654,32 @@ describe('RequestQueue (request lifecycle)', () => { expect(retrievedUrls.map((x) => new URL(x).pathname)).toEqual(Array.from({ length: 5 }, (_, i) => `/${i + 1}`)); }); }); + +describe('RequestQueue background batches', () => { + beforeEach(async () => { + serviceLocator.setStorageBackend(new MemoryStorageBackend()); + }); + + test('a failing background batch rejects instead of hanging, and stops blocking isFinished', async () => { + const queue = await RequestQueue.open(); + + let batches = 0; + const original = queue.addRequests.bind(queue); + vitest.spyOn(queue, 'addRequests').mockImplementation(async (requests, options) => { + if (++batches > 1) throw new Error('backend exploded'); + return original(requests, options); + }); + + const result = await queue.addRequestsBatched( + [{ url: 'https://example.com/1' }, { url: 'https://example.com/2' }], + { batchSize: 1, waitBetweenBatchesMillis: 0 }, + ); + + // Previously the async promise executor swallowed the throw: this promise never settled at all. + await expect(result.waitForAllRequestsToBeAdded).rejects.toThrow('backend exploded'); + + // ...and the in-flight batch counter stayed stuck, so the queue claimed to be unfinished forever. + await sleep(10); + expect(queue['inProgressRequestBatchCount']).toBe(0); + }, 10_000); +}); From ace845ec965c835203dfb3d120dacc70ecf28bc9 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sat, 8 Aug 2026 16:35:33 +0200 Subject: [PATCH 19/32] test(basic-crawler): assert the robots.txt crawl-delay actually reaches the manager The test asserted that no warning was logged and that `setCrawlDelay` still returned `true` - both of which hold when the domain is merely configured. It passed with `applyCrawlDelay` deleted outright. Now it checks the recorded delay and that dispatch is paced by it. --- test/core/crawlers/basic_crawler.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/core/crawlers/basic_crawler.test.ts b/test/core/crawlers/basic_crawler.test.ts index d5f29f4430c0..99e51377609c 100644 --- a/test/core/crawlers/basic_crawler.test.ts +++ b/test/core/crawlers/basic_crawler.test.ts @@ -2476,10 +2476,18 @@ describe('BasicCrawler', () => { const crawler = crawlerWithCrawlDelay({ requestManager }); const warning = vitest.spyOn(crawler.log, 'warning').mockImplementation(() => {}); - await crawler.addRequests(['http://example.com/1']); + await crawler.addRequests(['http://example.com/1', 'http://example.com/2']); expect(warning).not.toHaveBeenCalled(); - expect(requestManager.setCrawlDelay('http://example.com/1', 999)).toBe(true); + + // The robots.txt `Crawl-delay: 5` must have reached the manager, not merely been survivable. + await requestManager.fetchNextRequest(); + const state = (requestManager as any).domainStates.get('example.com'); + expect(state.crawlDelayMs).toBe(5_000); + + // ...and it paces dispatch: the next request is held back rather than served immediately. + expect(state.crawlDelayUntil).toBeGreaterThan(Date.now() + 4_000); + expect(await requestManager.fetchNextRequest()).toBeNull(); }); test('warns when the request manager cannot honour it', async () => { From 2e07215cf43c7a21e689dc4e4e6faf17b09eaca8 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sat, 8 Aug 2026 16:41:45 +0200 Subject: [PATCH 20/32] docs: spell out that throttled domains bypass `blockedStatusCodes` A 429 from a domain a `ThrottlingRequestManager` covers is treated as a rate limit before `blockedStatusCodes` is consulted, so the list only governs the domains it does not cover. Worth saying plainly: removing 429 from the list is the obvious-looking way to stop sessions being retired when adopting throttling, and it neither helps nor is needed. --- docs/guides/session_management.mdx | 2 ++ docs/upgrading/upgrading_v4.md | 2 +- packages/basic-crawler/src/internals/basic-crawler.ts | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/guides/session_management.mdx b/docs/guides/session_management.mdx index e4a831400104..46cd827037a5 100644 --- a/docs/guides/session_management.mdx +++ b/docs/guides/session_management.mdx @@ -217,6 +217,8 @@ For sites that respond with a `200` page that is actually a bot wall (Cloudflare Retiring a session on HTTP 429 burns proxies without slowing anything down — the site is asking you to wait, not telling you the session is unwelcome. Wrap your request manager in a `ThrottlingRequestManager` to back off per domain instead of rotating; see [per-domain throttling](./request-loaders#per-domain-throttling). Sessions are left untouched for the domains it covers. +Those domains are handled as rate limits before `blockedStatusCodes` is consulted, so leave 429 in the list — removing it only changes what happens for domains the manager does not cover. + ::: ## Sharing a session pool between crawlers diff --git a/docs/upgrading/upgrading_v4.md b/docs/upgrading/upgrading_v4.md index 9a8560c91ecc..b6a680b8a1fc 100644 --- a/docs/upgrading/upgrading_v4.md +++ b/docs/upgrading/upgrading_v4.md @@ -1188,7 +1188,7 @@ const crawler = new CheerioCrawler({ }); ``` -For the domains you list, a 429 honours `Retry-After` (or backs off exponentially), holds only that domain's requests back, and leaves both the session and the request's retry budget untouched. Because those retries are free, a domain that never stops rate-limiting would keep the crawl alive indefinitely — so one that goes `maxDomainStallSecs` (15 minutes by default) without letting a single request through shuts the crawl down with a `PersistentRateLimitError`, leaving its requests queued for a later run. +For the domains you list, a 429 is treated as a rate limit before `blockedStatusCodes` is consulted at all — it honours `Retry-After` (or backs off exponentially), holds only that domain's requests back, and leaves both the session and the request's retry budget untouched. Removing 429 from `blockedStatusCodes` therefore only affects domains the manager does not cover; you do not need to touch it to adopt throttling. Because those retries are free, a domain that never stops rate-limiting would keep the crawl alive indefinitely — so one that goes `maxDomainStallSecs` (15 minutes by default) without letting a single request through shuts the crawl down with a `PersistentRateLimitError`, leaving its requests queued for a later run. It is also what enforces robots.txt `Crawl-delay` directives — with `respectRobotsTxtFile` enabled and no throttling manager covering the domain, the directive is ignored and the crawler warns about it. See the [request loaders guide](../guides/request-loaders#per-domain-throttling). diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index 6965d3a162a9..dde67e43a18a 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -423,6 +423,10 @@ export interface BasicCrawlerOptions< /** * HTTP status codes that indicate the session should be retired. + * + * A 429 from a domain covered by a {@apilink ThrottlingRequestManager} is handled as a rate limit before + * this is consulted, so removing 429 here only affects domains that manager does not cover. + * * @default [401, 403, 429] */ blockedStatusCodes?: number[]; From 430cc3b0ba3e6e558ef1e5bea29f876485f1ba8d Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 10:17:37 +0200 Subject: [PATCH 21/32] refactor(core): tidy up the throttling manager's public surface - `parseRetryAfterHeader` was public API exported from a storages module despite being a plain HTTP header parser, so it moves to `http.ts` alongside its test. - Adds `innerManager`, so a caller that hands the wrapper its queue can still get the queue back, matching crawlee-python's `inner`. - `domains` now rejects empty strings instead of quietly dropping them, which turned a typo into a silently unthrottled domain. - Makes the two remaining time-sensitive tests deterministic: the backoff decay test slept out 20ms windows and flaked under load, and the purge test never checked the sub-queue was non-empty to begin with. --- docs/public-api/crawlee-core.api.md | 1 + packages/core/src/http.ts | 32 ++++++++++ packages/core/src/index.ts | 1 + .../storages/throttling_request_manager.ts | 62 +++++-------------- test/core/http.test.ts | 29 +++++++++ .../throttling_request_manager.test.ts | 61 +++++++----------- 6 files changed, 102 insertions(+), 84 deletions(-) create mode 100644 packages/core/src/http.ts create mode 100644 test/core/http.test.ts diff --git a/docs/public-api/crawlee-core.api.md b/docs/public-api/crawlee-core.api.md index 8e98a50e0973..0342cd723f81 100644 --- a/docs/public-api/crawlee-core.api.md +++ b/docs/public-api/crawlee-core.api.md @@ -2156,6 +2156,7 @@ export class ThrottlingRequestManager; // (undocumented) getTotalCount(): Promise; + get innerManager(): T; isEmpty(): Promise; isFinished(): Promise; // (undocumented) diff --git a/packages/core/src/http.ts b/packages/core/src/http.ts new file mode 100644 index 000000000000..4f3696d46ce1 --- /dev/null +++ b/packages/core/src/http.ts @@ -0,0 +1,32 @@ +/** + * Parses a `Retry-After` response header into a delay in milliseconds. + * + * The header holds either a non-negative number of seconds or an HTTP-date. + * See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Retry-After). + * + * @returns The delay in milliseconds, or `null` if the header is absent, unparseable, or already elapsed. + */ +export function parseRetryAfterHeader(value?: string | null): number | null { + if (!value) { + return null; + } + + const trimmed = value.trim(); + + // Per the spec this is a `delay-seconds`: digits only, so a negative or fractional value is not one. + if (/^\d+$/.test(trimmed)) { + // `Retry-After: 0` names no future deadline, same as an HTTP-date that has already passed. Reporting it + // as a zero delay would leave the domain unthrottled while still counting as a rate-limit event, so the + // caller would defer the request for free and re-send it immediately. + const delayMs = Number(trimmed) * 1000; + return delayMs > 0 ? delayMs : null; + } + + const date = Date.parse(trimmed); + if (!Number.isNaN(date)) { + const delayMs = date - Date.now(); + return delayMs > 0 ? delayMs : null; + } + + return null; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index dcf7df4a57d7..29a05a552e49 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -17,5 +17,6 @@ export * from './storages/index.js'; export * from './memory-storage/index.js'; export * from './validators.js'; export * from './cookie_utils.js'; +export * from './http.js'; export * from './recoverable_state.js'; export type { StorageBackend } from '@crawlee/types'; diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index f6ab4e5cc4ea..78a4beb2dd29 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -127,39 +127,6 @@ function throttledUntil(state: DomainState): number { return Math.max(state.backoffUntil, state.crawlDelayUntil); } -/** - * Parses a `Retry-After` response header into a delay in milliseconds. - * - * The header holds either a non-negative number of seconds or an HTTP-date. - * See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Retry-After). - * - * @returns The delay in milliseconds, or `null` if the header is absent, unparseable, or already elapsed. - */ -export function parseRetryAfterHeader(value?: string | null): number | null { - if (!value) { - return null; - } - - const trimmed = value.trim(); - - // Per the spec this is a `delay-seconds`: digits only, so a negative or fractional value is not one. - if (/^\d+$/.test(trimmed)) { - // `Retry-After: 0` names no future deadline, same as an HTTP-date that has already passed. Reporting it - // as a zero delay would leave the domain unthrottled while still counting as a rate-limit event, so the - // caller would defer the request for free and re-send it immediately. - const delayMs = Number(trimmed) * 1000; - return delayMs > 0 ? delayMs : null; - } - - const date = Date.parse(trimmed); - if (!Number.isNaN(date)) { - const delayMs = date - Date.now(); - return delayMs > 0 ? delayMs : null; - } - - return null; -} - /** * A request manager that wraps another one and paces requests per domain. * @@ -231,7 +198,7 @@ export class ThrottlingRequestManager { + expect(parseRetryAfterHeader('120')).toBe(120_000); + expect(parseRetryAfterHeader(' 5 ')).toBe(5000); + // Zero-padded values are valid `delay-seconds`. + expect(parseRetryAfterHeader('05')).toBe(5000); + + // A zero delay names no deadline; reporting it as one would leave the domain unthrottled and busy-loop. + expect(parseRetryAfterHeader('0')).toBeNull(); + expect(parseRetryAfterHeader('00')).toBeNull(); + + // date format + const futureDate = new Date(Date.now() + 5000).toUTCString(); + const delay = parseRetryAfterHeader(futureDate); + expect(delay).toBeGreaterThan(0); + expect(delay).toBeLessThanOrEqual(5500); + + // A date in the past means "no delay", not a negative one. + expect(parseRetryAfterHeader(new Date(Date.now() - 5000).toUTCString())).toBeNull(); + + expect(parseRetryAfterHeader(null)).toBeNull(); + expect(parseRetryAfterHeader(undefined)).toBeNull(); + expect(parseRetryAfterHeader('')).toBeNull(); + expect(parseRetryAfterHeader('invalid')).toBeNull(); + // Not `delay-seconds`; a negative delay would have suppressed the backoff entirely. + expect(parseRetryAfterHeader('-5')).toBeNull(); + expect(parseRetryAfterHeader('1.5')).toBeNull(); +}); diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index eb66c3a52613..e561ca584f14 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -1,8 +1,10 @@ -import { MemoryStorageBackend, PersistentRateLimitError, RequestQueue, serviceLocator } from '@crawlee/core'; import { + MemoryStorageBackend, + PersistentRateLimitError, + RequestQueue, + serviceLocator, ThrottlingRequestManager, - parseRetryAfterHeader, -} from '../../../packages/core/src/storages/throttling_request_manager.js'; +} from '@crawlee/core'; import { sleep } from '@crawlee/utils'; describe('ThrottlingRequestManager', () => { @@ -31,34 +33,6 @@ describe('ThrottlingRequestManager', () => { throw new Error('Timed out waiting for a request to become available'); } - test('parseRetryAfterHeader parsing seconds and date', () => { - expect(parseRetryAfterHeader('120')).toBe(120_000); - expect(parseRetryAfterHeader(' 5 ')).toBe(5000); - // Zero-padded values are valid `delay-seconds`. - expect(parseRetryAfterHeader('05')).toBe(5000); - - // A zero delay names no deadline; reporting it as one would leave the domain unthrottled and busy-loop. - expect(parseRetryAfterHeader('0')).toBeNull(); - expect(parseRetryAfterHeader('00')).toBeNull(); - - // date format - const futureDate = new Date(Date.now() + 5000).toUTCString(); - const delay = parseRetryAfterHeader(futureDate); - expect(delay).toBeGreaterThan(0); - expect(delay).toBeLessThanOrEqual(5500); - - // A date in the past means "no delay", not a negative one. - expect(parseRetryAfterHeader(new Date(Date.now() - 5000).toUTCString())).toBeNull(); - - expect(parseRetryAfterHeader(null)).toBeNull(); - expect(parseRetryAfterHeader(undefined)).toBeNull(); - expect(parseRetryAfterHeader('')).toBeNull(); - expect(parseRetryAfterHeader('invalid')).toBeNull(); - // Not `delay-seconds`; a negative delay would have suppressed the backoff entirely. - expect(parseRetryAfterHeader('-5')).toBeNull(); - expect(parseRetryAfterHeader('1.5')).toBeNull(); - }); - test('Routing: requests to configured domains route to sub-managers, others to inner queue', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ @@ -242,19 +216,28 @@ describe('ThrottlingRequestManager', () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: ['example.com'], - baseDelaySecs: 0.02, + baseDelaySecs: 10, maxDelaySecs: 60, }); + const state = domainState(manager, 'example.com'); manager.recordDomainDelay('https://example.com/1'); - await sleep(30); + + // Rewinding both clocks beats sleeping out real delays - a loaded CI box cannot race it. + const rewind = (ms: number) => { + state.backoffUntil -= ms; + state.backoffDecaysAt -= ms; + }; + + // Past the backoff but still inside the decay window: the next 429 continues the same burst. + rewind(11_000); manager.recordDomainDelay('https://example.com/1'); - expect(domainState(manager, 'example.com').consecutive429Count).toBe(2); + expect(state.consecutive429Count).toBe(2); - // Wait out the delay plus a full extra window with no further 429. - await sleep(120); + // Past the decay window as well: the domain is treated as recovered and the exponent restarts. + rewind(41_000); manager.recordDomainDelay('https://example.com/1'); - expect(domainState(manager, 'example.com').consecutive429Count).toBe(1); + expect(state.consecutive429Count).toBe(1); }); test('caps the delay at maxDelaySecs', async () => { @@ -307,10 +290,12 @@ describe('ThrottlingRequestManager', () => { const firstRun = new ThrottlingRequestManager({ inner: await createQueue(), domains }); await firstRun.addRequest({ url: 'https://example.com/stale' }); + const subQueue = await RequestQueue.open({ alias: 'throttled-example.com' }); + expect(await subQueue.getPendingCount()).toBe(1); + const secondRun = new ThrottlingRequestManager({ inner: await createQueue(), domains }); await secondRun.purge(); - const subQueue = await RequestQueue.open({ alias: 'throttled-example.com' }); expect(await subQueue.getPendingCount()).toBe(0); }); From ffe2ef1055f87ad39d1deff38766a3e1dec80b30 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 10:18:36 +0200 Subject: [PATCH 22/32] refactor(browser-crawler): declare `headers()` on `BaseResponse` Reading the `Retry-After` header cast the response to a shape asserting the method exists and then optional-called it anyway. `BaseResponse` describes what the crawler relies on, so the method belongs there - optional, because only the Playwright and Puppeteer responses are guaranteed to carry it. --- docs/public-api/crawlee-browser.api.md | 1 + packages/browser-crawler/src/internals/browser-crawler.ts | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/public-api/crawlee-browser.api.md b/docs/public-api/crawlee-browser.api.md index 11104518533e..9d202ad41375 100644 --- a/docs/public-api/crawlee-browser.api.md +++ b/docs/public-api/crawlee-browser.api.md @@ -41,6 +41,7 @@ import { StringPredicate } from 'ow'; // Not exported by the entry point; reachable only as a referenced type. // @public (undocumented) interface BaseResponse { + headers?(): Record; // (undocumented) status(): number; } diff --git a/packages/browser-crawler/src/internals/browser-crawler.ts b/packages/browser-crawler/src/internals/browser-crawler.ts index 875b5d2b7499..a14b276ca17c 100644 --- a/packages/browser-crawler/src/internals/browser-crawler.ts +++ b/packages/browser-crawler/src/internals/browser-crawler.ts @@ -61,6 +61,8 @@ import type { BrowserLaunchContext } from './browser-launcher.js'; interface BaseResponse { status(): number; + /** Optional because only Playwright and Puppeteer responses are guaranteed to carry it. */ + headers?(): Record; } /** @@ -837,7 +839,7 @@ export abstract class BrowserCrawler< // rate limit the domain should back off from. if (status === 429) { // Both drivers lower-case header names and join duplicates, so a plain lookup is enough. - const retryAfter = (response as { headers?(): Record }).headers?.()['retry-after']; + const retryAfter = response.headers?.()['retry-after']; if (this.recordDomainRateLimit(crawlingContext.request.url, retryAfter)) { throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); } From 3255a778a5eba225507368167f0aac248a3f695d Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 10:18:37 +0200 Subject: [PATCH 23/32] docs(core): note that `isEmpty` may disagree with `getPendingCount` `isEmpty` answers what the next fetch would return, not how much work is left, so a loader that withholds requests for a while - as `ThrottlingRequestManager` does for a rate-limited domain - reports empty while requests are still queued. --- packages/core/src/storages/request_loader.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/core/src/storages/request_loader.ts b/packages/core/src/storages/request_loader.ts index 32687f6c929c..f68988b35b72 100644 --- a/packages/core/src/storages/request_loader.ts +++ b/packages/core/src/storages/request_loader.ts @@ -60,6 +60,11 @@ export interface IRequestLoader { * Resolves to `true` if the next call to {@apilink IRequestLoader.fetchNextRequest} function * would return `null`, otherwise it resolves to `false`. * Note that even if the loader is empty, there might be some pending requests currently being processed. + * + * This is a statement about what the *next fetch* would return, not about how much work is left, so it + * may report `true` while {@apilink IRequestLoader.getPendingCount} is non-zero - a loader that withholds + * requests for a while (as {@apilink ThrottlingRequestManager} does for a rate-limited domain) is empty + * for as long as it will not hand anything over. Use `isFinished()` to ask whether the work is done. */ isEmpty(): Promise; From 337baf814ba45a68bbc35edd26a1429c0092c9dc Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 12:00:47 +0200 Subject: [PATCH 24/32] fix: Ensure that scraping is paced on a per-domain basis, not per-task --- .../storages/throttling_request_manager.ts | 21 +++++---- .../throttling_request_manager.test.ts | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 78a4beb2dd29..6ec7beba5988 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -308,13 +308,6 @@ export class ThrottlingRequestManager(); if (request) { - this.markDomainDispatched(domain); return request; } + + // No dispatch to pace, so the domain keeps its slot. + state.crawlDelayUntil = crawlDelayBefore; } return this.inner.fetchNextRequest(); diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index e561ca584f14..697ed7db2e47 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -375,6 +375,49 @@ describe('ThrottlingRequestManager', () => { expect(await manager.fetchNextRequest()).toBeNull(); }); + test('concurrent fetches cannot dispatch past the crawl-delay', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + // Holds every dispatch open long enough that the other callers demonstrably run while one is in + // flight, rather than leaving the overlap to microtask ordering. + requestManagerOpener: async (identifier, options) => { + const queue = await RequestQueue.open(identifier, options); + const fetch = queue.fetchNextRequest.bind(queue); + queue.fetchNextRequest = async (...args) => { + await sleep(50); + return fetch(...args); + }; + return queue; + }, + domains: ['example.com'], + }); + + manager.setCrawlDelay('https://example.com/1', 60); + for (let i = 0; i < 5; i++) { + await manager.addRequest({ url: `https://example.com/${i}` }); + } + + // The task loop runs several tasks at once, each pulling its own request. + const fetched = await Promise.all(Array.from({ length: 5 }, async () => manager.fetchNextRequest())); + + expect(fetched.filter(Boolean)).toHaveLength(1); + }); + + test('a domain that hands over nothing does not spend its crawl-delay slot', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + }); + + manager.setCrawlDelay('https://example.com/1', 60); + + // Nothing queued yet, so there is no dispatch for the delay to pace. + expect(await manager.fetchNextRequest()).toBeNull(); + + await manager.addRequest({ url: 'https://example.com/1' }); + expect((await manager.fetchNextRequest())!.url).toBe('https://example.com/1'); + }); + test('setCrawlDelay sets crawl-delay successfully', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ From 51592455435936541ca43cfd21cefc33b676cf91 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 12:22:55 +0200 Subject: [PATCH 25/32] fix: Normalize hostnames --- .../storages/throttling_request_manager.ts | 19 +++++++++++--- packages/core/src/url.ts | 11 ++++++++ .../throttling_request_manager.test.ts | 26 +++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/url.ts diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 6ec7beba5988..a40b244f2e00 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -8,6 +8,7 @@ import { asyncifyIterable } from '../iterables.js'; import type { CrawleeLogger } from '../log.js'; import type { Request, Source } from '../request.js'; import { serviceLocator } from '../service_locator.js'; +import { normalizeHostname } from '../url.js'; import { drainRequestBatches } from './batched_adds.js'; import type { IRequestManager, RequestsLike } from './request_manager.js'; import type { @@ -69,6 +70,9 @@ export interface ThrottlingRequestManagerOptions { expect(await manager.fetchNextRequest()).toBeNull(); }); + test('Routing: a configured domain matches however its hostname is spelled', async () => { + const inner = await createQueue(); + const manager = new ThrottlingRequestManager({ + inner, + // A unicode, a punycode and a root-dotted spelling. + domains: ['HÁČKY.cz', 'xn--bcher-kva.example', 'example.com.', '[::1]'], + }); + + await manager.addRequest({ url: 'https://xn--hky-ela4t.cz/punycode' }); + await manager.addRequest({ url: 'https://háčky.cz./unicode-with-root-dot' }); + await manager.addRequest({ url: 'https://bücher.example/unicode' }); + await manager.addRequest({ url: 'https://example.com/no-root-dot' }); + await manager.addRequest({ url: 'http://[::1]:8080/bracketed' }); + + // Every one of them belongs to a configured domain, so none of them fell through to the inner queue. + expect(await inner.getTotalCount()).toBe(0); + expect(await manager.getTotalCount()).toBe(5); + }); + + test('rejects a domain that is not a hostname', async () => { + const inner = await createQueue(); + + // Unbracketed IPv6 - previously accepted, then silently matched nothing. + expect(() => new ThrottlingRequestManager({ inner, domains: ['::1'] })).toThrow(/not a valid hostname/); + }); + test('addRequestsBatched routing', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ From 0002e6dd0547133e9d984d105697b8e217c6757a Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 12:29:29 +0200 Subject: [PATCH 26/32] fix: Do not give up on stalled domains when keepAlive is enabled --- docs/guides/request_loaders.mdx | 2 +- docs/upgrading/upgrading_v4.md | 2 +- .../src/internals/basic-crawler.ts | 2 +- .../storages/throttling_request_manager.ts | 3 ++ test/core/crawlers/http_crawler.test.ts | 37 +++++++++++++++++++ 5 files changed, 43 insertions(+), 3 deletions(-) diff --git a/docs/guides/request_loaders.mdx b/docs/guides/request_loaders.mdx index a270fd619e52..949643c7416e 100644 --- a/docs/guides/request_loaders.mdx +++ b/docs/guides/request_loaders.mdx @@ -160,7 +160,7 @@ const crawler = new CheerioCrawler({ Requests for a listed domain are routed into their own queue as they are added. When one of those domains answers with a 429, the crawler honours its `Retry-After` header — or backs off exponentially from `baseDelaySecs` up to `maxDelaySecs` if there is none — and holds that domain's requests back for the duration. Requests for every other domain keep flowing at full speed, the throttled request is retried later without counting against `maxRequestRetries`, and its session is left alone, because a rate limit says nothing about the session. -Because a throttled request costs no retries, a domain that never stops rate-limiting would otherwise keep the crawl alive forever. If one goes `maxDomainStallSecs` without letting a single request through, the crawl shuts down with a `PersistentRateLimitError` — at that point the concurrency is too high for that domain, or it has blocked you outright, and waiting longer will not help. Its requests are left in their queue on purpose, so re-running the crawl without purging storages resumes them if the rate limit lifts. +Because a throttled request costs no retries, a domain that never stops rate-limiting would otherwise keep the crawl alive forever. If one goes `maxDomainStallSecs` without letting a single request through, the crawl shuts down with a `PersistentRateLimitError` — at that point the concurrency is too high for that domain, or it has blocked you outright, and waiting longer will not help. Its requests are left in their queue on purpose, so re-running the crawl without purging storages resumes them if the rate limit lifts. A crawler running with `keepAlive` is exempt, since staying up regardless is what it was asked to do. This is opt-in and exact: only the domains you list are throttled, and matching is case-insensitive with no wildcard support, so list each subdomain you care about. diff --git a/docs/upgrading/upgrading_v4.md b/docs/upgrading/upgrading_v4.md index b6a680b8a1fc..e8fbf8540877 100644 --- a/docs/upgrading/upgrading_v4.md +++ b/docs/upgrading/upgrading_v4.md @@ -1188,7 +1188,7 @@ const crawler = new CheerioCrawler({ }); ``` -For the domains you list, a 429 is treated as a rate limit before `blockedStatusCodes` is consulted at all — it honours `Retry-After` (or backs off exponentially), holds only that domain's requests back, and leaves both the session and the request's retry budget untouched. Removing 429 from `blockedStatusCodes` therefore only affects domains the manager does not cover; you do not need to touch it to adopt throttling. Because those retries are free, a domain that never stops rate-limiting would keep the crawl alive indefinitely — so one that goes `maxDomainStallSecs` (15 minutes by default) without letting a single request through shuts the crawl down with a `PersistentRateLimitError`, leaving its requests queued for a later run. +For the domains you list, a 429 is treated as a rate limit before `blockedStatusCodes` is consulted at all — it honours `Retry-After` (or backs off exponentially), holds only that domain's requests back, and leaves both the session and the request's retry budget untouched. Removing 429 from `blockedStatusCodes` therefore only affects domains the manager does not cover; you do not need to touch it to adopt throttling. Because those retries are free, a domain that never stops rate-limiting would keep the crawl alive indefinitely — so one that goes `maxDomainStallSecs` (15 minutes by default) without letting a single request through shuts the crawl down with a `PersistentRateLimitError`, leaving its requests queued for a later run — unless `keepAlive` is set, which exempts the crawl. It is also what enforces robots.txt `Crawl-delay` directives — with `respectRobotsTxtFile` enabled and no throttling manager covering the domain, the directive is ignored and the crawler warns about it. See the [request loaders guide](../guides/request-loaders#per-domain-throttling). diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index dde67e43a18a..3781d7712baa 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -1232,7 +1232,7 @@ export class BasicCrawler< // Checked here because this runs only once nothing is in flight, which is exactly when a // crawl that cannot progress looks indistinguishable from one that is merely waiting. - if (supportsDomainThrottling(this.requestManager)) { + if (!keepAlive && supportsDomainThrottling(this.requestManager)) { await this.requestManager.assertNoStalledDomains(); } diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index a40b244f2e00..4ef383aaa156 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -102,6 +102,9 @@ export interface ThrottlingRequestManagerOptions(); @@ -652,3 +653,39 @@ test('a domain that never stops rate-limiting shuts the crawl down instead of ha // The request is deliberately left queued, so a later run can pick it up if the rate limit lifts. expect(await crawler.getRequestManager().then((manager) => manager.getPendingCount())).toBe(1); }, 30_000); + +test('`keepAlive` outlives a domain that never stops rate-limiting', async () => { + router.set('/always-429-keep-alive', (req, res) => { + res.statusCode = 429; + res.end(); + }); + + const crawler = new HttpCrawler({ + requestManager: new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['127.0.0.1'], + baseDelaySecs: 0.05, + maxDelaySecs: 0.1, + maxDomainStallSecs: 0.5, + }), + keepAlive: true, + maxRequestRetries: 0, + requestHandler: async () => {}, + }); + + const running = crawler.run([`${url}/always-429-keep-alive`]); + + // Several times the stall threshold - long enough that the shutdown would have fired by now. + const outcome = await Promise.race([ + running.then( + () => 'shut down', + () => 'threw', + ), + sleep(3000).then(() => 'still running' as const), + ]); + + expect(outcome).toBe('still running'); + + await crawler.teardown(); + await running; +}, 30_000); From acd50aca917adf4179f499302735d04e4ec50540 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 12:33:13 +0200 Subject: [PATCH 27/32] fix: Fix validation rules --- packages/core/src/storages/throttling_request_manager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 4ef383aaa156..39297e81da25 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -207,9 +207,9 @@ export class ThrottlingRequestManager Date: Sun, 9 Aug 2026 12:50:21 +0200 Subject: [PATCH 28/32] fix: Improve rate limit tracking --- .../storages/throttling_request_manager.ts | 20 +++++++++++++++++-- .../throttling_request_manager.test.ts | 13 ++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 39297e81da25..1aa5c4a4ceff 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -127,6 +127,11 @@ interface DomainState { crawlDelayMs: number | null; /** When this domain last let a request through, as a `Date.now()` timestamp. Drives stall detection. */ lastProgressAt: number; + /** + * When this domain last answered 429, as a `Date.now()` timestamp, or `0` if it never has. Read alongside + * `lastProgressAt` to tell a domain that is rate-limiting us from one that is merely being waited out. + */ + lastRateLimitedAt: number; } /** The moment a domain may be dispatched to again - whichever of its two independent clocks runs longer. */ @@ -244,6 +249,7 @@ export class ThrottlingRequestManager { await this.ensureSubManagers(); const now = Date.now(); const candidates = Array.from(this.domainStates.values()).filter( - (state) => state.consecutive429Count > 0 && now - state.lastProgressAt > this.maxDomainStallMs, + // Both clocks are read over the same window, so this says: it turned us away within the window, and + // over that whole window it never once let us through. + (state) => + now - state.lastRateLimitedAt <= this.maxDomainStallMs && + now - state.lastProgressAt > this.maxDomainStallMs, ); const stalled = ( @@ -589,6 +604,7 @@ export class ThrottlingRequestManager { await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); }); + + test('a domain that stopped rate-limiting a while ago is being waited out, not stalled', async () => { + const manager = await stallingManager(); + await manager.addRequest({ url: 'https://example.com/1' }); + + // A single old 429, and nothing since - which is what a `Crawl-delay` longer than the stall window + // looks like. The domain is not turning us away, we are keeping our distance from it. + manager.recordDomainDelay('https://example.com/1'); + stallFor(manager, 'example.com'); + domainState(manager, 'example.com').lastRateLimitedAt -= 60_000; + + await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + }); }); test('a crawl-delay does not swallow the 429 backoff', async () => { From 9d1cf48d4b359e19aac36fb80f5a021980256cd7 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 12:59:52 +0200 Subject: [PATCH 29/32] chore: Random cleanup --- .../src/internals/basic-crawler.ts | 19 +++++++---------- .../storages/throttling_request_manager.ts | 21 ++++++++++--------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index 3781d7712baa..b93244b61cce 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -1896,16 +1896,9 @@ export class BasicCrawler< }); } - private logOncePerRun(key: string, message: string): void { + private logOncePerRun(key: string, message: string, level: 'info' | 'warning' = 'info'): void { if (!this.#loggedPerRun.has(key)) { - this.log.info(message); - this.#loggedPerRun.add(key); - } - } - - private warnOncePerRun(key: string, message: string): void { - if (!this.#loggedPerRun.has(key)) { - this.log.warning(message); + this.log[level](message); this.#loggedPerRun.add(key); } } @@ -2273,12 +2266,13 @@ export class BasicCrawler< } const domain = hostnameOrUrl(url); - this.warnOncePerRun( + this.logOncePerRun( `rateLimitNotThrottled:${domain}`, `"${domain}" responded with HTTP 429 (Too Many Requests), but nothing is set up to back off from it, ` + - 'so the request will be retried without a per-domain delay and its session will be retired. ' + + 'so the response is handled like any other, with no per-domain delay. ' + `Pass a \`ThrottlingRequestManager\` as \`requestManager\` and include "${domain}" in its \`domains\` ` + 'option to honour `Retry-After` and apply exponential backoff instead.', + 'warning', ); return false; @@ -2296,11 +2290,12 @@ export class BasicCrawler< } const domain = hostnameOrUrl(url); - this.warnOncePerRun( + this.logOncePerRun( `crawlDelayIgnored:${domain}`, `robots.txt for "${domain}" defines a crawl-delay of ${delaySeconds}s, but nothing is set up to honour it, ` + 'so requests to that domain will not be paced. Pass a `ThrottlingRequestManager` as `requestManager` ' + `and include "${domain}" in its \`domains\` option to enforce the delay.`, + 'warning', ); } diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index 1aa5c4a4ceff..e483351adf31 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -259,11 +259,8 @@ export class ThrottlingRequestManager { const subManager = await this.requestManagerOpener( - { alias: `throttled-${domain}` }, + // Backends use the alias as a directory name, and an IPv6 literal is full of characters + // Windows will not accept. Ordinary hostnames survive this untouched. + { alias: `throttled-${encodeURIComponent(domain)}` }, { configuration: this.config }, ); this.subManagers.set(domain, subManager); @@ -471,7 +468,9 @@ export class ThrottlingRequestManager { - const manager = await this.selectManager(this.getUrlFromRequest(requestLike)); + this.warnIfNotRoutable(requestLike); + + const manager = await this.selectManager(requestLike.url ?? ''); return manager.addRequest(requestLike, options); } @@ -507,7 +506,9 @@ export class ThrottlingRequestManager { const byManager = new Map(); for (const request of chunk) { - const manager = this.managerForUrl(this.getUrlFromRequest(request)); + this.warnIfNotRoutable(request); + + const manager = this.managerForUrl(request.url ?? ''); const bucket = byManager.get(manager); if (bucket) { bucket.push(request); From d80e4a9d2ee39d2889d74792b61b52828f4ade29 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 13:26:52 +0200 Subject: [PATCH 30/32] fix: Improve domain stall detection --- .../storages/throttling_request_manager.ts | 34 +++++++++++-------- .../throttling_request_manager.test.ts | 26 ++++++++++++-- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index e483351adf31..6db5f7efc206 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -125,11 +125,15 @@ interface DomainState { consecutive429Count: number; /** Minimum interval between dispatches, from a robots.txt `Crawl-delay` directive. */ crawlDelayMs: number | null; - /** When this domain last let a request through, as a `Date.now()` timestamp. Drives stall detection. */ - lastProgressAt: number; + /** + * When the current unbroken run of 429s began, as a `Date.now()` timestamp, or `0` if the domain is not + * currently rate-limiting us. Cleared the moment a request gets through, so it measures how long we have + * been stonewalled rather than how long the domain has been quiet. + */ + rateLimitedSince: number; /** * When this domain last answered 429, as a `Date.now()` timestamp, or `0` if it never has. Read alongside - * `lastProgressAt` to tell a domain that is rate-limiting us from one that is merely being waited out. + * `rateLimitedSince` to tell a domain that is still turning us away from one that is merely being waited out. */ lastRateLimitedAt: number; } @@ -227,8 +231,6 @@ export class ThrottlingRequestManager + state.rateLimitedSince !== 0 && now - state.lastRateLimitedAt <= this.maxDomainStallMs && - now - state.lastProgressAt > this.maxDomainStallMs, + now - state.rateLimitedSince > this.maxDomainStallMs, ); const stalled = ( @@ -446,7 +453,7 @@ export class ThrottlingRequestManager `"${state.domain}" (${((now - state.lastProgressAt) / 1000).toFixed(0)}s)`) + .map((state) => `"${state.domain}" (${((now - state.rateLimitedSince) / 1000).toFixed(0)}s)`) .join(', '); throw new PersistentRateLimitError( @@ -457,11 +464,11 @@ export class ThrottlingRequestManager { await this.forEachManager((manager) => manager.purge?.()); - const now = Date.now(); for (const state of this.domainStates.values()) { state.consecutive429Count = 0; state.backoffUntil = 0; state.crawlDelayUntil = 0; state.backoffDecaysAt = 0; - state.lastProgressAt = now; + state.rateLimitedSince = 0; state.lastRateLimitedAt = 0; } } diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 1a87ae758720..7df219ccf19a 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -334,9 +334,12 @@ describe('ThrottlingRequestManager', () => { maxDomainStallSecs: 30, }); - /** Ages the domain past its stall threshold. Backdating beats sleeping - a loaded CI box cannot race it. */ + /** + * Ages the domain's ongoing run of 429s past the stall threshold. Backdating beats sleeping - a loaded + * CI box cannot race it. + */ const stallFor = (manager: ThrottlingRequestManager, domain: string) => { - domainState(manager, domain).lastProgressAt -= 60_000; + domainState(manager, domain).rateLimitedSince -= 60_000; }; test('gives up on a domain that never lets a request through', async () => { @@ -370,10 +373,27 @@ describe('ThrottlingRequestManager', () => { await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); }); + test('a domain that has been idle for longer than the window is not stalled by its first 429', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + baseDelaySecs: 0.01, + maxDomainStallSecs: 0.05, + }); + await manager.addRequest({ url: 'https://example.com/1' }); + + // The crawl spent longer than the whole stall window elsewhere before this domain was touched. + await sleep(100); + + // The first 429 starts the clock - it does not arrive with the idle time already on it. + manager.recordDomainDelay('https://example.com/1'); + + await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + }); + test('a domain that was never rate-limited is never stalled', async () => { const manager = await stallingManager(); await manager.addRequest({ url: 'https://example.com/1' }); - stallFor(manager, 'example.com'); await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); }); From a2b408c95fb0c1ee8cf31a021f1d0de50376a141 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 14:21:40 +0200 Subject: [PATCH 31/32] fix: Prevent race conditions in batched request addition --- packages/core/src/storages/batched_adds.ts | 38 ++++++++--------- packages/core/src/storages/request_queue.ts | 18 +------- .../throttling_request_manager.test.ts | 41 +++++++++++++++++++ 3 files changed, 61 insertions(+), 36 deletions(-) diff --git a/packages/core/src/storages/batched_adds.ts b/packages/core/src/storages/batched_adds.ts index 4372d0904d67..ecc51ffb8aa6 100644 --- a/packages/core/src/storages/batched_adds.ts +++ b/packages/core/src/storages/batched_adds.ts @@ -4,6 +4,7 @@ import type { ProcessedRequest } from '@crawlee/types'; import { chunkedAsyncIterable, peekableAsyncIterable } from '../iterables.js'; import type { Source } from '../request.js'; import type { AddRequestsBatchedResult } from './request_queue.js'; +import { activeStorageTransaction, withDirectStorageAccess } from './transaction.js'; export interface DrainRequestBatchesOptions { /** @@ -30,18 +31,6 @@ export interface DrainRequestBatchesOptions { * `isFinished` honest while batches are still landing. */ trackBackgroundBatches?: (batches: Promise) => void; - - /** - * Await the remaining chunks before returning even when the caller did not ask to. Set when the chunks - * carry state that must not outlive the current scope, regardless of what the caller wanted. - */ - alwaysAwaitRemainder?: boolean; - - /** - * Wraps the remaining chunks when they are *not* awaited, i.e. when they will outlive the caller's - * scope. Defaults to running them as-is. - */ - runDetachedRemainder?: (run: () => Promise) => Promise; } /** @@ -49,8 +38,10 @@ export interface DrainRequestBatchesOptions { * rest continue in the background, paced by `waitBetweenBatchesMillis`. * * Callers differ only in how a chunk is added and how the input is normalized, so that is all - * {@apilink DrainRequestBatchesOptions} asks for - the budget arithmetic, the lazy chunking and the - * over-limit reporting are identical for everyone and live here. + * {@apilink DrainRequestBatchesOptions} asks for - the budget arithmetic, the lazy chunking, the + * over-limit reporting and the transaction handling are identical for everyone and live here. In + * particular, every caller has to keep its background chunks out of a transaction they will outlive, + * so that is read from the ambient transaction rather than asked of the caller. */ export async function drainRequestBatches( options: DrainRequestBatchesOptions, @@ -63,10 +54,10 @@ export async function drainRequestBatches( maxNewRequests, processChunk, trackBackgroundBatches, - alwaysAwaitRemainder = false, - runDetachedRemainder = async (run) => run(), } = options; + const deferred = activeStorageTransaction()?.policy.requestQueue === 'deferred'; + let remainingBudget = maxNewRequests ?? Infinity; const requestsOverLimit: Source[] = []; @@ -117,14 +108,21 @@ export async function drainRequestBatches( const added: ProcessedRequest[] = []; for await (const chunk of chunks) { added.push(...(await addChunk(chunk, false))); - await sleep(waitBetweenBatchesMillis); + // Under `deferred` no chunk performs backend I/O, so pacing them would only stall the handler. + await sleep(deferred ? 0 : waitBetweenBatchesMillis); } return added; }; - // With a budget we must drain everything before we can report what went over it. - const awaitsRemainder = waitForAllRequestsToBeAdded || maxNewRequests !== undefined || alwaysAwaitRemainder; - const remainder = awaitsRemainder ? processRemainingChunks() : runDetachedRemainder(processRemainingChunks); + // With a budget we must drain everything before we can report what went over it; under `deferred` a + // writer that finishes after commit would have nowhere to put its journal entries. + const awaitsRemainder = waitForAllRequestsToBeAdded || maxNewRequests !== undefined || deferred; + + // An un-awaited writer outlives the transaction scope it inherits, so it must not record into a + // transaction that may already be closed. It writes directly - its write-through additions were never + // going to be rolled back anyway - which means the requests it adds are not journaled. + // See `StorageTransactionView.enqueuedUrls`. + const remainder = awaitsRemainder ? processRemainingChunks() : withDirectStorageAccess(processRemainingChunks); // The caller is not obliged to await `remainder`, so give it a handler of its own - an unhandled // rejection here would otherwise take the process down. diff --git a/packages/core/src/storages/request_queue.ts b/packages/core/src/storages/request_queue.ts index 3d4fcaf6b8f6..df28b5b55c8a 100644 --- a/packages/core/src/storages/request_queue.ts +++ b/packages/core/src/storages/request_queue.ts @@ -28,7 +28,7 @@ import type { InternalSource, RequestOptions, Source } from '../request.js'; import { Request } from '../request.js'; import { serviceLocator } from '../service_locator.js'; import type { JournalEntry, StorageTransaction } from './transaction.js'; -import { activeStorageTransaction, rejectOperationInTransaction, withDirectStorageAccess } from './transaction.js'; +import { activeStorageTransaction, rejectOperationInTransaction } from './transaction.js'; import { drainRequestBatches } from './batched_adds.js'; import type { IRequestManager, RequestsLike } from './request_manager.js'; import type { RequestQueueStats } from './storage_stats.js'; @@ -571,9 +571,6 @@ export class RequestQueue implements IStorage, IRequestManager { requests: ReadonlyDeep, options: AddRequestsBatchedOptions = {}, ): Promise { - const transaction = activeStorageTransaction(); - const deferred = transaction?.policy.requestQueue === 'deferred'; - ow( requests, ow.object @@ -633,8 +630,7 @@ export class RequestQueue implements IStorage, IRequestManager { return drainRequestBatches({ items: generateRequests(), batchSize: options.batchSize ?? 1000, - // Under `deferred` no chunk performs backend I/O, so pacing them would only stall the handler. - waitBetweenBatchesMillis: deferred ? 0 : (options.waitBetweenBatchesMillis ?? 1000), + waitBetweenBatchesMillis: options.waitBetweenBatchesMillis ?? 1000, waitForAllRequestsToBeAdded: options.waitForAllRequestsToBeAdded ?? false, maxNewRequests: options.maxNewRequests, @@ -661,16 +657,6 @@ export class RequestQueue implements IStorage, IRequestManager { return processedRequests; }, - // `deferred` must await the remainder - a writer that finishes after commit would have nowhere - // to put its journal entries. - alwaysAwaitRemainder: deferred, - - // An un-awaited writer outlives the transaction scope it inherits, so it must not record into a - // transaction that may already be closed. It writes directly - its write-through additions were - // never going to be rolled back anyway - which means the requests it adds are not journaled. - // See `StorageTransactionView.enqueuedUrls`. - runDetachedRemainder: withDirectStorageAccess, - trackBackgroundBatches: (batches) => { this.inProgressRequestBatchCount += 1; void batches.finally(() => { diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 7df219ccf19a..4b542ae2ca32 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -1,9 +1,11 @@ +import type { AddRequestsBatchedResult } from '@crawlee/core'; import { MemoryStorageBackend, PersistentRateLimitError, RequestQueue, serviceLocator, ThrottlingRequestManager, + withStorageTransaction, } from '@crawlee/core'; import { sleep } from '@crawlee/utils'; @@ -173,6 +175,45 @@ describe('ThrottlingRequestManager', () => { await expect(result.waitForAllRequestsToBeAdded).rejects.toThrow('backend exploded'); }); + describe('addRequestsBatched in a transaction', () => { + const sixRequests = Array.from({ length: 6 }, (_, i) => ({ url: `https://example.com/${i}` })); + + test('deferred: every batch is rolled back, not just the first', async () => { + const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: ['example.com'] }); + + let result!: AddRequestsBatchedResult; + await withStorageTransaction( + async (transaction) => { + result = await manager.addRequestsBatched(sixRequests, { + batchSize: 2, + waitBetweenBatchesMillis: 0, + }); + transaction.rollback(); + }, + { policy: { requestQueue: 'deferred' } }, + ); + await result.waitForAllRequestsToBeAdded; + + expect(await manager.getTotalCount()).toBe(0); + }); + + test('write-through: batches that outlive the transaction stay out of its journal', async () => { + const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: ['example.com'] }); + + await withStorageTransaction(async (transaction) => { + const result = await manager.addRequestsBatched(sixRequests, { + batchSize: 2, + waitBetweenBatchesMillis: 0, + }); + await result.waitForAllRequestsToBeAdded; + + // Only the initial batch was added within the transaction's scope; the rest writes directly, + // exactly as a bare `RequestQueue` does. + expect(transaction.enqueuedUrls).toHaveLength(2); + }); + }); + }); + test('warns that requestsFromUrl sources cannot be domain-routed', async () => { const manager = new ThrottlingRequestManager({ inner: await createQueue(), From d18d72c7ecd51c1d2e309bacf450af9231775ca1 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Sun, 9 Aug 2026 15:05:08 +0200 Subject: [PATCH 32/32] fix: Consume http response body streams of throttled requests --- packages/http-crawler/src/internals/http-crawler.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/http-crawler/src/internals/http-crawler.ts b/packages/http-crawler/src/internals/http-crawler.ts index f500d57c7bd4..e40ac25de9db 100644 --- a/packages/http-crawler/src/internals/http-crawler.ts +++ b/packages/http-crawler/src/internals/http-crawler.ts @@ -577,6 +577,9 @@ export class HttpCrawler< if (crawlingContext.response.status === 429) { const retryAfter = crawlingContext.response.headers.get('retry-after'); if (this.recordDomainRateLimit(crawlingContext.request.url, retryAfter)) { + // This is the one path that never reads the body, so cancel it to release the connection + // rather than leaving it to the garbage collector. + await crawlingContext.response.body?.cancel().catch(() => {}); throw new RequestThrottledError(`${crawlingContext.request.url} responded with 429.`); } }