fix: disable storage access on requestHandler timeouts (#3788)
Due to the nature of the concurrency model in JS, we cannot reliably abort the `requestHandler` execution on timeouts. This can lead to "race conditions" with, e.g., the `requestHandler` modifying the storages while the `errorHandler` / `failedRequestHandler` is already running. This PR adds a `tryCancel()` check throwing on elapsed timeouts before each storage method call. Closes https://github.com/apify/crawlee/issues/2889
This commit is contained in:
committed by
Martin Adámek
parent
1dc8812a4a
commit
4f0c4c452d
@@ -1,13 +1,17 @@
|
||||
import { AsyncLocalStorage } from 'node:async_hooks';
|
||||
|
||||
import type { Awaitable } from '../typedefs.js';
|
||||
import { tryCancel } from '@apify/timeout';
|
||||
|
||||
const storage = new AsyncLocalStorage<{ checkFunction: () => void }>();
|
||||
|
||||
/**
|
||||
* Invoke a storage access checker function defined using {@link withCheckedStorageAccess} higher up in the call stack.
|
||||
*/
|
||||
export const checkStorageAccess = () => storage.getStore()?.checkFunction();
|
||||
export const checkStorageAccess = () => {
|
||||
tryCancel();
|
||||
return storage.getStore()?.checkFunction();
|
||||
};
|
||||
|
||||
/**
|
||||
* Define a storage access checker function that should be used by calls to {@link checkStorageAccess} in the callbacks.
|
||||
|
||||
@@ -22,7 +22,7 @@ import {
|
||||
serviceLocator,
|
||||
SessionPool,
|
||||
} from '@crawlee/basic';
|
||||
import { RequestState, SessionPool, Statistics } from '@crawlee/core';
|
||||
import { Dataset, RequestState, SessionPool, Statistics } from '@crawlee/core';
|
||||
import { MemoryStorageClient } from '@crawlee/memory-storage';
|
||||
import type { ISession, ProxyInfo } from '@crawlee/types';
|
||||
import type { Dictionary } from '@crawlee/utils';
|
||||
@@ -1313,6 +1313,34 @@ describe('BasicCrawler', () => {
|
||||
results[0].errorMessages.forEach((msg) => expect(msg).toMatch('requestHandler timed out'));
|
||||
});
|
||||
|
||||
test('timeouted request should not access storages', async () => {
|
||||
const url = 'https://example.com';
|
||||
const requestList = await RequestList.open({ sources: [{ url }] });
|
||||
|
||||
const results: Request[] = [];
|
||||
const crawler = new BasicCrawler({
|
||||
requestList,
|
||||
requestHandlerTimeoutSecs: 0.01,
|
||||
maxRequestRetries: 0,
|
||||
requestHandler: async ({ pushData }) => {
|
||||
await sleep(10);
|
||||
await pushData({ foo: 'bar' });
|
||||
},
|
||||
failedRequestHandler: async ({ request }) => {
|
||||
results.push(request);
|
||||
await sleep(100);
|
||||
},
|
||||
});
|
||||
|
||||
await crawler.run();
|
||||
expect(results).toHaveLength(1);
|
||||
expect(results[0].url).toEqual(url);
|
||||
results[0].errorMessages.forEach((msg) => expect(msg).toMatch('requestHandler timed out'));
|
||||
|
||||
const dataset = await crawler.getDataset();
|
||||
expect((await dataset.getInfo()).itemCount).toBe(0);
|
||||
});
|
||||
|
||||
test('limits requestHandlerTimeoutSecs and derived vars to a valid value', async () => {
|
||||
const url = 'https://example.com';
|
||||
const requestList = await RequestList.open({ sources: [{ url }] });
|
||||
|
||||
Reference in New Issue
Block a user