diff --git a/docs/guides/request_loaders.mdx b/docs/guides/request_loaders.mdx index 57a9794dc729..00c53a5264a9 100644 --- a/docs/guides/request_loaders.mdx +++ b/docs/guides/request_loaders.mdx @@ -24,7 +24,7 @@ Request loaders extend the functionality of the `IRequestLoader`: The base interface for reading requests in a crawl. -- `IRequestManager`: Extends `IRequestLoader` with write capabilities (adding and reclaiming requests). +- `IRequestManager`: Extends `IRequestLoader` with write capabilities (adding and reclaiming requests), and with the pacing signals a crawler reports back. - `RequestManagerTandem`: Combines a read-only `IRequestLoader` with a writable `IRequestManager`. - `ThrottlingRequestManager`: Wraps a writable `IRequestManager` and paces requests per domain. @@ -55,8 +55,7 @@ class IRequestLoader { + getHandledCount() + fetchNextRequest() + markRequestAsHandled() - + isEmpty() - + isFinished() + + checkReadiness() + toTandem() } @@ -65,6 +64,7 @@ class IRequestManager { + addRequest() + addRequestsBatched() + reclaimRequest() + + recordPacingSignal() + purge() } @@ -102,7 +102,7 @@ A crawler reads its requests from a single `IRequestLoader` interface defines the foundation for fetching requests during a crawl. It provides methods for basic operations like retrieving the next request, marking requests as handled, and checking whether the loader is empty or finished. It is intentionally **read-only** — it does not allow adding new requests. Concrete implementations such as `RequestList` build on this interface to handle specific scenarios. You can create your own custom loader that reads from an external file, web endpoint, database, or any other data source. +The `IRequestLoader` interface defines the foundation for fetching requests during a crawl. It provides methods for basic operations like retrieving the next request, marking requests as handled, and reporting whether the loader has a request ready, is waiting on one, or is done. It is intentionally **read-only** — it does not allow adding new requests. Concrete implementations such as `RequestList` build on this interface to handle specific scenarios. You can create your own custom loader that reads from an external file, web endpoint, database, or any other data source. ### Request list @@ -134,18 +134,27 @@ 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. +### Pacing signals + +A manager decides *when* each request goes out, so it is also where a crawler reports back what a site said about the pace it wants to be crawled at. That arrives through one method, `recordPacingSignal()`: a source refused a request because we were going too fast (`reason: 'rateLimited'`, optionally carrying the wait it asked for), or a source declared a standing floor on how often it may be requested (`reason: 'minInterval'`, carrying that interval). Nothing in the payload names HTTP status codes, response headers or robots.txt — where a signal came from is the crawler's business, not the manager's. + +Every signal also carries a `scope`: how much of the URL space it covers. `'hostname'` and `'registrableDomain'` are what Crawlee's own reporters send, and the type suggests them, but it accepts any string — so a pacer keyed on something other than a host, an account or an API key say, can be reported to in its own vocabulary. A manager may apply a signal to a **wider** scope than it was given, since a floor that holds for one host still holds when a whole site is paced by it, but never to a narrower one, which would leave part of what the signal covers running unpaced. + +A third `reason`, `minIntervalEverywhere`, is a floor under the pace of **every** domain the manager dispatches to, declared by whoever owns the crawl rather than by a source — which is why it is the one variant carrying no `url`. It is how a crawler offers a manager its `sameDomainDelaySecs`: whatever paces takes the floor, and only when nothing does the crawler add a pacer of its own. + +Two things follow for implementors. The method is required, so reporting is never a question of support: a manager that does not pace — a plain queue — returns `false` and the crawler warns that the signal was dropped, while one that **wraps** another forwards it, as `RequestManagerTandem` does, or a nested pacer goes deaf. And forwarding needs no knowledge of the payload, which is why this is one method taking a value — and why what a signal applies to travels inside that value. + ## 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: +The `ThrottlingRequestManager` handles it at the scheduling layer instead. List the domains you want paced: ```ts -import { CheerioCrawler, RequestQueue, ThrottlingRequestManager } from 'crawlee'; +import { CheerioCrawler, ThrottlingRequestManager } from 'crawlee'; const crawler = new CheerioCrawler({ requestManager: new ThrottlingRequestManager({ - inner: await RequestQueue.open(), domains: ['api.example.com'], // optional, these are the defaults baseDelaySecs: 2, @@ -158,11 +167,15 @@ const crawler = new CheerioCrawler({ }); ``` +Requests for domains it does not pace go to the default request queue, opened on first use. Pass `inner` to wrap a manager of your own instead — a queue you opened, or a [tandem](#request-manager-tandem) over a `requestList`. + +The pacer works wherever you put it: pass it to the crawler directly as above, or nest it inside a [tandem](#request-manager-tandem) as the writable side of a loader — the tandem forwards the crawler's [pacing signals](#pacing-signals) to the manager it wraps, so listed domains are paced either way. + 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 with `purgeOnStart` disabled 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. -Matching is exact and case-insensitive, with no wildcard support, so list each subdomain you care about — or set `throttleBy: 'registrableDomain'`, which groups a site and all of its subdomains under a single set of clocks. +Matching is exact and case-insensitive, with no wildcard support, so list each subdomain you care about — or set `throttleBy: 'registrableDomain'`, which groups a site and all of its subdomains under a single set of clocks. That grouping is also the finest granularity this manager can pace at, since holding a domain back means holding its queue back: a [pacing signal](#pacing-signals) scoped more narrowly is applied to the whole group, and one scoped more widely throws rather than being quietly under-applied. ### The two clocks @@ -178,7 +191,6 @@ Every throttled domain runs two of them, and is dispatched to once both have run ```ts const crawler = new CheerioCrawler({ requestManager: new ThrottlingRequestManager({ - inner: await RequestQueue.open(), domains: 'all', minCrawlDelaySecs: 1, throttleBy: 'registrableDomain', @@ -189,7 +201,7 @@ const crawler = new CheerioCrawler({ }); ``` -This is what the crawler's own `sameDomainDelaySecs` option is built on — it wraps the crawler's request manager in a `ThrottlingRequestManager` configured exactly like the above. Dropping `minCrawlDelaySecs` is just as useful: 429 backoff and robots.txt `Crawl-delay` then apply to every domain, with no pacing of your own on top. +This is what the crawler's own `sameDomainDelaySecs` option is built on — with no manager of your own to report the floor to, it wraps the crawler's request manager in a `ThrottlingRequestManager` configured exactly like the above. Pass a manager like this one yourself and it takes the floor instead, so your configuration keeps the crawl to one clock per domain. Dropping `minCrawlDelaySecs` is just as useful: 429 backoff and robots.txt `Crawl-delay` then apply to every domain, with no pacing of your own on top. A queue per domain is not free, so a run may only throttle `maxThrottledDomains` of them (100 by default) before it throws. If you are crawling more domains than that, pace the crawl with `maxRequestsPerMinute` instead. The list of domains discovered so far is kept in the default key-value store, under `persistStateKey`, so that a restart reopens their queues rather than leaving whatever they still hold uncrawled. diff --git a/docs/public-api/crawlee-core.api.md b/docs/public-api/crawlee-core.api.md index e0ed42a631d4..785777d49b16 100644 --- a/docs/public-api/crawlee-core.api.md +++ b/docs/public-api/crawlee-core.api.md @@ -24,6 +24,7 @@ import type { ISession } from '@crawlee/types'; import type { ISessionPool } from '@crawlee/types'; import type { KeyValueStoreBackend } from '@crawlee/types'; import type { KeyValueStoreInfo } from '@crawlee/types'; +import type { LiteralUnion } from 'type-fest'; import { Log } from '@apify/log'; import log from '@apify/log'; import { Logger } from '@apify/log'; @@ -739,12 +740,11 @@ export interface IProxyConfiguration { // @public export interface IRequestLoader { [Symbol.asyncIterator](): AsyncGenerator; + checkReadiness(): Promise; fetchNextRequest(): Promise | null>; getHandledCount(): Promise; getPendingCount(): Promise; getTotalCount(): Promise; - isEmpty(): Promise; - isFinished(): Promise; markRequestAsHandled(request: Request_2): Promise; persistState?(): Promise; toTandem?(requestManager?: IRequestManager): Promise; @@ -758,6 +758,7 @@ export interface IRequestManager extends IRequestLoader { addRequestsBatched(requests: RequestsLike, options?: AddRequestsBatchedOptions): Promise; purge?(): Promise; reclaimRequest(request: Request_2, options?: RequestQueueOperationOptions): Promise; + recordPacingSignal(signal: PacingSignal): boolean; setExpectedRequestProcessingTimeSecs?(secs: number): Promise; } @@ -1031,6 +1032,26 @@ interface NewUrlOptions { export class NonRetryableError extends Error { } +// @public +export type PacingScope = LiteralUnion<'hostname' | 'registrableDomain', string>; + +// @public +export type PacingSignal = { + reason: 'rateLimited'; + url: string; + waitMs?: number; + scope?: PacingScope; +} | { + reason: 'minInterval'; + url: string; + intervalMs: number; + scope: PacingScope; +} | { + reason: 'minIntervalEverywhere'; + intervalMs: number; + scope: PacingScope; +}; + // @public export function parseRetryAfterHeader(value?: string | null): number | null; @@ -1179,6 +1200,8 @@ export class RequestList implements IRequestLoader { // (undocumented) [Symbol.asyncIterator](): AsyncGenerator, void, unknown>; // (undocumented) + checkReadiness(): Promise; + // (undocumented) fetchNextRequest(): Promise; // (undocumented) getHandledCount(): Promise; @@ -1186,10 +1209,6 @@ export class RequestList implements IRequestLoader { getState(): RequestListState; getTotalCount(): Promise; // (undocumented) - isEmpty(): Promise; - // (undocumented) - isFinished(): Promise; - // (undocumented) markRequestAsHandled(request: Request_2): Promise; static open(listNameOrOptions: string | null | RequestListOptions, sources?: RequestListSource[], options?: RequestListOptions): Promise; // (undocumented) @@ -1225,7 +1244,12 @@ export interface RequestListState { } // @public -export type RequestManagerOpener = (identifier: string | StorageIdentifier, options?: StorageOpenOptions) => Promise; +export type RequestLoaderStatus = Exclude; + +// @public +export type RequestManagerOpener = (identifier?: string | StorageIdentifier | null, options?: StorageOpenOptions) => Promise; // @public export class RequestManagerTandem implements IRequestManager { @@ -1236,6 +1260,7 @@ export class RequestManagerTandem implements IRequestManager { addRequest(requestLike: Source, options?: RequestQueueOperationOptions): Promise; // (undocumented) addRequestsBatched(requests: RequestsLike, options?: AddRequestsBatchedOptions): Promise; + checkReadiness(): Promise; fetchNextRequest(): Promise | null>; // (undocumented) getHandledCount(): Promise; @@ -1244,15 +1269,12 @@ export class RequestManagerTandem implements IRequestManager { // (undocumented) getTotalCount(): Promise; // (undocumented) - isEmpty(): Promise; - // (undocumented) - isFinished(): Promise; - // (undocumented) markRequestAsHandled(request: Request_2): Promise; persistState(): Promise; purge(): Promise; // (undocumented) reclaimRequest(request: Request_2, options?: RequestQueueOperationOptions): Promise; + recordPacingSignal(signal: PacingSignal): boolean; setExpectedRequestProcessingTimeSecs(secs: number): Promise; } @@ -1284,6 +1306,7 @@ export class RequestQueue implements IStorage, IRequestManager { addRequestsBatched(requests: ReadonlyDeep, options?: AddRequestsBatchedOptions): Promise; // (undocumented) readonly backend: RequestQueueBackend; + checkReadiness(): Promise; drop(): Promise; fetchNextRequest(): Promise | null>; getHandledCount(): Promise; @@ -1293,8 +1316,6 @@ export class RequestQueue implements IStorage, IRequestManager { getTotalCount(): Promise; // (undocumented) readonly id: string; - isEmpty(): Promise; - isFinished(): Promise; // (undocumented) readonly log: CrawleeLogger; markRequestAsHandled(request: Request_2): Promise; @@ -1303,6 +1324,7 @@ export class RequestQueue implements IStorage, IRequestManager { static open(identifier?: string | StorageIdentifier | null, options?: StorageOpenOptions): Promise; purge(): Promise; reclaimRequest(request: Request_2, options?: RequestQueueOperationOptions): Promise; + recordPacingSignal(_signal: PacingSignal): boolean; setExpectedRequestProcessingTimeSecs(secs: number): Promise; get stats(): RequestQueueStats; } @@ -1402,6 +1424,19 @@ export interface RequestQueueStats { // @public (undocumented) export type RequestsLike = AsyncIterable | Iterable | (Source | string)[]; +// @public +export type RequestSourceStatus = { + status: 'ready'; +} | { + status: 'waiting'; + readyAt?: number; +} | { + status: 'stalled'; + reason: string; +} | { + status: 'finished'; +}; + // @public (undocumented) export enum RequestState { // (undocumented) @@ -1725,6 +1760,8 @@ export class SitemapRequestLoader implements IRequestLoader { // (undocumented) [Symbol.asyncIterator](): AsyncGenerator, void, unknown>; // (undocumented) + checkReadiness(): Promise; + // (undocumented) fetchNextRequest(): Promise; // (undocumented) getHandledCount(): Promise; @@ -1732,10 +1769,6 @@ export class SitemapRequestLoader implements IRequestLoader { getPendingCount(): Promise; // (undocumented) getTotalCount(): Promise; - // (undocumented) - isEmpty(): Promise; - // (undocumented) - isFinished(): Promise; isSitemapFullyLoaded(): boolean; // (undocumented) markRequestAsHandled(request: Request_2): Promise; @@ -2031,19 +2064,6 @@ export interface StorageWritePolicy { requestQueue: StorageWriteMode; } -// @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 type SyncStateConversion = ((value: TFrom) => TTo) | StandardSchemaV1; @@ -2077,14 +2097,14 @@ export interface TaskLoopPredicates { } // @public -export class ThrottlingRequestManager implements IRequestManager, SupportsDomainThrottling { +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; - assertNoStalledDomains(): Promise; + checkReadiness(): Promise; // (undocumented) drop(): Promise; fetchNextRequest(): Promise | null>; @@ -2094,19 +2114,15 @@ export class ThrottlingRequestManager; // (undocumented) getTotalCount(): Promise; - get innerManager(): T; - isEmpty(): Promise; - isFinished(): Promise; + get innerManager(): T | undefined; // (undocumented) markRequestAsHandled(request: Request_2): Promise; // (undocumented) persistState(): Promise; purge(): Promise; - purgeDomainQueues(): Promise; // (undocumented) reclaimRequest(request: Request_2, options?: RequestQueueOperationOptions): Promise; - recordDomainDelay(url: string, retryAfterMs?: number | null): boolean; - setCrawlDelay(url: string, delaySeconds: number): boolean; + recordPacingSignal(signal: PacingSignal): boolean; // (undocumented) setExpectedRequestProcessingTimeSecs(secs: number): Promise; } @@ -2115,7 +2131,7 @@ export class ThrottlingRequestManager { baseDelaySecs?: number; domains: string[] | 'all'; - inner: T; + inner?: T | (() => T | Promise); maxDelaySecs?: number; maxDomainStallSecs?: number; maxThrottledDomains?: number; diff --git a/docs/public-api/crawlee-playwright.api.md b/docs/public-api/crawlee-playwright.api.md index c7c33ba67146..8aac6b5d7013 100644 --- a/docs/public-api/crawlee-playwright.api.md +++ b/docs/public-api/crawlee-playwright.api.md @@ -146,9 +146,6 @@ interface BlockRequestsOptions { // @public (undocumented) type ClickOptions = Parameters[1]; -// @public (undocumented) -function closeCookieModals(page: Page): Promise; - // @public (undocumented) type CompiledScriptFunction = (params: CompiledScriptParams) => Promise; @@ -296,7 +293,6 @@ declare namespace playwrightClickElements { // @public (undocumented) interface PlaywrightContextUtils { blockRequests(options?: BlockRequestsOptions): Promise; - closeCookieModals(): Promise; compileScript(scriptString: string, ctx?: Dictionary): CompiledScriptFunction; enqueueLinksByClickingElements(options: Omit): Promise; handleCloudflareChallenge(options?: HandleCloudflareChallengeOptions): Promise; @@ -364,7 +360,6 @@ declare namespace playwrightUtils { infiniteScroll, saveSnapshot, parseWithCheerio, - closeCookieModals, InjectFileOptions, BlockRequestsOptions, PlaywrightDirectNavigationOptions as DirectNavigationOptions, diff --git a/docs/public-api/crawlee-puppeteer.api.md b/docs/public-api/crawlee-puppeteer.api.md index 857ab7669db3..725adc754798 100644 --- a/docs/public-api/crawlee-puppeteer.api.md +++ b/docs/public-api/crawlee-puppeteer.api.md @@ -58,9 +58,6 @@ const blockResources: (page: Page, resourceTypes?: string[]) => Promise; // @public @deprecated function cacheResponses(page: Page, cache: Dictionary>, responseUrlRules: (string | RegExp)[]): Promise; -// @public (undocumented) -function closeCookieModals(page: Page): Promise; - // @public (undocumented) export type CompiledScriptFunction = (params: CompiledScriptParams) => Promise; @@ -175,7 +172,6 @@ declare namespace puppeteerClickElements { interface PuppeteerContextUtils { addInterceptRequestHandler(handler: InterceptHandler): Promise; blockRequests(options?: BlockRequestsOptions): Promise; - closeCookieModals(): Promise; compileScript(scriptString: string, ctx?: Dictionary): CompiledScriptFunction; enqueueLinksByClickingElements(options: Omit): Promise; infiniteScroll(options?: InfiniteScrollOptions): Promise; @@ -250,7 +246,6 @@ declare namespace puppeteerUtils { gotoExtended, infiniteScroll, saveSnapshot, - closeCookieModals, PuppeteerDirectNavigationOptions as DirectNavigationOptions, InjectFileOptions, BlockRequestsOptions, diff --git a/docs/upgrading/upgrading_v4.md b/docs/upgrading/upgrading_v4.md index 62f74fd28f54..b7312d3e5f7d 100644 --- a/docs/upgrading/upgrading_v4.md +++ b/docs/upgrading/upgrading_v4.md @@ -47,6 +47,7 @@ The purely mechanical renames, collected in one place. Where a row links to a se | `RobotsFile` | `RobotsTxtFile` | | `markRequestHandled()` | `markRequestAsHandled()` | | `requestList.length()` / `requestList.handledCount()` | `await getTotalCount()` / `await getHandledCount()` | +| `requestList.isEmpty()` / `requestList.isFinished()` | `await checkReadiness()` ([details](#isempty--isfinished-replaced-by-checkreadiness)) | | `Dataset.listItems()` | `Dataset.getData()` / `Dataset.values()` ([details](#datasetlistitems-replaced-by-datasetgetdata-and-datasetvalues)) | | crawler options `requestList` / `requestQueue` | `requestManager` ([details](#crawler-requestlist--requestqueue-options-deprecated-in-favor-of-requestmanager)) | | `enqueueLinks({ requestQueue })` | `enqueueLinks({ requestManager })` | @@ -555,6 +556,10 @@ const crawler = new CheerioCrawler({ `extendContext` runs **before navigation**, so the members it returns are visible to the `preNavigationHooks`, `postNavigationHooks`, and the `requestHandler` alike. As a consequence, the `context` passed to `extendContext` is the pre-navigation context and does **not** include navigation-dependent members (e.g. `page`, `response`, `$`, `body`). If your extension needs to read those, do it in a `postNavigationHook` or the `requestHandler` instead. +#### Crawling context no longer includes `closeCookieModals` + +The `closeCookieModals` context helper is removed from the Playwright and Puppeteer crawlers, along with the `playwrightUtils.closeCookieModals` / `puppeteerUtils.closeCookieModals` functions and the optional `idcac-playwright` peer dependency they were built on. + ### Crawling context is strictly typed Previously, the crawling context extended a `Record` type, allowing to access any property. This was changed to a strict type, which means that you can only access properties that are defined in the context. @@ -610,6 +615,8 @@ By default, only queues that the crawler created itself (the "owned" queue) are | `true` | Purged | Purged | | `false` | Not purged | Not purged | +One combination has no sensible default: `sameDomainDelaySecs` over a request manager you supplied that does not pace on its own. The per-domain queues that have to be emptied are the crawler's, the manager underneath them is yours, and a purge cannot respect both — so a repeated `run()` throws and asks you to pass `purgeRequestQueue` explicitly rather than guessing. A manager that takes the delay as a floor has nothing of ours underneath it, and is left alone like any other supplied manager. + ```typescript // The purge happens automatically between run() calls: const crawler = new BasicCrawler({ requestHandler: async ({ request }) => { /* ... */ } }); @@ -1389,6 +1396,7 @@ The harmonized loader interface differs from the old `IRequestList` in a few way | _(n/a)_ | `getPendingCount(): Promise` (new) | | `handledCount(): number` | `getHandledCount(): Promise` (renamed and now async) | | `markRequestHandled(request)` | `markRequestAsHandled(request)` (renamed) | +| `isEmpty(): Promise` and `isFinished(): Promise` | `checkReadiness(): Promise` ([details](#isempty--isfinished-replaced-by-checkreadiness)) | | `reclaimRequest()` on the interface | Removed from the read-only loaders entirely; reclaiming is a write operation that lives only on `IRequestManager` (e.g. `RequestQueue`, `RequestManagerTandem`) | | `inProgress: Set` on the interface | Removed from the interface | | `persistState(): Promise` (required) | `persistState?(): Promise` (optional) | @@ -1410,6 +1418,59 @@ const total = await requestList.getTotalCount(); const handled = await requestList.getHandledCount(); ``` +#### `IRequestManager` gained `recordPacingSignal()` + +v3 could not tell a request source that a domain wants to be left alone: a 429 only retired the session, and a robots.txt `Crawl-delay` was not enforced at all. One member now carries all of it: + +```typescript +recordPacingSignal(signal: PacingSignal): boolean; + +type PacingSignal = + // the source turned a request away because we were going too fast + | { reason: 'rateLimited'; url: string; waitMs?: number; scope?: PacingScope } + // it declared a standing floor on how often it may be requested + | { reason: 'minInterval'; url: string; intervalMs: number; scope: PacingScope } + // the operator asked for a floor under every domain, `sameDomainDelaySecs` being one + | { reason: 'minIntervalEverywhere'; intervalMs: number; scope: PacingScope }; + +// suggests the two Crawlee itself uses, accepts any string +type PacingScope = LiteralUnion<'hostname' | 'registrableDomain', string>; +``` + +The crawler reports a 429 (with `Retry-After` if the response carried one), a robots.txt `Crawl-delay`, and its own `sameDomainDelaySecs`. Nothing in the payload names the mechanism, so a manager never learns where a signal came from, and `true` means it took responsibility — which is how the crawler knows to treat a rate limit as a paced retry rather than a blocked response. Delays are in milliseconds. + +If you implement the interface: + +- Return `false` when you do not pace, and forward the value when you wrap a manager that might. The method is required so that reporting is never a question of support, and a pacer nested in a composition still has to hear about it. +- Apply a signal at a **wider** `scope` than you were given if you must — a per-host floor still holds when the whole site is paced by it — never a narrower one, and throw on a scope you cannot honour instead of under-applying it. `ThrottlingRequestManager` groups by `throttleBy`, so it widens `'hostname'` signals and throws on anything wider or on a vocabulary it does not speak. +- `minIntervalEverywhere` covers every domain you dispatch to, which is why it is the variant with no `url`. Take it only if you pace all of them; throw if you pace some. + +#### `isEmpty()` / `isFinished()` replaced by `checkReadiness()` + +The two predicates v3 put on `IRequestList` and `IRequestManager` (and on `RequestList`, `RequestQueue` and `RequestProvider`) are replaced by a single `checkReadiness()` call, on `IRequestLoader`, `IRequestManager` and every implementation: + +```typescript +type RequestSourceStatus = + | { status: 'ready' } // a fetch is expected to hand something over (v3: `!isEmpty()`) + | { status: 'waiting'; readyAt?: number } // nothing now, not done (v3: `isEmpty() && !isFinished()`) + | { status: 'stalled'; reason: string } // holding requests it cannot make progress on + | { status: 'finished' }; // nothing left at all (v3: `isFinished()`) +``` + +```diff +-if (!(await manager.isEmpty())) { /* fetch */ } ++if ((await manager.checkReadiness()).status === 'ready') { /* fetch */ } + +-if (await manager.isFinished()) { /* stop */ } ++if ((await manager.checkReadiness()).status === 'finished') { /* stop */ } +``` + +One probe instead of two, which a task loop runs several times a second, plus two answers the booleans could not express: `waiting` can name when it expects work again (`readyAt`) instead of leaving the caller to poll, and `stalled` reports requests a source cannot make progress on, which the crawler turns into a `PersistentRateLimitError`. + +If you implemented either interface, return `ready` without evaluating anything further — it is the most common answer and the only one a caller can act on immediately. Reading from two sources, the precedence is `ready` > `stalled` > `waiting` > `finished`, and a combined `waiting` carries the earlier `readyAt`. + +**Storage backends keep the two booleans** — see [`StorageBackend` interface simplified](#storagebackend-interface-simplified). + #### Combining a list and a queue: `toTandem()` `RequestList` and `SitemapRequestLoader` now expose a `toTandem()` helper that pairs the read-only loader with a writable request manager (the default `RequestQueue` if none is passed), producing a `RequestManagerTandem` you can hand to a crawler via the new `requestManager` option: @@ -1470,24 +1531,23 @@ A lone `requestList` now runs through a tandem over an auto-opened queue (rather ```typescript const crawler = new CheerioCrawler({ requestManager: new ThrottlingRequestManager({ - inner: await RequestQueue.open(), domains: ['api.example.com'], }), requestHandler, }); ``` -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. +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, where outliving such a domain is the point. 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). #### `sameDomainDelaySecs` is now backed by `ThrottlingRequestManager` -The option means the same thing as in v3 — subdomains included, it still paces a whole site rather than a single host — but it no longer holds delayed requests in memory and re-enqueues them. The crawler now wraps its request manager in a `ThrottlingRequestManager`, which gives every domain a request queue of its own so a delayed request waits in storage. Consequences worth knowing about: +`sameDomainDelaySecs` still works and still means what it did in v3 — subdomains included, it paces a whole registrable domain rather than a single host. Underneath, it is now a floor reported to the crawler's request manager as a [pacing signal](#irequestmanager-gained-recordpacingsignal); only when nothing there paces does the crawler wrap its manager in a `ThrottlingRequestManager`, which gives each domain a queue of its own so a delayed request waits in storage rather than in an in-memory map. Consequences worth knowing about: - A crawl that discovers more than `maxThrottledDomains` domains (100 by default) throws instead of quietly running out of steam. Pass your own `ThrottlingRequestManager` as `requestManager` to raise the ceiling — or crawl fewer sites. -- Combining `sameDomainDelaySecs` with a `requestManager` that throttles per domain on its own now throws. Configure the delay on that manager instead, via its `domains: 'all'` and `minCrawlDelaySecs` options. -- Requests that never pass through the request manager — those from the deprecated `requestList` option, or from a `requestsFromUrl` list — are not paced, and the crawler warns when it hands one out. +- Combining it with a manager that paces on its own no longer throws, and no longer gives one domain two clocks: a `ThrottlingRequestManager` with `domains: 'all'` and `throttleBy: 'registrableDomain'` takes the delay as its `minCrawlDelaySecs` floor, wherever it sits in a composition. One that paces only *some* domains throws instead — set `domains: 'all'`, or configure the delay there yourself and drop the option. +- Requests that never pass through the request manager — those from a `requestsFromUrl` list — are not paced, and the crawler warns when it hands one out. #### `BasicCrawler.requestList` and `BasicCrawler.requestQueue` fields removed @@ -1606,6 +1666,8 @@ Methods that may have "nothing" to return now consistently resolve to `undefined - `isEmpty()` is the weak check — `true` when the next `fetchNextRequest()` would return `undefined`, i.e. there is nothing left to fetch right now. Requests that are currently in progress (fetched but not yet handled or reclaimed) are **not** counted, because they are not fetchable. This is what drives the crawler's task scheduling. - `isFinished()` is the strong check — `true` only when there are no pending requests **and** no requests currently in progress (including those locked by other clients sharing the queue). This is what determines whether crawling is actually done. An in-progress request keeps the queue *empty but not finished*, which is what stops a crawler from shutting down while a request is still being processed. +The loader and manager frontends do **not** draw that distinction — `IRequestLoader` and `IRequestManager` answer both questions with one [`checkReadiness()`](#isempty--isfinished-replaced-by-checkreadiness) call. The split lives at the backend boundary because that is the layer where the two questions really are two separate storage lookups; a frontend that split them too would either probe twice per scheduling decision or lose the distinction, whereas a backend that answers one at a time costs its caller nothing. + The separate `RequestQueueV1`/`RequestQueueV2` classes (and the `RequestProvider` base class) have been removed. They no longer differ in behavior — request coordination is now internal to the storage backend — so they are merged into a single `RequestQueue` class. Replace any `RequestQueueV1`, `RequestQueueV2`, or `RequestProvider` imports with `RequestQueue`. The `requestLocking` crawler experiment has been removed, along with the `experiments` crawler option and the `CrawlerExperiments` type that contained it. Request locking has been the default since v3.10 and there is no longer an alternative implementation to opt out to, so the flag did nothing. Delete any `experiments: { requestLocking: ... }` from your crawler options: @@ -2065,6 +2127,7 @@ The full list of removed exports and members, for ctrl-F purposes. Where a repla - `FileDownloadOptions.streamHandler` - streaming should now be handled directly in the `requestHandler` instead - `playwrightUtils.registerUtilsToContext` and `puppeteerUtils.registerUtilsToContext` - this is now added to the context via `ContextPipeline` composition - `context.blockResources` and `context.cacheResponses` — no longer attached to the crawling context. The functionality is still available as deprecated functions, accessible both via the `puppeteerUtils` namespace (`puppeteerUtils.blockResources`, `puppeteerUtils.cacheResponses`) and as top-level exports from `@crawlee/puppeteer` (`import { blockResources, cacheResponses } from '@crawlee/puppeteer'`). Unlike the old context helpers, these take an explicit `page` argument — e.g. `await blockResources(page)`. Both are `@deprecated` and will be removed in a future release, so migrate away from them. +- `context.closeCookieModals`, `playwrightUtils.closeCookieModals` and `puppeteerUtils.closeCookieModals` — removed along with the optional `idcac-playwright` peer dependency (see [Crawling context no longer includes `closeCookieModals`](#crawling-context-no-longer-includes-closecookiemodals)) - `Configuration.systemInfoV2` / `CRAWLEE_SYSTEM_INFO_V2` environment variable — the v2 behavior is now the default (see [Available resource detection](#available-resource-detection)) - `checkAndSerialize` and `chunkBySize` functions (from `@crawlee/core`) — value (de)serialization now lives in the `KeyValueStore` frontend; use `serializeValue` / `parseValue` (see [`maybeStringify` is removed](#maybestringify-is-removed)) - `BASIC_CRAWLER_TIMEOUT_BUFFER_SECS` constant (from `@crawlee/basic`) — was an internal timeout buffer, no longer exported diff --git a/package.json b/package.json index a82cccc07cd4..9f26609e17f9 100644 --- a/package.json +++ b/package.json @@ -49,8 +49,8 @@ "publish:prod": "lerna publish from-package --contents dist --force-publish --yes", "release:prod": "pnpm build && pnpm publish:prod", "release:pin-versions": "turbo run copy -- -- --pin-versions", - "api:extract": "pnpm build && tsx scripts/api-extractor/run.ts", - "api:check": "tsx scripts/api-extractor/run.ts --verify", + "api:extract": "pnpm build && pnpm dlx github:apify/api-extractor-report --exclude=@crawlee/cli,@crawlee/templates --extract-command='pnpm api:extract'", + "api:check": "pnpm dlx github:apify/api-extractor-report --exclude=@crawlee/cli,@crawlee/templates --extract-command='pnpm api:extract' --verify", "lint": "oxlint packages test docs --tsconfig=tsconfig.json --type-aware", "lint:fix": "oxlint packages test docs --tsconfig=tsconfig.json --type-aware --fix", "format": "oxfmt packages test docs --write", @@ -74,7 +74,6 @@ "@crawlee/stagehand": "workspace:*", "@crawlee/types": "workspace:*", "@crawlee/utils": "workspace:*", - "@microsoft/api-extractor": "^7.58.9", "@oxlint/plugins": "^1.62.0", "@playwright/browser-chromium": "1.61.1", "@playwright/browser-firefox": "1.61.1", diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index c399229f0e37..64c7923ab5c0 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -63,6 +63,7 @@ import { NavigationSkippedError, NonRetryableError, OwnedOrInjected, + PersistentRateLimitError, purgeDefaultStorages, RequestHandlerError, parseRetryAfterHeader, @@ -71,7 +72,6 @@ import { RequestQueue, RequestState, RetryRequestError, - supportsDomainThrottling, Router, ServiceLocator, serviceLocator, @@ -352,8 +352,9 @@ export interface BasicCrawlerOptions< * Indicates how much time (in seconds) to wait before crawling another same domain request. Subdomains are * paced together with the site they belong to. * - * Wraps the crawler's request manager in a {@apilink ThrottlingRequestManager}; pass one as `requestManager` - * yourself to configure it further. + * Offered to the crawler's request manager as a `minIntervalEverywhere` {@apilink PacingSignal}; a manager that + * already paces every domain it dispatches to takes it, so no domain ends up with two clocks. Otherwise the + * crawler wraps its request manager in a {@apilink ThrottlingRequestManager} of its own. * @default 0 */ sameDomainDelaySecs?: number; @@ -749,13 +750,6 @@ export class BasicCrawler< return this.#sessionPoolDep.value; } - /** - * Tracks **only** the queue the crawler opens for itself — not the {@apilink RequestManagerTandem} that may wrap it - * around a user-supplied `requestList` — so the owned-only purge between repeated `run()` calls never reaches - * through to a borrowed loader. Filled lazily in {@apilink BasicCrawler.openOwnedRequestQueue|`openOwnedRequestQueue()`}. - */ - #ownedRequestQueue = OwnedOrInjected.resolve(); - /** * Whether the request-processing-time hint has already been forwarded to the request manager. The hint * derives only from `requestHandlerTimeoutMillis` (constant for the crawler's lifetime) and is raise-only, @@ -798,6 +792,12 @@ export class BasicCrawler< */ #autoscaledPool?: AutoscaledPool; + /** A pending nudge of the task loop, armed when the request manager announces when it will have work again. */ + #taskLoopWakeTimer?: NodeJS.Timeout; + + /** When the pending wake-up is due, so an earlier one can replace a later one. */ + #taskLoopWakeAt = 0; + /** * A reference to the underlying {@apilink IProxyConfiguration} instance that manages the crawler's proxies. * Only available if used by the crawler. @@ -858,7 +858,16 @@ export class BasicCrawler< protected readonly internalTimeoutMillis: number; readonly #maxRequestRetries: number; readonly #maxCrawlDepth?: number; - #sameDomainDelaySecs: number; + /** + * How much of {@apilink BasicCrawler.requestManager} the crawler may empty between repeated `run()` calls. + * + * - `all` — nothing under it came from the caller, so one `purge()` on the outside covers everything. + * - `none` — the caller supplied it and the crawler put nothing of its own inside. + * - `ambiguous` — the caller supplied it, but `sameDomainDelaySecs` put the crawler's own per-domain queues + * underneath: purging empties the caller's storage too, skipping leaves ours stale. A repeated `run()` asks + * rather than guessing. + */ + readonly #purgeableExtent: 'all' | 'none' | 'ambiguous'; readonly #maxRequestsPerCrawl?: number; private get handledRequestsCount(): number { @@ -1073,33 +1082,59 @@ export class BasicCrawler< const instanceIndex = BasicCrawler.instanceCount++; this.#identity = { instanceIndex, hasExplicitId: id !== undefined, id: id ?? String(instanceIndex) }; - if (requestManager !== undefined) { - if (requestList !== undefined || requestQueue !== undefined) { - throw new Error( - 'The `requestManager` option cannot be used in conjunction with `requestList` and/or `requestQueue`', - ); - } - // Both would pace the same domains, from different keys and with no idea of one another. - if (sameDomainDelaySecs > 0 && supportsDomainThrottling(requestManager)) { - throw new Error( - 'The `sameDomainDelaySecs` option cannot be combined with a `requestManager` that throttles ' + - 'per domain on its own. Configure the delay on the manager instead, via the ' + - '`minCrawlDelaySecs` option of `ThrottlingRequestManager`.', - ); - } + if (requestManager !== undefined && (requestList !== undefined || requestQueue !== undefined)) { + throw new Error( + 'The `requestManager` option cannot be used in conjunction with `requestList` and/or `requestQueue`', + ); + } + + const suppliedManager = requestManager ?? requestQueue; + + // Offered before building a pacer of our own: anything that paces takes the floor - through any + // number of wrappers, since they all forward - so no domain ends up with two clocks. + const floorTaken = + sameDomainDelaySecs > 0 && + (suppliedManager?.recordPacingSignal({ + reason: 'minIntervalEverywhere', + intervalMs: sameDomainDelaySecs * 1000, + // What `sameDomainDelaySecs` has always meant: one clock per site, subdomains included. + scope: 'registrableDomain', + }) ?? + false); + + const pacerNeeded = sameDomainDelaySecs > 0 && !floorTaken; + + // Our per-domain queues under a manager the caller owns is the case with no right answer; a floor + // it took leaves nothing of ours behind. See `#purgeableExtent`. + if (suppliedManager === undefined) { + this.#purgeableExtent = 'all'; + } else { + this.#purgeableExtent = pacerNeeded ? 'ambiguous' : 'none'; + } - this.requestManager = requestManager; - } else if (requestList !== undefined && requestQueue !== undefined) { - // Combine the read-only list with the writable queue into a tandem. - this.requestManager = new RequestManagerTandem(requestList, requestQueue); - } else if (requestQueue !== undefined) { + // Built here rather than at first use so it can sit *inside* the tandem below, which is where a + // loader's transferred requests pass through it. + const writableManager = pacerNeeded + ? new ThrottlingRequestManager({ + domains: 'all', + minCrawlDelaySecs: sameDomainDelaySecs, + throttleBy: 'registrableDomain', + persistStateKey: `CRAWLEE_THROTTLED_DOMAINS_${this.#identity.id}`, + // A factory, because the default queue is only opened on first use. + inner: suppliedManager ?? (() => this.openOwnedRequestQueue()), + }) + : suppliedManager; + + if (requestList !== undefined) { + // The list is read first, while new requests still have somewhere writable to go; the tandem also + // forwards `persistState()` to the loader. + this.requestManager = new RequestManagerTandem( + requestList, + writableManager ?? (() => this.openOwnedRequestQueue()), + ); + } else if (writableManager !== undefined) { // A RequestQueue is itself a request manager. - this.requestManager = requestQueue; - } else if (requestList !== undefined) { - // A lone read-only `requestList` (deprecated option) is combined with a lazily-opened default queue - // into a tandem, so that its requests are read first and new ones can still be enqueued during the - // crawl. The queue is opened on first use; the tandem also forwards `persistState()` to the loader. - this.requestManager = new RequestManagerTandem(requestList, () => this.openOwnedRequestQueue()); + this.requestManager = writableManager; } this.httpClient = httpClient ?? new LazyDefaultHttpClient({ logger: this.log }); @@ -1139,7 +1174,6 @@ export class BasicCrawler< this.#maxRequestRetries = maxRequestRetries; this.#maxCrawlDepth = maxCrawlDepth; - this.#sameDomainDelaySecs = sameDomainDelaySecs; this.#statisticsDep = OwnedOrInjected.resolve< IStatistics, Statistics @@ -1330,15 +1364,19 @@ 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 (!keepAlive && supportsDomainThrottling(this.requestManager)) { - await this.requestManager.assertNoStalledDomains(); + // `maybeFinish()` calls this only once nothing is in flight (`autoscaled_pool.ts`) - the point + // where a crawl that cannot progress becomes distinguishable from one that is merely waiting, + // and the only place where throwing does not abandon requests mid-processing. + const state = await this.requestManager?.checkReadiness(); + + // Under `keepAlive`, outliving a domain that will not let us through is the whole point. + if (state?.status === 'stalled' && !keepAlive) { + throw new PersistentRateLimitError(`Giving up: ${state.reason}`); } const isFinished = isFinishedFunction ? await isFinishedFunction() - : await this.defaultIsFinishedFunction(); + : state === undefined || state.status === 'finished'; if (isFinished) { const reason = isFinishedFunction @@ -1669,22 +1707,21 @@ export class BasicCrawler< // we need to purge the RQ to allow processing the same requests again — this is important so users can // pass in failed requests back to the `crawler.run()`, otherwise they would be considered as handled and // ignored — as a failed request is still handled. - // By default (`purgeRequestQueue` unset), only the queue we opened ourselves is purged. - // When `purgeRequestQueue` is explicitly `true`, we also purge a user-supplied manager. - // When `purgeRequestQueue` is explicitly `false`, nothing is purged. - const shouldPurge = purgeRequestQueue !== false; - const managerToPurge = - this.#ownedRequestQueue.maybeValue ?? (purgeRequestQueue === true ? this.requestManager : undefined); - - if (shouldPurge) { - await managerToPurge?.purge?.(); - - // The per-domain queues a `sameDomainDelaySecs` wrapper created are the crawler's own, whatever - // sits underneath them - so they are emptied even when the manager they wrap is spared. Purging - // the wrapper itself has already covered them. - if (this.requestManager instanceof ThrottlingRequestManager && managerToPurge !== this.requestManager) { - await this.requestManager.purgeDomainQueues(); - } + // `purgeRequestQueue` unset purges only storage the crawler opened itself (see `#purgeableExtent`); + // `true` also purges a caller-supplied manager, `false` purges nothing. + if (purgeRequestQueue === undefined && this.#purgeableExtent === 'ambiguous') { + throw new Error( + 'Cannot decide what to purge before running again: `sameDomainDelaySecs` paces the request ' + + 'manager you supplied, so the per-domain queues that have to be emptied are the ' + + "crawler's while the manager underneath them is yours. Say which you want: " + + '`run(requests, { purgeRequestQueue: true })` empties both, `false` empties neither.', + ); + } + + if (purgeRequestQueue !== false && (this.#purgeableExtent === 'all' || purgeRequestQueue === true)) { + // One call from the outside in reaches everything the manager wraps, a pacer's per-domain queues + // included. + await this.requestManager?.purge?.(); } // A supplied statistics instance keeps whatever state it was handed - only wipe a default we built. @@ -1855,20 +1892,6 @@ export class BasicCrawler< this.requestManager = await this.openOwnedRequestQueue(); } - // Wrapped here rather than in the constructor, because the manager being wrapped may only be opened at - // this point - and because everything that enqueues goes through here first, so nothing slips past the - // wrapper into the queue it hides. - if (this.#sameDomainDelaySecs > 0 && !supportsDomainThrottling(this.requestManager)) { - this.requestManager = new ThrottlingRequestManager({ - inner: this.requestManager, - domains: 'all', - minCrawlDelaySecs: this.#sameDomainDelaySecs, - // What `sameDomainDelaySecs` has always meant: one clock for a site, subdomains included. - throttleBy: 'registrableDomain', - persistStateKey: `CRAWLEE_THROTTLED_DOMAINS_${this.#identity.id}`, - }); - } - // Apply the processing-time hint here (an async lifecycle point) rather than in the constructor, // now that `setExpectedRequestProcessingTimeSecs` is async. The hint is raise-only and idempotent, // but guard so we do not re-issue it on every call. @@ -1889,8 +1912,7 @@ export class BasicCrawler< } /** - * Opens the default {@apilink RequestQueue}, applies the crawler's timeouts to it and records it as the - * crawler-owned queue (so it gets purged between repeated `run()` calls). + * Opens the default {@apilink RequestQueue} — the crawler's own, read from when the caller supplied nothing. * @private */ private async openOwnedRequestQueue(): Promise { @@ -1898,8 +1920,7 @@ export class BasicCrawler< // subsequent instances get their own queue via a unique alias so they don't collide. const identifier = this.#identity.instanceIndex === 0 ? null : { alias: `__default_${this.#identity.id}__` }; - const requestQueue = await RequestQueue.open(identifier, { configuration: serviceLocator.getConfiguration() }); - return this.#ownedRequestQueue.set(requestQueue); + return RequestQueue.open(identifier, { configuration: serviceLocator.getConfiguration() }); } /** @@ -2446,16 +2467,19 @@ export class BasicCrawler< } /** - * Records an HTTP 429 against the URL's domain so the request manager can pace the retry. + * Records an HTTP 429 against the URL's domain so the request manager can hold the retry back. * * @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 + * @returns `true` if the 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 { if ( - supportsDomainThrottling(this.requestManager) && - this.requestManager.recordDomainDelay(url, parseRetryAfterHeader(retryAfterHeader)) + this.requestManager?.recordPacingSignal({ + reason: 'rateLimited', + url, + waitMs: parseRetryAfterHeader(retryAfterHeader) ?? undefined, + }) ) { return true; } @@ -2463,33 +2487,38 @@ export class BasicCrawler< const domain = hostnameOrUrl(url); this.logOncePerRun( `rateLimitNotThrottled:${domain}`, - `"${domain}" responded with HTTP 429 (Too Many Requests), but nothing is set up to back off from it, ` + - '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.', + `"${domain}" responded with HTTP 429 (Too Many Requests), but the crawler's request manager does not ` + + 'pace that domain, so the response is handled like any other, with no per-domain delay. Set ' + + `\`sameDomainDelaySecs\`, or pass a \`ThrottlingRequestManager\` covering "${domain}" as ` + + '`requestManager`, to honour `Retry-After` and apply exponential backoff instead.', 'warning', ); return false; } - /** - * Hands a robots.txt `Crawl-delay` to the request manager, warning if nothing is able to honour it. - * - * 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. - */ + /** Hands a robots.txt `Crawl-delay` to the request manager, warning if it will not be honoured. */ private applyCrawlDelay(url: string, delaySeconds: number): void { - if (supportsDomainThrottling(this.requestManager) && this.requestManager.setCrawlDelay(url, delaySeconds)) { + // robots.txt is per-origin; `hostname` is the closest pacing scope and errs wide (http and https to one + // host share a clock). + if ( + this.requestManager?.recordPacingSignal({ + reason: 'minInterval', + url, + intervalMs: delaySeconds * 1000, + scope: 'hostname', + }) + ) { return; } const domain = hostnameOrUrl(url); 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.`, + `robots.txt for "${domain}" defines a crawl-delay of ${delaySeconds}s, but the crawler's request ` + + 'manager does not pace that domain, so its requests will not be paced. Set ' + + `\`sameDomainDelaySecs\`, or pass a \`ThrottlingRequestManager\` covering "${domain}" as ` + + '`requestManager`.', 'warning', ); } @@ -2533,10 +2562,10 @@ export class BasicCrawler< } const requestManagerPersistPromise = (async () => { - // The request manager persists its read-only loader's state, if it has one that supports persistence - // (e.g. a tandem wrapping a `RequestList`). For a plain `RequestQueue`, this is a no-op. + // The request manager persists its read-only loader's state, if it has one that supports + // persistence (e.g. a tandem wrapping a `RequestList`). For a plain `RequestQueue`, this is a no-op. if (this.requestManager?.persistState) { - if (await this.requestManager.isFinished()) return; + if ((await this.requestManager.checkReadiness()).status === 'finished') return; await this.requestManager.persistState().catch((err) => { if (err.message.includes('Cannot persist state.')) { this.log.error( @@ -2709,17 +2738,53 @@ export class BasicCrawler< } /** - * Returns true if either RequestList or RequestQueue have a request ready for processing. + * Whether the request manager has a request ready for processing. A manager that is only `waiting` also gets a + * wake-up scheduled, so a paced crawl resumes on its clock rather than on the task loop's polling interval. */ private async isTaskReadyFunction() { - return this.requestManager !== undefined && !(await this.requestManager.isEmpty()); + if (this.requestManager === undefined) { + return false; + } + + const state = await this.requestManager.checkReadiness(); + + if (state.status === 'waiting' && state.readyAt !== undefined) { + this.#scheduleTaskLoopWake(state.readyAt); + } + + return state.status === 'ready'; } /** - * Returns true if both RequestList and RequestQueue have all requests finished. + * Nudges the task loop at `readyAt`, on a single timer that only an earlier one replaces. The pool polls anyway + * every `maybeRunIntervalSecs` (0.5s by default), so this only shortens the wait - hence one timer rather than + * one per probe, and `unref`'d so it never keeps the process alive. */ - private async defaultIsFinishedFunction() { - return !this.requestManager || (await this.requestManager.isFinished()); + #scheduleTaskLoopWake(readyAt: number): void { + if (this.#taskLoopWakeTimer !== undefined) { + if (this.#taskLoopWakeAt <= readyAt) { + return; + } + clearTimeout(this.#taskLoopWakeTimer); + } + + this.#taskLoopWakeAt = readyAt; + this.#taskLoopWakeTimer = setTimeout( + () => { + this.#taskLoopWakeTimer = undefined; + void this.#autoscaledPool?.notify(); + }, + Math.max(0, readyAt - Date.now()), + ); + this.#taskLoopWakeTimer.unref(); + } + + /** Drops a pending task-loop wake-up, so a finished run leaves no timer behind. */ + #clearTaskLoopWake(): void { + if (this.#taskLoopWakeTimer !== undefined) { + clearTimeout(this.#taskLoopWakeTimer); + this.#taskLoopWakeTimer = undefined; + } } /** @@ -2904,6 +2969,7 @@ export class BasicCrawler< await serviceLocator.getEventManager().close(); } + this.#clearTaskLoopWake(); await this.#autoscaledPool?.abort(); await this.#concurrencySystemDep?.ifOwned((system) => system.stop()); } diff --git a/packages/core/src/storages/request_list.ts b/packages/core/src/storages/request_list.ts index 994955f37d2b..5fa875702e97 100644 --- a/packages/core/src/storages/request_list.ts +++ b/packages/core/src/storages/request_list.ts @@ -12,7 +12,7 @@ import { createDeserialize, serializeArray } from '../serialization.js'; import { serviceLocator } from '../service_locator.js'; import { parseArgument, schemas, validators } from '../validators.js'; import { KeyValueStore } from './key_value_store.js'; -import type { IRequestLoader } from './request_loader.js'; +import type { IRequestLoader, RequestLoaderStatus } from './request_loader.js'; import type { IRequestManager } from './request_manager.js'; import { purgeDefaultStorages } from './utils.js'; @@ -602,19 +602,15 @@ export class RequestList implements IRequestLoader { /** * @inheritDoc */ - async isEmpty(): Promise { + async checkReadiness(): Promise { this.ensureIsInitialized(); - return this.#requestsToRetry.length === 0 && this.#nextIndex >= this.requests.length; - } - - /** - * @inheritDoc - */ - async isFinished(): Promise { - this.ensureIsInitialized(); + if (this.#requestsToRetry.length > 0 || this.#nextIndex < this.requests.length) { + return { status: 'ready' }; + } - return this.inProgress.size === 0 && this.#nextIndex >= this.requests.length; + // `#requestsToRetry` is a subset of `inProgress`, so nothing in progress means nothing left to re-serve. + return this.inProgress.size === 0 ? { status: 'finished' } : { status: 'waiting' }; } /** diff --git a/packages/core/src/storages/request_loader.ts b/packages/core/src/storages/request_loader.ts index f68988b35b72..d1e607ba0f93 100644 --- a/packages/core/src/storages/request_loader.ts +++ b/packages/core/src/storages/request_loader.ts @@ -4,6 +4,72 @@ import type { Request } from '../request.js'; import type { IRequestManager } from './request_manager.js'; import type { RequestQueueOperationInfo } from './request_queue.js'; +/** + * A request source's own availability, in a single answer. + * + * - `ready` — the next {@apilink IRequestLoader.fetchNextRequest} is expected to hand something over. + * - `waiting` — nothing to fetch right now, but the source is not done: requests are in progress, are being + * added in the background, or are held back until `readyAt`. + * - `stalled` — the source holds requests it cannot make progress on. Only a manager that paces its own + * dispatch can reach this; see {@apilink ThrottlingRequestManager}. + * - `finished` — everything has been handled. + */ +export type RequestSourceStatus = + | { status: 'ready' } + | { + status: 'waiting'; + /** + * A `Date.now()` timestamp at which the source expects to become `ready`. Absent when the wait has + * no clock (an in-progress request, a background add), leaving a consumer to poll. + */ + readyAt?: number; + } + | { status: 'stalled'; reason: string } + | { status: 'finished' }; + +/** Loaders never stall — only a manager that paces its own dispatch can. */ +export type RequestLoaderStatus = Exclude; + +/** + * Combines two request sources' statuses, with the precedence `ready` > `stalled` > `waiting` > `finished`. + * + * Binary rather than variadic on purpose: it is on the task loop's probe path and folding a pair allocates + * nothing. + * + * @internal + */ +export function joinRequestSourceStatuses(a: RequestSourceStatus, b: RequestSourceStatus): RequestSourceStatus { + if (a.status === 'ready') { + return a; + } + if (b.status === 'ready') { + return b; + } + + // `ready` outranking `stalled` masks a stalled source while the other still has work. That is parity with + // the crawler before this was a single answer: the stall check was only reached from `isFinishedFunction`, + // which the task loop calls only when nothing is in flight and nothing is ready. "Fixing" the masking + // turns a crawl that is progressing elsewhere into a `PersistentRateLimitError`. `stalled` outranking + // `waiting` is the same parity - that call site fired regardless of other domains' clocks. + if (a.status === 'stalled') { + return a; + } + if (b.status === 'stalled') { + return b; + } + + if (a.status === 'waiting') { + // The earlier of the two known wake-up times - unknown only if neither source announced one. + if (b.status !== 'waiting' || b.readyAt === undefined) { + return a; + } + return a.readyAt !== undefined && a.readyAt <= b.readyAt ? a : b; + } + + // `a` is finished, so `b` decides. + return b; +} + /** * An abstract interface defining a read-only stream of requests to crawl. * @@ -26,7 +92,7 @@ import type { RequestQueueOperationInfo } from './request_queue.js'; * - **Restarts and migrations:** loaders that persist their state (see {@apilink IRequestLoader.persistState}) * treat in-progress requests as interrupted and re-serve them after a restart. A request that is fetched * but never marked handled will be crawled again. - * - **Termination detection:** {@apilink IRequestLoader.isFinished} only resolves to `true` once nothing is + * - **Termination detection:** {@apilink IRequestLoader.checkReadiness} only reports `finished` once nothing is * in progress. Leaving a request unmarked keeps the crawler running indefinitely. * - **Bookkeeping:** the handled and pending counts are derived from the set of in-progress requests, so * skipping {@apilink IRequestLoader.markRequestAsHandled} corrupts {@apilink IRequestLoader.getHandledCount} @@ -52,21 +118,13 @@ export interface IRequestLoader { getHandledCount(): Promise; /** - * Returns `true` if all requests were already handled and there are no more left. - */ - isFinished(): Promise; - - /** - * 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. + * Reports whether the loader has a request to hand over, is waiting on one, or is done — see + * {@apilink RequestSourceStatus}. * - * 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. + * A consumer's task loop is gated on this, so implementations MUST answer `ready` before evaluating + * anything else. `finished` may arrive late behind distributed storage, but it is never wrong. */ - isEmpty(): Promise; + checkReadiness(): Promise; /** * Gets the next {@apilink Request} to process, or `null` if there are no more pending requests. @@ -90,8 +148,9 @@ export interface IRequestLoader { * * Call this once you are done with the request — whether processing succeeded or was abandoned after * exhausting retries. Because a loader cannot take a request back, marking it handled is the only way to - * signal completion; failing to do so prevents {@apilink IRequestLoader.isFinished} from ever resolving to - * `true` and skews the handled and pending counts. See the request lifecycle contract on {@apilink IRequestLoader}. + * signal completion; failing to do so prevents {@apilink IRequestLoader.checkReadiness} from ever reporting + * `finished` and skews the handled and pending counts. See the request lifecycle contract on + * {@apilink IRequestLoader}. */ markRequestAsHandled(request: Request): Promise; diff --git a/packages/core/src/storages/request_manager.ts b/packages/core/src/storages/request_manager.ts index 601d84c7b0a0..cfa20dcfd77a 100644 --- a/packages/core/src/storages/request_manager.ts +++ b/packages/core/src/storages/request_manager.ts @@ -1,3 +1,5 @@ +import type { LiteralUnion } from 'type-fest'; + import type { Request, Source } from '../request.js'; import type { IRequestLoader } from './request_loader.js'; import type { @@ -41,4 +43,85 @@ export interface IRequestManager extends IRequestLoader { * this hint may leave it `undefined`. */ setExpectedRequestProcessingTimeSecs?(secs: number): Promise; + + /** + * Records something said about the pace requests should go out at, so that a manager which paces its own + * dispatch can hold requests back. + * + * Required rather than optional, so that a wrapping manager always forwards it and a pacer nested in a + * composition still receives it; a manager that does not pace returns `false`. + * + * @returns `true` if anything in the composition took responsibility for the signal. + */ + recordPacingSignal(signal: PacingSignal): boolean; } + +/** + * How much of the URL space a {@apilink PacingSignal} covers. + * + * Open on purpose: `'hostname'` and `'registrableDomain'` are what Crawlee's own reporters send and what + * {@apilink ThrottlingRequestManager} understands, but any string is accepted, so a pacer keyed on something + * else can be reported to in its own vocabulary. + */ +export type PacingScope = LiteralUnion<'hostname' | 'registrableDomain', string>; + +/** + * Something said about the pace requests should go out at, reported to a request manager through + * {@apilink IRequestManager.recordPacingSignal}. + * + * One shape rather than a method per channel: a pacing manager switches on `reason`, a wrapping one forwards the + * value without knowing what is in it, and a new kind of signal costs the interface nothing. The `url` travels + * inside the value because the crawl-wide variant has none. Nothing here names the mechanism a signal came + * from - status codes, headers and robots.txt are the crawler's business - and every delay is in milliseconds. + * + * ## Scope + * + * A manager may apply a signal to a **wider** scope than it was given - a floor that holds for one host still + * holds when a whole site is paced by it - but never to a narrower one, which would leave some of the URLs the + * signal covers running unpaced. A manager that can only do the latter, or that does not recognise the scope at + * all, MUST throw rather than quietly under-apply it. + */ +export type PacingSignal = + | { + /** + * The source turned a request away because we were going too fast — an HTTP 429 or 503, an exhausted + * quota. Reactive and transient: a pacer typically backs off while refusals continue, and lets that + * decay once they stop. + */ + reason: 'rateLimited'; + /** The URL that was turned away. */ + url: string; + /** How long the source asked us to wait before trying again, if it said. */ + waitMs?: number; + /** + * How far this refusal reaches, if the reporter can tell — a 429 rarely says. Left out, it asks the + * manager to apply the signal however it happens to group requests. + */ + scope?: PacingScope; + } + | { + /** + * The source declared a standing floor on how often it may be requested — a robots.txt `Crawl-delay`, + * a documented quota. A property of the source rather than of the run, so a pacer keeps it for the + * whole crawl. + */ + reason: 'minInterval'; + /** A URL of the source that declared the interval. */ + url: string; + /** The declared minimum interval between two requests to the source. */ + intervalMs: number; + /** Required, since whoever declares an interval knows what it applies to. */ + scope: PacingScope; + } + | { + /** + * A standing floor under the pace of **every** domain the manager dispatches to, declared by whoever + * owns the crawl rather than by a source — a crawler's `sameDomainDelaySecs`. A manager that paces only + * some of its domains MUST throw rather than under-apply it. + */ + reason: 'minIntervalEverywhere'; + /** The declared minimum interval between two requests to any one source. */ + intervalMs: number; + /** At what granularity the floor applies. */ + scope: PacingScope; + }; diff --git a/packages/core/src/storages/request_manager_tandem.ts b/packages/core/src/storages/request_manager_tandem.ts index 21aa2ab0f99b..205a2145c6a0 100644 --- a/packages/core/src/storages/request_manager_tandem.ts +++ b/packages/core/src/storages/request_manager_tandem.ts @@ -3,8 +3,9 @@ import type { Dictionary } from '@crawlee/types'; import type { CrawleeLogger } from '../log.js'; import type { Request, Source } from '../request.js'; import { serviceLocator } from '../service_locator.js'; -import type { IRequestLoader } from './request_loader.js'; -import type { IRequestManager, RequestsLike } from './request_manager.js'; +import type { IRequestLoader, RequestSourceStatus } from './request_loader.js'; +import { joinRequestSourceStatuses } from './request_loader.js'; +import type { IRequestManager, PacingSignal, RequestsLike } from './request_manager.js'; import type { AddRequestsBatchedOptions, AddRequestsBatchedResult, @@ -43,7 +44,14 @@ export class RequestManagerTandem implements IRequestManager { ) { this.#log = serviceLocator.getLogger().child({ prefix: 'RequestManagerTandem' }); this.#requestLoader = requestLoader; - this.#requestManagerFactory = typeof requestManager === 'function' ? requestManager : () => requestManager; + + if (typeof requestManager === 'function') { + this.#requestManagerFactory = requestManager; + } else { + // Nothing to open, so mark it resolved up front - synchronous pacing signals can then reach it. + this.#resolvedRequestManager = requestManager; + this.#requestManagerFactory = () => requestManager; + } } /** @@ -99,18 +107,13 @@ export class RequestManagerTandem implements IRequestManager { } /** - * Fetches the next request from the request manager. If the manager is empty and the loader - * is not finished, it will transfer a request from the loader to the manager first. + * Fetches the next request, transferring one from the loader first if the loader still has work. * @inheritdoc */ async fetchNextRequest(): Promise | null> { - // First, try to transfer a request from the requestList - const [listEmpty, listFinished] = await Promise.all([ - this.#requestLoader.isEmpty(), - this.#requestLoader.isFinished(), - ]); - - if (!listEmpty && !listFinished) { + // Only the loader's own state decides this: a manager waiting out a backoff must not freeze the + // loader's unrelated requests for the length of it. + if ((await this.#requestLoader.checkReadiness()).status === 'ready') { // If the transfer failed, the request was dropped; don't fetch from the manager this round (matching // crawlee-python behaviour). The next `fetchNextRequest()` call will pick up where we left off. if (!(await this.transferNextRequestToQueue())) { @@ -123,21 +126,17 @@ export class RequestManagerTandem implements IRequestManager { } /** + * The loader and the manager read as one source. * @inheritdoc */ - async isFinished(): Promise { + async checkReadiness(): Promise { const requestManager = await this.getRequestManager(); - const storagesFinished = await Promise.all([this.#requestLoader.isFinished(), requestManager.isFinished()]); - return storagesFinished.every(Boolean); - } + const [loaderStatus, managerStatus] = await Promise.all([ + this.#requestLoader.checkReadiness(), + requestManager.checkReadiness(), + ]); - /** - * @inheritdoc - */ - async isEmpty(): Promise { - const requestManager = await this.getRequestManager(); - const storagesEmpty = await Promise.all([this.#requestLoader.isEmpty(), requestManager.isEmpty()]); - return storagesEmpty.every(Boolean); + return joinRequestSourceStatuses(loaderStatus, managerStatus); } /** @@ -244,4 +243,14 @@ export class RequestManagerTandem implements IRequestManager { this.#expectedRequestProcessingSecs = secs; await this.#resolvedRequestManager?.setExpectedRequestProcessingTimeSecs?.(secs); } + + /** + * Forwards a pacing signal to the writable manager - the loader side is read-only and dispatches nothing of + * its own. Only a resolved manager is signalled; the tandem will not open a queue to answer a question about + * pacing. + * @inheritdoc + */ + recordPacingSignal(signal: PacingSignal): boolean { + return this.#resolvedRequestManager?.recordPacingSignal(signal) ?? false; + } } diff --git a/packages/core/src/storages/request_queue.ts b/packages/core/src/storages/request_queue.ts index baf50dcd4565..0ce947116f87 100644 --- a/packages/core/src/storages/request_queue.ts +++ b/packages/core/src/storages/request_queue.ts @@ -31,7 +31,8 @@ import { parseArgument, schemas, validators } from '../validators.js'; import type { JournalEntry, StorageTransaction } 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 { RequestLoaderStatus } from './request_loader.js'; +import type { IRequestManager, PacingSignal, RequestsLike } from './request_manager.js'; import type { RequestQueueStats } from './storage_stats.js'; import { StorageStatsTracker } from './storage_stats.js'; import type { IStorage, StorageIdentifier } from './storage_instance_manager.js'; @@ -699,7 +700,7 @@ export class RequestQueue implements IStorage, IRequestManager { * Note that the `null` return value doesn't mean the queue processing finished, * it means there are currently no pending requests. * To check whether all requests in queue were finished, - * use {@apilink RequestQueue.isFinished} instead. + * use {@apilink RequestQueue.checkReadiness} instead. * * @returns * Returns the request object or `null` if there are no more pending requests. @@ -801,48 +802,42 @@ export class RequestQueue implements IStorage, IRequestManager { } /** - * Resolves to `true` if the next call to {@apilink RequestQueue.fetchNextRequest} would return - * `null`, i.e. there are no pending requests to fetch right now. Otherwise it resolves to `false`. + * A queue hands requests out as fast as they are asked for; pacing is a job for a manager wrapped around it, + * such as {@apilink ThrottlingRequestManager}. + * @inheritdoc + */ + recordPacingSignal(_signal: PacingSignal): boolean { + return false; + } + + /** + * Reports whether the queue has a request to hand over, is waiting on one, or is done. * - * Note that even if the queue is empty, there might be some requests currently being processed - * (fetched but not yet handled or reclaimed). An empty queue therefore does not mean crawling is - * finished — those in-progress requests may still be reclaimed, and background tasks may still be - * adding more requests. To check whether all activity in the queue has finished, use - * {@apilink RequestQueue.isFinished}. + * `waiting` means requests are in progress (fetched but not yet handled or reclaimed, possibly by another + * client sharing the queue) or a background add is still landing; neither has a clock, so no `readyAt`. + * + * Due to the nature of distributed storage used by the queue, `finished` may occasionally arrive a probe or + * two late, but it is never reported early. */ - async isEmpty(): Promise { + async checkReadiness(): Promise { const transaction = activeStorageTransaction(); // Requests buffered by the active transaction count as pending from its point of view. if (transaction && this.bufferedRequests(transaction).size > 0) { - return false; + return { status: 'ready' }; } - return this.backend.isEmpty(); - } - - /** - * Resolves to `true` if all requests were already handled and there are no more left — including no - * requests currently in progress (fetched but not yet handled or reclaimed, including requests - * locked by other clients sharing the same queue) and no background add operations still in flight. - * - * Due to the nature of distributed storage used by the queue, the function may occasionally return - * a false negative, but it shall never return a false positive. - */ - async isFinished(): Promise { - const transaction = activeStorageTransaction(); + // Something fetchable outranks everything below, so this is the only backend call a probe needs. + if (!(await this.backend.isEmpty())) { + return { status: 'ready' }; + } // We are not finished if we're still adding new requests in the background. if (this.#inProgressRequestBatchCount > 0) { - return false; - } - - // Requests buffered by the active transaction count as pending from its point of view. - if (transaction && this.bufferedRequests(transaction).size > 0) { - return false; + return { status: 'waiting' }; } - return this.backend.isFinished(); + return (await this.backend.isFinished()) ? { status: 'finished' } : { status: 'waiting' }; } /** diff --git a/packages/core/src/storages/sitemap_request_loader.ts b/packages/core/src/storages/sitemap_request_loader.ts index f6f031600125..e3b236f3fca0 100644 --- a/packages/core/src/storages/sitemap_request_loader.ts +++ b/packages/core/src/storages/sitemap_request_loader.ts @@ -14,7 +14,7 @@ import { Request } from '../request.js'; import { serviceLocator } from '../service_locator.js'; import { parseArgument, schemas } from '../validators.js'; import { KeyValueStore } from './key_value_store.js'; -import type { IRequestLoader } from './request_loader.js'; +import type { IRequestLoader, RequestLoaderStatus } from './request_loader.js'; import type { IRequestManager } from './request_manager.js'; import { purgeDefaultStorages } from './utils.js'; @@ -181,7 +181,7 @@ export class SitemapRequestLoader implements IRequestLoader { * If the loading was aborted before the sitemaps were fully loaded, the request list might be missing some URLs. * The `isSitemapFullyLoaded` method can be used to check if the sitemaps were fully loaded. * - * If the loading is aborted and all the requests are handled, `isFinished()` will return `true`. + * If the loading is aborted and all the requests are handled, `checkReadiness()` will report `finished`. */ #abortLoading = false; @@ -470,17 +470,17 @@ export class SitemapRequestLoader implements IRequestLoader { /** * @inheritDoc */ - async isFinished(): Promise { - return ( - (await this.isEmpty()) && this.inProgress.size === 0 && (this.isSitemapFullyLoaded() || this.#abortLoading) - ); - } + async checkReadiness(): Promise { + if (this.#urlQueueStream.readableLength > 0) { + return { status: 'ready' }; + } - /** - * @inheritDoc - */ - async isEmpty(): Promise { - return this.#urlQueueStream.readableLength === 0; + // The parser is still running, so more URLs may yet arrive - on no schedule, hence no `readyAt`. + if (!this.isSitemapFullyLoaded() && !this.#abortLoading) { + return { status: 'waiting' }; + } + + return this.inProgress.size === 0 ? { status: 'finished' } : { status: 'waiting' }; } /** @@ -598,7 +598,7 @@ export class SitemapRequestLoader implements IRequestLoader { * @inheritDoc */ async *[Symbol.asyncIterator]() { - while (!(await this.isFinished())) { + while ((await this.checkReadiness()).status !== 'finished') { const request = await this.fetchNextRequest(); if (!request) break; diff --git a/packages/core/src/storages/throttling_request_manager.ts b/packages/core/src/storages/throttling_request_manager.ts index bd1b1b6fa696..36c51fa091ee 100644 --- a/packages/core/src/storages/throttling_request_manager.ts +++ b/packages/core/src/storages/throttling_request_manager.ts @@ -4,7 +4,6 @@ import { getDomain } from 'tldts'; import { z } from 'zod'; import type { Configuration } from '../configuration.js'; -import { PersistentRateLimitError } from '../errors.js'; import { asyncifyIterable } from '../iterables.js'; import type { CrawleeLogger } from '../log.js'; import type { Request, Source } from '../request.js'; @@ -13,7 +12,9 @@ import { normalizeHostname } from '../url.js'; import { parseArgument, schemas } from '../validators.js'; import { drainRequestBatches } from './batched_adds.js'; import { KeyValueStore } from './key_value_store.js'; -import type { IRequestManager, RequestsLike } from './request_manager.js'; +import type { RequestSourceStatus } from './request_loader.js'; +import { joinRequestSourceStatuses } from './request_loader.js'; +import type { IRequestManager, PacingScope, PacingSignal, RequestsLike } from './request_manager.js'; import type { AddRequestsBatchedOptions, AddRequestsBatchedResult, @@ -25,7 +26,7 @@ import type { StorageIdentifier } from './storage_instance_manager.js'; import type { StorageOpenOptions } from './utils.js'; const throttlingRequestManagerOptionsSchema = z.strictObject({ - inner: schemas.anyObject, + inner: z.union([schemas.anyObject, schemas.anyFunction]).optional(), domains: z.union([schemas.arrayOf(z.string().nonempty(), 'non-empty strings'), z.literal('all')]), requestManagerOpener: schemas.anyFunction.optional(), baseDelaySecs: schemas.anyNumber.refine((value) => value > 0, 'Expected a number greater than 0').optional(), @@ -47,43 +48,21 @@ const throttlingRequestManagerOptionsSchema = z.strictObject({ * concrete type and storage backend of the manager being wrapped. */ export type RequestManagerOpener = ( - identifier: string | StorageIdentifier, + identifier?: string | StorageIdentifier | null, 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 { /** * The request manager to wrap, usually a {@apilink RequestQueue}. Requests for domains that are not throttled - * are stored here. + * are stored here. May be a factory, so that the throttler can be constructed synchronously and the manager + * under it opened only on first use. + * + * Omitted, the default request queue is opened on first use through + * {@apilink ThrottlingRequestManagerOptions.requestManagerOpener|`requestManagerOpener`}. */ - inner: T; + inner?: T | (() => T | Promise); /** * Which domains to throttle: a list of hostnames, or `'all'` for every domain the crawl encounters. @@ -104,7 +83,8 @@ export interface ThrottlingRequestManagerOptions { ... }, @@ -266,15 +254,15 @@ const DEFAULT_PERSIST_STATE_KEY = 'CRAWLEE_THROTTLED_DOMAINS'; * * @category Sources */ -export class ThrottlingRequestManager - implements IRequestManager, SupportsDomainThrottling -{ - readonly #inner: T; +export class ThrottlingRequestManager implements IRequestManager { + readonly #innerFactory: () => T | Promise; + #innerPromise?: Promise; + #resolvedInner?: T; readonly #requestManagerOpener: RequestManagerOpener; readonly #baseDelayMs: number; readonly #maxDelayMs: number; readonly #maxDomainStallMs: number; - readonly #minCrawlDelayMs: number; + #minCrawlDelayMs: number; readonly #throttlesEveryDomain: boolean; readonly #throttleBy: 'hostname' | 'registrableDomain'; readonly #maxThrottledDomains: number; @@ -313,9 +301,12 @@ export class ThrottlingRequestManager; - /** Batches still being added in the background; keeps {@apilink ThrottlingRequestManager.isFinished} honest. */ + /** Batches still being added in the background; keeps {@apilink ThrottlingRequestManager.checkReadiness} honest. */ #inProgressBatchCount = 0; + /** The latest {@link setExpectedRequestProcessingTimeSecs} hint, kept for a wrapped manager resolved later. */ + #expectedRequestProcessingSecs?: number; + readonly #warnedAbout = new Set(); /** Whether any domain at all may end up throttled - listed up front, or discovered as the crawl runs. */ @@ -329,7 +320,18 @@ export class ThrottlingRequestManager this.#requestManagerOpener(null, { configuration: this.config }); + } else if (typeof options.inner === 'function') { + this.#innerFactory = options.inner; + } else { + // Nothing to open: resolved from the start, so `innerManager` and bookkeeping see it immediately. + this.#resolvedInner = options.inner; + this.#innerFactory = () => this.#resolvedInner!; + } + this.#requestManagerOpener = options.requestManagerOpener ?? ((idOrAlias, opts) => RequestQueue.open(idOrAlias, opts) as unknown as Promise); @@ -378,9 +380,33 @@ export class ThrottlingRequestManager { + if (this.#resolvedInner === undefined) { + this.#innerPromise ??= Promise.resolve(this.#innerFactory()); + this.#resolvedInner = await this.#innerPromise; + + if (this.#expectedRequestProcessingSecs !== undefined) { + await this.#resolvedInner.setExpectedRequestProcessingTimeSecs?.(this.#expectedRequestProcessingSecs); + } + } + + return this.#resolvedInner; } /** Warns once about sources that cannot be routed by domain, because their URLs are not known yet. */ @@ -429,7 +455,7 @@ export class ThrottlingRequestManager this.#maxDelayMs) { - const source = retryAfterGiven ? 'Retry-After header' : 'exponential backoff'; + const source = waitGiven ? 'requested wait' : 'exponential backoff'; this.log.warning( `Capping ${source} delay of ${(delayMs / 1000).toFixed(1)}s for domain "${state.domain}" ` + `to maxDelaySecs (${(this.#maxDelayMs / 1000).toFixed(1)}s); the domain may continue to rate-limit. ` + @@ -618,14 +727,14 @@ export class ThrottlingRequestManager { - await this.#ensureSubManagers(); - - const now = Date.now(); - const candidates = Array.from(this.domainStates.values()).filter( - // Together: it is still turning us away, and has been doing so without a break for longer than the - // window. A domain that has simply been idle starts this clock at its first 429 rather than - // arriving with the idle time already on it. - (state) => - state.rateLimitedSince !== 0 && - now - state.lastRateLimitedAt <= this.#maxDomainStallMs && - now - state.rateLimitedSince > this.#maxDomainStallMs, - ); - - const stalled = ( - await Promise.all( - candidates.map(async (state) => { - const subManager = await this.#subManagers.get(state.domain); - return subManager && !(await subManager.isEmpty()) ? state : null; - }), - ) - ).filter((state) => state !== null); - - if (stalled.length === 0) { - return; - } - - const summary = stalled - .map((state) => `"${state.domain}" (${((now - state.rateLimitedSince) / 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 ` + - `with \`purgeOnStart\` disabled will resume them if the rate limit lifts.`, - ); - } - /** Records that a domain let a request through, which ends any rate-limit run stall detection was timing. */ #recordProgress(url: string): void { const state = this.#getDomainState(url); @@ -812,8 +873,9 @@ export class ThrottlingRequestManager { const key = request.id ?? request.uniqueKey; + // Only a fetch from the wrapped manager puts a key here, so it is necessarily resolved already. if (this.#inFlightFromInner.delete(key)) { - return this.#inner; + return this.#resolvedInner!; } return this.#selectManagerOrThrow(request.url); @@ -832,29 +894,102 @@ export class ThrottlingRequestManager { + async checkReadiness(): Promise { await this.#ensureSubManagers(); - const fetchable = await Promise.all( - this.#fetchableDomains().map(async (domain) => this.#subManagers.get(domain)!), - ); - const results = await Promise.all([this.#inner, ...fetchable].map(async (manager) => manager.isEmpty())); + const now = Date.now(); + const dispatchable: Promise[] = []; + let readyAt: number | undefined; + let stallCandidates: DomainState[] | undefined; - return results.every(Boolean); - } + for (const state of this.domainStates.values()) { + // A `Crawl-delay` can give a domain a clock before its first request gives it a queue - nothing + // to fetch from and nothing to wait for until then. + if (!this.#subManagers.has(state.domain)) { + continue; + } - /** Unlike {@apilink ThrottlingRequestManager.isEmpty}, throttled requests still count as outstanding work. */ - async isFinished(): Promise { - if (this.#inProgressBatchCount > 0) { - return false; + if ( + // Together: still turning us away, and has been without a break for longer than the window - a + // domain that has merely been idle starts this clock at its first 429, not with idle time on it. + state.rateLimitedSince !== 0 && + now - state.lastRateLimitedAt <= this.#maxDomainStallMs && + now - state.rateLimitedSince > this.#maxDomainStallMs + ) { + // Deliberately not dispatchable: a domain that has refused every request for the whole window + // is not progress just because its backoff momentarily lapsed - that is what it does between 429s. + (stallCandidates ??= []).push(state); + continue; + } + + const until = throttledUntil(state); + + if (now >= until) { + dispatchable.push(this.#subManagers.get(state.domain)!); + } else if (readyAt === undefined || until < readyAt) { + readyAt = until; + } + } + + const probed = ( + await Promise.all([ + (await this.#getInner()).checkReadiness(), + ...dispatchable.map(async (subManager) => (await subManager).checkReadiness()), + ]) + ).reduce(joinRequestSourceStatuses); + + // Anything dispatchable outranks a stalled or throttled domain, so we stop here without touching them. + if (probed.status === 'ready') { + return probed; + } + + if (stallCandidates !== undefined) { + const stalled = ( + await Promise.all( + stallCandidates.map(async (state) => + (await (await this.#subManagers.get(state.domain)!).checkReadiness()).status === 'ready' + ? state + : null, + ), + ) + ).filter((state) => state !== null); + + if (stalled.length > 0) { + const summary = stalled + .map((state) => `"${state.domain}" (${((now - state.rateLimitedSince) / 1000).toFixed(0)}s)`) + .join(', '); + + return { + status: 'stalled', + reason: + `${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 with \`purgeOnStart\` disabled will resume them if the rate limit lifts.`, + }; + } + } + + if (readyAt !== undefined) { + return joinRequestSourceStatuses(probed, { status: 'waiting', readyAt }); } - return this.#everyManager((manager) => manager.isFinished()); + // Batches still landing in the background are work nobody can see in a queue yet. + if (this.#inProgressBatchCount > 0 && probed.status === 'finished') { + return { status: 'waiting' }; + } + + return probed; } /** @@ -862,17 +997,15 @@ export class ThrottlingRequestManager { - await this.#inner.purge?.(); - await this.purgeDomainQueues(); + await this.#purgeDomainQueues(); + await this.#resolvedInner?.purge?.(); } /** - * Empties the per-domain queues, leaving the wrapped manager alone. - * - * Those queues are this manager's own no matter who owns the one it wraps, which is what makes this safe to - * call where a full {@apilink ThrottlingRequestManager.purge|`purge()`} would not be. + * Empties the per-domain queues, leaving the wrapped manager alone - those queues are this manager's own no + * matter who owns the one it wraps. */ - async purgeDomainQueues(): Promise { + async #purgeDomainQueues(): Promise { const subManagers = await this.#getSubManagers(); await Promise.all(subManagers.map(async (manager) => manager.purge?.())); @@ -887,31 +1020,40 @@ export class ThrottlingRequestManager { + // Remembered so a manager opened later still gets the hint, without opening one now just to pass it along. + this.#expectedRequestProcessingSecs = secs; await this.#forEachManager((manager) => manager.setExpectedRequestProcessingTimeSecs?.(secs)); } + /** + * Runs `fn` over the sub-queues and, if it has been resolved, the wrapped manager - bookkeeping never forces + * a lazily-opened `inner`, since there is no point opening a queue purely to tell it something. + */ async #forEachManager(fn: (manager: T) => Promise | undefined): Promise { + const managers = await this.#getSubManagers(); + + if (this.#resolvedInner !== undefined) { + managers.push(this.#resolvedInner); + } + // `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))); + await Promise.all(managers.map(async (manager) => fn(manager))); } async #sumOverManagers(fn: (manager: T) => Promise): Promise { - const counts = await Promise.all([this.#inner, ...(await this.#getSubManagers())].map(fn)); + // Counts have to include the wrapped manager, so this one does resolve it. + const counts = await Promise.all([await this.#getInner(), ...(await this.#getSubManagers())].map(fn)); return counts.reduce((a, b) => a + b, 0); } - async #everyManager(fn: (manager: T) => Promise): Promise { - const results = await Promise.all([this.#inner, ...(await this.#getSubManagers())].map(fn)); - return results.every(Boolean); - } - /** * Returns the next request from a domain that is not backing off, or from the inner manager. * * Returns `null` while every remaining request belongs to a throttled domain - it never waits the backoff * out, because a consumer parked in here holds a concurrency slot, which the autoscaler reads as spare - * capacity and answers by scaling up. Callers poll instead, and {@apilink ThrottlingRequestManager.isEmpty} - * reports `true` meanwhile so the crawler's task loop idles rather than spins. + * capacity and answers by scaling up. Callers poll instead, and + * {@apilink ThrottlingRequestManager.checkReadiness|`checkReadiness()`} reports `waiting` meanwhile so the + * crawler's task loop idles rather than spins. */ async fetchNextRequest(): Promise | null> { await this.#ensureSubManagers(); @@ -937,7 +1079,7 @@ export class ThrottlingRequestManager(); + const request = await (await this.#getInner()).fetchNextRequest(); if (request !== null) { this.#inFlightFromInner.add(request.id ?? request.uniqueKey); diff --git a/packages/core/test/request-queue/request-queue.test.ts b/packages/core/test/request-queue/request-queue.test.ts index 1611303242d6..80e9eb6ea28f 100644 --- a/packages/core/test/request-queue/request-queue.test.ts +++ b/packages/core/test/request-queue/request-queue.test.ts @@ -41,39 +41,35 @@ describe('RequestQueue#fetchNextRequest delegates to the client', () => { }); }); -describe('RequestQueue#isEmpty and #isFinished treat in-progress requests differently', () => { +describe('RequestQueue#checkReadiness treats in-progress requests differently from handled ones', () => { let queue: RequestQueue; beforeAll(async () => { queue = await makeQueue('is-empty-vs-is-finished', 1); }); - test('a fetched (in-progress) request leaves the queue empty but not finished', async () => { + test('a fetched (in-progress) request leaves the queue waiting rather than finished', async () => { const request = await queue.fetchNextRequest(); expect(request).not.toBe(null); - // The fetched request is in progress (locked), not handled. There is nothing left to fetch, so - // the queue is empty (`isEmpty` is the "would fetchNextRequest return null" check). It is not - // finished though — the in-progress request might still be reclaimed — and that is what prevents - // a crawler from shutting down while a request is still being processed. - expect(await queue.isEmpty()).toBe(true); - expect(await queue.isFinished()).toBe(false); + // The in-progress request is locked, not handled, and might still be reclaimed — reporting `waiting` + // rather than `finished` is what prevents a crawler from shutting down while it is being processed. + expect((await queue.checkReadiness()).status).toBe('waiting'); }); test('handling the in-progress request finishes the queue', async () => { const request = await queue.getRequest('0'); await queue.markRequestAsHandled(request!); - expect(await queue.isEmpty()).toBe(true); - expect(await queue.isFinished()).toBe(true); + expect((await queue.checkReadiness()).status).toBe('finished'); }); }); -describe('RequestQueue#isFinished waits for background add operations', () => { - test('returns false while a background batch is still being added', async () => { +describe('RequestQueue#checkReadiness waits for background add operations', () => { + test('reports waiting while a background batch is still being added', async () => { const queue = await makeQueue('is-finished-background'); - expect(await queue.isFinished()).toBe(true); + expect((await queue.checkReadiness()).status).toBe('finished'); let callCount = 0; let resolveBatch!: () => void; @@ -99,8 +95,8 @@ describe('RequestQueue#isFinished waits for background add operations', () => { expect(req1).toBeDefined(); await queue.markRequestAsHandled(req1!); - // While the 2nd batch is in flight in the background, isFinished() reports false. - expect(await queue.isFinished()).toBe(false); + // The 2nd batch is still in flight in the background. + expect((await queue.checkReadiness()).status).toBe('waiting'); // Unblock the background batch and wait for it to complete. resolveBatch(); @@ -110,6 +106,6 @@ describe('RequestQueue#isFinished waits for background add operations', () => { expect(req2).toBeDefined(); await queue.markRequestAsHandled(req2!); - expect(await queue.isFinished()).toBe(true); + expect((await queue.checkReadiness()).status).toBe('finished'); }); }); diff --git a/packages/crawlee/package.json b/packages/crawlee/package.json index 11ad028cfebd..90aeebaf1eaa 100644 --- a/packages/crawlee/package.json +++ b/packages/crawlee/package.json @@ -66,14 +66,10 @@ "tslib": "^2.8.1" }, "peerDependencies": { - "idcac-playwright": "*", "playwright": "*", "puppeteer": "*" }, "peerDependenciesMeta": { - "idcac-playwright": { - "optional": true - }, "playwright": { "optional": true }, diff --git a/packages/fs-storage/package.json b/packages/fs-storage/package.json index 606a7e7852fe..233207eaf685 100644 --- a/packages/fs-storage/package.json +++ b/packages/fs-storage/package.json @@ -42,7 +42,7 @@ "access": "public" }, "dependencies": { - "@crawlee/fs-storage-native": "0.1.5-beta.18", + "@crawlee/fs-storage-native": "0.2.0", "@crawlee/types": "workspace:*", "@crawlee/utils": "workspace:*", "zod": "catalog:" diff --git a/packages/playwright-crawler/package.json b/packages/playwright-crawler/package.json index b1332db20834..36c1798bef89 100644 --- a/packages/playwright-crawler/package.json +++ b/packages/playwright-crawler/package.json @@ -66,13 +66,9 @@ "zod": "catalog:" }, "peerDependencies": { - "idcac-playwright": "^0.2.0", "playwright": "*" }, "peerDependenciesMeta": { - "idcac-playwright": { - "optional": true - }, "playwright": { "optional": true } diff --git a/packages/playwright-crawler/src/internals/playwright-crawler.ts b/packages/playwright-crawler/src/internals/playwright-crawler.ts index 0ccd7209be0a..1f35c39d4ffb 100644 --- a/packages/playwright-crawler/src/internals/playwright-crawler.ts +++ b/packages/playwright-crawler/src/internals/playwright-crawler.ts @@ -334,7 +334,6 @@ export class PlaywrightCrawler< requestManager: this.requestManager!, }), compileScript: (scriptString: string, ctx?: Dictionary) => playwrightUtils.compileScript(scriptString, ctx), - closeCookieModals: async () => playwrightUtils.closeCookieModals(context.page), handleCloudflareChallenge: async (options?: HandleCloudflareChallengeOptions) => { return playwrightUtils.handleCloudflareChallenge(context.page, context.request.url, options); }, diff --git a/packages/playwright-crawler/src/internals/utils/playwright-utils.ts b/packages/playwright-crawler/src/internals/utils/playwright-utils.ts index bc7fcb903acb..8e95548c262f 100644 --- a/packages/playwright-crawler/src/internals/utils/playwright-utils.ts +++ b/packages/playwright-crawler/src/internals/utils/playwright-utils.ts @@ -632,37 +632,6 @@ export async function parseWithCheerio( return $; } -let idcacPlaywright: null | { getInjectableScript: () => string } = null; -async function getIdcacPlaywright() { - if (idcacPlaywright) return idcacPlaywright; - - try { - idcacPlaywright = await import('idcac-playwright'); - } catch (error: any) { - getLog().warning(`Failed to import 'idcac-playwright'. - -We recently made idcac-playwright an optional dependency due to licensing issues. -To use this feature, please install it manually by running - -npm install idcac-playwright - -Original error message follows: - -${error.message} -`); - } - return idcacPlaywright; -} - -export async function closeCookieModals(page: Page): Promise { - parseArgument(page, validators.browserPage); - const idcac = await getIdcacPlaywright(); - - if (idcac?.getInjectableScript()) { - await page.evaluate(idcac.getInjectableScript()); - } -} - export interface HandleCloudflareChallengeOptions { /** Logging defaults to the `debug` level, use this flag to log to `info` level instead. */ verbose?: boolean; @@ -1006,20 +975,6 @@ export interface PlaywrightContextUtils { */ compileScript(scriptString: string, ctx?: Dictionary): CompiledScriptFunction; - /** - * Tries to close cookie consent modals on the page. Based on the I Don't Care About Cookies browser extension. - * - * Note that this method requires the idcac-playwright package to be installed. - * Crawlee does not include it by default due to licensing issues. - * - * To use this method, please install the package manually by running: - * - * ```bash - * npm install idcac-playwright - * ``` - */ - closeCookieModals(): Promise; - /** * This helper tries to solve the Cloudflare challenge automatically by clicking on the checkbox. * It will try to detect the Cloudflare page, click on the checkbox, and wait for 10 seconds (configurable @@ -1080,7 +1035,6 @@ export const playwrightUtils = { infiniteScroll, saveSnapshot, compileScript, - closeCookieModals, RenderingTypePredictor, handleCloudflareChallenge, }; diff --git a/packages/puppeteer-crawler/package.json b/packages/puppeteer-crawler/package.json index f0be0461a874..3e14f34722cc 100644 --- a/packages/puppeteer-crawler/package.json +++ b/packages/puppeteer-crawler/package.json @@ -60,13 +60,9 @@ "zod": "catalog:" }, "peerDependencies": { - "idcac-playwright": "^0.2.0", "puppeteer": "*" }, "peerDependenciesMeta": { - "idcac-playwright": { - "optional": true - }, "puppeteer": { "optional": true } diff --git a/packages/puppeteer-crawler/src/internals/puppeteer-crawler.ts b/packages/puppeteer-crawler/src/internals/puppeteer-crawler.ts index 6b1b39f70fa2..7d6292823ee9 100644 --- a/packages/puppeteer-crawler/src/internals/puppeteer-crawler.ts +++ b/packages/puppeteer-crawler/src/internals/puppeteer-crawler.ts @@ -314,7 +314,6 @@ export class PuppeteerCrawler< ...options, configuration: serviceLocator.getConfiguration(), }), - closeCookieModals: async () => puppeteerUtils.closeCookieModals(context.page), }; } diff --git a/packages/puppeteer-crawler/src/internals/utils/puppeteer_utils.ts b/packages/puppeteer-crawler/src/internals/utils/puppeteer_utils.ts index a69dc7ad548b..dfca18387a11 100644 --- a/packages/puppeteer-crawler/src/internals/utils/puppeteer_utils.ts +++ b/packages/puppeteer-crawler/src/internals/utils/puppeteer_utils.ts @@ -778,37 +778,6 @@ export async function saveSnapshot(page: Page, options: SaveSnapshotOptions = {} } } -let idcacPlaywright: null | { getInjectableScript: () => string } = null; -async function getIdcacPlaywright() { - if (idcacPlaywright) return idcacPlaywright; - - try { - idcacPlaywright = await import('idcac-playwright'); - } catch (error: any) { - getLog().warning(`Failed to import 'idcac-playwright'. - -We recently made idcac-playwright an optional dependency due to licensing issues. -To use this feature, please install it manually by running - -npm install idcac-playwright - -Original error message follows: - -${error.message} -`); - } - return idcacPlaywright; -} - -export async function closeCookieModals(page: Page): Promise { - parseArgument(page, validators.browserPage); - const idcac = await getIdcacPlaywright(); - - if (idcac?.getInjectableScript()) { - await page.evaluate(idcac.getInjectableScript()); - } -} - export interface PuppeteerContextUtils { /** * Injects a JavaScript file into current `page`. @@ -1053,20 +1022,6 @@ export interface PuppeteerContextUtils { * Saves a full screenshot and HTML of the current page into a Key-Value store. */ saveSnapshot(options?: SaveSnapshotOptions): Promise; - - /** - * Tries to close cookie consent modals on the page. Based on the I Don't Care About Cookies browser extension. - * - * Note that this method requires the idcac-playwright package to be installed. - * Crawlee does not include it by default due to licensing issues. - * - * To use this method, please install the package manually by running: - * - * ```bash - * npm install idcac-playwright - * ``` - */ - closeCookieModals(): Promise; } export { enqueueLinksByClickingElements, addInterceptRequestHandler, removeInterceptRequestHandler }; @@ -1084,5 +1039,4 @@ export const puppeteerUtils = { infiniteScroll, saveSnapshot, parseWithCheerio, - closeCookieModals, }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bceb7a212e14..8bdcd9e027b0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -281,9 +281,6 @@ importers: '@crawlee/utils': specifier: workspace:* version: link:packages/utils - '@microsoft/api-extractor': - specifier: ^7.58.9 - version: 7.58.9(@types/node@24.12.2) '@oxlint/plugins': specifier: ^1.62.0 version: 1.62.0 @@ -793,9 +790,6 @@ importers: '@crawlee/utils': specifier: workspace:* version: link:../utils - idcac-playwright: - specifier: '*' - version: 0.2.0 import-local: specifier: ^3.2.0 version: 3.2.0 @@ -812,8 +806,8 @@ importers: packages/fs-storage: dependencies: '@crawlee/fs-storage-native': - specifier: 0.1.5-beta.18 - version: 0.1.5-beta.18 + specifier: 0.2.0 + version: 0.2.0 '@crawlee/types': specifier: workspace:* version: link:../types @@ -1000,9 +994,6 @@ importers: cheerio: specifier: ^1.0.0 version: 1.2.0 - idcac-playwright: - specifier: ^0.2.0 - version: 0.2.0 jquery: specifier: ^3.7.1 version: 3.7.1 @@ -1054,9 +1045,6 @@ importers: devtools-protocol: specifier: '*' version: 0.0.1666840 - idcac-playwright: - specifier: ^0.2.0 - version: 0.2.0 jquery: specifier: ^3.7.1 version: 3.7.1 @@ -2284,33 +2272,60 @@ packages: resolution: {integrity: sha512-TzlTVpKPjaqW6qOYjQcYUDuGsLCNsvFHVBXkYGTAnf5V37jCWrE5haKNXzz0WZUtVHjrpV76L1buANjwXMfT8w==} engines: {node: '>=22'} - '@crawlee/fs-storage-native-darwin-arm64@0.1.5-beta.18': - resolution: {integrity: sha512-fYZbU61GoMw+3q1JXRvARa8f/A8qZo0BxBpGogzq8duhiOChs4v+uHjffuZjL4E6goFR5qjX2kwOAJzoEp2kZQ==} + '@crawlee/fs-storage-native-darwin-arm64@0.2.0': + resolution: {integrity: sha512-Jg4OjQVZWqQz6QCQ939iY/EJA55zAg2ZhB5JtTy0okhkfPc8kYHN43DRP2FswDHRACaftOEy8NaI1H3+V4Z4KQ==} engines: {node: '>= 20'} cpu: [arm64] os: [darwin] - '@crawlee/fs-storage-native-darwin-x64@0.1.5-beta.18': - resolution: {integrity: sha512-Wb/d6PLor34790ixEFsRADvTUXKzR846/1E2LUaDTLA3KQ5QUCwjQhs7VpfrgY9AzzgM/s8G+3TO2UI6L3eYkw==} + '@crawlee/fs-storage-native-darwin-x64@0.2.0': + resolution: {integrity: sha512-BnBgAVygRAaEBFcyTgSNr9DrquUuIe4O/jtgSPctQ/oUiZ+PkK2DKk+OEtpMXpC+wVzLsBWnSs2aFnFIf78PYA==} engines: {node: '>= 20'} cpu: [x64] os: [darwin] - '@crawlee/fs-storage-native-linux-x64-gnu@0.1.5-beta.18': - resolution: {integrity: sha512-J8qfCkj3i6ic87fiD4f05WSXLXaAFLZlboXeeH9nDhJv/6RLxjiwTqec/Ig9H3yon+v4VQUbSHIh3Mm6B0g3XQ==} + '@crawlee/fs-storage-native-linux-arm64-gnu@0.2.0': + resolution: {integrity: sha512-YWJEgDHLaN11chIGAvRJOkqgdxPmzkRTeEGG1ptg0VYQbNzNdXvq6ykiq1njDwWwgUzXTqjeRFF2/U44Hc4xXw==} + engines: {node: '>= 20'} + cpu: [arm64] + os: [linux] + libc: [glibc] + + '@crawlee/fs-storage-native-linux-arm64-musl@0.2.0': + resolution: {integrity: sha512-C+ED04FlzKNxECIl3miisbX9eC3pSV/yrFboCrUAycA8Vh01fRTfeREzS+hMOsCQsRXeCdQ5Ifihi1T/nJwUwQ==} + engines: {node: '>= 20'} + cpu: [arm64] + os: [linux] + libc: [musl] + + '@crawlee/fs-storage-native-linux-x64-gnu@0.2.0': + resolution: {integrity: sha512-NlZ3Ud9dBiqkXwFEJbEVOg6PcnYdUF+C+gR3LvD/H4FOr5noLn48Kp8ikbEMbGlPdyPtLvdsMp2MCQPlQEPYYw==} engines: {node: '>= 20'} cpu: [x64] os: [linux] libc: [glibc] - '@crawlee/fs-storage-native-win32-x64-msvc@0.1.5-beta.18': - resolution: {integrity: sha512-76uxjzcsD2E+mWqZ+HENpcSpJJAZVdWf3osKR3pA3wld+yzR9o0cckRysu4QTDhUeNtzL1KIq2NzOF1GPNLqiw==} + '@crawlee/fs-storage-native-linux-x64-musl@0.2.0': + resolution: {integrity: sha512-Jb78RXko15FIXQgyb7pdTgYVc8oLN+ltEDJt9QprJUh65kUrVmPc/d2kXJhtvkH7CGEYR6/QLjfRKvoD1eeWyg==} engines: {node: '>= 20'} cpu: [x64] + os: [linux] + libc: [musl] + + '@crawlee/fs-storage-native-win32-arm64-msvc@0.2.0': + resolution: {integrity: sha512-UPX2/G67OmUzqAnhKnUjMwxxhJzrmUEug8gSgIGLqYAV/B6wsCU7e5a59Ekc4vroVSZxmBJ3ahNu6lXL7oMhBA==} + engines: {node: '>= 20'} + cpu: [arm64] os: [win32] - '@crawlee/fs-storage-native@0.1.5-beta.18': - resolution: {integrity: sha512-G5+Alb5GDAZomtu+0mB8oeYA0BCMqOQIqNoTPtZxh2wLcb17CpMfJJvq6D/nT8vANAaBkrMR49bbrodtROV2ug==} + '@crawlee/fs-storage-native-win32-x64-msvc@0.2.0': + resolution: {integrity: sha512-jMafrxiPm/nU6rmW1PXVj5O5mnK8oz2nnf2aRaV6bpwWS9XJ7pHIJOo208o54pkNz0m7E6IKRaA248So8ei3KA==} + engines: {node: '>= 20'} + cpu: [x64] + os: [win32] + + '@crawlee/fs-storage-native@0.2.0': + resolution: {integrity: sha512-fLFhUZBBvgY2KmKaOcvJK1QaiwxoDt9XGaCbgG3xGxIZJfZW1OS71FKjAdxIv9QjcRWtUxD1QwD9kFHlpb5P2g==} engines: {node: '>= 20'} '@crawlee/types@3.16.0': @@ -3453,19 +3468,6 @@ packages: '@mermaid-js/parser@1.2.0': resolution: {integrity: sha512-oYPyv8A4As1yH5Bx+04iQEQxXuIQDe0GKCNSRgao6z8AM9jixXIfP0vsppRLvGf+nKIOb9/LdpWA4YuJiVvESA==} - '@microsoft/api-extractor-model@7.33.8': - resolution: {integrity: sha512-aIcoQggPyer3B6Ze3usz0YWC/oBwUHfRH5ETUsr+oT2BRA6SfTJl7IKPcPZkX4UR+PohowzW4uMxsvjrn8vm+w==} - - '@microsoft/api-extractor@7.58.9': - resolution: {integrity: sha512-S2UF4yza5GoxCmf7hJQNxJNZN9ltOVuOQv8Dy+Z21aol5ERoBNMdWcQHm4MJMPPItW4H/4rZD906iaf4mUojJA==} - hasBin: true - - '@microsoft/tsdoc-config@0.18.1': - resolution: {integrity: sha512-9brPoVdfN9k9g0dcWkFeA7IH9bbcttzDJlXvkf8b2OBzd5MueR1V2wkKBL0abn0otvmkHJC6aapBOTJDDeMCZg==} - - '@microsoft/tsdoc@0.16.0': - resolution: {integrity: sha512-xgAyonlVVS+q7Vc7qLW0UrJU7rSFcETRWsqdXZtjzRU8dF+6CkozTK4V4y1LwOX7j8r/vHphjDeMeGI4tNGeGA==} - '@modelcontextprotocol/sdk@1.29.0': resolution: {integrity: sha512-zo37mZA9hJWpULgkRpowewez1y6ML5GsXJPY8FI0tBBCd77HEvza4jDqRKOXgHNn867PVGCyTdzqpz0izu5ZjQ==} engines: {node: '>=18'} @@ -4299,36 +4301,6 @@ packages: '@rtsao/scc@1.1.0': resolution: {integrity: sha512-zt6OdqaDoOnJ1ZYsCYGt9YmWzDXl4vQdKTyJev62gFhRGKdx7mcT54V9KIjg+d2wi9EXsPvAPKe7i7WjfVWB8g==} - '@rushstack/node-core-library@5.23.1': - resolution: {integrity: sha512-wlKmIKIYCKuCASbITvOxLZXepPbwXvrv7S6ig6XNWFchSyhL/E2txmVXspHY49Wu2dzf7nI27a2k/yV5BA3EiA==} - peerDependencies: - '@types/node': '*' - peerDependenciesMeta: - '@types/node': - optional: true - - '@rushstack/problem-matcher@0.2.1': - resolution: {integrity: sha512-gulfhBs6n+I5b7DvjKRfhMGyUejtSgOHTclF/eONr8hcgF1APEDjhxIsfdUYYMzC3rvLwGluqLjbwCFZ8nxrog==} - peerDependencies: - '@types/node': '*' - peerDependenciesMeta: - '@types/node': - optional: true - - '@rushstack/rig-package@0.7.3': - resolution: {integrity: sha512-aAA518n6wxxjCfnTAOjQnm7ngNE0FVHxHAw2pxKlIhxrMn0XQjGcXKF0oKWpjBgJOmsaJpVob/v+zr3zxgPWuA==} - - '@rushstack/terminal@0.24.0': - resolution: {integrity: sha512-8ZQS4MMaGsv27EXCBiH7WMPkRZrffeDoIevs6z9TM5dzqiY6+Hn4evfK/G+gvgBTjfvfkHIZPQQmalmI2sM4TQ==} - peerDependencies: - '@types/node': '*' - peerDependenciesMeta: - '@types/node': - optional: true - - '@rushstack/ts-command-line@5.3.10': - resolution: {integrity: sha512-fwI076HYknC0IrMXdY6UmjDv+PH7NHhNJX3/pY2UblSE5XrXgndXZPiOe/6ZtuFpn6DvVDVNhtkIzQ+Qu/MhVQ==} - '@sapphire/async-queue@1.5.5': resolution: {integrity: sha512-cvGzxbba6sav2zZkH8GPf2oGk9yYoD5qrNWdu9fRehifgnFZJMV+nuy2nON2roRO4yQQ+v7MK/Pktl/HgfsUXg==} engines: {node: '>=v14.0.0', npm: '>=7.0.0'} @@ -4756,9 +4728,6 @@ packages: '@tybys/wasm-util@0.9.0': resolution: {integrity: sha512-6+7nlbMVX/PVDCwaIQ8nTOPveOcFLSt8GcXdx8hD0bt39uWxYT88uXzqTd4fTvqta7oeUJqudepapKNt2DYJFw==} - '@types/argparse@1.0.38': - resolution: {integrity: sha512-ebDJ9b0e702Yr7pWgB0jzm+CX4Srzz8RcXtLJDJB+BSccqMa36uyH/zUsSYao5+BD1ytv3k3rPYCq4mAE1hsXA==} - '@types/body-parser@1.19.6': resolution: {integrity: sha512-HLFeCYgz89uk22N5Qg3dvGvsv46B8GLvKKo1zKG4NybA8U2DiEO3w9lqGg29t/tfLRJpJ6iQxnVw4OnB7MoM9g==} @@ -5575,14 +5544,6 @@ packages: peerDependencies: zod: ^3.25.76 || ^4.1.8 - ajv-draft-04@1.0.0: - resolution: {integrity: sha512-mv00Te6nmYbRp5DCwclxtt7yV/joXJPGS7nM+97GdxvuttCOfgI3K4U25zboyeX0O+myI8ERluxQe5wljMmVIw==} - peerDependencies: - ajv: ^8.5.0 - peerDependenciesMeta: - ajv: - optional: true - ajv-formats@2.1.1: resolution: {integrity: sha512-Wx0Kx52hxE7C18hkMEggYlEifqWZtYaRgouJor+WMdPnQyEK13vgEWyVNup7SoeeoLMsr4kf5h6dOW11I15MUA==} peerDependencies: @@ -5684,9 +5645,6 @@ packages: arg@5.0.2: resolution: {integrity: sha512-PYjyFOLKQ9y57JvQ6QLo8dAgNqswh8M1RMJYdQduT6xbWSgK36P/Z/v+p888pM69jMMfS8Xd8F6I1kQ/I9HUGg==} - argparse@1.0.10: - resolution: {integrity: sha512-o5Roy6tNG4SL/FOkCAN6RzjiakZS25RLYFrcMttJqbdd8BWrnA+fGz57iN5Pb06pvBGvl5gQ0B48dJlslXvoTg==} - argparse@2.0.1: resolution: {integrity: sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==} @@ -7078,10 +7036,6 @@ packages: devtools-protocol@0.0.1666840: resolution: {integrity: sha512-gCcO42XCHKEs7Ag0S7aGYsnJ7hlgrO3qderYqeiY0Eqk+0GFfuvT13IA0hHreJTa2KCdDVyGMeOhdMNmrrTjVg==} - diff@8.0.4: - resolution: {integrity: sha512-DPi0FmjiSU5EvQV0++GFDOJ9ASQUVFh5kD+OzOnYdi7n3Wpm9hWWGfB/O2blfHcMVTL5WkQXSnRiK9makhrcnw==} - engines: {node: '>=0.3.1'} - diffie-hellman@5.0.3: resolution: {integrity: sha512-kqag/Nl+f3GwyK25fhUMYj81BUOrZ9IuJsjIcDE5icNM9FJHAVm3VcUDxdLPoQtTuUylWm6ZIknYJwwaPxsUzg==} @@ -8367,9 +8321,6 @@ packages: peerDependencies: postcss: ^8.1.0 - idcac-playwright@0.2.0: - resolution: {integrity: sha512-qJH7vQgq3TKnhea/3Z3jlEJL7NC9vK9BkLClAzQHVRepBtq1fWfSI4fSuMKcPq7nDUTTlIEIS+vU+GRwwR1BXw==} - identifier-regex@1.0.1: resolution: {integrity: sha512-ZrYyM0sozNPZlvBvE7Oq9Bn44n0qKGrYu5sQ0JzMUnjIhpgWYE2JB6aBoFwEYdPjqj7jPyxXTMJiHDOxDfd8yw==} engines: {node: '>=18'} @@ -8879,9 +8830,6 @@ packages: resolution: {integrity: sha512-ekilCSN1jwRvIbgeg/57YFh8qQDNbwDb9xT/qu2DAHbFFZUicIl4ygVaAvzveMhMVr3LnpSKTNnwt8PoOfmKhQ==} hasBin: true - jju@1.4.0: - resolution: {integrity: sha512-8wb9Yw966OSxApiCt0K3yNJL8pnNeIv+OEq2YMidz4FKP6nonSRoOXc80iXY4JaN2FC11B9qsNmDsm+ZOfMROA==} - joi@17.13.4: resolution: {integrity: sha512-1RuuER6kmt8K8I3nIWvPZKi5RQCb568ZPyY4Pwjlua+yo+63ZTmIwxLZH0heBmiKN4uxjvCiarDrjaeH84xicQ==} @@ -11909,9 +11857,6 @@ packages: split@1.0.1: resolution: {integrity: sha512-mTyOoPbrivtXnwnIxZRFYRrPNtEFKlpB2fvjSnCQUiAA6qAZzqwna5envK4uk6OIeP17CsdF3rSBGYVBsU0Tkg==} - sprintf-js@1.0.3: - resolution: {integrity: sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g==} - srcset@4.0.0: resolution: {integrity: sha512-wvLeHgcVHKO8Sc/H/5lkGreJQVeYMm9rlmt8PuR1xE31rIuXhuzznUUqAt8MqLhB3MqJdFzlNAfpcWnxiFUcPw==} engines: {node: '>=12'} @@ -14503,24 +14448,40 @@ snapshots: '@conventional-changelog/template@1.2.1': {} - '@crawlee/fs-storage-native-darwin-arm64@0.1.5-beta.18': + '@crawlee/fs-storage-native-darwin-arm64@0.2.0': optional: true - '@crawlee/fs-storage-native-darwin-x64@0.1.5-beta.18': + '@crawlee/fs-storage-native-darwin-x64@0.2.0': optional: true - '@crawlee/fs-storage-native-linux-x64-gnu@0.1.5-beta.18': + '@crawlee/fs-storage-native-linux-arm64-gnu@0.2.0': optional: true - '@crawlee/fs-storage-native-win32-x64-msvc@0.1.5-beta.18': + '@crawlee/fs-storage-native-linux-arm64-musl@0.2.0': optional: true - '@crawlee/fs-storage-native@0.1.5-beta.18': + '@crawlee/fs-storage-native-linux-x64-gnu@0.2.0': + optional: true + + '@crawlee/fs-storage-native-linux-x64-musl@0.2.0': + optional: true + + '@crawlee/fs-storage-native-win32-arm64-msvc@0.2.0': + optional: true + + '@crawlee/fs-storage-native-win32-x64-msvc@0.2.0': + optional: true + + '@crawlee/fs-storage-native@0.2.0': optionalDependencies: - '@crawlee/fs-storage-native-darwin-arm64': 0.1.5-beta.18 - '@crawlee/fs-storage-native-darwin-x64': 0.1.5-beta.18 - '@crawlee/fs-storage-native-linux-x64-gnu': 0.1.5-beta.18 - '@crawlee/fs-storage-native-win32-x64-msvc': 0.1.5-beta.18 + '@crawlee/fs-storage-native-darwin-arm64': 0.2.0 + '@crawlee/fs-storage-native-darwin-x64': 0.2.0 + '@crawlee/fs-storage-native-linux-arm64-gnu': 0.2.0 + '@crawlee/fs-storage-native-linux-arm64-musl': 0.2.0 + '@crawlee/fs-storage-native-linux-x64-gnu': 0.2.0 + '@crawlee/fs-storage-native-linux-x64-musl': 0.2.0 + '@crawlee/fs-storage-native-win32-arm64-msvc': 0.2.0 + '@crawlee/fs-storage-native-win32-x64-msvc': 0.2.0 '@crawlee/types@3.16.0': dependencies: @@ -16286,41 +16247,6 @@ snapshots: dependencies: '@chevrotain/types': 11.1.2 - '@microsoft/api-extractor-model@7.33.8(@types/node@24.12.2)': - dependencies: - '@microsoft/tsdoc': 0.16.0 - '@microsoft/tsdoc-config': 0.18.1 - '@rushstack/node-core-library': 5.23.1(@types/node@24.12.2) - transitivePeerDependencies: - - '@types/node' - - '@microsoft/api-extractor@7.58.9(@types/node@24.12.2)': - dependencies: - '@microsoft/api-extractor-model': 7.33.8(@types/node@24.12.2) - '@microsoft/tsdoc': 0.16.0 - '@microsoft/tsdoc-config': 0.18.1 - '@rushstack/node-core-library': 5.23.1(@types/node@24.12.2) - '@rushstack/rig-package': 0.7.3 - '@rushstack/terminal': 0.24.0(@types/node@24.12.2) - '@rushstack/ts-command-line': 5.3.10(@types/node@24.12.2) - diff: 8.0.4 - minimatch: 9.0.9 - resolve: 1.22.12 - semver: 7.7.4 - source-map: 0.6.1 - typescript: 5.9.3 - transitivePeerDependencies: - - '@types/node' - - '@microsoft/tsdoc-config@0.18.1': - dependencies: - '@microsoft/tsdoc': 0.16.0 - ajv: 8.18.0 - jju: 1.4.0 - resolve: 1.22.12 - - '@microsoft/tsdoc@0.16.0': {} - '@modelcontextprotocol/sdk@1.29.0(@cfworker/json-schema@4.1.1)(supports-color@7.2.0)(zod@4.4.3)': dependencies: '@hono/node-server': 1.19.17(hono@4.13.3) @@ -17074,45 +17000,6 @@ snapshots: '@rtsao/scc@1.1.0': {} - '@rushstack/node-core-library@5.23.1(@types/node@24.12.2)': - dependencies: - ajv: 8.18.0 - ajv-draft-04: 1.0.0(ajv@8.18.0) - ajv-formats: 3.0.1(ajv@8.18.0) - fs-extra: 11.3.4 - import-lazy: 4.0.0 - jju: 1.4.0 - resolve: 1.22.12 - semver: 7.7.4 - optionalDependencies: - '@types/node': 24.12.2 - - '@rushstack/problem-matcher@0.2.1(@types/node@24.12.2)': - optionalDependencies: - '@types/node': 24.12.2 - - '@rushstack/rig-package@0.7.3': - dependencies: - jju: 1.4.0 - resolve: 1.22.12 - - '@rushstack/terminal@0.24.0(@types/node@24.12.2)': - dependencies: - '@rushstack/node-core-library': 5.23.1(@types/node@24.12.2) - '@rushstack/problem-matcher': 0.2.1(@types/node@24.12.2) - supports-color: 8.1.1 - optionalDependencies: - '@types/node': 24.12.2 - - '@rushstack/ts-command-line@5.3.10(@types/node@24.12.2)': - dependencies: - '@rushstack/terminal': 0.24.0(@types/node@24.12.2) - '@types/argparse': 1.0.38 - argparse: 1.0.10 - string-argv: 0.3.2 - transitivePeerDependencies: - - '@types/node' - '@sapphire/async-queue@1.5.5': {} '@sec-ant/readable-stream@0.4.1': {} @@ -17508,8 +17395,6 @@ snapshots: dependencies: tslib: 2.8.1 - '@types/argparse@1.0.38': {} - '@types/body-parser@1.19.6': dependencies: '@types/connect': 3.4.38 @@ -18346,10 +18231,6 @@ snapshots: '@opentelemetry/api': 1.9.0 zod: 4.4.3 - ajv-draft-04@1.0.0(ajv@8.18.0): - optionalDependencies: - ajv: 8.18.0 - ajv-formats@2.1.1(ajv@8.18.0): optionalDependencies: ajv: 8.18.0 @@ -18520,10 +18401,6 @@ snapshots: arg@5.0.2: {} - argparse@1.0.10: - dependencies: - sprintf-js: 1.0.3 - argparse@2.0.1: {} args@5.0.3: @@ -20170,8 +20047,6 @@ snapshots: devtools-protocol@0.0.1666840: {} - diff@8.0.4: {} - diffie-hellman@5.0.3: dependencies: bn.js: 4.12.3 @@ -22074,8 +21949,6 @@ snapshots: dependencies: postcss: 8.5.26 - idcac-playwright@0.2.0: {} - identifier-regex@1.0.1: dependencies: reserved-identifiers: 1.2.0 @@ -22514,8 +22387,6 @@ snapshots: jiti@2.6.1: {} - jju@1.4.0: {} - joi@17.13.4: dependencies: '@hapi/hoek': 9.3.0 @@ -26596,8 +26467,6 @@ snapshots: dependencies: through: 2.3.8 - sprintf-js@1.0.3: {} - srcset@4.0.0: {} ssri@12.0.0: diff --git a/scripts/api-extractor/api-extractor.base.json b/scripts/api-extractor/api-extractor.base.json deleted file mode 100644 index de57e9e7e31a..000000000000 --- a/scripts/api-extractor/api-extractor.base.json +++ /dev/null @@ -1,60 +0,0 @@ -{ - "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", - - "newlineKind": "lf", - - "compiler": { - "overrideTsconfig": { - "compilerOptions": { - "module": "NodeNext", - "moduleResolution": "NodeNext", - "target": "ESNext", - "lib": ["DOM", "ES2023", "ES2024", "DOM.AsyncIterable"], - "skipLibCheck": true, - "strict": true - } - } - }, - - "apiReport": { - "enabled": true, - "reportFolder": "/../../docs/public-api", - "reportTempFolder": "/../../docs/public-api/temp" - }, - - "docModel": { - "enabled": false - }, - - "dtsRollup": { - "enabled": false - }, - - "tsdocMetadata": { - "enabled": false - }, - - "messages": { - "compilerMessageReporting": { - "default": { - "logLevel": "none" - } - }, - "extractorMessageReporting": { - "default": { - "logLevel": "none" - }, - "ae-incompatible-release-tags": { - "logLevel": "warning" - }, - "ae-forgotten-export": { - "logLevel": "warning" - } - }, - "tsdocMessageReporting": { - "default": { - "logLevel": "none" - } - } - } -} diff --git a/scripts/api-extractor/run.ts b/scripts/api-extractor/run.ts deleted file mode 100644 index ad0171f5d76f..000000000000 --- a/scripts/api-extractor/run.ts +++ /dev/null @@ -1,669 +0,0 @@ -/* eslint-disable no-console */ -import { spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { basename, dirname, relative, resolve } from 'node:path'; - -import { Extractor, ExtractorConfig, type IConfigFile } from '@microsoft/api-extractor'; -import { globbySync } from 'globby'; -// The workspace `typescript` is the TS 7 native compiler, which no longer ships the JS API -// this script parses reports with; `typescript-v6` is an npm alias to classic TypeScript. -import ts from 'typescript-v6'; - -/** - * Generates (`--verify` to check) a per-package map of the public type-level interface of - * each publishable `@crawlee/*` package, committed to `docs/public-api/.api.md`. - * These reports define where we promise backwards compatibility; changes must be reviewed. - * - * Before extraction we sanitize the `.d.ts` files (for the duration of the run, restoring them - * afterwards): (1) the build (`scripts/typescript_fixes.mjs`) injects `// @ts-ignore` comment - * lines that crash API Extractor's AST walker, so we strip them; (2) we rewrite the legacy - * JSDoc `@ignore` tag to `@internal`, since API Extractor only trims by release tag and would - * otherwise leak `@ignore`-d members into the `public` report. Rewriting `@ignore` also trims - * the members that were crashing API Extractor's AST walker (e.g. `BrowserLauncher`'s inline - * `import("ow")` `optionsShape`), so the affected packages now extract cleanly on the primary - * pass. A few packages may still re-export a comment-injected member across a package boundary - * and crash anyway; those are retried against a sanitized mirror of the dist tree with - * `@crawlee/*` deps remapped via tsconfig `paths`. - * - * A report should describe exactly the surface it maps: every name it mentions declared, and - * nothing declared that it does not mention. API Extractor gets us close but not there, because - * it decides what to *include* before it trims the non-`@public` declarations and never - * revisits that decision. Three things follow, and we handle each after extraction: - * - * 1. Imports of symbols only an `@internal` member used survive the trim and read as public - * surface (`Cookie` from `tough-cookie` in `@crawlee/core`). We drop them — - * see `pruneUnusedImports`. - * 2. Types the public API references but the entry point never exports would otherwise be - * named by the report and defined nowhere. `includeForgottenExports` emits them instead - * (without `export`, since they are observable but not importable) — the alternative was - * exporting ~38 types from their packages, committing us to names we never meant to - * publish. API Extractor labels them exactly like a real export, so we add a banner - * saying otherwise — see `annotateForgottenDeclarations`. The same pre-trim blind spot - * means it also emits declarations reachable only from trimmed members, which we drop — - * see `pruneDeadForgottenDeclarations`. - * 3. What (2) cannot supply, because the release-tag trim removes it first: a `@public` - * signature referencing an `@internal` type. That is a genuine tagging bug in the source, - * so the run fails — see `danglingReferences` and `INCOMPATIBLE_RELEASE_TAGS`. - * - * The pruning in (1) and (2) feed each other — dropping a declaration orphans its imports — so - * `pruneReport` runs them to a fixed point. - * - * When running under GitHub Actions (or with `--github`), failures are additionally emitted - * as workflow commands (`::error::`) so they show up as inline annotations in the CI run. - */ - -const root = resolve(import.meta.dirname, '..', '..'); -const baseConfigPath = resolve(import.meta.dirname, 'api-extractor.base.json'); -const baseConfig = JSON.parse(readFileSync(baseConfigPath, 'utf8')) as IConfigFile; -const reportFolder = resolve(root, 'docs', 'public-api'); -// API Extractor writes the "public" variant to a `.public.api.md` staging file here; we then -// promote it onto the committed `.api.md` ourselves (see `extract`), so the committed -// filenames stay stable while the report content is @public-only (no @internal symbols). -const stagingFolder = resolve(reportFolder, 'temp'); -const mirrorRoot = resolve(root, 'node_modules', '.cache', 'api-extractor-dts'); -const verify = process.argv.includes('--verify'); - -// Emit GitHub Actions workflow commands (annotations) when running in CI, so out-of-date -// reports and crashes surface as inline warnings/errors. Opt in with `--github` or force -// off with `--no-github` (auto-detected via the runner-set GITHUB_ACTIONS env var otherwise). -const github = process.argv.includes('--github') - || (process.env.GITHUB_ACTIONS === 'true' && !process.argv.includes('--no-github')); - -// GitHub workflow commands must escape `%`, `\r` and `\n` in the message. See -// https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands -const ghEscape = (message: string) => message.replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A'); -const ghCommand = (kind: 'error' | 'warning', message: string) => { - if (github) console.log(`::${kind}::${ghEscape(message)}`); -}; - -const TS_IGNORE_LINE = /^\s*\/\/ @ts-ignore optional peer dependency or compatibility with es2022\s*$/; -// `@ignore` is a legacy JSDoc/TypeDoc tag that API Extractor does not act on — unlike the -// release tags (`@internal`/`@alpha`/`@beta`), it does NOT trim the member from the report, -// so `@ignore`-d symbols wrongly leak into the `public` variant. There is no config knob for -// this, so we rewrite the tag to `@internal` in the (transient) `.d.ts`, letting API -// Extractor's real release-tag trimming drop them from the public surface map. We only rewrite -// `@ignore` when it sits directly behind a JSDoc gutter (`/**` or a leading `*`), which covers -// both the single-line `/** @ignore */` and multi-line ` * @ignore` forms while leaving prose -// or string literals that merely mention "@ignore" untouched. -const IGNORE_TAG = /(\/\*\*|\*)(\s*)@ignore\b/g; -// CLI binary and project scaffolding are tooling, not an importable API where we promise BC. -const EXCLUDED = new Set(['@crawlee/cli', '@crawlee/templates']); - -// The two API Extractor analyzer messages we act on (everything else is silenced to `none` -// in `api-extractor.base.json`); both describe a symbol the report references but never -// declares, which makes the committed surface map internally inconsistent. -// -// ae-incompatible-release-tags — a @public symbol's signature references an @internal one. -// The referenced type is trimmed from the @public report, so the surface map is left -// referring to a name that appears nowhere in it. Fatal: this is a genuine tagging bug, -// and the fix belongs in the source (drop the tag — untagged is implicitly public — or -// keep the type out of the public signature). -// ae-forgotten-export — a referenced symbol isn't exported from the entry point at all, so -// the report names a type consumers cannot import. Also fatal, but only once confirmed -// against the finished report: these messages are raised before the `@public` trim, so -// most of them concern symbols that never reach it (see `danglingReferences`). -const INCOMPATIBLE_RELEASE_TAGS = 'ae-incompatible-release-tags'; -const FORGOTTEN_EXPORT = 'ae-forgotten-export'; -type AnalyzerMessageId = typeof INCOMPATIBLE_RELEASE_TAGS | typeof FORGOTTEN_EXPORT; - -/** Pulls `Foo` out of `The symbol "Foo" needs to be exported by the entry point index.d.ts`. */ -const quotedSymbol = (text: string) => text.match(/"([^"]+)"/)?.[1] ?? text; - -interface PackageManifest { - name: string; - private?: boolean; - types?: string; - exports?: Record; -} - -const packageJsonPaths = globbySync('packages/*/package.json', { cwd: root, absolute: true }).sort(); - -function manifest(pkgJsonPath: string): PackageManifest { - return JSON.parse(readFileSync(pkgJsonPath, 'utf8')) as PackageManifest; -} - -function dtsEntry(pkgDir: string, pkg: PackageManifest): string | undefined { - const dot = pkg.exports?.['.']; - const candidate = (typeof dot === 'object' ? dot.types : undefined) ?? pkg.types ?? './dist/index.d.ts'; - const full = resolve(pkgDir, candidate); - return existsSync(full) ? full : undefined; -} - -const sanitizeDts = (content: string) => - content - .split('\n') - .filter((line) => !TS_IGNORE_LINE.test(line)) - .join('\n') - .replace(IGNORE_TAG, '$1$2@internal'); - -type ImportStatement = ts.ImportDeclaration | ts.ImportEqualsDeclaration; -const isImport = (node: ts.Node): node is ImportStatement => - ts.isImportDeclaration(node) || ts.isImportEqualsDeclaration(node); - -/** Local binding names introduced by an import statement (`[]` for a side-effect import). */ -function importBindings(node: ImportStatement): string[] { - if (ts.isImportEqualsDeclaration(node)) return [node.name.text]; - const clause = node.importClause; - if (!clause) return []; - const names = clause.name ? [clause.name.text] : []; - const bound = clause.namedBindings; - if (bound && ts.isNamespaceImport(bound)) names.push(bound.name.text); - if (bound && ts.isNamedImports(bound)) names.push(...bound.elements.map((element) => element.name.text)); - return names; -} - -/** - * Drops import statements whose binding is never referenced in the rest of the report. - * - * API Extractor collects the import list from the entry point *before* it trims the - * non-`@public` declarations, and never revisits it — deliberately, since a rollup may - * legitimately need an import that its release-tag filter would have dropped. In the API - * report, though, a type reachable only from an `@internal` member survives as a bare - * import and reads as public surface (e.g. `Cookie` from `tough-cookie` in `@crawlee/core`), - * producing review noise on changes that never touched the public API. There is no config - * knob for this, so we post-process the report. - * - * The report body is ordinary TypeScript, so we parse it rather than pattern-match lines: - * bindings come from real import nodes (covering every shape the emitter produces, including - * `import X = require(...)`), and usages from real identifier tokens — so a name occurring - * only in a string literal or a `// Warning:` comment correctly does not count as a use. - */ -function parseReport(report: string): { lines: string[]; open: number; close: number; source: ts.SourceFile } | undefined { - const lines = report.split('\n'); - // The report is a fixed markdown skeleton wrapping a single ```ts fence. - const open = lines.indexOf('```ts'); - const close = lines.lastIndexOf('```'); - if (open === -1 || close <= open) return undefined; - - const source = ts.createSourceFile( - 'report.ts', - lines.slice(open + 1, close).join('\n'), - ts.ScriptTarget.Latest, - true, - ts.ScriptKind.TS, - ); - // `parseDiagnostics` is internal, but there is no public per-file equivalent that doesn't - // require a whole Program. A report that doesn't parse means our assumptions are broken, - // so skip the analysis rather than act on a half-understood tree. - const diagnostics = (source as unknown as { parseDiagnostics?: readonly ts.Diagnostic[] }).parseDiagnostics; - if (diagnostics?.length) { - const message = `report did not parse (${diagnostics.length} syntax errors) — skipping import pruning and the dangling-reference check`; - console.warn(`! ${message}`); - ghCommand('warning', message); - return undefined; - } - return { lines, open, close, source }; -} - -function pruneUnusedImports(report: string): string { - const parsed = parseReport(report); - if (!parsed) return report; - const { lines, open, close, source } = parsed; - - const imports = source.statements.filter(isImport); - if (imports.length === 0) return report; - - const used = new Set(); - const collect = (node: ts.Node) => { - // Skip the import statements themselves so a binding never counts as its own usage. - if (isImport(node)) return; - if (ts.isIdentifier(node)) used.add(node.text); - node.forEachChild(collect); - }; - source.forEachChild(collect); - - // Offset back into the surrounding markdown; `getStart` skips leading trivia so we never - // swallow a comment sitting above an import. - const lineOf = (position: number) => open + 1 + source.getLineAndCharacterOfPosition(position).line; - const unused = imports.filter((node) => { - const bindings = importBindings(node); - return bindings.length > 0 && bindings.every((binding) => !used.has(binding)); - }); - if (unused.length === 0) return report; - - const dropped = new Set(); - for (const node of unused) { - for (let line = lineOf(node.getStart(source)); line <= lineOf(node.getEnd()); line++) dropped.add(line); - } - // Emptying the block entirely would leave the blank line that separated it from the - // declarations stacked on the one after the ```ts fence; drop it so the output matches - // what API Extractor emits for an import-less report. - const after = lineOf(unused[unused.length - 1].getEnd()) + 1; - if (unused.length === imports.length && lines[after] === '') dropped.add(after); - - return lines.filter((_, index) => !dropped.has(index)).join('\n'); -} - -/** - * Drops forgotten-export declarations that nothing in the report refers to. - * - * `includeForgottenExports` emits a declaration for every symbol the entry point failed to - * export, but API Extractor decides that before the `@public` trim — so it also emits the ones - * reachable only from members that never make it into the report. Those exist to back a - * reference that isn't there, and they drag their own imports back in with them. - * - * Eligibility comes from API Extractor's own `ae-forgotten-export` list rather than from the - * absence of an `export` keyword. Missing `export` is not sufficient: `export * as ns` is - * rendered as `declare namespace ns { export { ... } }`, which also carries no export modifier - * yet is a real part of the surface — and removing it would cascade into everything it names - * (`@crawlee/utils`' `social` namespace and all its members). Only symbols API Extractor could - * not export are candidates, so nothing that is genuinely reachable can be dropped. - */ -function pruneDeadForgottenDeclarations(report: string, forgotten: ReadonlySet): string { - if (forgotten.size === 0) return report; - const parsed = parseReport(report); - if (!parsed) return report; - const { lines, open, close, source } = parsed; - - // Positions, not counts: a declaration must not keep itself alive. Backend classes here - // name themselves (`static create(): Promise`), so a plain occurrence count - // would never let one go. - const occurrences = new Map(); - const record = (node: ts.Node) => { - // Only genuine references count. `storage.DatasetBackend` on an unrelated method must not - // keep the local `DatasetBackend` class alive, and nor must a member of the same name. - if (ts.isIdentifier(node) && !isDeclarationName(node)) { - const positions = occurrences.get(node.text); - if (positions) positions.push(node.getStart(source)); - else occurrences.set(node.text, [node.getStart(source)]); - } - node.forEachChild(record); - }; - source.forEachChild(record); - - const dead = source.statements.filter((statement) => { - if (isImport(statement) || ts.isExportDeclaration(statement)) return false; - const modifiers = ts.canHaveModifiers(statement) ? ts.getModifiers(statement) : undefined; - if (modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword)) return false; - const names = ts.isVariableStatement(statement) - ? statement.declarationList.declarations.map((declaration) => declaration.name) - : [(statement as { name?: ts.Node }).name]; - if (names.length === 0 || !names.every((name) => name && ts.isIdentifier(name))) return false; - const from = statement.getStart(source); - const to = statement.getEnd(); - return (names as ts.Identifier[]).every( - (name) => - forgotten.has(name.text) - && !(occurrences.get(name.text) ?? []).some((position) => position < from || position > to), - ); - }); - if (dead.length === 0) return report; - - const lineOf = (position: number) => open + 1 + source.getLineAndCharacterOfPosition(position).line; - const dropped = new Set(); - for (const statement of dead) { - // Take the `// @public (undocumented)` banner API Extractor writes above the declaration - // with it. Ask for the actual comment ranges rather than working back from - // `getFullStart()`, which sits at the end of the *previous* statement. - const comments = ts.getLeadingCommentRanges(source.text, statement.getFullStart()) ?? []; - const start = lineOf(comments[0]?.pos ?? statement.getStart(source)); - for (let line = start; line <= lineOf(statement.getEnd()); line++) dropped.add(line); - // Also take the blank line that separated it from the previous entry, so the neighbours - // don't end up glued together. - if (start - 1 > open && lines[start - 1].trim() === '') dropped.add(start - 1); - } - return lines.filter((_, index) => !dropped.has(index)).join('\n'); -} - -/** - * Removes everything the report carries but does not need: forgotten-export declarations - * nothing refers to, and imports nothing refers to. Each pass can expose more of the other - * (dropping a declaration orphans the imports it used), so this runs to a fixed point. It - * always terminates — every pass that changes anything strictly removes lines. - */ -function pruneReport(report: string, forgotten: ReadonlySet): string { - for (let current = report; ; ) { - const next = pruneUnusedImports(pruneDeadForgottenDeclarations(current, forgotten)); - if (next === current) return current; - current = next; - } -} - -/** Banner marking a declaration that is observable but not importable. */ -const NOT_EXPORTED_BANNER = '// Not exported by the entry point; reachable only as a referenced type.'; - -/** - * Labels the declarations `includeForgottenExports` contributed. - * - * Without this they render as `// @public (undocumented)`, identical to a genuine export apart - * from a missing `export` keyword — far too subtle to survive review. The distinction matters: - * their *shape* is part of the surface we promise not to break, but their *name* is not - * something a consumer can import. - */ -function annotateForgottenDeclarations(report: string, forgotten: ReadonlySet): string { - if (forgotten.size === 0) return report; - const parsed = parseReport(report); - if (!parsed) return report; - const { lines, open, source } = parsed; - - const lineOf = (position: number) => open + 1 + source.getLineAndCharacterOfPosition(position).line; - const marked = new Set(); - for (const statement of source.statements) { - if (isImport(statement) || ts.isExportDeclaration(statement)) continue; - const modifiers = ts.canHaveModifiers(statement) ? ts.getModifiers(statement) : undefined; - if (modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword)) continue; - const names = ts.isVariableStatement(statement) - ? statement.declarationList.declarations.map((declaration) => declaration.name) - : [(statement as { name?: ts.Node }).name]; - if (!names.length || !names.every((name) => name && ts.isIdentifier(name) && forgotten.has(name.text))) continue; - // Sit above API Extractor's own banner rather than replacing it, so its markers survive. - const comments = ts.getLeadingCommentRanges(source.text, statement.getFullStart()) ?? []; - const at = lineOf(comments[0]?.pos ?? statement.getStart(source)); - // Our banner becomes a leading comment itself, so skip anything already labelled. The - // staged report is regenerated from scratch each run and never arrives annotated, but - // that is a property of the caller, not of this function. - if (lines[at] !== NOT_EXPORTED_BANNER) marked.add(at); - } - if (marked.size === 0) return report; - - return lines.flatMap((line, index) => (marked.has(index) ? [NOT_EXPORTED_BANNER, line] : [line])).join('\n'); -} - -/** True when the identifier names something (a declaration, member or property) rather than referring to it. */ -function isDeclarationName(id: ts.Identifier): boolean { - const parent = id.parent as ts.Node | undefined; - if (!parent) return false; - // `a.b` / `A.B` — only the leftmost part resolves against the report's own scope. - if (ts.isQualifiedName(parent) && parent.right === id) return true; - if (ts.isPropertyAccessExpression(parent) && parent.name === id) return true; - // `export { x }` (as a `declare namespace` uses to re-expose its members) names the local - // binding, so it is a reference to it rather than a declaration of it. - if (ts.isExportSpecifier(parent)) return false; - return 'name' in parent && (parent as { name?: ts.Node }).name === id; -} - -/** Names the report introduces itself: top-level declarations plus whatever the imports bind. */ -function declaredNames(source: ts.SourceFile): Set { - const names = new Set(); - for (const statement of source.statements) { - if (isImport(statement)) { - for (const binding of importBindings(statement)) names.add(binding); - } else if (ts.isVariableStatement(statement)) { - for (const declaration of statement.declarationList.declarations) { - if (ts.isIdentifier(declaration.name)) names.add(declaration.name.text); - } - } else { - const name = (statement as { name?: ts.Node }).name; - if (name && ts.isIdentifier(name)) names.add(name.text); - } - } - return names; -} - -/** - * Names the report refers to but never declares or imports, leaving the committed surface map - * describing types a consumer cannot actually name. - * - * `candidates` comes from API Extractor's `ae-forgotten-export` messages. Restricting the search - * to those is what makes this check cheap and exact: deciding "is this identifier unresolved?" - * from scratch would mean re-implementing name resolution over the TypeScript globals, whereas - * API Extractor has already told us precisely which symbols it could not export. - * - * The filtering matters in the other direction too. Those messages are raised during analysis, - * *before* the `@public` trim, so most of them concern symbols reachable only from members that - * never make it into the report — harmless, and not something the source should be contorted to - * fix. Checking the finished artifact instead of the raw message list keeps only the real ones. - */ -function danglingReferences(report: string, candidates: Iterable): string[] { - const wanted = new Set(candidates); - if (wanted.size === 0) return []; - const parsed = parseReport(report); - if (!parsed) return []; - - const declared = declaredNames(parsed.source); - const dangling = new Set(); - const visit = (node: ts.Node) => { - if (ts.isIdentifier(node) && wanted.has(node.text) && !declared.has(node.text) && !isDeclarationName(node)) { - dangling.add(node.text); - } - node.forEachChild(visit); - }; - parsed.source.forEachChild(visit); - return [...dangling].sort(); -} - -const reportBaseName = (name: string) => name.replace('@', '').replace('/', '-'); -const reportFileName = (name: string) => `${reportBaseName(name)}.api.md`; -// With `reportVariants: ['public']`, API Extractor appends the variant kind to the file name, -// producing `.public.api.md`. We stage that, then promote it to `.api.md`. -const stagedFileName = (name: string) => `${reportBaseName(name)}.public.api.md`; - -/** Lazily built sanitized mirror of the dist tree, with a `@crawlee/*` -> mirror paths map. */ -let mirror: { packages: string; paths: Record } | undefined; -function getMirror() { - if (mirror) return mirror; - rmSync(mirrorRoot, { recursive: true, force: true }); - for (const file of globbySync('packages/*/dist/**/*.d.ts', { cwd: root, absolute: true })) { - const target = resolve(mirrorRoot, relative(root, file)); - mkdirSync(dirname(target), { recursive: true }); - writeFileSync(target, sanitizeDts(readFileSync(file, 'utf8'))); - } - const packages = resolve(mirrorRoot, 'packages'); - const paths: Record = {}; - for (const pkgJsonPath of packageJsonPaths) { - const dir = resolve(packages, relative(resolve(root, 'packages'), dirname(pkgJsonPath))); - if (existsSync(resolve(dir, 'dist/index.d.ts'))) paths[manifest(pkgJsonPath).name] = [resolve(dir, 'dist/index.d.ts')]; - } - mirror = { packages, paths }; - return mirror; -} - -function extract(pkgDir: string, pkgJsonPath: string, entry: string, paths?: Record) { - const name = manifest(pkgJsonPath).name; - const config = ExtractorConfig.prepare({ - configObjectFullPath: baseConfigPath, - packageJsonFullPath: pkgJsonPath, - configObject: { - ...baseConfig, - projectFolder: pkgDir, - mainEntryPointFilePath: entry, - compiler: paths - ? { overrideTsconfig: { compilerOptions: { baseUrl: root, paths } } } - : baseConfig.compiler, - apiReport: { - enabled: true, - // @public-only: drops @internal/@alpha/@beta symbols from the surface map. - reportVariants: ['public'], - // Emit declarations for symbols the public API references but the entry point - // never exports, instead of leaving the report referring to names that appear - // nowhere in it. They are emitted *without* `export`, which is the honest - // rendering: the shape is part of the observable surface and is tracked for BC, - // but the name is not importable. The alternative — exporting every such type - // from its package — would have added ~38 new public exports here, committing us - // to names we never meant to publish, so we track the shape instead. - includeForgottenExports: true, - reportFileName: reportFileName(name), - // Stage into temp; we promote the `.public.api.md` output onto the committed - // `.api.md` ourselves so the tracked filenames don't change. - reportFolder: stagingFolder, - reportTempFolder: stagingFolder, - }, - }, - }); - // Collected rather than printed, so `main` decides severity and the output stays grouped - // per package. Duplicates are common (one message per overload/declaration), hence the Set. - const diagnostics: Record> = { - [INCOMPATIBLE_RELEASE_TAGS]: new Set(), - [FORGOTTEN_EXPORT]: new Set(), - }; - // Let API Extractor always write the staged report (localBuild), then diff it against the - // committed report ourselves so `--verify` keys off the stable `.api.md` name. - Extractor.invoke(config, { - localBuild: true, - showVerboseMessages: false, - messageCallback: (message) => { - diagnostics[message.messageId as AnalyzerMessageId]?.add(message.text); - // Suppress API Extractor's own console output; everything else is already `none`. - message.handled = true; - }, - }); - - const forgotten = new Set([...diagnostics[FORGOTTEN_EXPORT]].map(quotedSymbol)); - const stagedPath = resolve(stagingFolder, stagedFileName(name)); - // Annotate after pruning, so declarations that are about to be dropped are never labelled. - const staged = annotateForgottenDeclarations(pruneReport(readFileSync(stagedPath, 'utf8'), forgotten), forgotten); - // Persist the pruned report so a `--verify` failure diffs against what we actually compare. - writeFileSync(stagedPath, staged); - const committedPath = resolve(reportFolder, reportFileName(name)); - const committed = existsSync(committedPath) ? readFileSync(committedPath, 'utf8') : undefined; - const apiReportChanged = staged !== committed; - if (apiReportChanged && !verify) writeFileSync(committedPath, staged); - - // Anything still referenced but neither declared nor imported after the pruning above — - // i.e. a forgotten export that `includeForgottenExports` could not supply because the - // release-tag trim dropped it first (see `danglingReferences`). - const unexported = danglingReferences(staged, forgotten); - return { apiReportChanged, committedPath, stagedPath, diagnostics, unexported }; -} - -type ExtractResult = ReturnType; - -// Render the surface diff between the committed report and the freshly staged one, so a -// failing `--verify` shows *what* changed rather than only telling you to re-run `api:extract`. -// Uses `git diff --no-index` (git is always present in CI) to avoid a diffing dependency. Runs -// from `root` with repo-relative paths and `committed`/`extracted` prefixes so the diff header -// reads cleanly instead of dumping absolute, machine-specific paths. -function reportDiff(committedPath: string, stagedPath: string): string { - const result = spawnSync( - 'git', - [ - '--no-pager', - 'diff', - '--no-index', - '--no-color', - '--src-prefix=committed/', - '--dst-prefix=extracted/', - '--', - relative(root, committedPath), - relative(root, stagedPath), - ], - { cwd: root, encoding: 'utf8', maxBuffer: 32 * 1024 * 1024 }, - ); - // `git diff --no-index` exits 1 when files differ (expected here); only bail on a real error. - return (result.stdout ?? '').trim() || (result.stderr ?? '').trim(); -} - -function main() { - let failed = 0; - - // Report filenames owned by an in-scope package. Any committed `*.api.md` not in this set - // is orphaned (e.g. its package was removed or renamed) and gets pruned in extract mode. - // Keyed off package existence, not per-run success, so a transient build/extract failure - // never deletes an otherwise-valid report. - const expectedReports = new Set( - packageJsonPaths - .map(manifest) - .filter((pkg) => !pkg.private && !EXCLUDED.has(pkg.name)) - .map((pkg) => reportFileName(pkg.name)), - ); - - // We rewrite the `.d.ts` files in place for the duration of the run (restored after) to: - // 1. strip the `// @ts-ignore` lines the build injects, which crash Extractor's AST walker; - // 2. rewrite `@ignore` -> `@internal` so those members get trimmed from the public report. - const originals = new Map(); - for (const file of globbySync('packages/*/dist/**/*.d.ts', { cwd: root, absolute: true })) { - const content = readFileSync(file, 'utf8'); - const sanitized = sanitizeDts(content); - if (sanitized !== content) { - originals.set(file, content); - writeFileSync(file, sanitized); - } - } - - try { - for (const pkgJsonPath of packageJsonPaths) { - const pkg = manifest(pkgJsonPath); - if (pkg.private || EXCLUDED.has(pkg.name)) continue; - - const pkgDir = dirname(pkgJsonPath); - const entry = dtsEntry(pkgDir, pkg); - if (!entry) { - const message = `${pkg.name}: no built dist/index.d.ts — run "pnpm build" first`; - console.error(`✗ ${message}`); - ghCommand('error', message); - failed++; - continue; - } - - // Up to date iff the committed report didn't change, and the report is internally - // consistent (no @public symbol referencing a trimmed @internal one). Both modes - // enforce consistency: unlike an out-of-date report, regenerating can't fix it. - const ok = (result: ExtractResult, via = '') => { - if (result.unexported.length > 0) { - const message = `${pkg.name}: referenced by the public API but not exported from the entry point: ${result.unexported.join(', ')} — export them, or keep them out of the public signature`; - console.error(`✗ ${message}`); - ghCommand('error', message); - failed++; - } - - const inconsistent = result.diagnostics[INCOMPATIBLE_RELEASE_TAGS]; - for (const text of inconsistent) { - const message = `${pkg.name}: ${text} — the referenced symbol is trimmed from the public report, leaving a dangling reference; drop the tag, or keep the type out of the public signature`; - console.error(`✗ ${message}`); - ghCommand('error', message); - failed++; - } - - if (verify && result.apiReportChanged) { - const message = `${pkg.name}: report out of date${via} — run "pnpm api:extract" and commit the changes in docs/public-api/`; - console.error(`✗ ${pkg.name}: report out of date${via}`); - // Print the actual surface diff so the failure is self-explanatory in the CI - // log; keep the concise message for the inline GitHub annotation. - const diff = reportDiff(result.committedPath, result.stagedPath); - if (diff) console.error(`${diff}\n`); - ghCommand('error', message); - failed++; - } else if (inconsistent.size === 0 && result.unexported.length === 0) { - console.log(`✓ ${pkg.name}${via}`); - } - }; - - // Fallback: retry against the sanitized mirror (dodges an API Extractor crash on - // cross-package re-exports of comment-injected members). - const viaMirror = () => { - const { packages, paths } = getMirror(); - const mirrorEntry = resolve(packages, relative(resolve(root, 'packages'), pkgDir), relative(pkgDir, entry)); - const { [pkg.name]: _self, ...deps } = paths; - return extract(pkgDir, pkgJsonPath, mirrorEntry, deps); - }; - - try { - ok(extract(pkgDir, pkgJsonPath, entry)); - } catch { - try { - ok(viaMirror(), ' (via mirror)'); - } catch (err) { - const message = `${pkg.name}: api-extractor crashed: ${(err as Error).message}`; - console.error(`✗ ${message}`); - ghCommand('error', message); - failed++; - } - } - } - } finally { - for (const [file, content] of originals) writeFileSync(file, content); - rmSync(mirrorRoot, { recursive: true, force: true }); - } - - // Prune orphaned reports: committed `*.api.md` files with no owning in-scope package - // (e.g. a removed/renamed package). Delete them in extract mode; flag them in verify mode. - for (const file of globbySync('*.api.md', { cwd: reportFolder, absolute: true })) { - if (expectedReports.has(basename(file))) continue; - if (verify) { - const message = `${basename(file)}: orphaned report (no matching package) — run "pnpm api:extract" to remove it`; - console.error(`✗ ${message}`); - ghCommand('error', message); - failed++; - } else { - rmSync(file); - console.log(`✓ removed orphaned report ${basename(file)}`); - } - } - - if (failed > 0) { - if (verify) console.error('\nRun "pnpm api:extract" and commit the changes in docs/public-api/.'); - process.exit(1); - } -} - -main(); diff --git a/test/core/crawlers/basic_crawler.test.ts b/test/core/crawlers/basic_crawler.test.ts index 8cb7190b2152..846e4d5bbb00 100644 --- a/test/core/crawlers/basic_crawler.test.ts +++ b/test/core/crawlers/basic_crawler.test.ts @@ -26,6 +26,7 @@ import { ProxyConfiguration, Request, RequestList, + RequestManagerTandem, RequestQueue, RequestValidationError, Router, @@ -199,8 +200,7 @@ describe('BasicCrawler', () => { expect((basicCrawler.concurrencySystem! as ConcurrencySystem).minConcurrency).toBe(25); expect(processed).toEqual(sourcesCopy); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); }); test('accepts a `requestManager` and crawls from it', async () => { @@ -697,8 +697,7 @@ describe('BasicCrawler', () => { expect(state.processed).toEqual(sourcesCopy); expect(state.processed).toBe(processed); expect(state.processed).toEqual(sourcesCopy); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); }); test('print a warning on sharing state between two crawlers', async () => { @@ -837,8 +836,7 @@ describe('BasicCrawler', () => { await persistPromise; expect(finished).toBe(false); - expect(await requestList.isFinished()).toBe(false); - expect(await requestList.isEmpty()).toBe(false); + expect((await requestList.checkReadiness()).status).toBe('ready'); expect(processed.length).toBe(200); expect(getValueSpy).toBeCalled(); @@ -890,8 +888,7 @@ describe('BasicCrawler', () => { expect(processed['http://example.com/2'].errorMessages).toHaveLength(11); expect(processed['http://example.com/2'].retryCount).toBe(10); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); }); test('should retry failed requests based on `request.maxRetries`', async () => { @@ -925,8 +922,7 @@ describe('BasicCrawler', () => { expect(processed['http://example.com/3'].errorMessages).toHaveLength(2); expect(processed['http://example.com/3'].retryCount).toBe(1); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); }); test('should not retry requests with noRetry set to true', async () => { @@ -977,8 +973,7 @@ describe('BasicCrawler', () => { expect(failedRequestHandlerCalls).toBe(3); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); }); test('should correctly track request.state', async () => { @@ -1091,8 +1086,7 @@ describe('BasicCrawler', () => { expect(failed['http://example.com/3'].retryCount).toBe(3); expect(Object.values(failed)).toHaveLength(3); expect(Object.values(processed)).toHaveLength(0); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); errors.forEach((error) => expect(error).toBeInstanceOf(Error)); }); @@ -1130,8 +1124,7 @@ describe('BasicCrawler', () => { expect(failed['http://example.com/3'].errorMessages).toHaveLength(1); expect(failed['http://example.com/3'].retryCount).toBe(0); expect(Object.values(failed)).toHaveLength(3); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); errors.forEach((error) => expect(error).toBeInstanceOf(NonRetryableError)); }); @@ -1181,7 +1174,7 @@ describe('BasicCrawler', () => { await expect(basicCrawler.run()).rejects.toThrow(CriticalError); expect(failedRequestHandler).not.toBeCalled(); - expect(await requestList.isFinished()).toBe(false); + expect((await requestList.checkReadiness()).status).toBe('ready'); }); test('should crash on MissingRouteError', async () => { @@ -1261,8 +1254,12 @@ describe('BasicCrawler', () => { .mockReturnValue(Promise.resolve() as any); const reclaimReq = vitest.spyOn(requestQueue, 'reclaimRequest').mockReturnValue(Promise.resolve() as any); - vitest.spyOn(requestQueue, 'isEmpty').mockImplementation(async () => queueContent.length <= 0); - vitest.spyOn(requestQueue, 'isFinished').mockResolvedValueOnce(true); + // The first probe reporting `finished` is masked by the request list, which still has requests to + // transfer into the queue at that point. + vitest + .spyOn(requestQueue, 'checkReadiness') + .mockImplementation(async () => (queueContent.length > 0 ? { status: 'ready' } : { status: 'finished' })) + .mockResolvedValueOnce({ status: 'finished' }); await basicCrawler.run(); @@ -1282,15 +1279,14 @@ describe('BasicCrawler', () => { expect(processed['http://example.com/1'].errorMessages).toHaveLength(4); expect(processed['http://example.com/1'].retryCount).toBe(3); - expect(await requestList.isFinished()).toBe(true); - expect(await requestList.isEmpty()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); vitest.restoreAllMocks(); }); test('should say that task is not ready requestList is not set and requestQueue is empty', async () => { const requestQueue = await RequestQueue.open({ id: 'xxx' }); - requestQueue.isEmpty = async () => Promise.resolve(true); + requestQueue.checkReadiness = async () => Promise.resolve({ status: 'waiting' }); const crawler = new BasicCrawler({ requestQueue, @@ -1348,10 +1344,10 @@ describe('BasicCrawler', () => { return Promise.resolve() as any; }); - const isFinishedOrig = vitest.spyOn(requestQueue, 'isFinished'); - + // The stub reports `finished` as soon as the queue runs dry; the crawl carries on because the task loop + // defers to the custom `isFinishedFunction`. requestQueue.fetchNextRequest = async () => queue.pop()!; - requestQueue.isEmpty = async () => Promise.resolve(!queue.length); + requestQueue.checkReadiness = async () => (queue.length ? { status: 'ready' } : { status: 'finished' }); // Add requests with buffer time for crawler startup. // Use longer delays to avoid flakiness under CPU load from parallel tests. @@ -1362,7 +1358,6 @@ describe('BasicCrawler', () => { expect(markRequestAsHandled).toBeCalledWith(request0); expect(markRequestAsHandled).toBeCalledWith(request1); - expect(isFinishedOrig).not.toBeCalled(); expect(isFinishedFunctionCalled).toBe(true); expect(isTaskReadyFunctionCalled).toBe(true); @@ -1397,10 +1392,10 @@ describe('BasicCrawler', () => { .spyOn(requestQueue, 'markRequestAsHandled') .mockReturnValue(Promise.resolve() as any); - const isFinishedOrig = vitest.spyOn(requestQueue, 'isFinished'); - + // The stub reports `finished` whenever the queue runs dry - `keepAlive` is what carries the crawler + // through those gaps, until `teardown()` ends the run. requestQueue.fetchNextRequest = async () => Promise.resolve(queue.pop()!); - requestQueue.isEmpty = async () => Promise.resolve(!queue.length); + requestQueue.checkReadiness = async () => (queue.length ? { status: 'ready' } : { status: 'finished' }); // Use longer delays to avoid flakiness under CPU load from parallel tests. setTimeout(() => queue.push(request0), 500); @@ -1413,7 +1408,6 @@ describe('BasicCrawler', () => { expect(markRequestAsHandled).toBeCalledWith(request0); expect(markRequestAsHandled).toBeCalledWith(request1); - expect(isFinishedOrig).not.toBeCalled(); // TODO: see why the request1 was passed as a second parameter to includes expect(processed.includes(request0)).toBe(true); @@ -2604,15 +2598,23 @@ describe('BasicCrawler', () => { // 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'); + // `domainStates` is TS-private, and deliberately not `#private`, so that tests can read it. + const { domainStates } = requestManager as unknown as { + domainStates: Map; + }; + const state = domainStates.get('example.com')!; expect(state.declaredCrawlDelayMs).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(); + + const readiness = await requestManager.checkReadiness(); + expect(readiness).toMatchObject({ status: 'waiting' }); + expect(readiness.status === 'waiting' && readiness.readyAt).toBeGreaterThan(Date.now() + 4_000); }); - test('warns when the request manager cannot honour it', async () => { + test('warns naming the options that would honour it when nothing paces the domain', async () => { const crawler = crawlerWithCrawlDelay({ requestQueue: await RequestQueue.open() }); const warning = vitest.spyOn(crawler.log, 'warning').mockImplementation(() => {}); @@ -2620,9 +2622,10 @@ describe('BasicCrawler', () => { expect(warning).toHaveBeenCalledTimes(1); expect(warning.mock.calls[0][0]).toMatch(/crawl-delay of 5s/); + expect(warning.mock.calls[0][0]).toMatch(/`sameDomainDelaySecs`.*`ThrottlingRequestManager`/s); }); - test('warns when the domain is missing from the manager `domains` list', async () => { + test('warns naming the domain when it is missing from the manager `domains` list', async () => { const requestManager = new ThrottlingRequestManager({ inner: await RequestQueue.open(), domains: ['some-other-domain.com'], @@ -2632,9 +2635,39 @@ describe('BasicCrawler', () => { await crawler.addRequests(['http://example.com/1']); + // Same warning as when nothing paces at all - the fix it names covers either case. expect(warning).toHaveBeenCalledTimes(1); + expect(warning.mock.calls[0][0]).toMatch(/does not pace that domain/); expect(warning.mock.calls[0][0]).toMatch(/example\.com/); }); + + test('is honoured for requests that started life in a `requestList`', async () => { + const visits: number[] = []; + const crawler = new (class MockedRobotsTxtCrawler extends BasicCrawler { + override async getRobotsTxtFileForUrl(_: string) { + return RobotsTxtFile.from('http://example.com/robots.txt', 'User-agent: *\nCrawl-delay: 0.5\n'); + } + })({ + respectRobotsTxtFile: true, + requestList: await RequestList.open(null, [ + 'http://example.com/1', + 'http://example.com/2', + 'http://example.com/3', + ]), + // Negligible on its own, so the delay observed below can only come from robots.txt. + sameDomainDelaySecs: 0.01, + requestHandler: async () => { + visits.push(Date.now()); + }, + }); + + await crawler.run(); + + // robots.txt is only read while the first request is in flight, so the delay it declares first + // bites between the second and the third. + expect(visits).toHaveLength(3); + expect(visits[2] - visits[1]).toBeGreaterThanOrEqual(400); + }); }); describe('sameDomainDelaySecs', () => { @@ -2691,13 +2724,14 @@ describe('BasicCrawler', () => { expect(reclaimed).toEqual([]); }); - test('a crawl fed by a requestList still finishes', async () => { - // Those requests are transferred straight into the wrapped manager, so they are never routed by - // domain - and have to be handed back to it rather than to the queue their domain would own. + test('paces requests that came from a `requestList`, and still finishes the crawl', async () => { + // The tandem is the only position from which a list's requests reach a per-domain queue, and + // once there they have to be handed back to it, or the crawl would never finish. const requestList = await RequestList.open(null, ['http://example.com/1', 'http://example.com/2']); - const { visits } = await crawlerVisiting([], { requestList, sameDomainDelaySecs: 0.1 }); + const { visits } = await crawlerVisiting([], { requestList, sameDomainDelaySecs: 0.5 }); expect(visits.map(({ url }) => url).sort()).toEqual(['http://example.com/1', 'http://example.com/2']); + expect(visits[1].at - visits[0].at).toBeGreaterThanOrEqual(400); }); test('a second run() crawls the same requests again', async () => { @@ -2712,10 +2746,171 @@ describe('BasicCrawler', () => { expect(crawler.statistics.state.requestsFinished).toBe(1); }); - test('refuses to be combined with a request manager that throttles on its own', async () => { + test('a second run() over a supplied manager asks which storage to purge', async () => { + // A purge cannot respect both the crawler's per-domain queues and the caller's manager + // underneath them, so rather than pick one silently, say so. + const crawler = new BasicCrawler({ + requestQueue: await RequestQueue.open(), + sameDomainDelaySecs: 0.05, + requestHandler: async () => {}, + }); + + await crawler.run(['http://example.com/1']); + + await expect(crawler.run(['http://example.com/1'])).rejects.toThrow( + /Cannot decide what to purge.*purgeRequestQueue/s, + ); + }); + + test.each([true, false])('a second run() with purgeRequestQueue: %s does not ask', async (purge) => { + const crawler = new BasicCrawler({ + requestQueue: await RequestQueue.open(), + sameDomainDelaySecs: 0.05, + requestHandler: async () => {}, + }); + + await crawler.run(['http://example.com/1']); + + await expect( + crawler.run(['http://example.com/1'], { purgeRequestQueue: purge }), + ).resolves.toBeDefined(); + }); + + test('a second run() asks even when the first routed nothing by domain', async () => { + // The guard fires on the shape of the configuration, not on what the queues happen to hold: + // keying it on an existing per-domain queue would make identical code throw or not depending + // on run history. + const requestQueue = await RequestQueue.open(); + await requestQueue.addRequest({ url: 'http://example.com/pre-added-1' }); + await requestQueue.addRequest({ url: 'http://example.com/pre-added-2' }); + + const visits: number[] = []; + const crawler = new BasicCrawler({ + requestQueue, + sameDomainDelaySecs: 2, + requestHandler: async () => { + visits.push(Date.now()); + }, + }); + + await crawler.run(); + + // Precondition, established rather than assumed: no per-domain queue was opened, since one + // would have paced these 2s apart. + expect(visits).toHaveLength(2); + expect(visits[1] - visits[0]).toBeLessThan(1000); + + await expect(crawler.run()).rejects.toThrow(/Cannot decide what to purge/); + }); + + test('a second run() asks for a supplied `requestManager`, not just a `requestQueue`', async () => { + const crawler = new BasicCrawler({ + requestManager: await RequestQueue.open(), + sameDomainDelaySecs: 0.05, + requestHandler: async () => {}, + }); + + await crawler.run(['http://example.com/1']); + + await expect(crawler.run(['http://example.com/1'])).rejects.toThrow(/Cannot decide what to purge/); + }); + + test('a second run() leaves a supplied manager alone when nothing paces it', async () => { + // The contrast that makes the question above worth asking: with no `sameDomainDelaySecs` the + // crawler puts nothing of its own inside the caller's manager, so there is nothing to ask about. + let visits = 0; + const crawler = new BasicCrawler({ + requestQueue: await RequestQueue.open(), + requestHandler: async () => { + visits += 1; + }, + }); + + await crawler.run(['http://example.com/1']); + await crawler.run(['http://example.com/1']); + + expect(visits).toBe(1); + }); + + test('a second run() purges everything the crawler opened itself, without being asked', async () => { + // Nothing came from the caller, so there is nothing to ask about - and the purge has to reach + // the per-domain queues, or the second run would crawl nothing. + let visits = 0; + const crawler = new BasicCrawler({ + sameDomainDelaySecs: 0.05, + requestHandler: async () => { + visits += 1; + }, + }); + + await crawler.run(['http://example.com/1']); + await crawler.run(['http://example.com/1']); + + expect(visits).toBe(2); + }); + + test('wraps a user `requestManager` rather than replacing it', async () => { + const requestManager = await RequestQueue.open(); + + const { crawler, visits } = await crawlerVisiting(['http://example.com/1', 'http://example.com/2'], { + requestManager, + sameDomainDelaySecs: 0.5, + }); + + const active = await crawler.getRequestManager(); + expect(active).toBeInstanceOf(ThrottlingRequestManager); + expect((active as ThrottlingRequestManager).innerManager).toBe(requestManager); + + expect(visits).toHaveLength(2); + expect(visits[1].at - visits[0].at).toBeGreaterThanOrEqual(400); + }); + + test('hands the delay to a manager that paces requests itself', async () => { + const requestManager = new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: 'all', + throttleBy: 'registrableDomain', + }); + + const { crawler, visits } = await crawlerVisiting(['http://example.com/1', 'http://example.com/2'], { + requestManager, + sameDomainDelaySecs: 0.5, + }); + + // Nothing was built around it, so the caller's manager is still the only thing pacing. + await expect(crawler.getRequestManager()).resolves.toBe(requestManager); + + expect(visits).toHaveLength(2); + expect(visits[1].at - visits[0].at).toBeGreaterThanOrEqual(400); + }); + + test('reaches a pacing manager through a wrapper', async () => { + // A tandem is not a pacer but forwards signals, so the throttler behind it takes the floor - + // no wrapper type is inspected on the way. + const throttler = new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: 'all', + throttleBy: 'registrableDomain', + }); + const requestManager = new RequestManagerTandem( + await RequestList.open(null, ['http://example.com/1', 'http://example.com/2']), + throttler, + ); + + const { crawler, visits } = await crawlerVisiting([], { requestManager, sameDomainDelaySecs: 0.5 }); + + await expect(crawler.getRequestManager()).resolves.toBe(requestManager); + + expect(visits).toHaveLength(2); + expect(visits[1].at - visits[0].at).toBeGreaterThanOrEqual(400); + }); + + test('refuses a manager that paces only some of the domains it holds', async () => { + // Taking a floor that covers every domain would leave everything but `example.com` unpaced. const requestManager = new ThrottlingRequestManager({ inner: await RequestQueue.open(), domains: ['example.com'], + throttleBy: 'registrableDomain', }); expect( @@ -2725,7 +2920,28 @@ describe('BasicCrawler', () => { sameDomainDelaySecs: 1, requestHandler: async () => {}, }), - ).toThrow(/ThrottlingRequestManager/); + ).toThrow(/domains: 'all'/); + }); + + test('a second run() leaves a manager that took the delay alone', async () => { + // The floor put no storage of ours underneath it, so there is no purge to ask about. + let visits = 0; + const crawler = new BasicCrawler({ + requestManager: new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: 'all', + throttleBy: 'registrableDomain', + }), + sameDomainDelaySecs: 0.05, + requestHandler: async () => { + visits += 1; + }, + }); + + await crawler.run(['http://example.com/1']); + await crawler.run(['http://example.com/1']); + + expect(visits).toBe(1); }); }); @@ -3553,7 +3769,7 @@ describe('BasicCrawler', () => { await crawler.addRequests([{ url: 'https://example.com/b', label: 'DETAIL', userData: { id: 'ok' } }]); const queue = await crawler.getRequestQueue(); - expect(await queue.isEmpty()).toBe(false); + expect((await queue.checkReadiness()).status).toBe('ready'); }); test('crawler.addRequests excludes the Crawlee-managed label when validating (strict schemas)', async () => { @@ -3567,7 +3783,7 @@ describe('BasicCrawler', () => { ]); const queue = await crawler.getRequestQueue(); - expect(await queue.isEmpty()).toBe(false); + expect((await queue.checkReadiness()).status).toBe('ready'); }); test('a schema that declares the label opts into validating it', async () => { @@ -3643,7 +3859,7 @@ describe('BasicCrawler', () => { { url: 'https://example.com/p', label: 'LIST', userData: { page: 2 } }, ] as never); const queue = await crawler.getRequestQueue(); - expect(await queue.isEmpty()).toBe(false); + expect((await queue.checkReadiness()).status).toBe('ready'); }); test('context.addRequests validates userData against the label schema', async () => { @@ -3674,7 +3890,7 @@ describe('BasicCrawler', () => { ] as never); const queue = await crawler.getRequestQueue(); - expect(await queue.isEmpty()).toBe(false); + expect((await queue.checkReadiness()).status).toBe('ready'); }); test('a plain (non-router) requestHandler skips validation entirely', async () => { @@ -3683,7 +3899,7 @@ describe('BasicCrawler', () => { await crawler.addRequests([{ url: 'https://example.com/e', label: 'DETAIL', userData: { id: 123 } }]); const queue = await crawler.getRequestQueue(); - expect(await queue.isEmpty()).toBe(false); + expect((await queue.checkReadiness()).status).toBe('ready'); }); test('validation runs at the crawler level; direct requestQueue calls bypass it', async () => { diff --git a/test/core/crawlers/http_crawler.test.ts b/test/core/crawlers/http_crawler.test.ts index d18cae419982..6aedcc2afee5 100644 --- a/test/core/crawlers/http_crawler.test.ts +++ b/test/core/crawlers/http_crawler.test.ts @@ -8,6 +8,7 @@ import { ConcurrencySystem, HttpCrawler, PersistentRateLimitError, + RequestList, RequestQueue, SessionPool, ThrottlingRequestManager, @@ -727,3 +728,70 @@ test('`keepAlive` outlives a domain that never stops rate-limiting', async () => await crawler.teardown(); await running; }, 30_000); + +test('a 429 on a request taken from a `requestList` is paced too', async () => { + const hits: number[] = []; + router.set('/429-then-ok-from-list', (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 handled: string[] = []; + const requestList = await RequestList.open(null, [`${url}/429-then-ok-from-list`]); + const throttler = new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['127.0.0.1'], + }); + + const crawler = new HttpCrawler({ + // The tandem forwards the 429 to the pacer nested inside it, so a request transferred out of the + // list is backed off rather than handed straight back to the handler. + requestManager: await requestList.toTandem(throttler), + maxRequestRetries: 0, + requestHandler: async ({ request }) => { + handled.push(request.url); + }, + }); + + const stats = await crawler.run(); + + // `maxRequestRetries: 0` would have failed the request outright had the 429 been charged as a retry. + expect(handled).toEqual([`${url}/429-then-ok-from-list`]); + expect(stats.requestsFailed).toBe(0); + + expect(hits).toHaveLength(2); + expect(hits[1] - hits[0]).toBeGreaterThanOrEqual(1000); +}, 30_000); + +test('an unthrottled 429 is handled like any other response, with a single warning', async () => { + let hits = 0; + router.set('/429-unthrottled', (req, res) => { + hits++; + res.statusCode = 429; + res.end(); + }); + + const crawler = new HttpCrawler({ + maxRequestRetries: 0, + requestHandler: async () => {}, + }); + + const warning = vitest.spyOn(crawler.log, 'warning').mockImplementation(() => {}); + + const stats = await crawler.run([`${url}/429-unthrottled`]); + + // No pacer, so the 429 stays a plain blocked response and costs the request its only retry. + expect(stats.requestsFailed).toBe(1); + expect(hits).toBe(1); + + const rateLimitWarnings = warning.mock.calls.filter(([message]) => message.includes('HTTP 429')); + expect(rateLimitWarnings).toHaveLength(1); + expect(rateLimitWarnings[0][0]).toMatch(/`sameDomainDelaySecs`.*`ThrottlingRequestManager`/s); +}, 30_000); diff --git a/test/core/request_list.test.ts b/test/core/request_list.test.ts index a58734d2f843..a659ea2a9994 100644 --- a/test/core/request_list.test.ts +++ b/test/core/request_list.test.ts @@ -66,19 +66,17 @@ describe('RequestList', () => { { url: 'https://example.com/1#same' }, ]); - expect(await requestList.isEmpty()).toBe(false); + expect((await requestList.checkReadiness()).status).toBe('ready'); const req = await requestList.fetchNextRequest(); expect(req!.url).toBe('https://example.com/1'); - expect(await requestList.isEmpty()).toBe(true); - expect(await requestList.isFinished()).toBe(false); + expect((await requestList.checkReadiness()).status).toBe('waiting'); expect(await requestList.fetchNextRequest()).toBe(null); await requestList.markRequestAsHandled(req!); - expect(await requestList.isEmpty()).toBe(true); - expect(await requestList.isFinished()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); }); test('must be initialized before using any of the methods', async () => { @@ -86,8 +84,7 @@ describe('RequestList', () => { const requestList = new RequestList({ sources: [{ url: 'https://example.com' }] }); const requestObj = new Request({ url: 'https://example.com' }); - await expect(requestList.isEmpty()).rejects.toThrow(); - await expect(requestList.isFinished()).rejects.toThrow(); + await expect(requestList.checkReadiness()).rejects.toThrow(); expect(() => requestList.getState()).toThrowError(); await expect(requestList.markRequestAsHandled(requestObj)).rejects.toThrow(); await expect(requestList.fetchNextRequest()).rejects.toThrow(); @@ -95,8 +92,7 @@ describe('RequestList', () => { // @ts-expect-error private method await requestList.initialize(); - await expect(requestList.isEmpty()).resolves.not.toThrow(); - await expect(requestList.isFinished()).resolves.not.toThrow(); + await expect(requestList.checkReadiness()).resolves.not.toThrow(); expect(() => requestList.getState()).not.toThrowError(); await expect(requestList.fetchNextRequest()).resolves.not.toThrow(); await expect(requestList.markRequestAsHandled(requestObj)).resolves.not.toThrow(); @@ -135,13 +131,14 @@ describe('RequestList', () => { state: originalList.getState(), }); - expect(await newList.isEmpty()).toBe(false); + expect((await newList.checkReadiness()).status).toBe('ready'); expect((await newList.fetchNextRequest())!.url).toBe('https://example.com/3'); expect((await newList.fetchNextRequest())!.url).toBe('https://example.com/5'); expect((await newList.fetchNextRequest())!.url).toBe('https://example.com/6'); expect((await newList.fetchNextRequest())!.url).toBe('https://example.com/7'); expect((await newList.fetchNextRequest())!.url).toBe('https://example.com/8'); - expect(await newList.isEmpty()).toBe(true); + // None of the five re-served requests was handled, so they are all still in progress. + expect((await newList.checkReadiness()).status).toBe('waiting'); }); test('`RequestList` is `for .. await` iterable', async () => { @@ -300,8 +297,7 @@ describe('RequestList', () => { nextIndex: 2, nextUniqueKey: 'https://example.com/3', }); - expect(await requestList.isEmpty()).toBe(false); - expect(await requestList.isFinished()).toBe(false); + expect((await requestList.checkReadiness()).status).toBe('ready'); expect(requestList.inProgress.size).toBe(2); await requestList.markRequestAsHandled(request1!); @@ -316,11 +312,10 @@ describe('RequestList', () => { const request3 = await requestList.fetchNextRequest(); expect(request3!.url).toBe('https://example.com/3'); expect(await requestList.fetchNextRequest()).toBe(null); - expect(await requestList.isEmpty()).toBe(true); - expect(await requestList.isFinished()).toBe(false); + expect((await requestList.checkReadiness()).status).toBe('waiting'); await requestList.markRequestAsHandled(request3!); - expect(await requestList.isFinished()).toBe(true); + expect((await requestList.checkReadiness()).status).toBe('finished'); }); test('should correctly persist its state when persistStateKey is set', async () => { @@ -588,7 +583,7 @@ describe('RequestList', () => { const rl = await RequestList.open(name, sources); expect(rl).toBeInstanceOf(RequestList); // An uninitialized list throws here, so this is the observable form of "open() initialized it". - await expect(rl.isEmpty()).resolves.toBe(false); + await expect(rl.checkReadiness()).resolves.toEqual({ status: 'ready' }); // The persistence keys are derived from the list name, which shows in the keys it reads and writes. expect(keysPassedTo(getValueSpy)).toEqual([ @@ -610,7 +605,7 @@ describe('RequestList', () => { const rl = await RequestList.open(name, sources); expect(rl).toBeInstanceOf(RequestList); expect(rl.requests).toEqual(requests); - await expect(rl.isEmpty()).resolves.toBe(false); + await expect(rl.checkReadiness()).resolves.toEqual({ status: 'ready' }); expect(keysPassedTo(getValueSpy)).toEqual([ `${CRAWLEE_KEY}-${STATE_PERSISTENCE_KEY}`, @@ -637,7 +632,7 @@ describe('RequestList', () => { expect(rl).toBeInstanceOf(RequestList); // The counter suffix on the unique key is what `keepDuplicateUrls: true` does. expect(rl.requests).toEqual(requests); - await expect(rl.isEmpty()).resolves.toBe(false); + await expect(rl.checkReadiness()).resolves.toEqual({ status: 'ready' }); // The list name wins over the `persistStateKey` option. expect(keysPassedTo(getValueSpy)).toEqual([ @@ -658,7 +653,7 @@ describe('RequestList', () => { const rl = await RequestList.open(name, sources); expect(rl).toBeInstanceOf(RequestList); expect(rl.requests).toEqual(requests); - await expect(rl.isEmpty()).resolves.toBe(false); + await expect(rl.checkReadiness()).resolves.toEqual({ status: 'ready' }); // A nameless list has no persistence keys, so it never touches the store. expect(getValueSpy).not.toBeCalled(); diff --git a/test/core/request_manager_tandem.test.ts b/test/core/request_manager_tandem.test.ts index a90a1c41653a..ebe94a881d3b 100644 --- a/test/core/request_manager_tandem.test.ts +++ b/test/core/request_manager_tandem.test.ts @@ -1,3 +1,4 @@ +import type { RequestLoaderStatus } from '@crawlee/core'; import { log, MemoryStorageBackend, @@ -6,6 +7,7 @@ import { RequestManagerTandem, RequestQueue, serviceLocator, + ThrottlingRequestManager, } from '@crawlee/core'; import { afterAll, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'; @@ -142,62 +144,38 @@ describe('RequestManagerTandem', () => { await expect(tandem.getTotalCount()).resolves.toBe(3); }); - test('isFinished returns true only when both list and queue are finished', async () => { + test('readiness joins the loader and the manager, and the manager never masks the loader', async () => { const requestList = await RequestList.open(null, [{ url: 'https://example.com/1' }]); const requestQueue = await RequestQueue.open(); const tandem = new RequestManagerTandem(requestList, requestQueue); - // Mock the isFinished methods - vi.spyOn(requestList, 'isFinished').mockResolvedValue(false); - vi.spyOn(requestQueue, 'isFinished').mockResolvedValue(false); + const listReadiness = vi.spyOn(requestList, 'checkReadiness'); + const queueReadiness = vi.spyOn(requestQueue, 'checkReadiness'); - // Neither is finished, so tandem should not be finished - expect(await tandem.isFinished()).toBe(false); + const readiness = async (list: RequestLoaderStatus, queue: RequestLoaderStatus) => { + listReadiness.mockResolvedValue(list); + queueReadiness.mockResolvedValue(queue); + return tandem.checkReadiness(); + }; - // Only list is finished - vi.spyOn(requestList, 'isFinished').mockResolvedValue(true); - vi.spyOn(requestQueue, 'isFinished').mockResolvedValue(false); - expect(await tandem.isFinished()).toBe(false); + await expect(readiness({ status: 'ready' }, { status: 'finished' })).resolves.toEqual({ status: 'ready' }); + await expect(readiness({ status: 'finished' }, { status: 'ready' })).resolves.toEqual({ status: 'ready' }); - // Only queue is finished - vi.spyOn(requestList, 'isFinished').mockResolvedValue(false); - vi.spyOn(requestQueue, 'isFinished').mockResolvedValue(true); - expect(await tandem.isFinished()).toBe(false); + // A backoff on the manager side must not freeze the loader's unrelated requests for the length of it. + await expect(readiness({ status: 'ready' }, { status: 'waiting', readyAt: 42 })).resolves.toEqual({ + status: 'ready', + }); - // Both are finished - vi.spyOn(requestList, 'isFinished').mockResolvedValue(true); - vi.spyOn(requestQueue, 'isFinished').mockResolvedValue(true); - expect(await tandem.isFinished()).toBe(true); - }); - - test('isEmpty returns true only when both list and queue are empty', async () => { - const requestList = await RequestList.open(null, [{ url: 'https://example.com/1' }]); - const requestQueue = await RequestQueue.open(); - - const tandem = new RequestManagerTandem(requestList, requestQueue); - - // Mock the isEmpty methods - vi.spyOn(requestList, 'isEmpty').mockResolvedValue(false); - vi.spyOn(requestQueue, 'isEmpty').mockResolvedValue(false); - - // Neither is empty, so tandem should not be empty - expect(await tandem.isEmpty()).toBe(false); + await expect(readiness({ status: 'waiting' }, { status: 'finished' })).resolves.toEqual({ status: 'waiting' }); + await expect(readiness({ status: 'finished' }, { status: 'waiting', readyAt: 42 })).resolves.toEqual({ + status: 'waiting', + readyAt: 42, + }); - // Only list is empty - vi.spyOn(requestList, 'isEmpty').mockResolvedValue(true); - vi.spyOn(requestQueue, 'isEmpty').mockResolvedValue(false); - expect(await tandem.isEmpty()).toBe(false); - - // Only queue is empty - vi.spyOn(requestList, 'isEmpty').mockResolvedValue(false); - vi.spyOn(requestQueue, 'isEmpty').mockResolvedValue(true); - expect(await tandem.isEmpty()).toBe(false); - - // Both are empty - vi.spyOn(requestList, 'isEmpty').mockResolvedValue(true); - vi.spyOn(requestQueue, 'isEmpty').mockResolvedValue(true); - expect(await tandem.isEmpty()).toBe(true); + await expect(readiness({ status: 'finished' }, { status: 'finished' })).resolves.toEqual({ + status: 'finished', + }); }); test('drops the request and marks it handled on the loader when transfer fails', async () => { @@ -281,10 +259,78 @@ describe('RequestManagerTandem', () => { expect(factory).toHaveBeenCalledTimes(1); // Subsequent operations reuse the same memoized queue. - await tandem.isFinished(); + await tandem.checkReadiness(); expect(factory).toHaveBeenCalledTimes(1); }); + test('forwards the pacing signals to the manager it wraps', async () => { + const requestList = await RequestList.open(null, [{ url: 'https://example.com/1' }]); + const throttler = new ThrottlingRequestManager({ + inner: await RequestQueue.open(), + domains: ['example.com'], + }); + + const tandem = new RequestManagerTandem(requestList, throttler); + + expect( + tandem.recordPacingSignal({ + url: 'https://example.com/1', + reason: 'minInterval', + intervalMs: 5_000, + scope: 'hostname', + }), + ).toBe(true); + expect(tandem.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', waitMs: 1_000 })).toBe( + true, + ); + + // A domain the pacer does not cover comes back as unpaced rather than silently swallowed, so the + // crawler can warn about it. + expect( + tandem.recordPacingSignal({ + url: 'https://other.com/1', + reason: 'minInterval', + intervalMs: 5_000, + scope: 'hostname', + }), + ).toBe(false); + expect(tandem.recordPacingSignal({ url: 'https://other.com/1', reason: 'rateLimited', waitMs: 1_000 })).toBe( + false, + ); + + // ...and the pacer acted on the signals rather than merely accepting them. + await expect(throttler.checkReadiness()).resolves.toMatchObject({ status: 'waiting' }); + + // A floor covering every domain reaches it the same way, and this pacer covers only `example.com`. + expect(() => + tandem.recordPacingSignal({ reason: 'minIntervalEverywhere', intervalMs: 5_000, scope: 'hostname' }), + ).toThrow(/domains: 'all'/); + }); + + test('reports the pacing signals as unhandled when the manager cannot pace', async () => { + const requestList = await RequestList.open(null, [{ url: 'https://example.com/1' }]); + const tandem = new RequestManagerTandem(requestList, await RequestQueue.open()); + + expect(tandem.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', waitMs: 1_000 })).toBe( + false, + ); + expect( + tandem.recordPacingSignal({ + url: 'https://example.com/1', + reason: 'minInterval', + intervalMs: 5_000, + scope: 'hostname', + }), + ).toBe(false); + expect( + tandem.recordPacingSignal({ + reason: 'minIntervalEverywhere', + intervalMs: 5_000, + scope: 'registrableDomain', + }), + ).toBe(false); + }); + test('persistState forwards to the read-only loader', async () => { const requestList = await RequestList.open(null, [{ url: 'https://example.com/1' }]); const requestQueue = await RequestQueue.open(); diff --git a/test/core/sitemap_request_loader.test.ts b/test/core/sitemap_request_loader.test.ts index e14a8cd353b5..031027c5f431 100644 --- a/test/core/sitemap_request_loader.test.ts +++ b/test/core/sitemap_request_loader.test.ts @@ -279,12 +279,13 @@ describe('SitemapRequestLoader', () => { enqueueStrategy: 'all', }); - while (await list.isEmpty()) { + while ((await list.checkReadiness()).status !== 'ready') { await sleep(20); } - await expect(list.isFinished(), 'list should not be finished').resolves.toBe(false); - await expect(list.isEmpty(), 'list should not be empty').resolves.toBe(false); + await expect(list.checkReadiness(), 'list should have a request available').resolves.toEqual({ + status: 'ready', + }); const firstRequest = await list.fetchNextRequest(); expect(firstRequest).not.toBe(null); @@ -349,7 +350,7 @@ describe('SitemapRequestLoader', () => { } expect(await list.getHandledCount()).toBe(2); - await expect(list.isFinished()).resolves.toBe(true); + await expect(list.checkReadiness()).resolves.toEqual({ status: 'finished' }); await expect(list.fetchNextRequest()).resolves.toBe(null); }); @@ -401,13 +402,13 @@ describe('SitemapRequestLoader', () => { enqueueStrategy: 'all', }); - while (await list.isEmpty()) { + while ((await list.checkReadiness()).status !== 'ready') { await sleep(20); } const firstBatch: Request[] = []; - while (!(await list.isEmpty())) { + while ((await list.checkReadiness()).status === 'ready') { const request = await list.fetchNextRequest(); firstBatch.push(request!); await list.markRequestAsHandled(request!); @@ -415,13 +416,13 @@ describe('SitemapRequestLoader', () => { expect(firstBatch).toHaveLength(2); - while (await list.isEmpty()) { + while ((await list.checkReadiness()).status !== 'ready') { await sleep(20); } const secondBatch: Request[] = []; - while (!(await list.isEmpty())) { + while ((await list.checkReadiness()).status === 'ready') { const request = await list.fetchNextRequest(); secondBatch.push(request!); await list.markRequestAsHandled(request!); @@ -429,7 +430,7 @@ describe('SitemapRequestLoader', () => { expect(secondBatch).toHaveLength(5); - await expect(list.isFinished()).resolves.toBe(true); + await expect(list.checkReadiness()).resolves.toEqual({ status: 'finished' }); expect(await list.getHandledCount()).toBe(7); }); @@ -443,7 +444,7 @@ describe('SitemapRequestLoader', () => { await list.markRequestAsHandled(request); } - await expect(list.isFinished()).resolves.toBe(true); + await expect(list.checkReadiness()).resolves.toEqual({ status: 'finished' }); expect(await list.getHandledCount()).toBe(7); }); @@ -457,8 +458,8 @@ describe('SitemapRequestLoader', () => { enqueueStrategy: 'all', }); - // Abort while the first sub-sitemap is still streaming - waiting for its URLs rather than for a fixed duration. - while (await list.isEmpty()) { + // Abort once the first sub-sitemap streamed a URL - waiting for that rather than for a fixed duration. + while ((await list.checkReadiness()).status !== 'ready') { await sleep(10); } @@ -468,7 +469,7 @@ describe('SitemapRequestLoader', () => { await list.markRequestAsHandled(request); } - await expect(list.isFinished()).resolves.toBe(true); + await expect(list.checkReadiness()).resolves.toEqual({ status: 'finished' }); expect(list.isSitemapFullyLoaded()).toBe(false); expect(await list.getHandledCount()).toBe(2); }); @@ -486,7 +487,7 @@ describe('SitemapRequestLoader', () => { await list.markRequestAsHandled(request); } - await expect(list.isFinished()).resolves.toBe(true); + await expect(list.checkReadiness()).resolves.toEqual({ status: 'finished' }); expect(list.isSitemapFullyLoaded()).toBe(false); expect(await list.getHandledCount()).toBe(2); }); @@ -504,7 +505,7 @@ describe('SitemapRequestLoader', () => { // Abort while the first sub-sitemap is still streaming, so the state is persisted with the // load aborted half-way through the index. - while (await list.isEmpty()) { + while ((await list.checkReadiness()).status !== 'ready') { await sleep(10); } @@ -526,16 +527,17 @@ describe('SitemapRequestLoader', () => { const list = await SitemapRequestLoader.open({ sitemapUrls: [`${url}/sitemap.xml`], enqueueStrategy: 'all' }); const requests: Request[] = []; - await expect(list.isFinished()).resolves.toBe(false); + // The sitemap may still be parsing at this point, so `ready` and `waiting` are both fine here. + expect((await list.checkReadiness()).status).not.toBe('finished'); - while (!(await list.isFinished())) { + while ((await list.checkReadiness()).status !== 'finished') { const request = await list.fetchNextRequest(); if (!request) break; await list.markRequestAsHandled(request); requests.push(request); } - await expect(list.isEmpty()).resolves.toBe(true); + await expect(list.checkReadiness()).resolves.toEqual({ status: 'finished' }); expect(requests.map((it) => it.url)).toEqual([ 'http://not-exists.com/', 'http://not-exists.com/catalog?item=12&desc=vacation_hawaii', @@ -561,9 +563,9 @@ describe('SitemapRequestLoader', () => { await list.persistState(); const newList = await SitemapRequestLoader.open(options); - await expect(newList.isEmpty()).resolves.toBe(false); + await expect(newList.checkReadiness()).resolves.toEqual({ status: 'ready' }); - while (!(await newList.isFinished())) { + while ((await newList.checkReadiness()).status !== 'finished') { const request = await newList.fetchNextRequest(); if (!request) break; await newList.markRequestAsHandled(request); @@ -683,7 +685,7 @@ describe('SitemapRequestLoader', () => { }); // Wait until the first URL is buffered, i.e. the loader is parked on backpressure. - while (await list.isEmpty()) { + while ((await list.checkReadiness()).status !== 'ready') { await sleep(20); } @@ -696,7 +698,7 @@ describe('SitemapRequestLoader', () => { } expect(list.isSitemapFullyLoaded()).toBe(true); - await expect(list.isFinished()).resolves.toBe(true); + await expect(list.checkReadiness()).resolves.toEqual({ status: 'finished' }); expect(urls).toEqual( new Set([ 'http://not-exists.com/', diff --git a/test/core/storages/request_queue.test.ts b/test/core/storages/request_queue.test.ts index a115f5d8bccf..ec0f82db163b 100644 --- a/test/core/storages/request_queue.test.ts +++ b/test/core/storages/request_queue.test.ts @@ -70,7 +70,7 @@ describe('RequestQueue remote', () => { expect(await queue.fetchNextRequest()).toBeNull(); }); - test('a handled request is not fetched again and isFinished() becomes true', async () => { + test('a handled request is not fetched again and checkReadiness() reports finished', async () => { const queue = await RequestQueue.open(); await queue.addRequest({ url: 'http://example.com/a' }); @@ -81,7 +81,7 @@ describe('RequestQueue remote', () => { await queue.markRequestAsHandled(fetched!); expect(await queue.fetchNextRequest()).toBeNull(); - expect(await queue.isFinished()).toBe(true); + expect((await queue.checkReadiness()).status).toBe('finished'); }); test('a reclaimed request is fetched again; reclaim with forefront returns it to the front', async () => { @@ -220,25 +220,44 @@ describe('RequestQueue remote', () => { expect(retrievedUrls.map((x) => new URL(x).pathname)).toEqual(['/1', '/2', '/3', '/4', '/5', '/6']); }); - test('isEmpty() reflects fetchable requests while isFinished() accounts for in-progress ones', async () => { + test('checkReadiness() distinguishes a fetchable queue from an in-progress one', async () => { const queue = await RequestQueue.open(); await queue.addRequest({ url: 'http://example.com/a' }); - // There is a pending request, so the queue is neither empty nor finished. - expect(await queue.isEmpty()).toBe(false); - expect(await queue.isFinished()).toBe(false); + expect((await queue.checkReadiness()).status).toBe('ready'); const fetched = await queue.fetchNextRequest(); - // The request is now in progress (locked), not handled. There is nothing left to fetch, so the - // queue is empty — but it is not finished, since the in-progress request might still be - // reclaimed. That "not finished" signal keeps a crawler running while the request is processed. - expect(await queue.isEmpty()).toBe(true); - expect(await queue.isFinished()).toBe(false); + // The in-progress request is locked, not handled, and might still be reclaimed - `waiting` rather than + // `finished` is what keeps a crawler running while the request is processed. + expect((await queue.checkReadiness()).status).toBe('waiting'); await queue.markRequestAsHandled(fetched!); - // Now the request is handled and gone, so the queue is both empty and finished. - expect(await queue.isEmpty()).toBe(true); - expect(await queue.isFinished()).toBe(true); + expect((await queue.checkReadiness()).status).toBe('finished'); + }); + + test('recordPacingSignal() reports that a plain queue paces nothing', async () => { + const queue = await RequestQueue.open(); + + // Required on `IRequestManager`, so `false` never means "unsupported" - it means nothing here paces, + // which is what lets a crawler warn that the signal had nowhere to go. + expect(queue.recordPacingSignal({ url: 'http://example.com/a', reason: 'rateLimited', waitMs: 1_000 })).toBe( + false, + ); + expect( + queue.recordPacingSignal({ + url: 'http://example.com/a', + reason: 'minInterval', + intervalMs: 1_000, + scope: 'hostname', + }), + ).toBe(false); + expect( + queue.recordPacingSignal({ + reason: 'minIntervalEverywhere', + intervalMs: 1_000, + scope: 'registrableDomain', + }), + ).toBe(false); }); test('should accept plain object in addRequest()', async () => { @@ -590,7 +609,7 @@ describe('RequestQueue (request lifecycle)', () => { await queue.markRequestAsHandled(first!); expect(await queue.fetchNextRequest()).toBeNull(); - expect(await queue.isFinished()).toBe(true); + expect((await queue.checkReadiness()).status).toBe('finished'); }); test('`fetchNextRequest` order respects `forefront` enqueues', async () => { @@ -659,7 +678,7 @@ describe('RequestQueue background batches', () => { serviceLocator.setStorageBackend(new MemoryStorageBackend()); }); - test('a failing background batch rejects instead of hanging, and stops blocking isFinished', async () => { + test('a failing background batch rejects instead of hanging, and stops blocking checkReadiness()', async () => { const queue = await RequestQueue.open(); let batches = 0; @@ -681,6 +700,6 @@ describe('RequestQueue background batches', () => { const req = await queue.fetchNextRequest(); expect(req).toBeDefined(); await queue.markRequestAsHandled(req!); - expect(await queue.isFinished()).toBe(true); + expect((await queue.checkReadiness()).status).toBe('finished'); }, 10_000); }); diff --git a/test/core/storages/request_source_status.test.ts b/test/core/storages/request_source_status.test.ts new file mode 100644 index 000000000000..cdcda425696c --- /dev/null +++ b/test/core/storages/request_source_status.test.ts @@ -0,0 +1,65 @@ +import type { RequestSourceStatus } from '@crawlee/core'; +import { describe, expect, test } from 'vitest'; + +// Not part of the public surface, so it is reached the way the other internal helpers are tested. +import { joinRequestSourceStatuses } from '../../../packages/core/src/storages/request_loader.js'; + +describe('joinRequestSourceStatuses', () => { + const ready = { status: 'ready' } as const satisfies RequestSourceStatus; + const stalled = { + status: 'stalled', + reason: 'example.com is stonewalling us', + } as const satisfies RequestSourceStatus; + const waiting = { status: 'waiting' } as const satisfies RequestSourceStatus; + const finished = { status: 'finished' } as const satisfies RequestSourceStatus; + + // Both directions of every pair, because the join has to be commutative for two sources read as one. + test.each([ + [ready, ready, ready], + [ready, stalled, ready], + [ready, waiting, ready], + [ready, finished, ready], + [stalled, stalled, stalled], + [stalled, waiting, stalled], + [stalled, finished, stalled], + [waiting, waiting, waiting], + [waiting, finished, waiting], + [finished, finished, finished], + ] satisfies [RequestSourceStatus, RequestSourceStatus, RequestSourceStatus][])( + '$0.status + $1.status -> $2.status', + (a, b, expected) => { + expect(joinRequestSourceStatuses(a, b)).toEqual(expected); + expect(joinRequestSourceStatuses(b, a)).toEqual(expected); + }, + ); + + test('a stalled source is masked while the other one still has work', () => { + // The crawler turns `stalled` into a `PersistentRateLimitError`, so this precedence is what keeps a + // crawl that is making progress elsewhere from being abandoned over one hopeless domain. + expect(joinRequestSourceStatuses(stalled, ready)).toEqual(ready); + }); + + test('the earlier of two known wake-up times wins', () => { + expect( + joinRequestSourceStatuses({ status: 'waiting', readyAt: 200 }, { status: 'waiting', readyAt: 100 }), + ).toEqual({ status: 'waiting', readyAt: 100 }); + expect( + joinRequestSourceStatuses({ status: 'waiting', readyAt: 100 }, { status: 'waiting', readyAt: 200 }), + ).toEqual({ status: 'waiting', readyAt: 100 }); + }); + + test('a known wake-up time survives a source that has none', () => { + expect(joinRequestSourceStatuses(waiting, { status: 'waiting', readyAt: 100 })).toEqual({ + status: 'waiting', + readyAt: 100, + }); + expect(joinRequestSourceStatuses({ status: 'waiting', readyAt: 100 }, finished)).toEqual({ + status: 'waiting', + readyAt: 100, + }); + }); + + test('no wake-up time is invented when neither source knows one', () => { + expect(joinRequestSourceStatuses(waiting, waiting)).toEqual({ status: 'waiting' }); + }); +}); diff --git a/test/core/storages/storage_purge.test.ts b/test/core/storages/storage_purge.test.ts index d3d69e727b4c..7f11890a8e67 100644 --- a/test/core/storages/storage_purge.test.ts +++ b/test/core/storages/storage_purge.test.ts @@ -170,7 +170,7 @@ describe('purgeDefaultStorages', () => { await defaultQueue.addRequest({ url: 'https://example.com/stale' }); await purgeDefaultStorages(); - expect(await defaultQueue.isEmpty()).toBe(true); + expect((await defaultQueue.checkReadiness()).status).toBe('finished'); }); test('clears a crawler-owned alias queue left behind by a previous run', async () => { @@ -185,6 +185,6 @@ describe('purgeDefaultStorages', () => { await purgeDefaultStorages(); const secondRun = await RequestQueue.open({ alias: '__default_1__' }); - expect(await secondRun.isEmpty()).toBe(true); + expect((await secondRun.checkReadiness()).status).toBe('finished'); }); }); diff --git a/test/core/storages/storage_transaction.test.ts b/test/core/storages/storage_transaction.test.ts index f191c72a4114..87fdd6e23fdf 100644 --- a/test/core/storages/storage_transaction.test.ts +++ b/test/core/storages/storage_transaction.test.ts @@ -636,7 +636,7 @@ describe('RequestQueue in a transaction', () => { await expect(queue.getRequest('https://example.com/a')).resolves.toMatchObject({ url: 'https://example.com/a', }); - await expect(queue.isEmpty()).resolves.toBe(false); + await expect(queue.checkReadiness()).resolves.toEqual({ status: 'ready' }); const queueInfo = await queue.getInfo(); expect(queueInfo.pendingRequestCount).toBe(1); diff --git a/test/core/storages/throttling_request_manager.test.ts b/test/core/storages/throttling_request_manager.test.ts index 5bb99752d274..9c9b57c4a2a5 100644 --- a/test/core/storages/throttling_request_manager.test.ts +++ b/test/core/storages/throttling_request_manager.test.ts @@ -1,8 +1,7 @@ -import type { AddRequestsBatchedResult } from '@crawlee/core'; +import type { AddRequestsBatchedResult, StorageIdentifier, ThrottlingRequestManagerOptions } from '@crawlee/core'; import { KeyValueStore, MemoryStorageBackend, - PersistentRateLimitError, RequestQueue, serviceLocator, ThrottlingRequestManager, @@ -126,15 +125,14 @@ describe('ThrottlingRequestManager', () => { expect(produced).toBe(10); }); - test('addRequestsBatched keeps isFinished false while background batches are landing', async () => { + test('addRequestsBatched keeps checkReadiness() from reporting finished 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, + checkReadiness: async () => ({ status: 'finished' }) as const, } as unknown as RequestQueue; const manager = new ThrottlingRequestManager({ inner: eagerlyFinished, domains: [] }); @@ -146,7 +144,7 @@ describe('ThrottlingRequestManager', () => { // 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); + expect((await manager.checkReadiness()).status).toBe('waiting'); await result.waitForAllRequestsToBeAdded; expect(await manager.getTotalCount()).toBe(2); @@ -243,11 +241,142 @@ describe('ThrottlingRequestManager', () => { await manager.markRequestAsHandled(request); // Marking it handled in its domain's sub-queue instead would leave the inner manager serving it forever. - expect(await manager.isFinished()).toBe(true); + expect((await manager.checkReadiness()).status).toBe('finished'); expect(await inner.getPendingCount()).toBe(0); }); - test('recordDomainDelay enforces throttling and fair scheduling', async () => { + describe('a lazily-opened inner manager', () => { + const throttling = { domains: ['example.com'] } satisfies Omit< + ThrottlingRequestManagerOptions, + 'inner' + >; + + test('is resolved once, so a whole batch lands in - and is handed back to - one instance', async () => { + const factory = vitest.fn(async () => createQueue()); + const manager = new ThrottlingRequestManager({ ...throttling, inner: factory }); + + await manager.addRequestsBatched([ + { url: 'https://example.com/1' }, + { url: 'https://other.com/1' }, + { url: 'https://example.com/2' }, + ]); + + const inner = manager.innerManager!; + expect(await inner.getTotalCount()).toBe(1); + expect(await manager.getTotalCount()).toBe(3); + + // The inner-routed request is the one that matters: handed back to a second instance, it would leave + // this one serving it forever. + for (let i = 0; i < 3; i++) { + await manager.markRequestAsHandled(await pollForNextRequest(manager)); + } + + expect((await manager.checkReadiness()).status).toBe('finished'); + expect(await inner.getPendingCount()).toBe(0); + expect(factory).toHaveBeenCalledTimes(1); + }); + + test('is not forced by bookkeeping', async () => { + const factory = vitest.fn(async () => createQueue()); + const manager = new ThrottlingRequestManager({ ...throttling, inner: factory }); + + await manager.purge(); + await manager.persistState(); + await manager.setExpectedRequestProcessingTimeSecs(600); + await manager.drop(); + + expect(factory).not.toHaveBeenCalled(); + expect(manager.innerManager).toBeUndefined(); + }); + + test('receives an expected-processing-time hint given before it was resolved', async () => { + const inner = await createQueue(); + const setHint = vitest.spyOn(inner, 'setExpectedRequestProcessingTimeSecs'); + const manager = new ThrottlingRequestManager({ ...throttling, inner: () => inner }); + + await manager.setExpectedRequestProcessingTimeSecs(600); + expect(setHint).not.toHaveBeenCalled(); + + await manager.fetchNextRequest(); + + expect(setHint).toHaveBeenCalledWith(600); + }); + + test('is opened by checkReadiness(), so requests left in it by a previous run are not missed', async () => { + // Without purgeOnStart, an unopened queue may still hold work nobody would ever look at. + await (await createQueue('leftover-inner')).addRequest({ url: 'https://other.com/left-behind' }); + + const manager = new ThrottlingRequestManager({ + ...throttling, + inner: () => createQueue('leftover-inner'), + }); + + expect((await manager.checkReadiness()).status).toBe('ready'); + expect(manager.innerManager).toBeDefined(); + }); + + test('is exposed by innerManager only once resolved, which reading it never triggers', async () => { + const inner = await createQueue(); + const factory = vitest.fn(() => inner); + const manager = new ThrottlingRequestManager({ ...throttling, inner: factory }); + + expect(manager.innerManager).toBeUndefined(); + expect(factory).not.toHaveBeenCalled(); + + await manager.addRequest({ url: 'https://other.com/1' }); + + expect(manager.innerManager).toBe(inner); + expect(factory).toHaveBeenCalledTimes(1); + }); + + describe('omitted entirely', () => { + test('opens the default queue on first use, and not before', async () => { + const manager = new ThrottlingRequestManager({ domains: ['example.com'] }); + + expect(manager.innerManager).toBeUndefined(); + + await manager.addRequest({ url: 'https://other.com/1' }); + + expect(manager.innerManager).toBeInstanceOf(RequestQueue); + expect((await manager.fetchNextRequest())!.url).toBe('https://other.com/1'); + }); + + test('routes throttled domains by domain as usual', async () => { + const manager = new ThrottlingRequestManager({ + domains: 'all', + throttleBy: 'registrableDomain', + minCrawlDelaySecs: 60, + }); + + await manager.addRequest({ url: 'https://example.com/1' }); + await manager.addRequest({ url: 'https://example.com/2' }); + + expect((await manager.fetchNextRequest())!.url).toBe('https://example.com/1'); + // Paced, so nothing of the second request until the delay runs out - and the default queue, + // opened for the requests nobody routed, is empty rather than serving it. + expect(await manager.fetchNextRequest()).toBeNull(); + expect(await manager.innerManager!.getTotalCount()).toBe(0); + }); + + test('is honoured by `requestManagerOpener`, so the default is not hardcoded', async () => { + const opener = vitest.fn(async (identifier?: string | StorageIdentifier | null) => + RequestQueue.open(identifier ?? 'substituted-default'), + ); + const manager = new ThrottlingRequestManager({ + domains: ['example.com'], + requestManagerOpener: opener, + }); + + await manager.addRequest({ url: 'https://other.com/1' }); + + // Called with no identifier for the wrapped manager, and with an alias per throttled domain. + expect(opener).toHaveBeenCalledWith(null, expect.anything()); + expect(manager.innerManager!.name).toBe('substituted-default'); + }); + }); + }); + + test('recordPacingSignal enforces throttling and fair scheduling', async () => { const inner = await createQueue(); const manager = new ThrottlingRequestManager({ inner, @@ -258,26 +387,23 @@ describe('ThrottlingRequestManager', () => { 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); + // Long enough that a loaded box cannot race the assertions below - the backoff is zeroed, never waited out. + expect( + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', waitMs: 10_000 }), + ).toBe(true); - // 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'); + // The throttled domain is skipped rather than holding the crawl up. + expect((await manager.fetchNextRequest())!.url).toBe('https://foo.com/1'); - // 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. + // Nothing left to hand over - and the manager says so rather than blocking 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 waiting = await manager.checkReadiness(); + // ...while still reporting the throttled request as outstanding work, due once its backoff runs out. + expect(waiting).toMatchObject({ status: 'waiting' }); + expect(waiting.status === 'waiting' && waiting.readyAt).toBeGreaterThan(Date.now()); - const start = Date.now(); - const req2 = await pollForNextRequest(manager); - - expect(Date.now() - start).toBeGreaterThanOrEqual(400); - expect(req2.url).toBe('https://example.com/1'); + domainState(manager, 'example.com').backoffUntil = 0; + expect((await manager.fetchNextRequest())!.url).toBe('https://example.com/1'); }); test('a burst of concurrent 429s advances the backoff only once', async () => { @@ -290,7 +416,7 @@ describe('ThrottlingRequestManager', () => { // 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(manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' })).toBe(true); } expect(domainState(manager, 'example.com').consecutive429Count).toBe(1); @@ -305,7 +431,7 @@ describe('ThrottlingRequestManager', () => { }); const state = domainState(manager, 'example.com'); - manager.recordDomainDelay('https://example.com/1'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); // Rewinding both clocks beats sleeping out real delays - a loaded CI box cannot race it. const rewind = (ms: number) => { @@ -315,12 +441,12 @@ describe('ThrottlingRequestManager', () => { // 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'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); expect(state.consecutive429Count).toBe(2); // 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'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); expect(state.consecutive429Count).toBe(1); }); @@ -331,7 +457,7 @@ describe('ThrottlingRequestManager', () => { maxDelaySecs: 1, }); - manager.recordDomainDelay('https://example.com/1', 3_600_000); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', waitMs: 3_600_000 }); expect(domainState(manager, 'example.com').backoffUntil).toBeLessThanOrEqual(Date.now() + 1000); }); @@ -344,7 +470,7 @@ describe('ThrottlingRequestManager', () => { }); await manager.addRequest({ url: 'https://example.com/1' }); - manager.recordDomainDelay('https://example.com/1', 60_000); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', waitMs: 60_000 }); const start = Date.now(); expect(await manager.fetchNextRequest()).toBeNull(); @@ -362,8 +488,7 @@ describe('ThrottlingRequestManager', () => { // 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.checkReadiness()).status).toBe('ready'); expect(await secondRun.getPendingCount()).toBe(1); expect((await secondRun.fetchNextRequest())!.url).toBe('https://example.com/left-behind'); }); @@ -383,6 +508,21 @@ describe('ThrottlingRequestManager', () => { expect(await subQueue.getPendingCount()).toBe(0); }); + test('purges its own queues even when the wrapped manager refuses', async () => { + // A backend that cannot empty a queue in place refuses outright rather than quietly doing nothing. + const inner = await createQueue(); + const manager = new ThrottlingRequestManager({ inner, domains: ['example.com'] }); + await manager.addRequest({ url: 'https://example.com/stale' }); + + const subQueue = await RequestQueue.open({ alias: 'throttled-example.com' }); + expect(await subQueue.getPendingCount()).toBe(1); + + vitest.spyOn(inner, 'purge').mockRejectedValue(new Error('cannot empty a request queue in place')); + + await expect(manager.purge()).rejects.toThrow('cannot empty a request queue in place'); + expect(await subQueue.getPendingCount()).toBe(0); + }); + describe('stall detection', () => { const stallingManager = async () => new ThrottlingRequestManager({ @@ -403,32 +543,62 @@ describe('ThrottlingRequestManager', () => { 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'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); + + expect((await manager.checkReadiness()).status).not.toBe('stalled'); - await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + stallFor(manager, 'example.com'); + expect(await manager.checkReadiness()).toMatchObject({ + status: 'stalled', + reason: expect.stringContaining('example.com'), + }); + }); + test('a lapsed backoff does not let a stalling domain pass for progress', async () => { + // A stonewalling domain is dispatchable between one 429 and the next, so whether a probe lands in + // that window is a race. It must not decide whether the crawl gives up. + const manager = await stallingManager(); + await manager.addRequest({ url: 'https://example.com/1' }); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); stallFor(manager, 'example.com'); - await expect(manager.assertNoStalledDomains()).rejects.toThrow(PersistentRateLimitError); + + // Its backoff has run out, so its own queue would happily hand the request over. + domainState(manager, 'example.com').backoffUntil = 0; + const subQueue = await RequestQueue.open({ alias: 'throttled-example.com' }); + await expect(subQueue.checkReadiness()).resolves.toEqual({ status: 'ready' }); + + expect((await manager.checkReadiness()).status).toBe('stalled'); + }); + + test('work elsewhere outranks a stalling domain', async () => { + const manager = await stallingManager(); + await manager.addRequest({ url: 'https://example.com/1' }); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); + stallFor(manager, 'example.com'); + + await manager.addRequest({ url: 'https://other.com/1' }); + + expect((await manager.checkReadiness()).status).toBe('ready'); }); 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'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); stallFor(manager, 'example.com'); await manager.markRequestAsHandled((await pollForNextRequest(manager))!); - await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + expect((await manager.checkReadiness()).status).not.toBe('stalled'); }); test('a domain that has run out of work is finished, not stalled', async () => { const manager = await stallingManager(); - manager.recordDomainDelay('https://example.com/1'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); stallFor(manager, 'example.com'); - await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + expect((await manager.checkReadiness()).status).not.toBe('stalled'); }); test('a domain that has been idle for longer than the window is not stalled by its first 429', async () => { @@ -444,16 +614,16 @@ describe('ThrottlingRequestManager', () => { 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'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); - await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + expect((await manager.checkReadiness()).status).not.toBe('stalled'); }); test('a domain that was never rate-limited is never stalled', async () => { const manager = await stallingManager(); await manager.addRequest({ url: 'https://example.com/1' }); - await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + expect((await manager.checkReadiness()).status).not.toBe('stalled'); }); test('a domain that stopped rate-limiting a while ago is being waited out, not stalled', async () => { @@ -462,11 +632,11 @@ describe('ThrottlingRequestManager', () => { // 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'); + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited' }); stallFor(manager, 'example.com'); domainState(manager, 'example.com').lastRateLimitedAt -= 60_000; - await expect(manager.assertNoStalledDomains()).resolves.toBeUndefined(); + expect((await manager.checkReadiness()).status).not.toBe('stalled'); }); }); @@ -476,13 +646,20 @@ describe('ThrottlingRequestManager', () => { domains: ['example.com'], }); - manager.setCrawlDelay('https://example.com/1', 5); + manager.recordPacingSignal({ + url: 'https://example.com/1', + reason: 'minInterval', + intervalMs: 5_000, + scope: 'hostname', + }); 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); + expect( + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', waitMs: 30_000 }), + ).toBe(true); const state = domainState(manager, 'example.com'); expect(state.consecutive429Count).toBe(1); @@ -509,7 +686,12 @@ describe('ThrottlingRequestManager', () => { domains: ['example.com'], }); - manager.setCrawlDelay('https://example.com/1', 60); + manager.recordPacingSignal({ + url: 'https://example.com/1', + reason: 'minInterval', + intervalMs: 60_000, + scope: 'hostname', + }); for (let i = 0; i < 5; i++) { await manager.addRequest({ url: `https://example.com/${i}` }); } @@ -526,7 +708,12 @@ describe('ThrottlingRequestManager', () => { domains: ['example.com'], }); - manager.setCrawlDelay('https://example.com/1', 60); + manager.recordPacingSignal({ + url: 'https://example.com/1', + reason: 'minInterval', + intervalMs: 60_000, + scope: 'hostname', + }); // Nothing queued yet, so there is no dispatch for the delay to pace. expect(await manager.fetchNextRequest()).toBeNull(); @@ -555,7 +742,9 @@ describe('ThrottlingRequestManager', () => { await manager.addRequest({ url: 'https://example.com/1' }); - expect(manager.recordDomainDelay('https://example.com/1', 60_000)).toBe(true); + expect( + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', waitMs: 60_000 }), + ).toBe(true); expect(await manager.fetchNextRequest()).toBeNull(); }); @@ -592,7 +781,14 @@ describe('ThrottlingRequestManager', () => { // Robots.txt is read before the domain's first request is enqueued, so the delay lands on a domain // the manager has never seen. - expect(manager.setCrawlDelay('https://example.com/robots.txt', 60)).toBe(true); + expect( + manager.recordPacingSignal({ + url: 'https://example.com/robots.txt', + reason: 'minInterval', + intervalMs: 60_000, + scope: 'hostname', + }), + ).toBe(true); await manager.addRequest({ url: 'https://example.com/1' }); await manager.addRequest({ url: 'https://example.com/2' }); @@ -704,14 +900,19 @@ describe('ThrottlingRequestManager', () => { }); }); - test('setCrawlDelay sets crawl-delay successfully', async () => { + test('recordPacingSignal 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 + manager.recordPacingSignal({ + url: 'https://example.com/1', + reason: 'minInterval', + intervalMs: 200, + scope: 'hostname', + }); await manager.addRequest({ url: 'https://example.com/1' }); await manager.addRequest({ url: 'https://example.com/2' }); @@ -726,4 +927,130 @@ describe('ThrottlingRequestManager', () => { expect(Date.now() - start).toBeGreaterThanOrEqual(150); expect(req2.url).toBe('https://example.com/2'); }); + + describe('pacing signal scope', () => { + test('a signal narrower than the grouping is honoured across the whole group', async () => { + // robots.txt is per-origin, so a `Crawl-delay` read from one subdomain is hostname-scoped - and the + // manager holds one queue per group, so it cannot hold a single host back. + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: 'all', + throttleBy: 'registrableDomain', + }); + + expect( + manager.recordPacingSignal({ + url: 'https://a.example.com/robots.txt', + reason: 'minInterval', + intervalMs: 60_000, + scope: 'hostname', + }), + ).toBe(true); + + // The sibling subdomain is paced by it too - over-applied, never under-applied. + await manager.addRequest({ url: 'https://b.example.com/1' }); + await manager.addRequest({ url: 'https://b.example.com/2' }); + + expect((await manager.fetchNextRequest())!.url).toBe('https://b.example.com/1'); + expect(await manager.fetchNextRequest()).toBeNull(); + }); + + test('a signal wider than the grouping throws instead of being under-applied', async () => { + // Pacing one host would leave its siblings running flat out, so the manager refuses rather than pretend. + const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: 'all' }); + + expect(() => + manager.recordPacingSignal({ + url: 'https://a.example.com/1', + reason: 'minInterval', + intervalMs: 60_000, + scope: 'registrableDomain', + }), + ).toThrow(/groups requests by "hostname".*throttleBy: "registrableDomain"/s); + }); + + test('a scope it does not speak throws', async () => { + const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: 'all' }); + + // A per-account limit is a real thing to be told about, but this manager keys on hostnames. + expect(() => + manager.recordPacingSignal({ url: 'https://example.com/1', reason: 'rateLimited', scope: 'account' }), + ).toThrow(/only understands the scopes "hostname" and "registrableDomain"/); + }); + + test('an unscoped signal is applied at the grouping, whatever that is', async () => { + // A 429 does not say whether the limit was per host, per account or per address, so nothing is declared. + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: 'all', + throttleBy: 'registrableDomain', + }); + + await manager.addRequest({ url: 'https://a.example.com/1' }); + expect(manager.recordPacingSignal({ url: 'https://a.example.com/1', reason: 'rateLimited' })).toBe(true); + + await expect(manager.checkReadiness()).resolves.toMatchObject({ status: 'waiting' }); + }); + }); + + describe("reason: 'minIntervalEverywhere'", () => { + const floor = (intervalMs: number, scope = 'registrableDomain' as const) => + ({ reason: 'minIntervalEverywhere', intervalMs, scope }) as const; + + test('paces every domain the manager covers, declared or not', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: 'all', + throttleBy: 'registrableDomain', + }); + + expect(manager.recordPacingSignal(floor(60_000))).toBe(true); + + await manager.addRequest({ url: 'https://example.com/1' }); + await manager.addRequest({ url: 'https://example.com/2' }); + + expect((await manager.fetchNextRequest())!.url).toBe('https://example.com/1'); + expect(await manager.fetchNextRequest()).toBeNull(); + }); + + test('is a floor, so a longer configured delay stands', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: 'all', + throttleBy: 'registrableDomain', + minCrawlDelaySecs: 60, + }); + + expect(manager.recordPacingSignal(floor(1_000))).toBe(true); + + await manager.addRequest({ url: 'https://example.com/1' }); + await manager.fetchNextRequest(); + + // The dispatch armed the configured minute, not the second it was just handed. + expect(domainState(manager, 'example.com').crawlDelayUntil).toBeGreaterThan(Date.now() + 30_000); + }); + + test('a manager that paces nothing reports it as unhandled', async () => { + // Answering `false` rather than throwing is what lets a caller pace it from the outside instead. + const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: [] }); + + expect(manager.recordPacingSignal(floor(1_000))).toBe(false); + }); + + test('a manager that paces only some domains throws instead of leaving the rest unpaced', async () => { + const manager = new ThrottlingRequestManager({ + inner: await createQueue(), + domains: ['example.com'], + throttleBy: 'registrableDomain', + }); + + expect(() => manager.recordPacingSignal(floor(1_000))).toThrow(/domains: 'all'/); + }); + + test('a floor at a finer grain than the grouping throws, like any other signal', async () => { + const manager = new ThrottlingRequestManager({ inner: await createQueue(), domains: 'all' }); + + expect(() => manager.recordPacingSignal(floor(1_000))).toThrow(/groups requests by "hostname"/); + }); + }); }); diff --git a/test/e2e/playwright-enqueue-links/actor/main.js b/test/e2e/playwright-enqueue-links/actor/main.js index 221b413b8c49..ea500d77faba 100644 --- a/test/e2e/playwright-enqueue-links/actor/main.js +++ b/test/e2e/playwright-enqueue-links/actor/main.js @@ -14,14 +14,12 @@ const mainOptions = { await Actor.main(async () => { const crawler = new PlaywrightCrawler({ maxRequestsPerCrawl: 30, - requestHandler: async ({ page, request, enqueueLinks, closeCookieModals }) => { + requestHandler: async ({ page, request, enqueueLinks }) => { const { url, loadedUrl } = request; const pageTitle = await page.title(); log.info(`URL: ${url}; LOADED_URL: ${loadedUrl}; TITLE: ${pageTitle}`); - await closeCookieModals(); - const results = await enqueueLinks(); if (loadedUrl.startsWith('https://drive')) { diff --git a/test/e2e/playwright-enqueue-links/actor/package.json b/test/e2e/playwright-enqueue-links/actor/package.json index ce445f0f5f9e..197d3d7ef654 100644 --- a/test/e2e/playwright-enqueue-links/actor/package.json +++ b/test/e2e/playwright-enqueue-links/actor/package.json @@ -11,7 +11,6 @@ "@crawlee/playwright": "file:./packages/playwright-crawler", "@crawlee/types": "file:./packages/types", "@crawlee/utils": "file:./packages/utils", - "idcac-playwright": "^0.2.0", "playwright": "1.58.2" }, "overrides": { diff --git a/test/e2e/puppeteer-enqueue-links/actor/main.js b/test/e2e/puppeteer-enqueue-links/actor/main.js index 48415382f3a5..226e5e70f9f9 100644 --- a/test/e2e/puppeteer-enqueue-links/actor/main.js +++ b/test/e2e/puppeteer-enqueue-links/actor/main.js @@ -12,14 +12,12 @@ const mainOptions = { await Actor.main(async () => { const crawler = new PuppeteerCrawler({ maxRequestsPerCrawl: 30, - async requestHandler({ page, enqueueLinks, request, log, closeCookieModals }) { + async requestHandler({ page, enqueueLinks, request, log }) { const { url, loadedUrl } = request; const pageTitle = await page.title(); log.info(`URL: ${url}; LOADED_URL: ${loadedUrl}; TITLE: ${pageTitle}`); - await closeCookieModals(); - const results = await enqueueLinks(); if (loadedUrl.startsWith('https://drive')) { diff --git a/test/e2e/puppeteer-enqueue-links/actor/package.json b/test/e2e/puppeteer-enqueue-links/actor/package.json index f058749f4858..e683d0b38737 100644 --- a/test/e2e/puppeteer-enqueue-links/actor/package.json +++ b/test/e2e/puppeteer-enqueue-links/actor/package.json @@ -11,7 +11,6 @@ "@crawlee/puppeteer": "file:./packages/puppeteer-crawler", "@crawlee/types": "file:./packages/types", "@crawlee/utils": "file:./packages/utils", - "idcac-playwright": "^0.2.0", "puppeteer": "*" }, "overrides": {