Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions docs/guides/result_storage.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,30 @@ This is **on by default** for every crawler and covers the whole request lifecyc

- **Deferred cleanups.** Callbacks registered with `registerDeferredCleanup()` run after the transaction is closed, so their writes land immediately and are not rolled back. In <ApiLink to="playwright-crawler/class/AdaptivePlaywrightCrawler">`AdaptivePlaywrightCrawler`</ApiLink> they run once per request handler attempt, so a write there can land twice for one request — push your results from the request handler instead.

### Reacting to a commit

A write made inside a transaction cannot fail where it is made — `pushData()` only records it, and the failure, if any, happens at commit time. `afterStorageCommit()` is where you get it back: the callback runs once the request's writes have been committed, or once committing them has failed, and receives the error in the latter case.

```typescript
async function requestHandler({ pushData, afterStorageCommit, useState }) {
const state = await useState({ itemCount: 0 });

await pushData(item);
afterStorageCommit((error) => {
if (error) {
throw new NonRetryableError('The page is too big to store', { cause: error });
}
state.itemCount++;
});
}
```

Both halves of that callback need the commit. Incrementing the counter in the handler would over-count, because `useState()` is not transactional: a request that pushes an item and then fails keeps the increment but loses the item. And a commit failure left alone is just an ordinary request error — retried to exhaustion, with nothing tying it to the write that caused it.

Callbacks run in registration order, after the commit and before the request is marked as handled. The first one to throw propagates and the rest do not run; on a failed commit its error replaces the commit error, which is how the example above decides what the request fails with. They are *not* run when the request handler itself fails — nothing was written, and that is what `errorHandler` / `failedRequestHandler` are for.

In <ApiLink to="playwright-crawler/class/AdaptivePlaywrightCrawler">`AdaptivePlaywrightCrawler`</ApiLink> a callback belongs to the request handler attempt that registered it, so it runs only for the attempt whose writes are the ones being committed. Under `transactionalStorage: false` there is no commit to react to — writes are applied as they are made and throw at the call site on their own — and `afterStorageCommit()` throws.

### Escape hatches

The feature is escapable at three granularities:
Expand Down
2 changes: 2 additions & 0 deletions docs/public-api/crawlee-core.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ export { CrawleeLoggerOptions }

// @public (undocumented)
export interface CrawlingContext<UserData extends Dictionary = Dictionary> extends RestrictedCrawlingContext<UserData> {
afterStorageCommit(callback: (error?: Error) => Awaitable<void>): void;
extendTimeout(secs: number): void;
registerDeferredCleanup(cleanup: () => Promise<unknown>): void;
sendRequest: (requestOverrides?: Partial<HttpRequestOptions>, optionsOverrides?: SendRequestOptions) => Promise<Response>;
Expand Down Expand Up @@ -2005,6 +2006,7 @@ export class StorageStatsTracker<T extends Record<keyof T, number>> {

// @public
export class StorageTransaction implements StorageTransactionView {
afterCommit(callback: (error?: Error) => Awaitable<void>): void;
commit(): Promise<void>;
// (undocumented)
get datasetItems(): {
Expand Down
5 changes: 3 additions & 2 deletions docs/upgrading/upgrading_v4.md
Original file line number Diff line number Diff line change
Expand Up @@ -671,9 +671,10 @@ Every crawler now wraps each request in a **storage transaction** (see the [Tran
The observable behavior of a *successful* handler is unchanged (reads within a handler see its own writes), but several things differ on the failure path and around handler boundaries:

- **Uncommitted writes are invisible to other handlers.** Using the key-value store as a live channel between concurrently running handlers no longer works — one handler's `setValue()` only becomes visible to others once its request succeeds. Use `useState()` for cross-handler communication.
- **`useState()` / `getAutoSavedValue()` are *not* transactional.** The shared state object stays live; mutations of it are not rolled back when a handler fails.
- **`useState()` / `getAutoSavedValue()` are *not* transactional.** The shared state object stays live; mutations of it are not rolled back when a handler fails. Side effects that have to match the writes that actually landed — result counters above all — belong in a callback registered with the new `afterStorageCommit()` context helper.
- **Request queue additions are applied immediately by default** (the `writeThrough` policy) and are not rolled back — deduplication by `uniqueKey` keeps retries idempotent. Pass `transactionalStorage: { requestQueue: 'deferred' }` for strict all-or-nothing enqueues.
- **Commit is at-least-once.** It spans multiple storages, so a commit that fails partway fails the request; the retry may re-apply writes that already landed.
- **A write cannot fail where it is made.** `pushData()` records the item and returns; if the storage backend rejects it, that happens at commit time, and a `try`/`catch` around the call never sees it. Register an `afterStorageCommit()` callback next to the write instead — it receives the commit error, and an error it throws replaces it, so a rejected write can still be turned into a `NonRetryableError` of your own.
- **`KeyValueStore.setValue()` with a stream value throws inside a request handler.** A stream can only be consumed once, so it cannot be buffered. Wrap the call in `withDirectStorageAccess()` to write it immediately:

```typescript
Expand All @@ -690,7 +691,7 @@ The mechanism can be disabled entirely with `transactionalStorage: false` on any
#### Removed symbols and options

- `checkStorageAccess` and `withCheckedStorageAccess` are superseded by the transaction mechanism; the per-call-site helper is now `withDirectStorageAccess()`.
- The experimental `AdaptivePlaywrightCrawler` no longer needs its bespoke write-buffering machinery: the `preventDirectStorageAccess` option is gone (direct storage calls are now captured by the per-attempt transaction instead of throwing), and `RequestHandlerResult` is replaced by the read-only `StorageTransactionView`, which the `resultChecker` / `resultComparator` callbacks (and `fullResultComparator`) now receive. The view keeps the familiar accessors (`datasetItems`, `enqueuedUrls`, `keyValueStoreChanges`), so most callbacks only need a type change. The `calls` and `enqueuedUrlLists` accessors are gone — `requestsFromUrl` sources are now expanded when added, so the fetched URLs appear in `enqueuedUrls` (and are what `fullResultComparator` compares).
- The experimental `AdaptivePlaywrightCrawler` no longer needs its bespoke write-buffering machinery: the `preventDirectStorageAccess` option is gone (direct storage calls are now captured by the per-attempt transaction instead of throwing), and `RequestHandlerResult` is replaced by the read-only `StorageTransactionView`, which the `resultChecker` / `resultComparator` callbacks (and `fullResultComparator`) now receive. The view keeps the familiar accessors (`datasetItems`, `enqueuedUrls`, `keyValueStoreChanges`), so most callbacks only need a type change. The `calls` and `enqueuedUrlLists` accessors are gone — `requestsFromUrl` sources are now expanded when added, so the fetched URLs appear in `enqueuedUrls` (and are what `fullResultComparator` compares). The `commitResult` override point is gone too; use an `afterStorageCommit()` callback to run logic once the winning attempt's writes have landed (or failed to).

### `storageObject` is removed from storage classes

Expand Down
13 changes: 13 additions & 0 deletions packages/basic-crawler/src/internals/basic-crawler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1472,6 +1472,19 @@ export class BasicCrawler<
registerDeferredCleanup: (cleanup: () => Promise<unknown>) => {
deferredCleanup.push(cleanup);
},
afterStorageCommit: (callback: (error?: Error) => Awaitable<void>) => {
const transaction = currentStorageTransaction();

if (transaction === undefined) {
throw new Error(
'afterStorageCommit() needs an active storage transaction, and there is none. ' +
"With `transactionalStorage: false` the request's writes are applied as they are " +
'made, so they throw at the call site instead of at commit time.',
);
}

transaction.afterCommit(callback);
},
extendTimeout: (secs: number) => {
const extraMillis = secs * 1000;

Expand Down
46 changes: 45 additions & 1 deletion packages/core/src/crawlers/crawler_commons.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import type { Dictionary, HttpRequestOptions, ISession, ProxyInfo, SendRequestOptions } from '@crawlee/types';
import type {
Awaitable,
Dictionary,
HttpRequestOptions,
ISession,
ProxyInfo,
SendRequestOptions,
} from '@crawlee/types';
import type { ReadonlyDeep } from 'type-fest';

import type { EnqueueUrlsOptions } from '../enqueue_links/enqueue_links.js';
Expand Down Expand Up @@ -183,6 +190,43 @@ export interface CrawlingContext<UserData extends Dictionary = Dictionary> exten
*/
registerDeferredCleanup(cleanup: () => Promise<unknown>): void;

/**
* Registers `callback` to run once the request's storage writes have been committed, or once
* committing them has failed - the point where a write made in the handler can finally be reacted
* to where it was made. Two things belong here: side effects that must agree with what actually
* landed (result counters, progress reporting), and handling of a write that did not land at all.
*
* ```ts
* async requestHandler({ pushData, afterStorageCommit, useState }) {
* const state = await useState({ itemCount: 0 });
*
* await pushData(item);
* afterStorageCommit((error) => {
* if (error) {
* throw new NonRetryableError('The page is too big to store', { cause: error });
* }
Comment on lines +205 to +207

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we throw a retriable error, does it mean we'll process the same request again? meaning

  1. run requestHandler
  2. commit dataset writes
  3. throw from afterStorageCommit
  4. (?) back to 1. and 2., creating duplicate dataset items?

Should we make every afterStorageCommit error non-retriable? Or maybe just document this better?

* state.itemCount++;
* });
* }
* ```
*
* `useState()` is deliberately not transactional, which is what makes the counter case necessary:
* incrementing in the handler would keep the increment for a request that pushes an item and then
* fails, while the item itself is rolled back.
*
* Callbacks run in registration order, after the commit and before the request is marked as handled.
* The first one to throw propagates and the rest do not run; on a failed commit its error replaces
* the commit error, so a callback can decide what the request fails with. Callbacks are *not* run
* when the request handler itself fails - nothing was written, and `errorHandler` /
* `failedRequestHandler` cover that.
*
* Throws when storage is not transactional (`transactionalStorage: false`) - writes are then applied
* as they are made and throw at the call site on their own. In {@apilink AdaptivePlaywrightCrawler}
* the callback belongs to the current request handler attempt, so it runs only for the attempt whose
* writes are the ones being committed.
*/
afterStorageCommit(callback: (error?: Error) => Awaitable<void>): void;

/**
* Gives the current request `secs` more seconds to finish, for when how long it needs is only apparent
* once it is already running - a listing page that turns out to have far more to scroll through than
Expand Down
46 changes: 41 additions & 5 deletions packages/core/src/storages/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ export class StorageTransaction implements StorageTransactionView {

#disposed = false;

/** Callbacks registered via `afterCommit()`, run in registration order once a commit has been attempted. */
readonly #afterCommitCallbacks: ((error?: Error) => Awaitable<void>)[] = [];

/** @internal */
constructor(options: StorageTransactionOptions = {}) {
this.policy = { ...DEFAULT_STORAGE_WRITE_POLICY, ...options.policy };
Expand Down Expand Up @@ -210,6 +213,20 @@ export class StorageTransaction implements StorageTransactionView {
this.journal.push(entry);
}

/**
* Registers a callback to run once a commit has been attempted, whether it succeeded (`error` is
* `undefined`) or failed - the only point at which a deferred write can be reacted to where it was
* made. Callbacks run in registration order; the first to throw propagates, the rest do not run, and
* on a failed commit its error replaces the commit error. Not run on rollback: nothing was written.
*/
afterCommit(callback: (error?: Error) => Awaitable<void>): void {
if (!this.isActive) {
throw new Error(`Cannot register a commit callback on a transaction in the '${this.#state}' state`);
}

this.#afterCommitCallbacks.push(callback);
}

/**
* Replays the journaled writes into real storage. A no-op unless the transaction is `open`.
*
Expand Down Expand Up @@ -237,11 +254,28 @@ export class StorageTransaction implements StorageTransactionView {
`Committing the storage transaction timed out after ${this.#commitTimeoutMillis / 1000} seconds.`,
),
);
this.#state = 'committed';
} catch (error) {
this.#state = 'failed';
throw error;
// Callbacks are handed an `Error`, so a backend that rejects with anything else gets wrapped.
const commitError = error instanceof Error ? error : new Error(String(error), { cause: error });

// A callback that throws here propagates instead: it was handed the failure and decided what
// the request should fail with.
await this.#runAfterCommitCallbacks(commitError);
throw commitError;
}

this.#state = 'committed';
await this.#runAfterCommitCallbacks();
}

/** Fresh timeout context for the same reason as the flush. */
async #runAfterCommitCallbacks(error?: Error): Promise<void> {
await timeoutStorage.exit(async () => {
for (const callback of this.#afterCommitCallbacks) {
await callback(error);
}
});
}

private async flush(): Promise<void> {
Expand Down Expand Up @@ -282,9 +316,10 @@ export class StorageTransaction implements StorageTransactionView {
}

/**
* Releases the journal and the write-time snapshots it holds. Must be called for *every* terminal
* state, `failed` included. Idempotent, never throws, and does not change `state`. Any
* {@apilink StorageTransactionView} of this transaction is only valid until this is called.
* Releases the journal, the write-time snapshots it holds and any registered commit callbacks. Must
* be called for *every* terminal state, `failed` included. Idempotent, never throws, and does not
* change `state`. Any {@apilink StorageTransactionView} of this transaction is only valid until this
* is called.
*/
dispose(): void {
if (this.#disposed) {
Expand All @@ -305,6 +340,7 @@ export class StorageTransaction implements StorageTransactionView {

this.#disposed = true;
this.journal.length = 0;
this.#afterCommitCallbacks.length = 0;
}

get datasetItems(): { item: Dictionary; datasetId: string }[] {
Expand Down
37 changes: 37 additions & 0 deletions test/core/crawlers/adaptive_playwright_crawler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,43 @@ describe('AdaptivePlaywrightCrawler', () => {
expect(await store.getValue('1')).toEqual({ content: 42 });
});

test('should run afterStorageCommit callbacks only for the committed attempt', async () => {
// Always detect: the browser attempt is committed, then a static attempt runs purely for the
// comparison and is discarded. So the handler runs twice for one request.
const renderingTypePredictor = makeRiggedRenderingTypePredictor({
detectionProbabilityRecommendation: 1,
renderingType: 'clientOnly',
});

let handlerRuns = 0;
const committed: number[] = [];

const requestHandler: AdaptivePlaywrightCrawlerOptions['requestHandler'] = async ({
pushData,
afterStorageCommit,
}) => {
const run = ++handlerRuns;
await pushData({ run });
afterStorageCommit(() => void committed.push(run));
};

const crawler = await makeOneshotCrawler(
{
requestHandler,
renderingTypePredictor,
maxRequestsPerCrawl: 1,
maxRequestRetries: 0,
},
[`http://${HOSTNAME}:${port}/static`],
);

await crawler.run();

expect(handlerRuns).toBe(2);
expect(committed).toEqual([1]);
expect((await Dataset.getData()).items).toEqual([{ run: 1 }]);
});

test('should persist RenderingTypePredictor state on PERSIST_STATE events', async () => {
const requestHandler: AdaptivePlaywrightCrawlerOptions['requestHandler'] = vi.fn(async ({ pushData }) => {
await pushData({ content: 'test data' });
Expand Down
Loading
Loading