feat: implement per-domain request throttling (ThrottlingRequestManager) - #3741
Merged
Conversation
(cherry picked from commit 5422b40)
`requestManager` was missing from `optionsShape`, so `ow.object.exactShape` rejected the documented option outright; nothing covered it. Adds the regression test the drive-by fix in this branch never got.
…azily Sub-queues live under a stable `throttled-<domain>` alias and outlive the process, but the map tracking them was only filled on insert - so a restart saw none, reported the crawl finished, and stranded everything a previous run had left throttled. Also fixes `purge` and `persistState` skipping them.
`fetchNextRequest` slept until the backoff expired (up to `maxDelayMs`), holding a task slot the autoscaler reads as spare capacity - so it scaled up toward `maxConcurrency` and released the lot at once, and `stop()` could not complete. It now returns `null` and `isEmpty()` reports throttled requests as unavailable, so the task loop idles instead. Drops the single-waiter wake primitive, which could only ever wake one of several concurrent callers.
The hand-rolled `addRequestsBatched` drained its whole input up front, ignored `maxNewRequests`, never reported `requestsOverLimit`, dropped the in-flight batch count `isFinished()` depends on, and left its background promise unhandled so a backend error killed the process. It now chunks lazily, routes each chunk by domain, and hands the slices to the target managers' own `addRequestsBatched`. Drops the public `addRequests`, which only existed to support the duck-typed fallback and is not part of `IRequestManager`.
A URL list's contents are unknown until the owning manager expands it, so those requests always land in the inner manager and silently escape throttling. Say so once instead of pretending they were routed.
…onse Every already-in-flight request comes back 429, so counting each one made the exponent track concurrency (8 parallel requests jumped straight to 2^7). The counter now advances once per event and decays on its own after a quiet window, which also removes the `errorMessages`/`retryCount` success guess - that read as success for skipped requests and silently flattened the curve.
A plain `Error` charged the 429 to `maxRequestRetries` and to the session's error score, so a healthy but rate-limited domain still failed its requests and still burned proxies - the two things the throttling manager exists to avoid. Throwing `RequestThrottledError` instead reclaims the request without recording a failure, and the manager paces the retry behind the backoff.
…opped The startup warning read a tandem's unresolved inner manager, so it fired for correctly configured crawlers whenever `run()` got no URLs - while staying silent when a `ThrottlingRequestManager` was configured without the crawled domain in its `domains` list. `setCrawlDelay` now reports whether it took effect and the warning follows that, naming the domain at fault.
The `'x' in y && typeof (y as any).x === 'function'` dance was repeated at both call sites, and the browser one hand-rolled a case-insensitive header scan plus an array unwrap that neither Playwright nor Puppeteer can produce. Also records the rate limit before the error-status throw, so opting 429 into `additionalHttpErrorStatusCodes` no longer skips the backoff.
`String(parseInt(v)) === v.trim()` rejected valid zero-padded values like `05` and accepted `-5`, which set a delay in the past - reporting the domain as throttled while applying no backoff at all. Also drops a try/catch around `Date.parse`, which does not throw.
A 526-line public class shipped with no doc comment at all, so its defaults, its opt-in nature and the meaning of `recordDomainDelay`'s return value were undiscoverable. Also removes a field only the constructor read.
A new public request manager was absent from the request-loaders guide that indexes them, and it silently changes what a 429 does - which until now the session-management guide was the only place to describe.
`pnpm api:check` runs in CI and the new exports were never recorded, so the check failed deterministically. Also narrows the constructor's `config` parameter to private - it was leaking into the public surface unintentionally.
A throttled 429 costs no retry, so a domain that never recovered kept the crawl alive forever - `run()` simply never resolved, while the statistics reported zero requests throughout. Each domain now carries a stall clock, reset whenever one of its requests is handled, and the crawler aborts with a `PersistentRateLimitError` once a domain that still has queued work has been rate-limiting for `maxDomainStallSecs`. Those requests are deliberately left in their queue, so re-running without purging storages resumes them if the limit lifts. Also renames the manager's delay options to seconds, matching the rest of the crawler options, and replaces the `Partial<ThrottlingRequestManager>` casts in `BasicCrawler` with a `SupportsDomainThrottling` guard. That drops the tandem's two forwarded methods: a tandem-wrapped throttler no longer throttles, but the existing warnings say so rather than leaving it silent.
This was referenced Aug 8, 2026
`ThrottlingRequestManager.addRequestsBatched` was a near-verbatim copy of `RequestQueue`'s: same budget-derived chunk size, same peek/next dance, same over-limit drain. The copy had already fallen behind - the transaction handling `RequestQueue` gained never reached it - so the loop now lives in `drainRequestBatches` and both callers supply only what actually differs, namely how a chunk is added and how the input is normalized. Fixes a latent bug in the process: `RequestQueue` ran its background batches in an `async` promise executor, which swallows throws. A failing background batch therefore left `waitForAllRequestsToBeAdded` pending forever and never decremented `inProgressRequestBatchCount`, so the queue reported itself unfinished indefinitely - now it rejects and settles.
…es the manager The test asserted that no warning was logged and that `setCrawlDelay` still returned `true` - both of which hold when the domain is merely configured. It passed with `applyCrawlDelay` deleted outright. Now it checks the recorded delay and that dispatch is paced by it.
A 429 from a domain a `ThrottlingRequestManager` covers is treated as a rate limit before `blockedStatusCodes` is consulted, so the list only governs the domains it does not cover. Worth saying plainly: removing 429 from the list is the obvious-looking way to stop sessions being retired when adopting throttling, and it neither helps nor is needed.
- `parseRetryAfterHeader` was public API exported from a storages module despite being a plain HTTP header parser, so it moves to `http.ts` alongside its test. - Adds `innerManager`, so a caller that hands the wrapper its queue can still get the queue back, matching crawlee-python's `inner`. - `domains` now rejects empty strings instead of quietly dropping them, which turned a typo into a silently unthrottled domain. - Makes the two remaining time-sensitive tests deterministic: the backoff decay test slept out 20ms windows and flaked under load, and the purge test never checked the sub-queue was non-empty to begin with.
Reading the `Retry-After` header cast the response to a shape asserting the method exists and then optional-called it anyway. `BaseResponse` describes what the crawler relies on, so the method belongs there - optional, because only the Playwright and Puppeteer responses are guaranteed to carry it.
`isEmpty` answers what the next fetch would return, not how much work is left, so a loader that withholds requests for a while - as `ThrottlingRequestManager` does for a rate-limited domain - reports empty while requests are still queued.
This was referenced Aug 9, 2026
Open
janbuchar
approved these changes
Aug 9, 2026
janbuchar
left a comment
janbuchar
left a comment
Contributor
There was a problem hiding this comment.
LGTM, but I changed the original a lot @harryautomazione, can you check it? @barjin, please, can you provide a fresh pair of eyes?
barjin
approved these changes
Aug 10, 2026
barjin
left a comment
barjin
left a comment
Member
There was a problem hiding this comment.
Thank you both! I don't have anything to add (other than the existing issues). Approving 👍
janbuchar
added a commit
that referenced
this pull request
Aug 12, 2026
- closes #3998 Unnamed storages (default and aliased) are now purged on start, named ones are not — crawlee-python's rule. The fs backend sweeps the storage directories, so leftovers from a previous process are caught too. Two adjacent bugs, one commit each: - `createDatasetBackend()` / `({})` didn't open the default storage, though `StorageIdentifier` says they do - the `__default__` sentinel leaked into the directory name, so default storages lived in `storage/datasets/__default__` rather than `default` `ThrottlingRequestManager` (#3741) sub-queues are alias-keyed, so they now only survive a restart with `purgeOnStart` off — as its docs already said.
B4nan
pushed a commit
that referenced
this pull request
Aug 12, 2026
…er) (#3741) Co-authored-by: Jan Buchar <jan@buchar.dev>
B4nan
pushed a commit
that referenced
this pull request
Aug 12, 2026
- closes #3998 Unnamed storages (default and aliased) are now purged on start, named ones are not — crawlee-python's rule. The fs backend sweeps the storage directories, so leftovers from a previous process are caught too. Two adjacent bugs, one commit each: - `createDatasetBackend()` / `({})` didn't open the default storage, though `StorageIdentifier` says they do - the `__default__` sentinel leaked into the directory name, so default storages lived in `storage/datasets/__default__` rather than `default` `ThrottlingRequestManager` (#3741) sub-queues are alias-keyed, so they now only survive a restart with `purgeOnStart` off — as its docs already said.
B4nan
pushed a commit
that referenced
this pull request
Aug 18, 2026
…er) (#3741) Co-authored-by: Jan Buchar <jan@buchar.dev>
B4nan
pushed a commit
that referenced
this pull request
Aug 18, 2026
- closes #3998 Unnamed storages (default and aliased) are now purged on start, named ones are not — crawlee-python's rule. The fs backend sweeps the storage directories, so leftovers from a previous process are caught too. Two adjacent bugs, one commit each: - `createDatasetBackend()` / `({})` didn't open the default storage, though `StorageIdentifier` says they do - the `__default__` sentinel leaked into the directory name, so default storages lived in `storage/datasets/__default__` rather than `default` `ThrottlingRequestManager` (#3741) sub-queues are alias-keyed, so they now only survive a restart with `purgeOnStart` off — as its docs already said.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements per-domain request rate-limiting and delay logic at the
RequestManagerlayer. It ports the design from Python's Crawlee (PR #1762) to the TypeScriptv4branch, addressing the architectural feedback from PR #3737.Context & Rationale
Previously, receiving an HTTP 429 status code triggered a
SessionErrorwhich retired and rotated the proxy/session immediately, leading to high IP churn and fast session depletion.By moving the throttling to the request manager layer:
Retry-Afterheaders and applies exponential backoff delays precisely to the affected domains.Key Changes
ThrottlingRequestManager(packages/core/src/storages/throttling_request_manager.ts): Implemented a new storage wrapper that implementsIRequestManager. It routes domain-specific requests to sub-managers, manages backoff delays, and runs a wake/sleep loop infetchNextRequestto avoid busy waiting.HttpCrawler&BrowserCrawler: Intercepted HTTP 429 status codes, extracted retry-after, registered the delay withThrottlingRequestManager, and forced a standard retry (avoiding session retirement).BasicCrawler: Integrated withRobotsTxtFileto register thecrawl-delaywhenrespectRobotsTxtFileis active.RobotsTxtFile(packages/utils/src/internals/robots.ts): ExposedgetCrawlDelayfromrobots-parser.Verification Results
test/core/storages/throttling_request_manager.test.tscovering routing, exponential backoff, headers parsing, and crawl-delay settings (5/5 passed).test/core/crawlers/http_crawler.test.tsverifying that HTTP 429 delays are respected and session is not retired (17/17 passed).