diff --git a/packages/basic-crawler/src/internals/basic-crawler.ts b/packages/basic-crawler/src/internals/basic-crawler.ts index 76f8dbabde55..a1e9c3393982 100644 --- a/packages/basic-crawler/src/internals/basic-crawler.ts +++ b/packages/basic-crawler/src/internals/basic-crawler.ts @@ -15,7 +15,6 @@ import type { IRequestManager, LoadedContext, ProxyInfo, - Request, RequestsLike, RequestTransform, RestrictedCrawlingContext, @@ -41,6 +40,7 @@ import { mergeCookies, NonRetryableError, purgeDefaultStorages, + Request, RequestListAdapter, RequestManagerTandem, RequestProvider, @@ -1209,7 +1209,7 @@ export class BasicCrawler[0]): Promise { + protected async handleSkippedRequest(options: Omit[0], 'url'>): Promise { if (options.reason === 'limit') { this.logOncePerRun( 'maxRequestsPerCrawl', @@ -1225,7 +1225,7 @@ export class BasicCrawler(); - const skippedBecauseOfMaxCrawlDepth = new Set(); + const skippedBecauseOfRobots = new Map(); + const skippedBecauseOfMaxCrawlDepth = new Map(); + + const normalizeSkippedRequest = (request: string | Source): Request => { + return request instanceof Request + ? request + : new Request(typeof request === 'string' ? { url: request } : { ...request, url: request.url! }); + }; const isAllowedBasedOnRobotsTxtFile = this.isAllowedBasedOnRobotsTxtFile.bind(this); const maxCrawlDepth = this.maxCrawlDepth; @@ -1271,9 +1277,10 @@ export class BasicCrawler maxCrawlDepth) { - skippedBecauseOfMaxCrawlDepth.add(url); + skippedBecauseOfMaxCrawlDepth.set(url, skippedRequest); continue; } @@ -1281,7 +1288,7 @@ export class BasicCrawler 0 ) { await Promise.all( - [...skippedBecauseOfRobots] - .map((url) => { - return this.handleSkippedRequest({ url, reason: 'robotsTxt' }); + [...skippedBecauseOfRobots.values()] + .map((request) => { + return this.handleSkippedRequest({ request, reason: 'robotsTxt' }); }) .concat( skippedBecauseOfLimit.map((request) => { - const url = typeof request === 'string' ? request : request.url!; - return this.handleSkippedRequest({ url, reason: 'limit' }); + return this.handleSkippedRequest({ + request: normalizeSkippedRequest(request), + reason: 'limit', + }); }), - [...skippedBecauseOfMaxCrawlDepth].map((url) => { - return this.handleSkippedRequest({ url, reason: 'depth' }); + [...skippedBecauseOfMaxCrawlDepth.values()].map((request) => { + return this.handleSkippedRequest({ request, reason: 'depth' }); }), ), ); @@ -1632,7 +1641,7 @@ export class BasicCrawler 0) { await Promise.all( skippedRequests.map((request) => { + const skippedRequest = + request instanceof Request + ? request + : new Request( + typeof request === 'string' ? { url: request } : { ...request, url: request.url! }, + ); + return onSkippedRequest({ - url: request.url, - reason: request.skippedReason ?? reason, + url: skippedRequest.url, + request: skippedRequest, + reason: (request as { skippedReason?: SkippedRequestReason }).skippedReason ?? reason, }) as Promise; }), ); @@ -418,7 +423,7 @@ export async function enqueueLinks( } async function createFilteredRequests() { - const skippedRequests: string[] = []; + const skippedRequests: Request[] = []; // No user provided patterns means we can skip an extra filtering step if (urlPatternObjects.length === 0) { @@ -427,7 +432,7 @@ export async function enqueueLinks( enqueueStrategyPatterns, urlExcludePatternObjects, options.strategy, - (url) => skippedRequests.push(url), + (request) => skippedRequests.push(request), ); } @@ -437,17 +442,16 @@ export async function enqueueLinks( urlPatternObjects, urlExcludePatternObjects, options.strategy, - (url) => skippedRequests.push(url), + (request) => skippedRequests.push(request), ); // ...then filter them by the enqueue links strategy (making this an AND check) - const filtered = filterRequestsByPatterns(generatedRequestsFromUserFilters, enqueueStrategyPatterns, (url) => - skippedRequests.push(url), + const filtered = filterRequestsByPatterns( + generatedRequestsFromUserFilters, + enqueueStrategyPatterns, + (request) => skippedRequests.push(request), ); - await reportSkippedRequests( - skippedRequests.map((url) => ({ url })), - 'filters', - ); + await reportSkippedRequests(skippedRequests, 'filters'); return filtered; } @@ -460,7 +464,7 @@ export async function enqueueLinks( if (requestsOverLimit?.length !== undefined && requestsOverLimit.length > 0) { await reportSkippedRequests( - requestsOverLimit.map((r) => ({ url: typeof r === 'string' ? r : r.url! })), + requestsOverLimit.map((request) => (typeof request === 'string' ? { url: request } : request)), 'enqueueLimit', ); } diff --git a/packages/core/src/enqueue_links/shared.ts b/packages/core/src/enqueue_links/shared.ts index 93fea17c6268..be80e53b87a3 100644 --- a/packages/core/src/enqueue_links/shared.ts +++ b/packages/core/src/enqueue_links/shared.ts @@ -49,7 +49,12 @@ export type RegExpInput = RegExp | RegExpObject; export type SkippedRequestReason = 'robotsTxt' | 'limit' | 'enqueueLimit' | 'filters' | 'redirect' | 'depth'; -export type SkippedRequestCallback = (args: { url: string; reason: SkippedRequestReason }) => Awaitable; +export type SkippedRequestCallback = (args: { + /** @deprecated Use `request.url` instead. */ + url: string; + request: Request; + reason: SkippedRequestReason; +}) => Awaitable; /** * @ignore @@ -171,18 +176,20 @@ export function createRequests( urlPatternObjects?: UrlPatternObject[], excludePatternObjects: UrlPatternObject[] = [], strategy?: EnqueueLinksOptions['strategy'], - onSkippedUrl?: (url: string) => void, + onSkippedRequest?: (request: Request) => void, ): Request[] { const excludePatternObjectMatchers = excludePatternObjects.map(createPatternObjectMatcher); const urlPatternObjectMatchers = urlPatternObjects?.map(createPatternObjectMatcher); return requestOptions .map((opts) => ({ url: typeof opts === 'string' ? opts : opts.url, opts })) - .filter(({ url }) => { + .filter(({ url, opts }) => { const matchesExcludePatterns = excludePatternObjectMatchers.some(({ match }) => match(url)); if (matchesExcludePatterns) { - onSkippedUrl?.(url); + onSkippedRequest?.( + new Request(typeof opts === 'string' ? { url: opts, enqueueStrategy: strategy } : opts), + ); } return !matchesExcludePatterns; @@ -205,7 +212,7 @@ export function createRequests( } // didn't match any positive pattern - onSkippedUrl?.(url); + onSkippedRequest?.(new Request(typeof opts === 'string' ? { url: opts, enqueueStrategy: strategy } : opts)); return null; }) .filter((request) => request) as Request[]; @@ -214,7 +221,7 @@ export function createRequests( export function filterRequestsByPatterns( requests: Request[], patterns?: UrlPatternObject[], - onSkippedUrl?: (url: string) => void, + onSkippedRequest?: (request: Request) => void, ): Request[] { if (!patterns?.length) { return requests; @@ -229,7 +236,7 @@ export function filterRequestsByPatterns( if (matchingPattern !== undefined) { filtered.push(request); } else { - onSkippedUrl?.(request.url); + onSkippedRequest?.(request); } } diff --git a/packages/http-crawler/src/internals/http-crawler.ts b/packages/http-crawler/src/internals/http-crawler.ts index be969884d92e..1aeadecbd331 100644 --- a/packages/http-crawler/src/internals/http-crawler.ts +++ b/packages/http-crawler/src/internals/http-crawler.ts @@ -562,7 +562,7 @@ export class HttpCrawler< request.noRetry = true; request.state = RequestState.SKIPPED; - await this.handleSkippedRequest({ url: request.url, reason: 'redirect' }); + await this.handleSkippedRequest({ request, reason: 'redirect' }); return; } diff --git a/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts b/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts index 6610362358a8..19d41b57ade6 100644 --- a/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts +++ b/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts @@ -658,7 +658,7 @@ export class AdaptivePlaywrightCrawler extends PlaywrightCrawler { request.noRetry = true; request.state = RequestState.SKIPPED; - await this.handleSkippedRequest({ url: request.url, reason: 'redirect' }); + await this.handleSkippedRequest({ request, reason: 'redirect' }); return; } diff --git a/test/core/crawlers/basic_crawler.test.ts b/test/core/crawlers/basic_crawler.test.ts index 676ab290918e..164e198644ad 100644 --- a/test/core/crawlers/basic_crawler.test.ts +++ b/test/core/crawlers/basic_crawler.test.ts @@ -264,6 +264,7 @@ describe('BasicCrawler', () => { options = { urls: ['https://example.com/1/', 'https://example.com/2/'], onSkippedRequest: onSkippedRequestMock, + userData: { source: 'crawl-depth-test' }, }; request = new Request({ url: 'https://example.com/', crawlDepth: 2 }); requestQueue = { @@ -291,8 +292,20 @@ describe('BasicCrawler', () => { const skippedRequests = onSkippedRequestMock.mock.calls.map((call) => call[0]); expect(skippedRequests).toHaveLength(2); - expect(skippedRequests[0]).toStrictEqual({ url: 'https://example.com/1/', reason: 'depth' }); - expect(skippedRequests[1]).toStrictEqual({ url: 'https://example.com/2/', reason: 'depth' }); + expect(skippedRequests[0]).toMatchObject({ + url: 'https://example.com/1/', + reason: 'depth', + request: { url: 'https://example.com/1/' }, + }); + expect(skippedRequests[1]).toMatchObject({ + url: 'https://example.com/2/', + reason: 'depth', + request: { url: 'https://example.com/2/' }, + }); + expect(skippedRequests[0].request).toBeInstanceOf(Request); + expect(skippedRequests[1].request).toBeInstanceOf(Request); + expect(skippedRequests[0].request.userData).toMatchObject({ source: 'crawl-depth-test' }); + expect(skippedRequests[1].request.userData).toMatchObject({ source: 'crawl-depth-test' }); }); it('should respect user provided transformRequestFunction', async () => { @@ -323,8 +336,10 @@ describe('BasicCrawler', () => { const skippedRequests = onSkippedRequestMock.mock.calls.map((call) => call[0]); expect(skippedRequests).toHaveLength(2); - expect(skippedRequests[0]).toStrictEqual({ url: 'https://example.com/1/', reason: 'filters' }); - expect(skippedRequests[1]).toStrictEqual({ url: 'https://example.com/2/', reason: 'filters' }); + expect(skippedRequests[0]).toMatchObject({ url: 'https://example.com/1/', reason: 'filters' }); + expect(skippedRequests[1]).toMatchObject({ url: 'https://example.com/2/', reason: 'filters' }); + expect(skippedRequests[0].request).toBeInstanceOf(Request); + expect(skippedRequests[1].request).toBeInstanceOf(Request); }); }); @@ -2233,9 +2248,10 @@ describe('BasicCrawler', () => { ]; for (const mock of [crawlerOnSkippedRequest, userOnSkippedRequest]) { - expect(mock.mock.calls.map((call) => call[0]).sort((a, b) => a.url.localeCompare(b.url))).toEqual( - skipped, - ); + const calls = mock.mock.calls.map((call) => call[0]).sort((a, b) => a.url.localeCompare(b.url)); + expect(calls).toMatchObject(skipped); + expect(calls[0].request).toBeInstanceOf(Request); + expect(calls[1].request).toBeInstanceOf(Request); } });