Skip to content

Commit 0761b67

Browse files
authored
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 #2889
1 parent 7acf908 commit 0761b67

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

packages/core/src/storages/access_checking.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
import { AsyncLocalStorage } from 'node:async_hooks';
22

33
import type { Awaitable } from '../typedefs.js';
4+
import { tryCancel } from '@apify/timeout';
45

56
const storage = new AsyncLocalStorage<{ checkFunction: () => void }>();
67

78
/**
89
* Invoke a storage access checker function defined using {@link withCheckedStorageAccess} higher up in the call stack.
910
*/
10-
export const checkStorageAccess = () => storage.getStore()?.checkFunction();
11+
export const checkStorageAccess = () => {
12+
tryCancel();
13+
return storage.getStore()?.checkFunction();
14+
};
1115

1216
/**
1317
* Define a storage access checker function that should be used by calls to {@link checkStorageAccess} in the callbacks.

test/core/crawlers/basic_crawler.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
serviceLocator,
2020
SessionPool,
2121
} from '@crawlee/basic';
22-
import { RequestState } from '@crawlee/core';
22+
import { Dataset, RequestState } from '@crawlee/core';
2323
import { MemoryStorageClient } from '@crawlee/memory-storage';
2424
import type { ISession, ProxyInfo } from '@crawlee/types';
2525
import type { Dictionary } from '@crawlee/utils';
@@ -1287,6 +1287,34 @@ describe('BasicCrawler', () => {
12871287
results[0].errorMessages.forEach((msg) => expect(msg).toMatch('requestHandler timed out'));
12881288
});
12891289

1290+
test('timeouted request should not access storages', async () => {
1291+
const url = 'https://example.com';
1292+
const requestList = await RequestList.open({ sources: [{ url }] });
1293+
1294+
const results: Request[] = [];
1295+
const crawler = new BasicCrawler({
1296+
requestList,
1297+
requestHandlerTimeoutSecs: 0.01,
1298+
maxRequestRetries: 0,
1299+
requestHandler: async ({ pushData }) => {
1300+
await sleep(10);
1301+
await pushData({ foo: 'bar' });
1302+
},
1303+
failedRequestHandler: async ({ request }) => {
1304+
results.push(request);
1305+
await sleep(100);
1306+
},
1307+
});
1308+
1309+
await crawler.run();
1310+
expect(results).toHaveLength(1);
1311+
expect(results[0].url).toEqual(url);
1312+
results[0].errorMessages.forEach((msg) => expect(msg).toMatch('requestHandler timed out'));
1313+
1314+
const dataset = await crawler.getDataset();
1315+
expect((await dataset.getInfo()).itemCount).toBe(0);
1316+
});
1317+
12901318
test('limits requestHandlerTimeoutSecs and derived vars to a valid value', async () => {
12911319
const url = 'https://example.com';
12921320
const requestList = await RequestList.open({ sources: [{ url }] });

0 commit comments

Comments
 (0)