refactor: add ArgumentValidationError to @crawlee/core#3716
Open
B4nan wants to merge 100 commits into
Open
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 #2479 - closes #3106 - closes #3107 - closes #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 #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 #3198
Phasing out `got-scraping`-specific interfaces in favour of native `fetch` API. Related to #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).
Related to #3275 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…3611) ## Summary When a commit only touches files under `test/e2e/**` (skipped by oxlint's `ignorePatterns`), `oxlint --fix` exits with an error because no files match, breaking the lint-staged hook. Mirrors the flag already used for `oxfmt` so the hook succeeds in this case. Closes #3610 🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Summary
- **Replaces `get()`/`set()` with direct property access** —
`config.headless` instead of `config.get('headless')`. Both methods are
removed (v4 breaking change).
- **Constructor options now take highest priority** — `new
Configuration({ headless: false })` works even when
`CRAWLEE_HEADLESS=true` is set. New priority: constructor > env vars >
crawlee.json > schema defaults.
- **Immutable instances** — assigning to a config property throws
`TypeError`.
- **Zod-based field definitions** — schema, env var mapping, and
defaults defined in one place (`crawleeConfigFields`).
- **Extensible via subclassing** — subclasses override `protected static
fields` to register additional config fields (e.g. Apify SDK).
Closes #3080
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The publish step was silently emitting `changed_packages=0` because lerna 9.x crashed with `import_minimatch2.default is not a function` and `echo "x=$(... | wc -l)" | tee` swallowed the non-zero exit. - pnpm-workspace: scope `lerna>minimatch` to ^3.1.4 so lerna keeps the CJS-default-export shape its bundled code expects, while everything else still dedups to the global minimatch ^9.0.0 override. - test-ci: `set -eo pipefail` plus an explicit assignment so a lerna crash now fails the step instead of being masked by the outer pipe.
The publish workflow had `inputs.dist-tag == 'next'` gates on the bump, commit, and `pnpm publish:next` steps. v4 passes `dist-tag: 'v4'`, which silently skipped all four steps — the workflow reported success but nothing was published. Switching to `!= 'prod'` lets any canary tag (`next`, `v4`, future release lines) flow through. The actual npm dist-tag is still set per branch by the `publish:next` script (`--dist-tag next` on master, `--dist-tag v4` on v4).
pnpm 10 emits `EDOUBLEDASH` and no longer forwards args after `--`, so `pnpm publish:next -- --yes` left lerna prompting interactively and the publish step blocked indefinitely. Moving `--yes` into the script itself makes it pnpm-version-agnostic; dropped the now-dead `-- --yes` from the workflow command for symmetry.
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 #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 #3595
…ious fixes (#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 #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 #3502
## 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 (#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 #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 #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 #3649 and is removed with the revert) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
) 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) #3617
…om implementations (#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 #3702
A shared error class for schema (zod) validation failures. Its `message` is a human-readable sentence naming the offending field and the value it received, while the structured zod `issues` are exposed directly on the error (and the original `ZodError` on `cause`). This is the foundation for migrating Crawlee's argument validation from `ow` to `zod`; the Apify SDK re-exports it for the same purpose.
B4nan
changed the title
Member
Author
|
Now I realized we wanted to drop this SDK<->crawlee dependency, but even in that case, we want the same error handling experience (as well as zod instead of ow), so I'd merge this in regarldess. |
Contributor
Agreed, but maybe we could put this in a package smaller than core? If SDK ends up depending on utils, for instance, nobody will bat an eye... |
Member
Author
|
We might want to take a closer look at this, and maybe even introduce some |
Contributor
|
Yeah, makes sense. |
|
I noticed I would add a regression test for an optional/string field receiving |
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.
Adds a shared
ArgumentValidationError(exported from@crawlee/coreviaerrors.ts) for schema (zod) validation failures.message— a plain, human-readable sentence naming the offending field and the value it received, e.g.Invalid string: must match pattern /^[A-Z]{2}$/ at `countryCode`, got `CZE`. Not a JSON dump.issues— the structured zod issues, exposed directly on the error so consumers can branch on them without digging intocause.cause— the originalZodError, kept for completeness.The constructor takes
(error: ZodError, value: unknown)and formats the message itself (thevalueis walked by each issue'spathto surface the offending value).Why
This is the foundation for migrating Crawlee's argument validation from
owtozod. The Apify SDK has just done the same migration and currently defines this class itself (apify-sdk-js#636) — once this lands and releases, the SDK will re-export it from@crawlee/coreinstead, so the two stay in lockstep.Scope is intentionally just the error class (+ its formatter and a test) — no
owcall sites are migrated here.@crawlee/corealready depends onzod; built in dependency order + lint + the new test all pass.🤖 Generated with Claude Code