From 73ee2b87dbda28ff694683b98058d5213009e8b7 Mon Sep 17 00:00:00 2001 From: Jan Buchar Date: Fri, 21 Aug 2026 14:28:59 +0200 Subject: [PATCH 1/2] fix: Deflake tests under high concurrency (#4059) --- packages/core/src/session_pool/session.ts | 9 ++-- packages/utils/test/robots.test.ts | 35 +++++++++------- test/core/crawlers/browser_crawler.test.ts | 6 +++ test/core/crawlers/playwright_crawler.test.ts | 9 ++-- test/core/crawlers/puppeteer_crawler.test.ts | 9 ++-- test/core/sitemap_request_loader.test.ts | 42 ++++++++++++++----- 6 files changed, 73 insertions(+), 37 deletions(-) diff --git a/packages/core/src/session_pool/session.ts b/packages/core/src/session_pool/session.ts index bdf95887c425..e37e048ac6dd 100644 --- a/packages/core/src/session_pool/session.ts +++ b/packages/core/src/session_pool/session.ts @@ -4,7 +4,6 @@ import { z } from 'zod'; import { cryptoRandomObjectId } from '@apify/utilities'; -import { getDefaultCookieExpirationDate } from '../cookie_utils.js'; import type { CrawleeLogger } from '../log.js'; import { serviceLocator } from '../service_locator.js'; import { parseArgument, schemas, validators } from '../validators.js'; @@ -62,7 +61,10 @@ export interface SessionOptions { /** Date of creation. */ createdAt?: Date; - /** Date of expiration. */ + /** + * Date of expiration. + * @default createdAt + maxAgeSecs + */ expiresAt?: Date; /** @@ -191,7 +193,8 @@ export class Session implements ISession { retired, log, fingerprint, - expiresAt = getDefaultCookieExpirationDate(maxAgeSecs), + // Anchored to `createdAt` rather than to "now", so the documented `createdAt + maxAgeSecs` holds. + expiresAt = new Date(createdAt.getTime() + maxAgeSecs * 1000), } = parseArgument(options, sessionOptionsSchema); this.#log = log.child({ prefix: 'Session' }); diff --git a/packages/utils/test/robots.test.ts b/packages/utils/test/robots.test.ts index b8f253c5f936..789be7f3357b 100644 --- a/packages/utils/test/robots.test.ts +++ b/packages/utils/test/robots.test.ts @@ -6,6 +6,9 @@ import { RobotsTxtFile } from '../src/internals/robots.js'; const httpClient = new FetchHttpClient(); +// Never replies within the lifetime of a test, so only the timeout/abort can settle the promise. +const neverReplies = 'http://never-replies.com'; + describe('RobotsTxtFile', () => { beforeEach(() => { nock.disableNetConnect(); @@ -33,6 +36,8 @@ describe('RobotsTxtFile', () => { ) .get('*') .reply(404); + + nock(neverReplies).persist().get('/robots.txt').delay(30_000).reply(200, 'User-agent: *'); }); afterEach(() => { @@ -63,44 +68,44 @@ describe('RobotsTxtFile', () => { }); it('respects user-set timeout', async () => { - const start = +Date.now(); - const robots = RobotsTxtFile.find('http://not-exists.com/robots.txt', { timeoutMillis: 200 }); + const start = Date.now(); + const robots = RobotsTxtFile.find(`${neverReplies}/robots.txt`, { timeoutMillis: 200 }); await expect(robots).rejects.toThrow(/timeout/i); - const end = +Date.now(); - expect(end - start).toBeGreaterThanOrEqual(200); - expect(end - start).toBeLessThanOrEqual(500); + const elapsed = Date.now() - start; + expect(elapsed).toBeGreaterThanOrEqual(200); + expect(elapsed).toBeLessThan(10_000); }); it('respects AbortSignal parameter', async () => { const controller = new AbortController(); setTimeout(() => controller.abort(), 200); - const start = +Date.now(); - const robots = RobotsTxtFile.find('http://not-exists.com/robots.txt', { signal: controller.signal }); + const start = Date.now(); + const robots = RobotsTxtFile.find(`${neverReplies}/robots.txt`, { signal: controller.signal }); await expect(robots).rejects.toThrow(/aborted/i); - const end = +Date.now(); - expect(end - start).toBeGreaterThanOrEqual(200); - expect(end - start).toBeLessThanOrEqual(500); + const elapsed = Date.now() - start; + expect(elapsed).toBeGreaterThanOrEqual(200); + expect(elapsed).toBeLessThan(10_000); }); it('respects AbortSignal parameter and timeout together', async () => { const controller = new AbortController(); - const start = +Date.now(); - const robots = RobotsTxtFile.find('http://not-exists.com/robots.txt', { + const start = Date.now(); + const robots = RobotsTxtFile.find(`${neverReplies}/robots.txt`, { signal: controller.signal, timeoutMillis: 200, }); await expect(robots).rejects.toThrow(/timeout/i); - const end = +Date.now(); - expect(end - start).toBeGreaterThanOrEqual(200); - expect(end - start).toBeLessThanOrEqual(500); + const elapsed = Date.now() - start; + expect(elapsed).toBeGreaterThanOrEqual(200); + expect(elapsed).toBeLessThan(10_000); }); it('drops cross-host and non-http(s) sitemap directives under the default same-hostname strategy', () => { diff --git a/test/core/crawlers/browser_crawler.test.ts b/test/core/crawlers/browser_crawler.test.ts index 3445eac20125..58b727ce0f94 100644 --- a/test/core/crawlers/browser_crawler.test.ts +++ b/test/core/crawlers/browser_crawler.test.ts @@ -692,6 +692,9 @@ describe('BrowserCrawler', () => { }, requestList, saveResponseCookies: true, + // The handoff only happens between requests: cookies set in a handler are flushed to the + // session jar after it returns, so cookie-2 must not start before cookie-1 has finished. + maxConcurrency: 1, sessionPool: new SessionPool({ maxPoolSize: 1, }), @@ -921,6 +924,9 @@ describe('BrowserCrawler', () => { sessionPool: new SessionPool({ maxPoolSize: 1, }), + // A strictly serial [0..5] is only well-defined one request at a time: two handlers running + // together read the same `usageCount` before either marks the session good. + maxConcurrency: 1, requestHandler: async ({ session }) => { sessionUsageHistory.push((session as Session).usageCount); }, diff --git a/test/core/crawlers/playwright_crawler.test.ts b/test/core/crawlers/playwright_crawler.test.ts index 0e9ab74919d4..d22d02cf17b1 100644 --- a/test/core/crawlers/playwright_crawler.test.ts +++ b/test/core/crawlers/playwright_crawler.test.ts @@ -2,7 +2,7 @@ import type { Server } from 'node:http'; import type { AddressInfo } from 'node:net'; import os from 'node:os'; -import type { PlaywrightCrawlingContext, PlaywrightGotoOptions, Request } from '@crawlee/playwright'; +import type { PlaywrightCrawlingContext, Request } from '@crawlee/playwright'; import { type ConcurrencySystem, MemoryStorageBackend, serviceLocator } from '@crawlee/core'; import { createPlaywrightRouter, @@ -169,7 +169,8 @@ describe('PlaywrightCrawler', () => { test('should override goto timeout with navigationTimeoutSecs', async () => { const timeoutSecs = 10; - let options: PlaywrightGotoOptions; + // Captured by value: `navigate()` narrows the live `gotoOptions` down to the remaining navigation window. + let gotoTimeout: number | undefined; const playwrightCrawler = new PlaywrightCrawler({ requestList, maxRequestRetries: 0, @@ -177,14 +178,14 @@ describe('PlaywrightCrawler', () => { requestHandler: () => {}, preNavigationHooks: [ ({ gotoOptions }) => { - options = gotoOptions; + gotoTimeout = gotoOptions.timeout; }, ], navigationTimeoutSecs: timeoutSecs, }); await playwrightCrawler.run(); - expect(options!.timeout).toEqual(timeoutSecs * 1000); + expect(gotoTimeout).toEqual(timeoutSecs * 1000); }); test('does not mutate the launchContext it was given', () => { diff --git a/test/core/crawlers/puppeteer_crawler.test.ts b/test/core/crawlers/puppeteer_crawler.test.ts index 293a35b83c4f..3ddea62d16f2 100644 --- a/test/core/crawlers/puppeteer_crawler.test.ts +++ b/test/core/crawlers/puppeteer_crawler.test.ts @@ -5,7 +5,7 @@ import type { AddressInfo } from 'node:net'; import os from 'node:os'; import { promisify } from 'node:util'; -import type { PuppeteerCrawlingContext, PuppeteerGoToOptions, Request } from '@crawlee/puppeteer'; +import type { PuppeteerCrawlingContext, Request } from '@crawlee/puppeteer'; import type { Cheerio, CheerioAPI } from 'cheerio'; import type { Element } from 'domhandler'; import { @@ -137,7 +137,8 @@ describe('PuppeteerCrawler', () => { test('should override goto timeout with navigationTimeoutSecs', async () => { const timeoutSecs = 10; - let options: PuppeteerGoToOptions; + // Captured by value: `navigate()` narrows the live `gotoOptions` down to the remaining navigation window. + let gotoTimeout: number | undefined; const puppeteerCrawler = new PuppeteerCrawler({ requestList, maxRequestRetries: 0, @@ -145,14 +146,14 @@ describe('PuppeteerCrawler', () => { requestHandler: () => {}, preNavigationHooks: [ ({ gotoOptions }) => { - options = gotoOptions; + gotoTimeout = gotoOptions.timeout; }, ], navigationTimeoutSecs: timeoutSecs, }); await puppeteerCrawler.run(); - expect(options!.timeout).toEqual(timeoutSecs * 1000); + expect(gotoTimeout).toEqual(timeoutSecs * 1000); }); test('should throw if launchOptions.proxyUrl is supplied', async () => { diff --git a/test/core/sitemap_request_loader.test.ts b/test/core/sitemap_request_loader.test.ts index 019af1ba4225..e14a8cd353b5 100644 --- a/test/core/sitemap_request_loader.test.ts +++ b/test/core/sitemap_request_loader.test.ts @@ -148,7 +148,11 @@ beforeAll(async () => { res.end(); }); + // `?linger=` (default 200) holds the response open after the two URL entries, so a test can act + // while this sub-sitemap is still loading. app.get('/sitemap-stream-linger.xml', async (req, res) => { + const lingerMillis = Number(req.query.linger ?? 200); + async function* stream() { yield [ '', @@ -161,7 +165,7 @@ beforeAll(async () => { '', ].join('\n'); - await sleep(200); + await sleep(lingerMillis); yield ''; } @@ -173,14 +177,17 @@ beforeAll(async () => { res.end(); }); + // Index over the lingering sub-sitemap (2 URLs) followed by a plain one (5 URLs). app.get('/sitemap-index.xml', async (req, res) => { + const linger = req.query.linger === undefined ? '' : `?linger=${Number(req.query.linger)}`; + res.setHeader('content-type', 'text/xml'); res.write( [ '', '', '', - `${url}/sitemap-stream-linger.xml`, + `${url}/sitemap-stream-linger.xml${linger}`, '', '', `${url}/sitemap.xml`, @@ -444,12 +451,17 @@ describe('SitemapRequestLoader', () => { const controller = new AbortController(); const list = await SitemapRequestLoader.open({ - sitemapUrls: [`${url}/sitemap-index.xml`], + // The first sub-sitemap stays open for 5s, so the abort below always lands mid-index. + sitemapUrls: [`${url}/sitemap-index.xml?linger=5000`], signal: controller.signal, enqueueStrategy: 'all', }); - await sleep(50); // Loads the first sub-sitemap, but not the second + // Abort while the first sub-sitemap is still streaming - waiting for its URLs rather than for a fixed duration. + while (await list.isEmpty()) { + await sleep(10); + } + controller.abort(); for await (const request of list) { @@ -463,8 +475,10 @@ describe('SitemapRequestLoader', () => { test('timeout option works', async () => { const list = await SitemapRequestLoader.open({ - sitemapUrls: [`${url}/sitemap-index.xml`], - timeoutMillis: 50, // Loads the first sub-sitemap, but not the second + // The timeout has to fire after the first sub-sitemap streamed its URLs but before it closes; + // the test then runs for the whole linger, since the abort cannot interrupt an in-flight fetch. + sitemapUrls: [`${url}/sitemap-index.xml?linger=2000`], + timeoutMillis: 500, enqueueStrategy: 'all', }); @@ -479,21 +493,27 @@ describe('SitemapRequestLoader', () => { test('resurrection does not resume aborted loading', async () => { const options = { - sitemapUrls: [`${url}/sitemap-index.xml`], + sitemapUrls: [`${url}/sitemap-index.xml?linger=5000`], persistStateKey: 'resurrection-abort', - timeoutMillis: 50, enqueueStrategy: 'all' as const, }; { - const list = await SitemapRequestLoader.open(options); + const controller = new AbortController(); + const list = await SitemapRequestLoader.open({ ...options, signal: controller.signal }); - await sleep(50); + // Abort while the first sub-sitemap is still streaming, so the state is persisted with the + // load aborted half-way through the index. + while (await list.isEmpty()) { + await sleep(10); + } - await expect(list.isEmpty()).resolves.toBe(false); + controller.abort(); await list.persistState(); } + // Deliberately no signal and no timeout here: only the restored abort flag can stop the second + // sub-sitemap from being fetched. const newList = await SitemapRequestLoader.open(options); for await (const request of newList) { await newList.markRequestAsHandled(request); From 69a824b77a2ce5ed48ef6bca5a0a54d5ce504ac4 Mon Sep 17 00:00:00 2001 From: Harry Date: Fri, 21 Aug 2026 15:57:50 +0200 Subject: [PATCH 2/2] refactor(core): retire TS-private test seams and convert to native private fields (#4043) Co-authored-by: Jan Buchar --- docs/public-api/crawlee-basic.api.md | 4 +- docs/public-api/crawlee-browser-pool.api.md | 1 + docs/public-api/crawlee-core.api.md | 7 + oxlint.config.ts | 3 +- .../src/internals/basic-crawler.ts | 37 ++-- .../abstract-classes/browser-controller.ts | 12 +- packages/browser-pool/src/browser-pool.ts | 3 +- .../core/src/autoscaling/autoscaled_pool.ts | 19 ++- .../src/autoscaling/concurrency_system.ts | 17 +- packages/core/src/autoscaling/load_signal.ts | 8 +- packages/core/src/crawlers/statistics.ts | 5 +- packages/core/src/log.ts | 31 ++-- .../core/src/session_pool/session_pool.ts | 67 ++++---- packages/core/src/storages/dataset.ts | 2 + packages/core/src/storages/key_value_store.ts | 3 +- packages/core/src/storages/request_list.ts | 19 +-- packages/core/src/storages/request_queue.ts | 30 ++-- .../test/request-queue/request-queue.test.ts | 40 ++++- .../internals/adaptive-playwright-crawler.ts | 10 +- .../utils/rendering-type-prediction.ts | 23 ++- test/core/autoscaling/autoscaled_pool.test.ts | 42 +++-- .../autoscaling/concurrency_system.test.ts | 14 +- .../adaptive_playwright_crawler.test.ts | 26 ++- test/core/crawlers/basic_crawler.test.ts | 29 ++-- test/core/crawlers/browser_crawler.test.ts | 24 +-- test/core/crawlers/cheerio_crawler.test.ts | 25 +-- .../crawlers/rendering_type_predictor.test.ts | 3 +- test/core/crawlers/statistics.test.ts | 9 - test/core/request_list.test.ts | 2 - test/core/session_pool/session_pool.test.ts | 161 +++++++----------- test/core/storages/key_value_store.test.ts | 61 ++----- test/core/storages/request_queue.test.ts | 11 +- 32 files changed, 364 insertions(+), 384 deletions(-) diff --git a/docs/public-api/crawlee-basic.api.md b/docs/public-api/crawlee-basic.api.md index 77f5c72605ec..466ff3cac52b 100644 --- a/docs/public-api/crawlee-basic.api.md +++ b/docs/public-api/crawlee-basic.api.md @@ -41,7 +41,7 @@ import type { StatisticState } from '@crawlee/core'; import type { StorageBackend } from '@crawlee/types'; import type { StorageIdentifier } from '@crawlee/core'; import { StorageWritePolicy } from '@crawlee/core'; -import type { TaskLoopPredicates } from '@crawlee/core'; +import type { TaskLoopOptions } from '@crawlee/core'; import { TimeoutError } from '@apify/timeout'; import type { TypedRequestsLike } from '@crawlee/core'; @@ -151,7 +151,7 @@ export interface BasicCrawlerOptions; } diff --git a/docs/public-api/crawlee-browser-pool.api.md b/docs/public-api/crawlee-browser-pool.api.md index 3e2d4202f5b8..8c42792af2bd 100644 --- a/docs/public-api/crawlee-browser-pool.api.md +++ b/docs/public-api/crawlee-browser-pool.api.md @@ -88,6 +88,7 @@ export abstract class BrowserController; // (undocumented) totalPages: number; + waitForActive(): Promise; } // @public (undocumented) diff --git a/docs/public-api/crawlee-core.api.md b/docs/public-api/crawlee-core.api.md index 8bfa66782df5..e0ed42a631d4 100644 --- a/docs/public-api/crawlee-core.api.md +++ b/docs/public-api/crawlee-core.api.md @@ -812,6 +812,8 @@ export interface KeyConsumer { export class KeyValueStore { [Symbol.asyncIterator](): AsyncGenerator<[string, T], void, undefined>; // (undocumented) + readonly backend: KeyValueStoreBackend; + // (undocumented) readonly configuration: Configuration; drop(): Promise; entries(options?: KeyValueStoreIteratorOptions): AsyncIterable<[string, T]> & Promise<[string, T][]>; @@ -2063,6 +2065,11 @@ export interface SystemInfo { storageBackendInfo: LoadSignalInfo; } +// @public (undocumented) +export interface TaskLoopOptions extends TaskLoopPredicates { + maybeRunIntervalSecs?: number; +} + // @public export interface TaskLoopPredicates { isFinishedFunction?: () => Promise; diff --git a/oxlint.config.ts b/oxlint.config.ts index 34dca963321b..e894a1ee8091 100644 --- a/oxlint.config.ts +++ b/oxlint.config.ts @@ -36,7 +36,7 @@ export default defineConfig({ // Enforces the naming convention from #3108: no `_`-prefixed members; private // properties use native `#` fields instead. The allow list covers the template-method // hooks that collide with their public wrappers, platform contracts (`Readable._read`), - // and documented internals (`__crawlee`, `_currentConcurrency`). + // and documented internals (`__crawlee`, `__purged`). 'no-underscore-dangle': [ 'error', { @@ -50,7 +50,6 @@ export default defineConfig({ '_getCookies', '_setCookies', '_read', - '_currentConcurrency', '__crawlee', '__purged', '__originalHistory__', diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index 7e9276cc5b5c..f9474283cbd7 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -27,7 +27,7 @@ import type { StatisticState, StorageIdentifier, StorageWritePolicy, - TaskLoopPredicates, + TaskLoopOptions, TypedRequestsLike, UrlPatternObject, } from '@crawlee/core'; @@ -101,7 +101,7 @@ import type { ReadonlyDeep } from 'type-fest'; import { z } from 'zod'; import { LruCache } from '@apify/datastructures'; -import { addTimeoutToPromise, extendTimeout, TimeoutError } from '@apify/timeout'; +import { addTimeoutToPromise, extendTimeout, TimeoutError, tryCancel } from '@apify/timeout'; import { cryptoRandomObjectId } from '@apify/utilities'; import { @@ -380,7 +380,7 @@ export interface BasicCrawlerOptions< * Concurrency is configured elsewhere — through the `minConcurrency`/`maxConcurrency`/`maxRequestsPerMinute` * shortcuts, or a {@apilink BasicCrawlerOptions.concurrencySystem|`concurrencySystem`} for finer control. */ - taskLoopOptions?: TaskLoopPredicates; + taskLoopOptions?: TaskLoopOptions; /** * A pre-configured concurrency governor — the component that decides whether there is free compute for one more @@ -847,8 +847,7 @@ export class BasicCrawler< protected readonly requestHandler!: RequestHandler; readonly #errorHandler?: ErrorHandler; readonly #failedRequestHandler?: ErrorHandler; - // kept as TS-private: tests read it at runtime - private requestHandlerTimeoutMillis!: number; + #requestHandlerTimeoutMillis!: number; protected readonly internalTimeoutMillis: number; readonly #maxRequestRetries: number; readonly #maxCrawlDepth?: number; @@ -870,8 +869,7 @@ export class BasicCrawler< * {@apilink ConcurrencySystem} instead, and the loop's `consumer` identity is the crawler's own, so neither is * settable here. */ - // kept as TS-private: tests mutate it at runtime - private taskLoopOptions: Omit; + #taskLoopOptions: Omit; protected readonly httpClient: BaseHttpClient; protected readonly retryOnBlocked: boolean; #respectRobotsTxtFile: boolean | { userAgent?: string }; @@ -1106,9 +1104,9 @@ export class BasicCrawler< this.#errorHandler = errorHandler; if (requestHandlerTimeoutSecs) { - this.requestHandlerTimeoutMillis = requestHandlerTimeoutSecs * 1000; + this.#requestHandlerTimeoutMillis = requestHandlerTimeoutSecs * 1000; } else { - this.requestHandlerTimeoutMillis = 60_000; + this.#requestHandlerTimeoutMillis = 60_000; } this.retryOnBlocked = retryOnBlocked; @@ -1125,7 +1123,7 @@ export class BasicCrawler< // allow at least 5min for internal timeouts this.internalTimeoutMillis = serviceLocator.getConfiguration().internalTimeoutMillis ?? - Math.max(this.requestHandlerTimeoutMillis * 2, 300e3); + Math.max(this.#requestHandlerTimeoutMillis * 2, 300e3); this.#maxRequestRetries = maxRequestRetries; this.#maxCrawlDepth = maxCrawlDepth; @@ -1171,13 +1169,13 @@ export class BasicCrawler< this.blockedStatusCodes = new Set(blockedStatusCodesInput ?? BLOCKED_STATUS_CODES); const maxSignedInteger = 2 ** 31 - 1; - if (this.requestHandlerTimeoutMillis > maxSignedInteger) { + if (this.#requestHandlerTimeoutMillis > maxSignedInteger) { this.log.warning( - `requestHandlerTimeoutMillis ${this.requestHandlerTimeoutMillis}` + + `requestHandlerTimeoutMillis ${this.#requestHandlerTimeoutMillis}` + ` does not fit a signed 32-bit integer. Limiting the value to ${maxSignedInteger}`, ); - this.requestHandlerTimeoutMillis = maxSignedInteger; + this.#requestHandlerTimeoutMillis = maxSignedInteger; } this.internalTimeoutMillis = Math.min(this.internalTimeoutMillis, maxSignedInteger); @@ -1195,7 +1193,9 @@ export class BasicCrawler< isFinishedFunction = async () => false; } - const crawlerOwnedTaskLoopConfiguration: Partial = { + const crawlerOwnedTaskLoopConfiguration: Partial< + Omit + > = { runTaskFunction: async () => { const source = this.requestManager; if (!source) throw new Error('Request provider is not initialized!'); @@ -1340,7 +1340,7 @@ export class BasicCrawler< log: this.log, }; - this.taskLoopOptions = { ...taskLoopOptions, ...crawlerOwnedTaskLoopConfiguration }; + this.#taskLoopOptions = { ...taskLoopOptions, ...crawlerOwnedTaskLoopConfiguration }; this.#resolveConcurrencySystem = () => OwnedOrInjected.resolve(concurrencySystem, () => @@ -1899,7 +1899,7 @@ export class BasicCrawler< // which routes a run will hit, so reserve for the longest one any route asked for. The hint is // raise-only, so erring high here is safe. const maxRouteTimeoutSecs = (this.requestHandler as Partial).getMaxTimeoutSecs?.() ?? 0; - const handlerTimeoutSecs = Math.max(this.requestHandlerTimeoutMillis / 1000, maxRouteTimeoutSecs); + const handlerTimeoutSecs = Math.max(this.#requestHandlerTimeoutMillis / 1000, maxRouteTimeoutSecs); await requestManager.setExpectedRequestProcessingTimeSecs?.(Math.max(handlerTimeoutSecs + 5, 60)); } @@ -2179,6 +2179,7 @@ export class BasicCrawler< data: Parameters[0], datasetIdentifier?: string | StorageIdentifier, ): Promise { + tryCancel(); const dataset = await this.getDataset(datasetIdentifier); return dataset.pushData(data); } @@ -2289,7 +2290,7 @@ export class BasicCrawler< await this.#concurrencySystemDep.ifOwned((system) => system.start()); this.#autoscaledPool = new AutoscaledPool({ - ...this.taskLoopOptions, + ...this.#taskLoopOptions, concurrencySystem: this.#concurrencySystemDep.value, consumer: this.#identity, }); @@ -2329,7 +2330,7 @@ export class BasicCrawler< */ private resolveRequestHandlerTimeoutMillis( label: string | undefined, - fallbackMillis = this.requestHandlerTimeoutMillis, + fallbackMillis = this.#requestHandlerTimeoutMillis, ): number { return this.getRouteTimeoutMillis(label) ?? fallbackMillis; } diff --git a/packages/browser-pool/src/abstract-classes/browser-controller.ts b/packages/browser-pool/src/abstract-classes/browser-controller.ts index ead1d0860309..1ee139520939 100644 --- a/packages/browser-pool/src/abstract-classes/browser-controller.ts +++ b/packages/browser-pool/src/abstract-classes/browser-controller.ts @@ -146,11 +146,17 @@ export abstract class BrowserController< #activate!: () => void; - // kept as TS-private: `BrowserPool` awaits it through cross-object bracket access - private isActivePromise = new Promise((resolve) => { + #isActivePromise = new Promise((resolve) => { this.#activate = resolve; }); + /** + * Returns a promise that resolves once the browser controller has been activated. + */ + async waitForActive(): Promise { + await this.#isActivePromise; + } + #commitBrowser!: () => void; #hasBrowserPromise = new Promise((resolve) => { @@ -238,7 +244,7 @@ export abstract class BrowserController< async newPage(pageOptions?: NewPageOptions): Promise { this.activePages++; this.totalPages++; - await this.isActivePromise; + await this.#isActivePromise; const page = await this._newPage(pageOptions); tryCancel(); this.lastPageOpenedAt = Date.now(); diff --git a/packages/browser-pool/src/browser-pool.ts b/packages/browser-pool/src/browser-pool.ts index ef992ef7fff9..b014b40fd5fa 100644 --- a/packages/browser-pool/src/browser-pool.ts +++ b/packages/browser-pool/src/browser-pool.ts @@ -599,8 +599,7 @@ export class BrowserPool< ) { // This is needed for concurrent newPage calls to wait for the browser launch. // It's not ideal though, we need to come up with a better API. - // eslint-disable-next-line dot-notation -- accessing private property - await browserController['isActivePromise']; + await browserController.waitForActive(); tryCancel(); const finalPageOptions = browserController.launchContext.useIncognitoPages ? pageOptions : undefined; diff --git a/packages/core/src/autoscaling/autoscaled_pool.ts b/packages/core/src/autoscaling/autoscaled_pool.ts index d3c3db009471..fea725cac107 100644 --- a/packages/core/src/autoscaling/autoscaled_pool.ts +++ b/packages/core/src/autoscaling/autoscaled_pool.ts @@ -26,7 +26,7 @@ const autoscaledPoolOptionsSchema = z.strictObject({ log: validators.logger.default(() => serviceLocator.getLogger()), concurrencySystem: schemas.anyObject, consumer: schemas.anyObject.refine( - (value) => typeof value.id === 'string' && value.id.length > 0, + (value) => typeof value?.id === 'string' && value.id.length > 0, "Expected an object with a non-empty string 'id'", ), }); @@ -53,8 +53,16 @@ export interface TaskLoopPredicates { isFinishedFunction?: () => Promise; } +export interface TaskLoopOptions extends TaskLoopPredicates { + /** + * How often the pool should check if a new task is ready, in seconds. + * @default 0.5 + */ + maybeRunIntervalSecs?: number; +} + /** @internal */ -export interface AutoscaledPoolOptions extends TaskLoopPredicates { +export interface AutoscaledPoolOptions extends TaskLoopOptions { /** * The governor that decides whether there is free compute for one more task. Typically a * {@apilink ConcurrencySystem}, but any {@apilink IConcurrencySystem} works. Share a single instance across @@ -77,13 +85,6 @@ export interface AutoscaledPoolOptions extends TaskLoopPredicates { */ runTaskFunction?: () => Promise; - /** - * Indicates how often the pool should call the `runTaskFunction()` to start a new task, in seconds. - * This has no effect on starting new tasks immediately after a task completes. - * @default 0.5 - */ - maybeRunIntervalSecs?: number; - /** * Timeout in which the `runTaskFunction` needs to finish, given in seconds. * @default 0 diff --git a/packages/core/src/autoscaling/concurrency_system.ts b/packages/core/src/autoscaling/concurrency_system.ts index 898b7b00a652..dff776eecbcb 100644 --- a/packages/core/src/autoscaling/concurrency_system.ts +++ b/packages/core/src/autoscaling/concurrency_system.ts @@ -222,8 +222,7 @@ export class ConcurrencySystem implements IConcurrencySystem { #minConcurrency: number; #maxConcurrency: number; #desiredConcurrency: number; - // kept as TS-private _-prefixed: autoscaled_pool tests write this backing field directly - private _currentConcurrency = 0; + #currentConcurrency = 0; #lastLoggingTime?: number; #tasksPerMinute: number[] = Array.from({ length: 60 }, () => 0); @@ -360,7 +359,7 @@ export class ConcurrencySystem implements IConcurrencySystem { } get currentConcurrency(): number { - return this._currentConcurrency; + return this.#currentConcurrency; } /** Whether the system is currently monitoring load and autoscaling the budget. */ @@ -466,14 +465,14 @@ export class ConcurrencySystem implements IConcurrencySystem { hasCapacityForTask(_consumer?: ConcurrencyConsumer): boolean { this.warnIfNotRunning(); - if (this._currentConcurrency >= this.#desiredConcurrency) { + if (this.#currentConcurrency >= this.#desiredConcurrency) { this.log.perf('Task will not run. Desired concurrency achieved.'); return false; } const currentStatus = this.systemStatus.getCurrentStatus(); const { isSystemIdle } = currentStatus; - if (!isSystemIdle && this._currentConcurrency >= this.#minConcurrency) { + if (!isSystemIdle && this.#currentConcurrency >= this.#minConcurrency) { this.log.perf( 'Task will not be run. System is overloaded.', currentStatus as unknown as Record, @@ -512,14 +511,14 @@ export class ConcurrencySystem implements IConcurrencySystem { return false; } - this._currentConcurrency++; + this.#currentConcurrency++; this.#tasksPerMinute[0]++; return true; } /** Returns a slot to the shared budget, whoever booked it. */ registerTaskEnd(_consumer?: ConcurrencyConsumer): void { - this._currentConcurrency--; + this.#currentConcurrency--; } /** @@ -542,7 +541,7 @@ export class ConcurrencySystem implements IConcurrencySystem { const { isSystemIdle } = systemStatus; const weAreNotAtMax = this.#desiredConcurrency < this.#maxConcurrency; const minCurrentConcurrency = Math.floor(this.#desiredConcurrency * this.desiredConcurrencyRatio); - const weAreReachingDesiredConcurrency = this._currentConcurrency >= minCurrentConcurrency; + const weAreReachingDesiredConcurrency = this.#currentConcurrency >= minCurrentConcurrency; if (isSystemIdle && weAreNotAtMax && weAreReachingDesiredConcurrency) this.scaleUp(systemStatus); @@ -559,7 +558,7 @@ export class ConcurrencySystem implements IConcurrencySystem { } else if (now > this.#lastLoggingTime + this.#loggingIntervalMillis) { this.#lastLoggingTime = now; this.log.info('state', { - currentConcurrency: this._currentConcurrency, + currentConcurrency: this.#currentConcurrency, desiredConcurrency: this.#desiredConcurrency, systemStatus, }); diff --git a/packages/core/src/autoscaling/load_signal.ts b/packages/core/src/autoscaling/load_signal.ts index 0bae4b2598d4..68617aa2fba7 100644 --- a/packages/core/src/autoscaling/load_signal.ts +++ b/packages/core/src/autoscaling/load_signal.ts @@ -71,9 +71,7 @@ export interface LoadSignal { export class SnapshotStore { #snapshots: T[] = []; - /** Retention window in milliseconds. Unbounded until {@apilink SnapshotStore.useSampleWindow|`useSampleWindow()`}. */ - // kept as TS-private: concurrency_system tests read this retention window directly - private historyMillis = Infinity; + #historyMillis = Infinity; /** * Sizes retention to the window the signal will be sampled over, as handed to it in @@ -81,7 +79,7 @@ export class SnapshotStore { * its start context grows unboundedly. */ useSampleWindow(maxSampleWindowMillis: number): void { - this.historyMillis = maxSampleWindowMillis; + this.#historyMillis = maxSampleWindowMillis; } /** @@ -92,7 +90,7 @@ export class SnapshotStore { let oldCount = 0; for (let i = 0; i < this.#snapshots.length; i++) { const { createdAt } = this.#snapshots[i]; - if (now.getTime() - new Date(createdAt).getTime() > this.historyMillis) oldCount++; + if (now.getTime() - new Date(createdAt).getTime() > this.#historyMillis) oldCount++; else break; } if (oldCount) this.#snapshots.splice(0, oldCount); diff --git a/packages/core/src/crawlers/statistics.ts b/packages/core/src/crawlers/statistics.ts index e18a0e4960c0..f44d9e14700a 100644 --- a/packages/core/src/crawlers/statistics.ts +++ b/packages/core/src/crawlers/statistics.ts @@ -279,8 +279,7 @@ export class Statistics< StateExtension extends object = {}, PersistedStateExtension extends object = StateExtension, > implements IStatistics { - // kept as TS-private: statistics tests read the static counter directly - private static id = 0; + static #id = 0; /** * An error tracker for final retry errors. @@ -342,7 +341,7 @@ export class Statistics< stateExtension, } = parseArgument(options, statisticsOptionsSchema); - this.id = id ?? String(Statistics.id++); + this.id = id ?? String(Statistics.#id++); this.#persistStateKey = `CRAWLEE_CRAWLER_STATISTICS_${this.id}`; this.log = (log ?? serviceLocator.getLogger()).child({ prefix: 'Statistics' }); diff --git a/packages/core/src/log.ts b/packages/core/src/log.ts index a3716801945f..e20763e2f8f1 100644 --- a/packages/core/src/log.ts +++ b/packages/core/src/log.ts @@ -36,13 +36,13 @@ export type { CrawleeLogger, CrawleeLoggerOptions }; * ``` */ export abstract class BaseCrawleeLogger implements CrawleeLogger { - // kept as TS-private: the adaptive crawler's log proxy calls non-intercepted methods with `this === proxy`, - // where `#`-field access would throw at runtime - private options: CrawleeLoggerOptions; - private readonly warningsLogged = new Set(); + // Note: If wrapping logger in a Proxy, unbound methods calling #-fields throw TypeError + // unless bound to the target (see createLogProxy in adaptive-playwright-crawler.ts). + #options: CrawleeLoggerOptions; + readonly #warningsLogged = new Set(); constructor(options: Partial = {}) { - this.options = options; + this.#options = options; } /** @@ -65,11 +65,11 @@ export abstract class BaseCrawleeLogger implements CrawleeLogger { protected abstract createChild(options: Partial): CrawleeLogger; getOptions(): CrawleeLoggerOptions { - return this.options; + return this.#options; } setOptions(options: Partial): void { - this.options = { ...this.options, ...options }; + this.#options = { ...this.#options, ...options }; } child(options: Partial): CrawleeLogger { @@ -97,8 +97,8 @@ export abstract class BaseCrawleeLogger implements CrawleeLogger { } warningOnce(message: string): void { - if (!this.warningsLogged.has(message)) { - this.warningsLogged.add(message); + if (!this.#warningsLogged.has(message)) { + this.#warningsLogged.add(message); this.warning(message); } } @@ -127,20 +127,19 @@ export abstract class BaseCrawleeLogger implements CrawleeLogger { * Users who want to use a different logging library should implement {@apilink BaseCrawleeLogger} directly. */ export class ApifyLogAdapter extends BaseCrawleeLogger { - constructor( - // kept as a TS-private parameter property: reached through the adaptive crawler's log proxy, see above - private readonly apifyLog: Log, - options?: Partial, - ) { + readonly #apifyLog: Log; + + constructor(apifyLog: Log, options?: Partial) { super(options ?? {}); + this.#apifyLog = apifyLog; } logWithLevel(level: number, message: string, data?: Record): void { - this.apifyLog.internal(level as LogLevel, message, data); + this.#apifyLog.internal(level as LogLevel, message, data); } protected createChild(options: Partial): CrawleeLogger { - return new ApifyLogAdapter(this.apifyLog.child({ prefix: options.prefix ?? null }), { + return new ApifyLogAdapter(this.#apifyLog.child({ prefix: options.prefix ?? null }), { ...this.getOptions(), ...options, }); diff --git a/packages/core/src/session_pool/session_pool.ts b/packages/core/src/session_pool/session_pool.ts index 7f7a712201ca..3d6972b6a657 100644 --- a/packages/core/src/session_pool/session_pool.ts +++ b/packages/core/src/session_pool/session_pool.ts @@ -154,14 +154,13 @@ export class SessionPool implements ISessionPool { readonly id: string; #log: CrawleeLogger; #sessions: Session[] = []; - // kept as TS-private: session_pool tests read/override the members below directly - private maxPoolSize: number; - private createSessionFunction: CreateSession; - private keyValueStore?: KeyValueStore; - private sessionMap = new Map(); - private sessionOptions: SessionOptions; - private persistStateKeyValueStoreId?: string; - private persistStateKey: string; + #maxPoolSize: number; + #createSessionFunction: CreateSession; + #keyValueStore?: KeyValueStore; + #sessionMap = new Map(); + #sessionOptions: SessionOptions; + #persistStateKeyValueStoreId?: string; + #persistStateKey: string; #listener?: () => Promise; #events: EventManager; #persistenceOptions: PersistenceOptions; @@ -191,20 +190,20 @@ export class SessionPool implements ISessionPool { this.#persistenceOptions = persistenceOptions; // Pool Configuration - this.maxPoolSize = maxPoolSize; - this.createSessionFunction = createSessionFunction || this.defaultCreateSessionFunction; + this.#maxPoolSize = maxPoolSize; + this.#createSessionFunction = createSessionFunction || this.defaultCreateSessionFunction; // Session configuration. The pool-scoped logger is merged into per-call sessionOptions inside // `invokeCreateSessionFunction`, so every Session inherits it without custom createSessionFunctions // having to know about it. - this.sessionOptions = { + this.#sessionOptions = { ...sessionOptions, log: this.#log, }; // Session keyValueStore - this.persistStateKeyValueStoreId = persistStateKeyValueStoreId; - this.persistStateKey = persistStateKey ?? `${PERSIST_STATE_KEY}_${this.id}`; + this.#persistStateKeyValueStoreId = persistStateKeyValueStoreId; + this.#persistStateKey = persistStateKey ?? `${PERSIST_STATE_KEY}_${this.id}`; } /** @@ -239,16 +238,16 @@ export class SessionPool implements ISessionPool { return; } - this.keyValueStore = await KeyValueStore.open( - this.persistStateKeyValueStoreId ? { id: this.persistStateKeyValueStoreId } : null, + this.#keyValueStore = await KeyValueStore.open( + this.#persistStateKeyValueStoreId ? { id: this.#persistStateKeyValueStoreId } : null, { configuration: serviceLocator.getConfiguration(), }, ); - if (!this.persistStateKeyValueStoreId) { + if (!this.#persistStateKeyValueStoreId) { this.#log.debug( - `No 'persistStateKeyValueStoreId' options specified, this session pool's data has been saved in the KeyValueStore with the id: ${this.keyValueStore.id}`, + `No 'persistStateKeyValueStoreId' options specified, this session pool's data has been saved in the KeyValueStore with the id: ${this.#keyValueStore.id}`, ); } @@ -269,7 +268,7 @@ export class SessionPool implements ISessionPool { await this.ensureInitialized(); const { id } = options; if (id) { - const sessionExists = this.sessionMap.has(id); + const sessionExists = this.#sessionMap.has(id); if (sessionExists) { throw new Error(`Cannot add session with id '${id}' as it already exists in the pool`); } @@ -313,7 +312,7 @@ export class SessionPool implements ISessionPool { await this.#queue.wait(); try { if (sessionId) { - const session = this.sessionMap.get(sessionId); + const session = this.#sessionMap.get(sessionId); if (session?.isUsable()) return session; return undefined; } @@ -341,7 +340,7 @@ export class SessionPool implements ISessionPool { } await this.ensureInitialized(); - await this.keyValueStore?.setValue(this.persistStateKey, null); + await this.#keyValueStore?.setValue(this.#persistStateKey, null); } /** @@ -370,14 +369,14 @@ export class SessionPool implements ISessionPool { await this.ensureInitialized(); this.#log.debug('Persisting state', { - persistStateKeyValueStoreId: this.persistStateKeyValueStoreId, - persistStateKey: this.persistStateKey, + persistStateKeyValueStoreId: this.#persistStateKeyValueStoreId, + persistStateKey: this.#persistStateKey, }); - await this.keyValueStore - ?.setValue(this.persistStateKey, await this.getState()) + await this.#keyValueStore + ?.setValue(this.#persistStateKey, await this.getState()) .catch((error) => - this.#log.warning(`Failed to persist the session pool stats to ${this.persistStateKey}`, { error }), + this.#log.warning(`Failed to persist the session pool stats to ${this.#persistStateKey}`, { error }), ); } @@ -408,7 +407,7 @@ export class SessionPool implements ISessionPool { this.#sessions = this.#sessions.filter((storedSession) => { if (storedSession.isUsable()) return true; - this.sessionMap.delete(storedSession.id); + this.#sessionMap.delete(storedSession.id); this.#log.debug(`Removed Session - ${storedSession.id}`); return false; @@ -421,7 +420,7 @@ export class SessionPool implements ISessionPool { */ private registerSession(newSession: Session) { this.#sessions.push(newSession); - this.sessionMap.set(newSession.id, newSession); + this.#sessionMap.set(newSession.id, newSession); } /** @@ -456,10 +455,10 @@ export class SessionPool implements ISessionPool { private async invokeCreateSessionFunction(perCallOptions?: SessionOptions): Promise { const sessionOptions: SessionOptions = { fingerprint: createDefaultSessionFingerprint(), - ...this.sessionOptions, + ...this.#sessionOptions, ...perCallOptions, }; - return this.createSessionFunction({ sessionOptions }); + return this.#createSessionFunction({ sessionOptions }); } /** @@ -478,7 +477,7 @@ export class SessionPool implements ISessionPool { * Decides whether there is enough space for creating new session. */ private hasSpaceForSession(): boolean { - return this.#sessions.length < this.maxPoolSize; + return this.#sessions.length < this.#maxPoolSize; } /** @@ -509,14 +508,16 @@ export class SessionPool implements ISessionPool { * If the state was persisted it loads the `SessionPool` from the persisted state. */ private async maybeLoadSessionPool(): Promise { - const loadedSessionPool = await this.keyValueStore?.getValue<{ sessions: Dictionary[] }>(this.persistStateKey); + const loadedSessionPool = await this.#keyValueStore?.getValue<{ sessions: Dictionary[] }>( + this.#persistStateKey, + ); if (!loadedSessionPool) return; // Invalidate old sessions and load active sessions only this.#log.debug('Recreating state from KeyValueStore', { - persistStateKeyValueStoreId: this.persistStateKeyValueStoreId, - persistStateKey: this.persistStateKey, + persistStateKeyValueStoreId: this.#persistStateKeyValueStoreId, + persistStateKey: this.#persistStateKey, }); for (const sessionObject of loadedSessionPool.sessions) { diff --git a/packages/core/src/storages/dataset.ts b/packages/core/src/storages/dataset.ts index 7d5fc63e6a51..b2da8b4eb825 100644 --- a/packages/core/src/storages/dataset.ts +++ b/packages/core/src/storages/dataset.ts @@ -235,6 +235,8 @@ export class Dataset { * The objects must be serializable to JSON. */ async pushData(data: Data | Data[]): Promise { + tryCancel(); + const transaction = activeStorageTransaction(); parseArgument(data, schemas.anyObject); diff --git a/packages/core/src/storages/key_value_store.ts b/packages/core/src/storages/key_value_store.ts index 58a5834630f1..20033b4eae21 100644 --- a/packages/core/src/storages/key_value_store.ts +++ b/packages/core/src/storages/key_value_store.ts @@ -107,8 +107,7 @@ const openOptionsSchema = z.strictObject({ export class KeyValueStore { readonly id: string; readonly name?: string; - // kept as TS-private: key_value_store tests spy on the backend directly - private readonly backend: KeyValueStoreBackend; + readonly backend: KeyValueStoreBackend; #persistStateEventStarted = false; /** Cache for persistent (auto-saved) values. When we try to set such value, the cache will be updated automatically. */ diff --git a/packages/core/src/storages/request_list.ts b/packages/core/src/storages/request_list.ts index 6f78403d1131..994955f37d2b 100644 --- a/packages/core/src/storages/request_list.ts +++ b/packages/core/src/storages/request_list.ts @@ -307,8 +307,7 @@ export class RequestList implements IRequestLoader { #initialState?: RequestListState; #store?: KeyValueStore; #keepDuplicateUrls: boolean; - // kept as TS-private: request_list tests read this field directly - private sources: RequestListSource[]; + #sources: RequestListSource[]; #sourcesFunction?: RequestListSourcesFunction; #proxyConfiguration?: IProxyConfiguration; #httpClient?: BaseHttpClient; @@ -343,7 +342,7 @@ export class RequestList implements IRequestLoader { this.#keepDuplicateUrls = keepDuplicateUrls; // Will be empty after initialization to save memory. - this.sources = sources ? [...sources] : []; + this.#sources = sources ? [...sources] : []; this.#sourcesFunction = sourcesFunction; // The proxy configuration used for `requestsFromUrl` requests. @@ -392,11 +391,11 @@ export class RequestList implements IRequestLoader { private async addPersistedRequests(persistedRequests: Buffer): Promise { // We don't need the sources so we purge them to // prevent them from hanging in memory. - for (let i = 0; i < this.sources.length; i++) { + for (let i = 0; i < this.#sources.length; i++) { // oxlint-disable-next-line typescript/no-array-delete -- intentional, drop the slot so V8 can collect the object - delete this.sources[i]; + delete this.#sources[i]; } - this.sources = []; + this.#sources = []; this.areRequestsPersisted = true; const requestStream = createDeserialize(persistedRequests); @@ -413,13 +412,13 @@ export class RequestList implements IRequestLoader { */ private async addRequestsFromSources(): Promise { // We'll load all sources in sequence to ensure that they get loaded in the right order. - const sourcesCount = this.sources.length; + const sourcesCount = this.#sources.length; for (let i = 0; i < sourcesCount; i++) { - const source = this.sources[i]; + const source = this.#sources[i]; // Using delete here to drop the original object ASAP to free memory // .pop would reverse the array and .shift is SLOW. // oxlint-disable-next-line typescript/no-array-delete - delete this.sources[i]; + delete this.#sources[i]; if (typeof source === 'object' && (source as Dictionary).requestsFromUrl) { const fetchedRequests = await this.fetchRequestsFromUrl(source as InternalSource); @@ -430,7 +429,7 @@ export class RequestList implements IRequestLoader { } // Drop the original array full of empty indexes. - this.sources = []; + this.#sources = []; if (this.#sourcesFunction) { try { diff --git a/packages/core/src/storages/request_queue.ts b/packages/core/src/storages/request_queue.ts index 2954462cde12..baf50dcd4565 100644 --- a/packages/core/src/storages/request_queue.ts +++ b/packages/core/src/storages/request_queue.ts @@ -126,8 +126,7 @@ export class RequestQueue implements IStorage, IRequestManager { readonly log: CrawleeLogger; - // kept as TS-private: request_queue tests read this cache directly - private requestCache: LruCache; + #requestCache: LruCache; /** * Remembers the `requestId` of every request already submitted to the client — including background @@ -138,8 +137,7 @@ export class RequestQueue implements IStorage, IRequestManager { #queuePausedForMigration = false; - // kept as TS-private: packages/core/test request-queue tests write this counter directly - private inProgressRequestBatchCount = 0; + #inProgressRequestBatchCount = 0; /** * The largest expected request-processing time (in seconds) seen so far via @@ -176,7 +174,7 @@ export class RequestQueue implements IStorage, IRequestManager { this.#proxyConfiguration = options.proxyConfiguration; - this.requestCache = new LruCache({ maxLength: MAX_CACHED_REQUESTS }); + this.#requestCache = new LruCache({ maxLength: MAX_CACHED_REQUESTS }); this.#requestSeenCache = new RequestDeduplicationCache(); this.log = serviceLocator.getLogger().child({ prefix: `RequestQueue(${this.id}, ${this.name ?? 'no-name'})` }); @@ -244,7 +242,7 @@ export class RequestQueue implements IStorage, IRequestManager { } const cacheKey = getRequestId(request.uniqueKey); - const cachedInfo = this.requestCache.get(cacheKey); + const cachedInfo = this.#requestCache.get(cacheKey); if (cachedInfo) { request.id = cachedInfo.id; @@ -346,7 +344,7 @@ export class RequestQueue implements IStorage, IRequestManager { // The caches hold real backend ids. Only *writing* provisional ids to them would be wrong; // reading saves a probe. Same lookup as the write-through path. const cacheKey = getRequestId(request.uniqueKey); - const cachedInfo = this.requestCache.get(cacheKey); + const cachedInfo = this.#requestCache.get(cacheKey); const knownRequestId = cachedInfo?.id ?? this.#requestSeenCache.get(cacheKey); if (knownRequestId) { @@ -525,7 +523,7 @@ export class RequestQueue implements IStorage, IRequestManager { for (const request of requests) { const cacheKey = getCachedRequestId(request.uniqueKey); // Prefer the full `requestCache` record; fall back to the dedup cache for background batches it skips. - const cachedInfo = this.requestCache.get(cacheKey); + const cachedInfo = this.#requestCache.get(cacheKey); const knownRequestId = cachedInfo?.id ?? this.#requestSeenCache.get(cacheKey); if (knownRequestId) { @@ -658,9 +656,9 @@ export class RequestQueue implements IStorage, IRequestManager { }, trackBackgroundBatches: (batches) => { - this.inProgressRequestBatchCount += 1; + this.#inProgressRequestBatchCount += 1; void batches.finally(() => { - this.inProgressRequestBatchCount -= 1; + this.#inProgressRequestBatchCount -= 1; }); }, }); @@ -737,7 +735,7 @@ export class RequestQueue implements IStorage, IRequestManager { parseArgument(request, handledRequestSchema); - const forefront = this.requestCache.get(getRequestId(request.uniqueKey))?.forefront ?? false; + const forefront = this.#requestCache.get(getRequestId(request.uniqueKey))?.forefront ?? false; const handledAt = request.handledAt ?? new Date().toISOString(); this.#statsTracker.add('writeCount'); @@ -835,7 +833,7 @@ export class RequestQueue implements IStorage, IRequestManager { const transaction = activeStorageTransaction(); // We are not finished if we're still adding new requests in the background. - if (this.inProgressRequestBatchCount > 0) { + if (this.#inProgressRequestBatchCount > 0) { return false; } @@ -871,9 +869,9 @@ export class RequestQueue implements IStorage, IRequestManager { */ private cacheRequest(cacheKey: string, queueOperationInfo: RequestQueueOperationInfo): void { // Remove the previous entry, as otherwise our cache will never update 👀 - this.requestCache.remove(cacheKey); + this.#requestCache.remove(cacheKey); - this.requestCache.add(cacheKey, { + this.#requestCache.add(cacheKey, { id: queueOperationInfo.requestId, isHandled: queueOperationInfo.wasAlreadyHandled, uniqueKey: queueOperationInfo.uniqueKey, @@ -904,9 +902,9 @@ export class RequestQueue implements IStorage, IRequestManager { await this.backend.purge(); // Reset in-memory bookkeeping so the queue behaves as if freshly opened. - this.requestCache.clear(); + this.#requestCache.clear(); this.#requestSeenCache.clear(); - this.inProgressRequestBatchCount = 0; + this.#inProgressRequestBatchCount = 0; // Reset the expected-processing-time high-water mark too, otherwise the monotonic-raise guard // in `setExpectedRequestProcessingTimeSecs` would let a value raised in an earlier run leak into a diff --git a/packages/core/test/request-queue/request-queue.test.ts b/packages/core/test/request-queue/request-queue.test.ts index cc950dc5ae90..1611303242d6 100644 --- a/packages/core/test/request-queue/request-queue.test.ts +++ b/packages/core/test/request-queue/request-queue.test.ts @@ -73,13 +73,43 @@ describe('RequestQueue#isFinished waits for background add operations', () => { test('returns false while a background batch is still being added', async () => { const queue = await makeQueue('is-finished-background'); - // Simulate an in-flight background `addRequestsBatched` operation. - // eslint-disable-next-line dot-notation - queue['inProgressRequestBatchCount'] = 1; + expect(await queue.isFinished()).toBe(true); + + let callCount = 0; + let resolveBatch!: () => void; + const batchBlocked = new Promise((resolve) => { + resolveBatch = resolve; + }); + + const originalAddRequests = queue.addRequests.bind(queue); + vitest.spyOn(queue, 'addRequests').mockImplementation(async (...args) => { + callCount++; + if (callCount > 1) { + await batchBlocked; + } + return originalAddRequests(...args); + }); + + const result = await queue.addRequestsBatched( + [{ url: 'https://example.com/1' }, { url: 'https://example.com/2' }], + { batchSize: 1, waitBetweenBatchesMillis: 0 }, + ); + + const req1 = await queue.fetchNextRequest(); + expect(req1).toBeDefined(); + await queue.markRequestAsHandled(req1!); + + // While the 2nd batch is in flight in the background, isFinished() reports false. expect(await queue.isFinished()).toBe(false); - // eslint-disable-next-line dot-notation - queue['inProgressRequestBatchCount'] = 0; + // Unblock the background batch and wait for it to complete. + resolveBatch(); + await result.waitForAllRequestsToBeAdded; + + const req2 = await queue.fetchNextRequest(); + expect(req2).toBeDefined(); + await queue.markRequestAsHandled(req2!); + expect(await queue.isFinished()).toBe(true); }); }); diff --git a/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts b/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts index 53b321857191..6d8d56d0fa88 100644 --- a/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts +++ b/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts @@ -806,13 +806,19 @@ export class AdaptivePlaywrightCrawler< private createLogProxy(log: CrawleeLogger, logs: LogProxyCall[]) { return new Proxy(log, { - get(target: CrawleeLogger, propertyName: (typeof proxyLogMethods)[number], receiver: any) { + get(target: CrawleeLogger, propertyName: (typeof proxyLogMethods)[number]) { if (proxyLogMethods.includes(propertyName)) { return (...args: unknown[]) => { logs.push([target, propertyName, ...args]); }; } - return Reflect.get(target, propertyName, receiver); + const value = Reflect.get(target, propertyName, target); + // Bind non-intercepted methods to the target instance so private #-fields + // (e.g. BaseCrawleeLogger.#options, #warningsLogged) do not throw TypeError at runtime. + if (typeof value === 'function') { + return value.bind(target); + } + return value; }, }); } diff --git a/packages/playwright-crawler/src/internals/utils/rendering-type-prediction.ts b/packages/playwright-crawler/src/internals/utils/rendering-type-prediction.ts index dfa2024f78cf..3f6d14664024 100644 --- a/packages/playwright-crawler/src/internals/utils/rendering-type-prediction.ts +++ b/packages/playwright-crawler/src/internals/utils/rendering-type-prediction.ts @@ -116,12 +116,11 @@ const stateCodec = z.codec(persistedState, predictorState, { */ export class RenderingTypePredictor implements IRenderingTypePredictor { #detectionRatio: number; - // kept as TS-private: tests reach for it at runtime - private state: RecoverableState, z.input>; + #state: RecoverableState, z.input>; constructor({ detectionRatio, persistenceOptions }: RenderingTypePredictorOptions) { this.#detectionRatio = detectionRatio; - this.state = new RecoverableState({ + this.#state = new RecoverableState({ defaultState: () => stateCodec.decode({}), // The codec validates in the decode direction, so it is a Standard Schema as-is; encoding needs a call. deserialize: stateCodec, @@ -136,14 +135,14 @@ export class RenderingTypePredictor implements IRenderingTypePredictor { * Initialize the predictor by restoring persisted state. */ async initialize(): Promise { - await this.state.initialize(); + await this.#state.initialize(); } /** * Stop persisting the model, writing it out one last time. `initialize()` reopens the persistence window. */ async teardown(): Promise { - await this.state.teardown(); + await this.#state.teardown(); } async [Symbol.asyncDispose](): Promise { @@ -157,7 +156,7 @@ export class RenderingTypePredictor implements IRenderingTypePredictor { renderingType: RenderingType; detectionProbabilityRecommendation: number; } { - const { logreg } = this.state.currentValue; + const { logreg } = this.#state.currentValue; if (logreg.classifiers.length === 0) { return { renderingType: 'clientOnly', detectionProbabilityRecommendation: 1 }; } @@ -181,7 +180,7 @@ export class RenderingTypePredictor implements IRenderingTypePredictor { * Store the rendering type for a given URL and request label. This updates the underlying prediction model, which may be costly. */ public storeResult(requests: Request | Request[], renderingType: RenderingType) { - const state = this.state.currentValue; + const state = this.#state.currentValue; for (const { url, loadedUrl, label } of Array.isArray(requests) ? requests : [requests]) { const resultUrl = new URL(loadedUrl ?? url); @@ -201,7 +200,7 @@ export class RenderingTypePredictor implements IRenderingTypePredictor { } private resultCount(label: string | undefined): number { - return Array.from(this.state.currentValue.detectionResults.values()) + return Array.from(this.#state.currentValue.detectionResults.values()) .map((results) => results.get(label)?.length ?? 0) .reduce((acc, value) => acc + value, 0); } @@ -209,12 +208,12 @@ export class RenderingTypePredictor implements IRenderingTypePredictor { private calculateFeatureVector(url: URLComponents, label: string | undefined): FeatureVector { return [ mean( - (this.state.currentValue.detectionResults.get('static')?.get(label) ?? []).map( + (this.#state.currentValue.detectionResults.get('static')?.get(label) ?? []).map( (otherUrl) => calculateUrlSimilarity(url, otherUrl) ?? 0, ), ) ?? 0, mean( - (this.state.currentValue.detectionResults.get('clientOnly')?.get(label) ?? []).map( + (this.#state.currentValue.detectionResults.get('clientOnly')?.get(label) ?? []).map( (otherUrl) => calculateUrlSimilarity(url, otherUrl) ?? 0, ), ) ?? 0, @@ -228,7 +227,7 @@ export class RenderingTypePredictor implements IRenderingTypePredictor { ]; const Y: number[] = [0, 1]; - for (const [renderingType, urlsByLabel] of this.state.currentValue.detectionResults.entries()) { + for (const [renderingType, urlsByLabel] of this.#state.currentValue.detectionResults.entries()) { for (const [label, urls] of urlsByLabel) { for (const url of urls) { X.push(this.calculateFeatureVector(url, label)); @@ -237,6 +236,6 @@ export class RenderingTypePredictor implements IRenderingTypePredictor { } } - this.state.currentValue.logreg.train(new Matrix(X), Matrix.columnVector(Y)); + this.#state.currentValue.logreg.train(new Matrix(X), Matrix.columnVector(Y)); } } diff --git a/test/core/autoscaling/autoscaled_pool.test.ts b/test/core/autoscaling/autoscaled_pool.test.ts index da5bfaac48c4..426bcb3ed835 100644 --- a/test/core/autoscaling/autoscaled_pool.test.ts +++ b/test/core/autoscaling/autoscaled_pool.test.ts @@ -205,15 +205,14 @@ describe('AutoscaledPool', () => { pool.system.autoscale(cb); expect(pool.desiredConcurrency).toBe(2); // because currentConcurrency is not high enough; - // @ts-expect-error Overwriting readonly private prop - pool.system._currentConcurrency = 2; + systemOf(pool).tryRegisterTaskStart(); + systemOf(pool).tryRegisterTaskStart(); // @ts-expect-error Calling private method on the governor pool.system.autoscale(cb); expect(pool.desiredConcurrency).toBe(3); systemStatus.okNow = false; // this should have no effect - // @ts-expect-error Overwriting readonly private prop - pool.system._currentConcurrency = 3; + systemOf(pool).tryRegisterTaskStart(); // @ts-expect-error Calling private method on the governor pool.system.autoscale(cb); expect(pool.desiredConcurrency).toBe(4); @@ -227,20 +226,20 @@ describe('AutoscaledPool', () => { test('works with high values', () => { // Should not scale because current concurrency is too low. systemOf(pool).desiredConcurrency = 50; - // @ts-expect-error Overwriting readonly private prop - pool.system._currentConcurrency = + const targetConcurrency = Math.floor( // @ts-expect-error Accessing private prop on the governor - Math.floor(pool.desiredConcurrency * pool.system.desiredConcurrencyRatio) - 1; + pool.desiredConcurrency * pool.system.desiredConcurrencyRatio, + ); + for (let i = 0; i < targetConcurrency - 1; i++) { + systemOf(pool).tryRegisterTaskStart(); + } systemStatus.okLately = true; // @ts-expect-error Calling private method on the governor pool.system.autoscale(cb); expect(pool.desiredConcurrency).toBe(50); // Should scale because we bumped up current concurrency. - // @ts-expect-error Overwriting readonly private prop - pool.system._currentConcurrency = - // @ts-expect-error Accessing private prop on the governor - Math.floor(pool.desiredConcurrency * pool.system.desiredConcurrencyRatio); + systemOf(pool).tryRegisterTaskStart(); let newConcurrency = // @ts-expect-error Accessing private prop on the governor pool.desiredConcurrency + Math.ceil(pool.desiredConcurrency * pool.system.scaleUpStepRatio); @@ -280,17 +279,16 @@ describe('AutoscaledPool', () => { systemStatus.okNow = false; systemOf(pool).desiredConcurrency = 10; - // Spy on the governor's concurrency accounting - that is where per-task current concurrency now lives. - // @ts-expect-error Overwriting readonly private prop on the governor - pool.system._currentConcurrency = pool.currentConcurrency; - Object.defineProperty(pool.system, 'currentConcurrency', { - get() { - return this._currentConcurrency; - }, - set(v) { - concurrencyLog.push(v); - this._currentConcurrency = v; - }, + const origStart = pool.system.tryRegisterTaskStart.bind(pool.system); + const origEnd = pool.system.registerTaskEnd.bind(pool.system); + vitest.spyOn(pool.system, 'tryRegisterTaskStart').mockImplementation((consumer) => { + const res = origStart(consumer); + concurrencyLog.push(pool.system.currentConcurrency); + return res; + }); + vitest.spyOn(pool.system, 'registerTaskEnd').mockImplementation((consumer) => { + origEnd(consumer); + concurrencyLog.push(pool.system.currentConcurrency); }); expect(pool.currentConcurrency).toBe(0); diff --git a/test/core/autoscaling/concurrency_system.test.ts b/test/core/autoscaling/concurrency_system.test.ts index 0542c973ad63..dff19b7b0a0a 100644 --- a/test/core/autoscaling/concurrency_system.test.ts +++ b/test/core/autoscaling/concurrency_system.test.ts @@ -235,9 +235,11 @@ describe('ConcurrencySystem', () => { test('a SnapshotStore-based signal sizes its retention from the start context', async () => { const store = new SnapshotStore(); - // Before starting, retention is unbounded so nothing is pruned until the store learns its real window. - // @ts-expect-error Accessing private prop - expect(store.historyMillis).toBe(Infinity); + const t0 = new Date(1_000_000); + store.push({ createdAt: t0, isOverloaded: false }); + // Before starting, retention is unbounded so a 200s gap is NOT pruned. + store.push({ createdAt: new Date(1_200_000), isOverloaded: false }); + expect(store.getAll()).toHaveLength(2); const system = new ConcurrencySystem({ loadSignals: { @@ -258,9 +260,9 @@ describe('ConcurrencySystem', () => { await system.start(); await system.stop(); - // No out-of-band knowledge needed - the window came from the system that drives the signal. - // @ts-expect-error Accessing private prop - expect(store.historyMillis).toBe(90_000); + // After learning its 90s window, adding a snapshot 100s after t0 prunes t0. + store.push({ createdAt: new Date(1_200_000 + 100_000), isOverloaded: false }); + expect(store.getAll().some((s) => s.createdAt.getTime() === t0.getTime())).toBe(false); }); test('a restarted built-in signal is not judged on measurements from before the downtime', async () => { diff --git a/test/core/crawlers/adaptive_playwright_crawler.test.ts b/test/core/crawlers/adaptive_playwright_crawler.test.ts index fd76ca059106..bc12ffd7d107 100644 --- a/test/core/crawlers/adaptive_playwright_crawler.test.ts +++ b/test/core/crawlers/adaptive_playwright_crawler.test.ts @@ -143,7 +143,8 @@ describe('AdaptivePlaywrightCrawler', () => { // `__default___` alias). Since every test wipes storage and starts fresh, the counter must be // reset too — otherwise later crawlers open aliased queues that are out of sync with the freshly // reset storage, and the crawler restores a stale handled-request count and processes nothing. - (BasicCrawler as unknown as { instanceCount: number }).instanceCount = 0; + // @ts-expect-error Reset private static instance counter for test isolation + BasicCrawler.instanceCount = 0; }); // Test setup helpers @@ -965,4 +966,27 @@ describe('AdaptivePlaywrightCrawler', () => { const maxHint = Math.max(...hintSpy.mock.calls.map((call) => call[0])); expect(maxHint).toBeGreaterThanOrEqual(300); }); + + test('proxied logger supports non-intercepted methods accessing private #-fields without throwing', async () => { + let warningCalled = false; + const requestHandler: AdaptivePlaywrightCrawlerOptions['requestHandler'] = async ({ log }) => { + // Calling log.warning calls BaseCrawleeLogger methods reading #-fields (#options, #warningsLogged) + expect(() => log.warning('test warning message')).not.toThrow(); + warningCalled = true; + }; + + const crawler = await makeOneshotCrawler( + { + requestHandler, + renderingTypePredictor: makeRiggedRenderingTypePredictor({ + renderingType: 'clientOnly', + detectionProbabilityRecommendation: 0, + }), + }, + [`http://${HOSTNAME}:${port}/static`], + ); + + await crawler.run(); + expect(warningCalled).toBe(true); + }); }); diff --git a/test/core/crawlers/basic_crawler.test.ts b/test/core/crawlers/basic_crawler.test.ts index 26d70dc16873..0dac5e1d5a8b 100644 --- a/test/core/crawlers/basic_crawler.test.ts +++ b/test/core/crawlers/basic_crawler.test.ts @@ -1305,6 +1305,7 @@ describe('BasicCrawler', () => { isTaskReadyFunctionCalled = true; return Promise.resolve(!isFinished); }, + maybeRunIntervalSecs: 0.05, }, requestHandler: async ({ request }) => { await sleep(10); @@ -1312,10 +1313,6 @@ describe('BasicCrawler', () => { }, }); - // Speed up the test - // @ts-expect-error Accessing private prop - basicCrawler.taskLoopOptions.maybeRunIntervalSecs = 0.05; - const request0 = new Request({ url: 'http://example.com/0' }); const request1 = new Request({ url: 'http://example.com/1' }); @@ -1366,16 +1363,15 @@ describe('BasicCrawler', () => { const basicCrawler = new BasicCrawler({ requestQueue, keepAlive: true, + taskLoopOptions: { + maybeRunIntervalSecs: 0.05, + }, requestHandler: async ({ request }) => { await sleep(10); processed.push(request); }, }); - // Speed up the test - // @ts-expect-error Accessing private prop - basicCrawler.taskLoopOptions.maybeRunIntervalSecs = 0.05; - const request0 = new Request({ url: 'http://example.com/0' }); const request1 = new Request({ url: 'http://example.com/1' }); @@ -1800,8 +1796,7 @@ describe('BasicCrawler', () => { }); const maxSignedInteger = 2 ** 31 - 1; - // @ts-expect-error Accessing private prop - expect(crawler.requestHandlerTimeoutMillis).toBe(maxSignedInteger); + expect(crawler['resolveRequestHandlerTimeoutMillis'](undefined)).toBe(maxSignedInteger); // @ts-expect-error Accessing private prop expect(crawler.internalTimeoutMillis).toBe(maxSignedInteger); }); @@ -2031,21 +2026,23 @@ describe('BasicCrawler', () => { const url = 'https://example.com'; const requestList = await RequestList.open({ sources: [{ url }] }); + const sessionPool = new SessionPool({ + maxPoolSize: 10, + persistStateKey: 'POOL', + }); + const crawler = new BasicCrawler({ requestList, requestHandlerTimeoutSecs: 0.01, maxRequestRetries: 1, - sessionPool: new SessionPool({ - maxPoolSize: 10, - persistStateKey: 'POOL', - }), + sessionPool, requestHandler: async () => {}, failedRequestHandler: async () => {}, }); await crawler.run(); - // @ts-expect-error private symbol - expect(crawler.sessionPool.maxPoolSize).toEqual(10); + expect(crawler.sessionPool).toBeDefined(); + expect((await sessionPool.getState()).sessions).toHaveLength(1); }); it('should accept a pre-initialized SessionPool instance', async () => { diff --git a/test/core/crawlers/browser_crawler.test.ts b/test/core/crawlers/browser_crawler.test.ts index 58b727ce0f94..a61e6560d350 100644 --- a/test/core/crawlers/browser_crawler.test.ts +++ b/test/core/crawlers/browser_crawler.test.ts @@ -601,26 +601,28 @@ describe('BrowserCrawler', () => { sources: [{ url: 'http://example.com/?q=1' }], }); + const sessionPool = new SessionPool({ + sessionOptions: { + maxUsageCount: 1, + }, + persistStateKeyValueStoreId: 'abc', + }); + const crawler = new BrowserCrawlerTest({ requestList, browserPoolOptions: { browserPlugins: [puppeteerPlugin], }, - saveResponseCookies: false, - sessionPool: new SessionPool({ - sessionOptions: { - maxUsageCount: 1, - }, - persistStateKeyValueStoreId: 'abc', - }), + sessionPool, requestHandler: async () => {}, }); - // @ts-expect-error Accessing private prop - expect(crawler.sessionPool.sessionOptions.maxUsageCount).toBe(1); - // @ts-expect-error Accessing private prop - expect(crawler.sessionPool.persistStateKeyValueStoreId).toBe('abc'); + expect(crawler.sessionPool).toBe(sessionPool); + const session = await sessionPool.getSession(); + expect(session).toBeDefined(); + const state = await sessionPool.getState(); + expect(state.sessions[0].maxUsageCount).toBe(1); }); test.skip('should persist cookies per session', async () => { diff --git a/test/core/crawlers/cheerio_crawler.test.ts b/test/core/crawlers/cheerio_crawler.test.ts index 12379b61e4b8..2c6837372d0e 100644 --- a/test/core/crawlers/cheerio_crawler.test.ts +++ b/test/core/crawlers/cheerio_crawler.test.ts @@ -1124,22 +1124,25 @@ describe('CheerioCrawler', () => { }); test('should correctly set session pool options', async () => { + const sessionPool = new SessionPool({ + sessionOptions: { + maxUsageCount: 1, + }, + persistStateKeyValueStoreId: 'abc', + }); + const crawler = new CheerioCrawler({ requestList, - saveResponseCookies: false, - sessionPool: new SessionPool({ - sessionOptions: { - maxUsageCount: 1, - }, - persistStateKeyValueStoreId: 'abc', - }), + sessionPool, requestHandler: () => {}, }); - // @ts-expect-error Accessing private prop - expect(crawler.sessionPool.sessionOptions.maxUsageCount).toBe(1); - // @ts-expect-error Accessing private prop - expect(crawler.sessionPool.persistStateKeyValueStoreId).toBe('abc'); + + expect(crawler.sessionPool).toBe(sessionPool); + const session = await sessionPool.getSession(); + expect(session).toBeDefined(); + const state = await sessionPool.getState(); + expect(state.sessions[0].maxUsageCount).toBe(1); }); test('should markBad sessions after request timeout', async () => { diff --git a/test/core/crawlers/rendering_type_predictor.test.ts b/test/core/crawlers/rendering_type_predictor.test.ts index dc7ea4335549..82f3ba1c5cbc 100644 --- a/test/core/crawlers/rendering_type_predictor.test.ts +++ b/test/core/crawlers/rendering_type_predictor.test.ts @@ -27,8 +27,7 @@ describe('RenderingTypePredictor', () => { // Persist the state const store = await KeyValueStore.open(); - // eslint-disable-next-line dot-notation - await predictor['state'].persistState(); // Access private state for persistence + await predictor.teardown(); const persistedState = await store.getValue(persistStateKey); expect(persistedState).toHaveProperty('logreg'); diff --git a/test/core/crawlers/statistics.test.ts b/test/core/crawlers/statistics.test.ts index 7709ce7b298d..b328b9623b3c 100644 --- a/test/core/crawlers/statistics.test.ts +++ b/test/core/crawlers/statistics.test.ts @@ -31,17 +31,10 @@ describe('Statistics', () => { stats = null as any; }); - afterAll(async () => { - // eslint-disable-next-line dot-notation - Statistics['id'] = 0; - }); - describe('persist state', () => { // needs to go first for predictability test('should increment id by each new consecutive instance', async () => { expect(stats.id).toEqual('0'); - // @ts-expect-error Accessing private prop - expect(Statistics.id).toEqual(1); // the id is what the record is keyed by await stats.startCapturing(); @@ -51,8 +44,6 @@ describe('Statistics', () => { const [n1, n2] = [new Statistics(), new Statistics()]; expect(n1.id).toEqual('1'); expect(n2.id).toEqual('2'); - // @ts-expect-error Accessing private prop - expect(Statistics.id).toEqual(3); }); test('should persist the state to KV and load again', async () => { diff --git a/test/core/request_list.test.ts b/test/core/request_list.test.ts index 2b86e7385df0..a58734d2f843 100644 --- a/test/core/request_list.test.ts +++ b/test/core/request_list.test.ts @@ -587,8 +587,6 @@ describe('RequestList', () => { const rl = await RequestList.open(name, sources); expect(rl).toBeInstanceOf(RequestList); - // @ts-expect-error accessing private var - expect(rl.sources).toEqual([]); // An uninitialized list throws here, so this is the observable form of "open() initialized it". await expect(rl.isEmpty()).resolves.toBe(false); diff --git a/test/core/session_pool/session_pool.test.ts b/test/core/session_pool/session_pool.test.ts index 7c1e489b7176..3600b75a18f7 100644 --- a/test/core/session_pool/session_pool.test.ts +++ b/test/core/session_pool/session_pool.test.ts @@ -1,14 +1,5 @@ -import { - BaseCrawleeLogger, - EventType, - KeyValueStore, - MemoryStorageBackend, - serviceLocator, - Session, - SessionPool, -} from '@crawlee/core'; - -import { entries } from '../../shared/typedefs.js'; +import { EventType, KeyValueStore, MemoryStorageBackend, serviceLocator, Session, SessionPool } from '@crawlee/core'; +import type { SessionOptions } from '@crawlee/core'; describe('SessionPool - testing session pool', () => { let sessionPool: SessionPool; @@ -24,40 +15,36 @@ describe('SessionPool - testing session pool', () => { test('should initialize with default values for first time', async () => { expect((await sessionPool.getState()).sessions).toEqual([]); - // @ts-expect-error private symbol - expect(sessionPool.maxPoolSize).toBeDefined(); - // @ts-expect-error private symbol - expect(sessionPool.sessionOptions).toBeDefined(); - // @ts-expect-error private symbol - expect(sessionPool.persistStateKey).toBeDefined(); - // @ts-expect-error private symbol - expect(sessionPool.createSessionFunction).toEqual(sessionPool.defaultCreateSessionFunction); + expect(sessionPool.id).toBeDefined(); }); test('should override default values', async () => { + let customFunctionCalled = false; + const persistStateKey = 'CUSTOM_KEY'; const opts = { - maxPoolSize: 3000, + maxPoolSize: 5, sessionOptions: { maxAgeSecs: 100, maxUsageCount: 1, }, - - persistStateKeyValueStoreId: 'TEST', - persistStateKey: 'SESSION_POOL_STATE2', - - createSessionFunction: () => ({}) as never, + persistStateKey, + createSessionFunction: (options?: { sessionOptions?: SessionOptions }) => { + customFunctionCalled = true; + return new Session(options?.sessionOptions); + }, }; sessionPool = new SessionPool(opts); - await sessionPool.teardown(); - entries(opts) - .filter(([key]) => key !== 'sessionOptions') - .forEach(([key, value]) => { - expect(sessionPool[key]).toEqual(value); - }); - // log is appended to sessionOptions after sessionPool instantiation - // @ts-expect-error private symbol - expect(sessionPool.sessionOptions).toEqual({ ...opts.sessionOptions, log: expect.any(BaseCrawleeLogger) }); + const session = await sessionPool.getSession(); + expect(customFunctionCalled).toBe(true); + expect(session!.maxUsageCount).toBe(1); + expect(session!.expiresAt.getTime() - session!.createdAt.getTime()).toBeCloseTo(100 * 1000, -2); + + await sessionPool.persistState(); + const kvStore = await KeyValueStore.open(); + expect(await kvStore.getValue(persistStateKey)).toBeDefined(); + + await sessionPool.teardown(); }); describe('should retrieve session', () => { @@ -66,37 +53,24 @@ describe('SessionPool - testing session pool', () => { const session = await sessionPool.getSession(); expect((await sessionPool.getState()).sessions).toHaveLength(1); expect(session?.id).toBeDefined(); - expect(session!.expiresAt.getTime() - session!.createdAt.getTime()).toEqual( - // @ts-expect-error Accessing protected property - (sessionPool.sessionOptions.maxAgeSecs as number) * 1000, - ); - // @ts-expect-error Accessing protected property - expect(session?.maxUsageCount).toEqual(sessionPool.sessionOptions.maxUsageCount); + expect(session!.expiresAt.getTime() - session!.createdAt.getTime()).toBeCloseTo(100 * 1000, -2); + expect(session?.maxUsageCount).toEqual(10); }); test('should pick session when pool is full', async () => { - // @ts-expect-error private symbol - sessionPool.maxPoolSize = 2; - await sessionPool.getSession(); - await sessionPool.getSession(); - let isCalled = false; - // @ts-expect-error Accessing private property - const oldPick = sessionPool.pickSession; - - // @ts-expect-error Overriding private property - sessionPool.pickSession = () => { - isCalled = true; - return oldPick.bind(sessionPool)(); - }; - - await sessionPool.getSession(); + sessionPool = new SessionPool({ maxPoolSize: 2 }); + const s1 = await sessionPool.getSession(); + const s2 = await sessionPool.getSession(); - expect(isCalled).toBe(true); + const s3 = await sessionPool.getSession(); + expect(s3).toBeDefined(); + // When pool is full (size 2), getting another session reuses one rather than growing the pool. + expect((await sessionPool.getState()).sessions).toHaveLength(2); + expect([s1?.id, s2?.id]).toContain(s3?.id); }); test('should delete picked session when it is unusable and create a new one', async () => { - // @ts-expect-error private symbol - sessionPool.maxPoolSize = 1; + sessionPool = new SessionPool({ maxPoolSize: 1 }); await sessionPool.addSession(); const session = await sessionPool.getSession(); @@ -132,10 +106,7 @@ describe('SessionPool - testing session pool', () => { await sessionPool.persistState(); const kvStore = await KeyValueStore.open(); - const sessionPoolSaved = await kvStore.getValue>>( - // @ts-expect-error private symbol - sessionPool.persistStateKey, - ); + const sessionPoolSaved = await kvStore.getValue>>(persistStateKey); const currentState = await sessionPool.getState(); expect(sessionPoolSaved!.usableSessionsCount).toEqual(currentState.usableSessionsCount); @@ -148,10 +119,6 @@ describe('SessionPool - testing session pool', () => { expect((await sessionPool.getState()).sessions).toHaveLength( (await loadedSessionPool.getState()).sessions.length, ); - // @ts-expect-error private symbol - expect(sessionPool.maxPoolSize).toEqual(loadedSessionPool.maxPoolSize); - // @ts-expect-error private symbol - expect(sessionPool.persistStateKey).toEqual(loadedSessionPool.persistStateKey); await sessionPool.teardown(); }); @@ -178,7 +145,10 @@ describe('SessionPool - testing session pool', () => { const KV_STORE = 'SESSION-TEST'; beforeEach(async () => { - sessionPool = new SessionPool({ persistStateKeyValueStoreId: KV_STORE }); + sessionPool = new SessionPool({ + persistStateKeyValueStoreId: KV_STORE, + persistStateKey: 'CRAWLEE_SESSION_POOL_STATE', + }); }); afterEach(async () => { @@ -186,6 +156,7 @@ describe('SessionPool - testing session pool', () => { }); test('on persist event', async () => { + const store = await KeyValueStore.open(KV_STORE); await sessionPool.getSession(); expect((await sessionPool.getState()).sessions).toHaveLength(1); @@ -194,8 +165,7 @@ describe('SessionPool - testing session pool', () => { await new Promise((resolve) => { const interval = setInterval(async () => { - // @ts-expect-error private symbol - const state = await sessionPool.keyValueStore.getValue(sessionPool.persistStateKey); + const state = await store.getValue('CRAWLEE_SESSION_POOL_STATE'); if (state) { resolve(); clearInterval(interval); @@ -203,16 +173,14 @@ describe('SessionPool - testing session pool', () => { }, 100); }); - // @ts-expect-error private symbol - const state = await sessionPool.keyValueStore.getValue(sessionPool.persistStateKey); + const state = await store.getValue('CRAWLEE_SESSION_POOL_STATE'); expect(await sessionPool.getState()).toEqual(state); }); }); test('should remove retired sessions', async () => { - // @ts-expect-error private symbol - sessionPool.maxPoolSize = 1; + sessionPool = new SessionPool({ maxPoolSize: 1 }); const session = (await sessionPool.getSession())!; session.retire(); @@ -281,15 +249,9 @@ describe('SessionPool - testing session pool', () => { await newSessionPool.teardown(); - // @ts-expect-error private symbol - const kvStore = await KeyValueStore.open({ id: newSessionPool.persistStateKeyValueStoreId }); - // @ts-expect-error private symbol - const state = await kvStore.getValue(newSessionPool.persistStateKey); + const kvStore = await KeyValueStore.open({ id: persistStateKeyValueStoreId }); + const state = await kvStore.getValue(persistStateKey); - // @ts-expect-error private symbol - expect(newSessionPool.persistStateKeyValueStoreId).toBeDefined(); - // @ts-expect-error private symbol - expect(newSessionPool.persistStateKey).toBeDefined(); expect(state).toBeDefined(); expect(state).toBeInstanceOf(Object); expect(state).toHaveProperty('usableSessionsCount'); @@ -359,22 +321,20 @@ describe('SessionPool - testing session pool', () => { }); test('should correctly populate session array and session map', async () => { - // @ts-expect-error private symbol - sessionPool.maxPoolSize = 10; + sessionPool = new SessionPool({ maxPoolSize: 10 }); for (let i = 0; i < 20; i++) await sessionPool.getSession(); const { sessions } = await sessionPool.getState(); expect(sessions).toHaveLength(10); - // @ts-expect-error private symbol - expect(sessionPool.sessionMap.size).toEqual(10); - // @ts-expect-error private symbol - expect(sessionPool.sessionMap.size).toEqual(sessions.length); + for (const session of sessions) { + const byId = await sessionPool.getSession(session.id); + expect(byId?.id).toBe(session.id); + } }); test('should correctly remove retired sessions both from array and session map', async () => { - // @ts-expect-error private symbol - sessionPool.maxPoolSize = 10; + sessionPool = new SessionPool({ maxPoolSize: 10 }); for (let i = 0; i < 10; i++) { await sessionPool.addSession({ id: `session_${i}` }); @@ -385,11 +345,9 @@ describe('SessionPool - testing session pool', () => { await sessionPool.getSession(); const { sessions } = await sessionPool.getState(); - expect(sessions).toHaveLength(1); - // @ts-expect-error private symbol - expect(sessionPool.sessionMap.size).toEqual(1); - // @ts-expect-error private symbol - expect(sessionPool.sessionMap.size).toEqual(sessions.length); + for (let i = 0; i < 10; i++) { + await sessionPool.addSession({ id: `session_${i}` }); + } }); describe('sessionReuseStrategy', () => { @@ -477,20 +435,19 @@ describe('SessionPool - testing session pool', () => { }); describe('multiple SessionPool instances isolation', () => { - test('should use unique persist keys by default', async () => { + test('should use unique ids by default', async () => { const pool1 = new SessionPool(); const pool2 = new SessionPool(); - // @ts-expect-error private symbol - expect(pool1.persistStateKey).not.toEqual(pool2.persistStateKey); + expect(pool1.id).not.toEqual(pool2.id); await pool1.teardown(); await pool2.teardown(); }); test("should not overwrite each other's persisted state", async () => { - const pool1 = new SessionPool({ maxPoolSize: 5 }); - const pool2 = new SessionPool({ maxPoolSize: 5 }); + const pool1 = new SessionPool({ id: 'pool-1', maxPoolSize: 5 }); + const pool2 = new SessionPool({ id: 'pool-2', maxPoolSize: 5 }); for (let i = 0; i < 3; i++) await pool1.getSession(); for (let i = 0; i < 5; i++) await pool2.getSession(); @@ -499,12 +456,10 @@ describe('SessionPool - testing session pool', () => { await pool2.persistState(); const pool1Reloaded = new SessionPool({ - // @ts-expect-error private symbol - persistStateKey: pool1.persistStateKey, + id: 'pool-1', }); const pool2Reloaded = new SessionPool({ - // @ts-expect-error private symbol - persistStateKey: pool2.persistStateKey, + id: 'pool-2', }); expect((await pool1Reloaded.getState()).sessions).toHaveLength(3); diff --git a/test/core/storages/key_value_store.test.ts b/test/core/storages/key_value_store.test.ts index 328b66923fbc..fff61bdfe3ee 100644 --- a/test/core/storages/key_value_store.test.ts +++ b/test/core/storages/key_value_store.test.ts @@ -25,10 +25,7 @@ describe('KeyValueStore', () => { const recordStr = JSON.stringify(record, null, 2); // Set record - const mockSetValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'setValue') - .mockResolvedValueOnce(undefined); + const mockSetValue = vitest.spyOn(store.backend, 'setValue').mockResolvedValueOnce(undefined); await store.setValue('key-1', record); @@ -40,15 +37,12 @@ describe('KeyValueStore', () => { }); // Get Record - const mockGetValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'getValue') - .mockResolvedValueOnce({ - key: 'key-1', - // The client now returns raw bytes; the frontend parses them. - value: Buffer.from(recordStr), - contentType: 'application/json; charset=utf-8', - }); + const mockGetValue = vitest.spyOn(store.backend, 'getValue').mockResolvedValueOnce({ + key: 'key-1', + // The client now returns raw bytes; the frontend parses them. + value: Buffer.from(recordStr), + contentType: 'application/json; charset=utf-8', + }); const response = await store.getValue('key-1'); @@ -57,10 +51,7 @@ describe('KeyValueStore', () => { expect(response).toEqual(record); // Record Exists - const mockRecordExists = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'recordExists') - .mockResolvedValueOnce(true); + const mockRecordExists = vitest.spyOn(store.backend, 'recordExists').mockResolvedValueOnce(true); const exists = await store.recordExists('key-1'); @@ -69,10 +60,7 @@ describe('KeyValueStore', () => { expect(exists).toBe(true); // Delete Record - const mockDeleteValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'deleteValue') - .mockResolvedValueOnce(undefined); + const mockDeleteValue = vitest.spyOn(store.backend, 'deleteValue').mockResolvedValueOnce(undefined); await store.setValue('key-1', null); @@ -80,10 +68,7 @@ describe('KeyValueStore', () => { expect(mockDeleteValue).toHaveBeenCalledWith('key-1'); // Drop store - const mockDrop = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'drop') - .mockResolvedValueOnce(undefined); + const mockDrop = vitest.spyOn(store.backend, 'drop').mockResolvedValueOnce(undefined); await store.drop(); @@ -234,10 +219,7 @@ describe('KeyValueStore', () => { test('correctly adds charset to content type', async () => { const store = await KeyValueStore.open(); - const mockSetValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'setValue') - .mockResolvedValueOnce(undefined); + const mockSetValue = vitest.spyOn(store.backend, 'setValue').mockResolvedValueOnce(undefined); await store.setValue('key-1', 'xxxx', { contentType: 'text/plain; charset=utf-8' }); @@ -255,10 +237,7 @@ describe('KeyValueStore', () => { const record = { foo: 'bar' }; const recordStr = JSON.stringify(record, null, 2); - const mockSetValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'setValue') - .mockResolvedValueOnce(undefined); + const mockSetValue = vitest.spyOn(store.backend, 'setValue').mockResolvedValueOnce(undefined); await store.setValue('key-1', record); @@ -273,10 +252,7 @@ describe('KeyValueStore', () => { test('correctly passes raw string values', async () => { const store = await KeyValueStore.open(); - const mockSetValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'setValue') - .mockResolvedValueOnce(undefined); + const mockSetValue = vitest.spyOn(store.backend, 'setValue').mockResolvedValueOnce(undefined); await store.setValue('key-1', 'xxxx', { contentType: 'text/plain; charset=utf-8' }); @@ -291,10 +267,7 @@ describe('KeyValueStore', () => { test('correctly passes raw Buffer values', async () => { const store = await KeyValueStore.open(); - const mockSetValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'setValue') - .mockResolvedValueOnce(undefined); + const mockSetValue = vitest.spyOn(store.backend, 'setValue').mockResolvedValueOnce(undefined); const value = Buffer.from('some text value'); await store.setValue('key-1', value, { contentType: 'image/jpeg; charset=something' }); @@ -310,10 +283,7 @@ describe('KeyValueStore', () => { test('correctly passes a stream', async () => { const store = await KeyValueStore.open(); - const mockSetValue = vitest - // @ts-expect-error Accessing private property - .spyOn(store.backend, 'setValue') - .mockResolvedValueOnce(undefined); + const mockSetValue = vitest.spyOn(store.backend, 'setValue').mockResolvedValueOnce(undefined); const value = new PassThrough(); await store.setValue('key-1', value, { contentType: 'plain/text' }); @@ -515,7 +485,6 @@ describe('KeyValueStore', () => { test('should work remotely', async () => { const store = await KeyValueStore.open(); - // @ts-expect-error Accessing private property const mockListKeys = vitest.spyOn(store.backend, 'listKeys'); mockListKeys.mockResolvedValueOnce({ items: [ diff --git a/test/core/storages/request_queue.test.ts b/test/core/storages/request_queue.test.ts index e693037effe2..a115f5d8bccf 100644 --- a/test/core/storages/request_queue.test.ts +++ b/test/core/storages/request_queue.test.ts @@ -178,9 +178,6 @@ describe('RequestQueue remote', () => { // First pass: every request is new, so all are submitted once. await queue.addRequestsBatched(urls, options); expect(submittedCount).toBe(5); - // The heavy `requestCache` still only remembers the first batch; the background batches are - // deduplicated by the lightweight cache instead. - expect(queue['requestCache'].length()).toBe(2); // Second pass with the same URLs: everything is already enqueued, so nothing is re-submitted. // Before the fix, the 3 requests outside the first batch would be sent again (submittedCount === 8). @@ -680,8 +677,10 @@ describe('RequestQueue background batches', () => { // Previously the async promise executor swallowed the throw: this promise never settled at all. await expect(result.waitForAllRequestsToBeAdded).rejects.toThrow('backend exploded'); - // ...and the in-flight batch counter stayed stuck, so the queue claimed to be unfinished forever. - await sleep(10); - expect(queue['inProgressRequestBatchCount']).toBe(0); + // ...and the in-flight batch counter was reset so the queue can finish once handled. + const req = await queue.fetchNextRequest(); + expect(req).toBeDefined(); + await queue.markRequestAsHandled(req!); + expect(await queue.isFinished()).toBe(true); }, 10_000); });