refactor: remove browserPoolOptions from BrowserCrawler#3785
Open
yuvrajsingh2428 wants to merge 106 commits into
Open
refactor: remove browserPoolOptions from BrowserCrawler#3785yuvrajsingh2428 wants to merge 106 commits into
yuvrajsingh2428 wants to merge 106 commits into
Conversation
BREAKING CHANGE: The project is now native ESM without a CJS alternative. This is fine since all supported node versions allow `require(esm)`. Also all the dependencies are updated to the latest versions, including cheerio v1.
BREAKING CHANGE: The crawler following options are removed: - `handleRequestFunction` -> `requestHandler` - `handlePageFunction` -> `requestHandler` - `handleRequestTimeoutSecs` -> `requestHandlerTimeoutSecs` - `handleFailedRequestFunction` -> `failedRequestHandler`
BREAKING CHANGE: The crawling context no longer includes the `Error` object for failed requests. Use the second parameter of the `errorHandler` or `failedRequestHandler` callbacks to access the error. 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.
….retireOnBlockedStatusCodes` BREAKING CHANGE: `additionalBlockedStatusCodes` parameter of `Session.retireOnBlockedStatusCodes` method is removed. Use the `blockedStatusCodes` crawler option instead.
….retireOnBlockedStatusCodes` BREAKING CHANGE: `additionalBlockedStatusCodes` parameter of `Session.retireOnBlockedStatusCodes` method is removed. Use the `blockedStatusCodes` crawler option instead.
also tries to bump better-sqlite3 to latest version to have prebuilds for node 22
- closes apify#2479 - closes apify#3106 - closes apify#3107 - closes apify#3078 In my opinion, it makes a lot of sense to do the remaining changes in a separate PR. - [x] Introduce a `ContextPipeline` abstraction - [x] Update crawlers to use it - [x] Make sure that existing tests pass - [ ] Refine the `ContextPipeline.compose` signature and the semantics of `BasicCrawlerOptions.contextPipelineEnhancer` to maximize DX - [x] Write tests for the `contextPipelineEnhancer` - [x] Resolve added TODO comments (fix immediately or make issues) - [ ] Update documentation The `context-pipeline` branch introduces a fundamental architectural change to how Crawlee crawlers build and enhance the crawling context passed to request handlers. The core motivation is to fix the composition and extensibility nightmare in the current crawler hierarchy. 1. **Rigid inheritance hierarchy**: Crawlers were stuck in a brittle inheritance chain where each layer manipulated the context object while assuming that it already satisfied its final type. Multiple overrides of `BasicCrawler` lifecycle methods made the execution flow even harder to follow. 2. **Context enhancement via monkey-patching**: Manual property assignment (`crawlingContext.page = page`, `crawlingContext.$ = $`) scattered everywhere. It was a mess to follow and impossible to reason about. 3. **Cleanup coordination**: Resource cleanup was handled by separate `_cleanupContext` methods that were not co-located with the initialization. 4. **Extension mechanism was broken**: The `CrawlerExtension.use()` API tried to let you extend crawlers (the ones based on `HttpCrawler`) by overwriting properties - completely type-unsafe and fragile as hell. Introduces `ContextPipeline` - a **middleware-based composition pattern** where: - Each crawler layer defines how it enhances the context through explicit `action` functions - Cleanup logic is co-located with initialization via optional `cleanup` functions - Type safety is maintained through TypeScript generics that track context transformations - The pipeline executes middleware sequentially with proper error handling and guaranteed cleanup Declarative middleware composition with co-located cleanup: ```typescript contextPipeline.compose({ action: async (context) => ({ page, $ }), cleanup: async (context) => { await page.close(); } }) ``` The `ContextPipeline<TBase, TFinal>` tracks type transformations through the chain: ```typescript ContextPipeline<CrawlingContext, CrawlingContext> .compose<{ page: Page }>(...) // ContextPipeline<CrawlingContext, CrawlingContext & { page: Page }> .compose<{ $: CheerioAPI }>(...) // ContextPipeline<CrawlingContext, CrawlingContext & { page: Page, $: CheerioAPI }> ``` The `CrawlerExtension.use()` is gone. New approach via `contextPipelineEnhancer`: ```typescript new BasicCrawler({ contextPipelineEnhancer: (pipeline) => pipeline.compose({ action: async (context) => ({ myCustomProp: ... }) }) }) ``` The current way to express a context pipeline middleware has some shortcomings (`ContextPipeline.compose`, `BasicCrawlerOptions.contextPipelineEnhancer`). I suggest resolving this in another PR. For most legitimate use cases, this should be non-breaking. Those who extend the Crawler classes in non-trivial ways may need to adjust their code though - the non-public interface of `BasicCrawler` and `HttpCrawler` changed quite a bit. The pipeline uses `Object.defineProperties` for each middleware. Is this a serious performance consideration? --------- Co-authored-by: Martin Adámek <banan23@gmail.com>
Extracts `ProxyConfiguration` to `BasicCrawler` (related to discussion under apify#2917). Pass the `ProxyConfiguration` instance to the `SessionPool` for new `Session` object creation. Store and read the `ProxyInfo` from the `Session` instance instead of calling the `ProxyConfiguration` methods in the crawlers. closes apify#3198
Phasing out `got-scraping`-specific interfaces in favour of native `fetch` API. Related to apify#3071
Fixes build toolchain errors caused by the recent rebase onto the current `master` ([more details here](https://apify.slack.com/archives/C02JQSN79V4/p1764373034961859)). The largest thing is probably updating the dependency versions in `package.json` - if `turborepo` doesn't find the matching version in the local workspace, it will build against the package pulled from `npm` (which doesn't match the v4 API at this point).
…client (apify#3286) Removes incorrect implementation `KVS.getPublicUrl()` implementation from `@crawlee/core` and proxies the call to the storage client. Closes apify#3272 Closes apify#3076
…rfaces (apify#3295) Works towards removing `got-scraping` as a direct Crawlee dependency. Related to apify#3275 Related to apify#3071
Related to apify#3275 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The pnpm-migration commented this block out and then dropped it. The intent was to disable it on v4 (where v4-beta isn't the Docker image to build), not to retire it permanently. Reinstating it with `if: github.ref == 'refs/heads/master'` so the block lives in v4's workflow file but only fires once these changes land on master.
The feature has seen little adoption, and the tier rotation bled into APIs that otherwise had no business knowing about it (`ProxyConfiguration` pulling a `Request`, `newProxyInfo` returning a tier, browser controllers tracking a tier, etc.). In `v4`, the main rotation unit is the `Session`, so proxy-level tier rotation is redundant. The behaviour can still be emulated idiomatically with `Session` and `SessionPool` classes (see the Upgrading guide). Closes apify#3597
## Summary `packages/linkedom-crawler/src/internals/linkedom-crawler.ts` imports `cheerio` (`import * as cheerio from 'cheerio'`) but `@crawlee/linkedom`'s `package.json` doesn't list it as a dependency. It works inside the monorepo because cheerio is hoisted to the workspace root via other packages (`@crawlee/cheerio`, `@crawlee/utils`, `@crawlee/http`, …), so Node always finds it. **Downstream installs that depend only on `crawlee`** (which re-exports `@crawlee/linkedom`) **and don't pull any cheerio-using sibling** fail at runtime: ``` Error: Cannot find package 'cheerio' imported from .../node_modules/@crawlee/linkedom/internals/linkedom-crawler.js ``` This bit the apify-sdk-js v4 catch-up PRs (apify/apify-sdk-js#597) on a clean CI install — without this fix, every consumer has to ship a `cheerio` dev-dep workaround. The fix is one-line: declare `cheerio: "^1.0.0"` (matching what `@crawlee/cheerio` already pins).
Makes `BasicCrawler` (and its subclasses) reuse `Session` instances according to the `sessionReuseStrategy`. Before, the `BasicCrawler` would create a single-use `Session` for each `Request` unless instructed otherwise (e.g., via `request.sessionId`). Refactors small bits around the `Session` lifecycle in `BasicCrawler`. Closes apify#3595
…ious fixes (apify#3606) The flag had become a no-op after cookie handling moved into `BaseHttpClient` - response `Set-Cookie` headers were always written back into the session jar regardless of the flag's value. Pass a per-request clone of the jar when the flag is off, so reads keep working, but writes are discarded. This PR also adds several regression tests for cookie-related behaviour. Together with the other `Session`-related fixes, this closes apify#3271 in `v4`.
Drops the `maxSessionRotation` option and related tooling, simplifying the request retry logic. Blocked requests now get retried the same way as any other failing requests. Closes apify#3502
… to centralize storage instance caching (apify#3584) - depends on apify#3570 - closes apify#3592
## Summary - Adds `Configuration.reset()` as a discoverable static method on the new v4 `Configuration` class. Delegates to `serviceLocator.reset()` so the cached configuration, event manager, storage client, and logger are all dropped together. - Adds a test verifying that after `Configuration.reset()`, the next `Configuration.getGlobalConfig()` re-reads `process.env`. ## Motivation v4 `Configuration` resolves env vars eagerly at construction, so tests that mutate `process.env` after the global singleton has been resolved need a way to drop the cached instance. Today that's either `serviceLocator.reset()` (works, but the name suggests "service tree" rather than "the config I just changed env vars for") or reaching into the private `Configuration.globalConfig` via type assertions (currently happening across multiple test files in `apify-sdk-js`). A class-side `Configuration.reset()` makes the intent obvious from the call site and lives next to `Configuration.getGlobalConfig()` in autocomplete. The implementation just delegates to `serviceLocator.reset()` rather than resetting only the configuration — resetting only the config would leave a stale `EventManager` / `StorageClient` / logger holding the old config, which is the wrong contract for callers wanting to "start fresh". ## Test plan - [x] `vitest run packages/core/test/core/configuration.test.ts -t "reset"` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## What changes - Bump `packageManager` pnpm@10.24.0 → pnpm@11.0.9 (and the matching `volta` hint). - Drop `preinstall: npx only-allow pnpm` (ships in publish tarballs and breaks downstream npm consumers — see apify/apify-client-js#893; `only-allow`'s own `node_modules` guard catches the case but the `npx` bootstrap fails first in `npm install -g` context). - Add `devEngines.packageManager: { name: pnpm, version: 11.0.9, onFail: error }`. pnpm v11 reimplemented `pnpm config / version / pkg` as native subcommands (v10 still shells to npm), so `error` is safe here. - Migrate `pnpm-workspace.yaml` from `onlyBuiltDependencies` (pnpm 10 list) to `allowBuilds` (pnpm 11 explicit map). Add `strictDepBuilds: false` so install still proceeds if a new build-requesting dep shows up without an explicit entry; existing entries all flipped to `true` to preserve current build behavior. - `pnpm-lock.yaml`: minor inline of `patchedDependencies` (one-line hash) — pnpm 11 lockfile reader normalizes the old `hash:` / `path:` map. Lockfile version stays at 9.0. ## Why on v4 specifically All v4 packages require Node 22+ (the exception is `stagehand-crawler` at `>=16`, untouched here). pnpm v11 also requires Node 22+, so v4 is the natural place to land the upgrade — `master` (still supporting Node 18/20) stays on pnpm 10. ## Verified locally - `corepack pnpm --version` after the change → `11.0.9` (Corepack picks it up from `packageManager`). - `pnpm install --frozen-lockfile` succeeds. - `pnpm rebuild` runs all builds in the `allowBuilds` map without errors (better-sqlite3, nx postinstall, puppeteer, etc.). ## CI notes - All workflows use `apify/workflows/pnpm-install@main`, which corepacks pnpm from `packageManager` — picks up v11 automatically. - Releases use `lerna version`, not `pnpm version`, so the v10 npm-shellout trap from apify/apify-client-js#895 doesn't apply. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: B4nan <615580+B4nan@users.noreply.github.com>
…opy (apify#3650) ## What broke The v4 publish workflow died on the canary-version-bump step: https://github.com/apify/crawlee/actions/runs/25744567788/job/75604092585 \`\`\` ERROR @crawlee/utils#copy: command (packages/utils) pnpm run copy --canary=major --preid=beta exited (1) [ERROR] Command was killed with SIGINT (User interruption with CTRL-C): /home/runner/setup-pnpm/node_modules/.bin/global/...pnpm.mjs install at runDepsStatusCheck (pnpm.mjs:211947:7) \`\`\` ## Why pnpm 11 added \`verifyDepsBeforeRun\` (default: \`warn\`), which silently runs \`pnpm install\` before every \`pnpm run X\` to make sure installed deps match the lockfile. The publish workflow runs \`pnpm turbo copy --force -- --canary=major --preid=beta\`, which fans \`pnpm run copy\` out across all 23 workspace packages **in parallel**. So pnpm fires off 23 concurrent installs that fight over the store and the lockfile — one wins, the rest get killed with SIGINT, turbo aborts. Regressed in apify#3645 (pnpm 10 → 11). pnpm 10 had no auto-install hook. ## Fix \`\`\`yaml # pnpm-workspace.yaml verifyDepsBeforeRun: false \`\`\` CI already runs \`pnpm install --frozen-lockfile\` once at job start (via \`apify/workflows/pnpm-install\`), so disabling the per-script auto-install just removes the parallel contention without losing any guarantee.
## Summary Reverts apify#3649. The added `Configuration.reset()` is a thin alias for `serviceLocator.reset()` — same scope, less honest name. The method lives on `Configuration` but doesn't reset anything *on* the Configuration; cached env-resolved values on existing instances are untouched, and the actual effect is "drop the configuration + event manager + storage client + logger that the service locator caches." Callers who want that should keep calling `serviceLocator.reset()` directly, where the name matches the behavior. A narrower, Configuration-only reset (drop just the cached config, leave other services alone) would technically belong on `Configuration`, but it's not what callers want in practice — surviving services would still hold the stale config, leaving the runtime in an inconsistent state. The whole-tree drop is the right operation, and `serviceLocator.reset()` already provides it. ## Test plan - [x] Existing crawlee tests still pass (the only test for `Configuration.reset()` was added in apify#3649 and is removed with the revert) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…ify#3643) Removes `extends EventEmitter` from `SessionPool`. The event system was originally only used to retire browsers on expiring `Session`s. `BrowserCrawler` now retires the browser at the end of `Request` processing, if the `Session` proves to be retired. This leads to a simpler API. This refactor simplifies the `SessionPool` interface and allows us to drop the `SessionPool` references from different parts of the codebase. Related to (prerequisite of) apify#3617
…om implementations (apify#3644) Exports `ISessionPool` interface that can be used to implement custom `SessionPool` implementations. Adds documentation.
Bumps Playwright and Puppeteer dependencies to the latest versions to ensure compatibility with Node 24.16. Closes apify#3702
… with their Python counterparts (apify#3627) closes apify#3075
Adds a `fingerprint` field to `Session` so repeated requests with the same session can stay consistent on user-agent, headers, and TLS profile across HTTP clients and browser crawlers. Closes apify#3628.
…ify#3672) `preNavigationHooks` and `postNavigationHooks` may now return a `Partial<Context>`; returned own properties are merged into the crawling context. Closes apify#3660
…erpart (apify#3729) Closes apify#3075 closes apify#2840
yuvrajsingh2428
changed the title
B4nan
added
the
t-tooling
barjin
reviewed
Jul 1, 2026
barjin
left a comment
barjin
left a comment
Member
There was a problem hiding this comment.
Thank you @yuvrajsingh2428 for drafting this PR.
I believe the original request feature has design flaws. Let us discuss this before proceeding with work on this PR.
Cheers!
| // Disable the default fingerprint spoofing to avoid conflicts with Camoufox. | ||
| useFingerprints: false, | ||
| }, | ||
| }), |
Member
There was a problem hiding this comment.
This replaces PlaywrightCrawler's implicit BrowserPool with the right browserPlugins: [playwrightLauncher.createBrowserPlugin()] (the custom instance doesn't have any browserPlugins)
I'm afraid the current BrowserPool design (with library-specific launchers) forbids us from dropping the browserPoolOptions from BrowserCrawler. Any user trying to customize BrowserPool options for a crawler would have to pass the right browserPlugins (and make sure these match the BrowserCrawler subclass they are using).
In the end, I'd now rather keep browserPoolOptions / browserPool for simplicity.
cc @janbuchar wdyt ?
Author
There was a problem hiding this comment.
Thanks for the review!
That makes sense. I followed the direction from the issue discussion and assumed that requiring a pre-instantiated browserPool would be preferable to exposing two configuration paths.
I agree that, with the current BrowserPool design, users would also need to know how to provide the correct browserPlugins for the crawler they're using, which leaks implementation details and makes simple customization harder.
I'm happy to adapt the PR based on the outcome of the discussion between you and @janbuchar, or close it if the consensus is to keep supporting both browserPoolOptions and browserPool.
Author
|
@janbuchar @barjin Just following up on the discussion above. I'm happy to either adapt this PR to a revised approach or close it if the consensus is to keep supporting both |
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
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
browserPoolOptionsfromBrowserCrawlerbrowserPoolwhen custom browser pool configuration is neededPlaywrightCrawler,PuppeteerCrawler, andStagehandCrawlerto create a defaultBrowserPoolwhen one is not providedbrowserPoolCloses #3728