refactor: use BasicCrawler session in browser launch context (#3489)

Before, `BrowserCrawler` instances could generate a new `Session`,
overwriting the one created in `BasicCrawler`'s pipeline. The changes
from this PR enforce the use of the `BasicCrawler` session, aligning the
`BrowserCrawler` behaviour with `HttpCrawler` and
`AdaptivePlaywrightCrawler`.

Prerequisite: #3478

Author: barjin

packages/browser-crawler/src/internals/browser-crawler.ts

@@ -37,7 +37,7 @@ import type {
     LaunchContext,
 } from '@crawlee/browser-pool';
 import { BROWSER_CONTROLLER_EVENTS, BrowserPool } from '@crawlee/browser-pool';
-import type { BatchAddRequestsResult, Cookie as CookieObject, ProxyInfo } from '@crawlee/types';
+import type { BatchAddRequestsResult, Cookie as CookieObject } from '@crawlee/types';
 import type { RobotsTxtFile } from '@crawlee/utils';
 import { CLOUDFLARE_RETRY_CSS_SELECTORS, RETRY_CSS_SELECTORS, sleep } from '@crawlee/utils';
 import ow from 'ow';
@@ -391,12 +391,8 @@ export abstract class BrowserCrawler<
             browserPoolOptions.useFingerprints = false;
         }
 
-        const { preLaunchHooks = [], postLaunchHooks = [], ...rest } = browserPoolOptions;
-
         this.browserPool = new BrowserPool<InternalBrowserPoolOptions>({
-            ...(rest as any),
-            preLaunchHooks: [this._extendLaunchContext.bind(this), ...preLaunchHooks],
-            postLaunchHooks: [this._maybeAddSessionRetiredListener.bind(this), ...postLaunchHooks],
+            ...(browserPoolOptions as any),
         });
     }
 
@@ -469,25 +465,12 @@ export abstract class BrowserCrawler<
             id: crawlingContext.id,
         };
 
-        const useIncognitoPages = this.launchContext?.useIncognitoPages;
-
         if (crawlingContext.session?.proxyInfo) {
             const proxyInfo = crawlingContext.session.proxyInfo;
-            crawlingContext.proxyInfo = proxyInfo;
 
             newPageOptions.proxyUrl = proxyInfo?.url;
             newPageOptions.proxyTier = proxyInfo?.proxyTier;
-
-            if (proxyInfo?.ignoreTlsErrors) {
-                /**
-                 * @see https://playwright.dev/docs/api/class-browser/#browser-new-context
-                 * @see https://github.com/puppeteer/puppeteer/blob/main/docs/api.md
-                 */
-                newPageOptions.pageOptions = {
-                    ignoreHTTPSErrors: true,
-                    acceptInsecureCerts: true,
-                };
-            }
+            newPageOptions.ignoreTlsErrors = proxyInfo?.ignoreTlsErrors;
         }
 
         const page = (await this.browserPool.newPage(newPageOptions)) as Page;
@@ -497,11 +480,9 @@ export abstract class BrowserCrawler<
             page as any,
         ) as ProvidedController;
 
-        const contextEnqueueLinks = crawlingContext.enqueueLinks;
+        this.addSessionRetiredListener(crawlingContext.session, browserControllerInstance);
 
-        const session = useIncognitoPages
-            ? crawlingContext.session
-            : (browserControllerInstance.launchContext.session as Session);
+        const contextEnqueueLinks = crawlingContext.enqueueLinks;
 
         return {
             page,
@@ -511,8 +492,6 @@ export abstract class BrowserCrawler<
                 );
             },
             browserController: browserControllerInstance,
-            session,
-            proxyInfo: session?.proxyInfo,
             enqueueLinks: async (enqueueOptions: EnqueueLinksOptions = {}) => {
                 return (await browserCrawlerEnqueueLinks({
                     options: { ...enqueueOptions, limit: this.calculateEnqueuedRequestLimit(enqueueOptions?.limit) },
@@ -685,57 +664,37 @@ export abstract class BrowserCrawler<
         request.loadedUrl = await page.url();
     }
 
-    protected async _extendLaunchContext(_pageId: string, launchContext: LaunchContext): Promise<void> {
-        const launchContextExtends: { session?: Session; proxyInfo?: ProxyInfo } = {};
-
-        // Hacky access from `AdaptivePlaywrightCrawler` calls this without calling `.init()`.
-        // This is the only case where this.sessionPool is accessed without being initialized.
-        if (this.sessionPool) {
-            launchContextExtends.session = await this.sessionPool.newSession({
-                proxyInfo: await this.proxyConfiguration?.newProxyInfo({
-                    // cannot pass a request here, since session is created on browser launch
-                }),
-            });
-        }
+    private browserSessionIds = new WeakMap<Context['browserController'], Set<string>>();
 
-        if (!launchContext.proxyUrl && launchContextExtends.session?.proxyInfo) {
-            const proxyInfo = launchContextExtends.session.proxyInfo;
+    private addSessionRetiredListener(session: Session, browserController: Context['browserController']): void {
+        if (!this.sessionPool) {
+            return;
+        }
 
-            launchContext.proxyUrl = proxyInfo?.url;
-            launchContextExtends.proxyInfo = proxyInfo;
+        let sessionIds = this.browserSessionIds.get(browserController);
 
-            // Disable SSL verification for MITM proxies
-            if (proxyInfo?.ignoreTlsErrors) {
-                /**
-                 * @see https://playwright.dev/docs/api/class-browser/#browser-new-context
-                 * @see https://github.com/puppeteer/puppeteer/blob/main/docs/api.md
-                 */
-                (launchContext.launchOptions as Dictionary).ignoreHTTPSErrors = true;
-                (launchContext.launchOptions as Dictionary).acceptInsecureCerts = true;
-            }
+        if (sessionIds) {
+            sessionIds.add(session.id);
+            return;
         }
 
-        launchContext.extend(launchContextExtends);
-    }
+        sessionIds = new Set([session.id]);
+        this.browserSessionIds.set(browserController, sessionIds);
 
-    protected _maybeAddSessionRetiredListener(_pageId: string, browserController: Context['browserController']): void {
-        if (this.sessionPool) {
-            const listener = (session: Session) => {
-                const { launchContext } = browserController;
-                if (session.id === (launchContext.session as Session).id) {
-                    this.browserPool.retireBrowserController(
-                        browserController as Parameters<
-                            BrowserPool<InternalBrowserPoolOptions>['retireBrowserController']
-                        >[0],
-                    );
-                }
-            };
+        const listener = (retired: Session) => {
+            if (this.browserSessionIds.get(browserController)?.has(retired.id)) {
+                this.browserPool.retireBrowserController(
+                    browserController as Parameters<
+                        BrowserPool<InternalBrowserPoolOptions>['retireBrowserController']
+                    >[0],
+                );
+            }
+        };
 
-            this.sessionPool.on(EVENT_SESSION_RETIRED, listener);
-            browserController.on(BROWSER_CONTROLLER_EVENTS.BROWSER_CLOSED, () => {
-                return this.sessionPool!.removeListener(EVENT_SESSION_RETIRED, listener);
-            });
-        }
+        this.sessionPool.on(EVENT_SESSION_RETIRED, listener);
+        browserController.on(BROWSER_CONTROLLER_EVENTS.BROWSER_CLOSED, () => {
+            return this.sessionPool!.removeListener(EVENT_SESSION_RETIRED, listener);
+        });
     }
 
     /**

packages/browser-pool/src/browser-pool.ts

@@ -436,7 +436,14 @@ export class BrowserPool<
      * or their page limits have been exceeded.
      */
     async newPage(options: BrowserPoolNewPageOptions<PageOptions, BrowserPlugins[number]> = {}): Promise<PageReturn> {
-        const { id = nanoid(), pageOptions, browserPlugin = this._pickBrowserPlugin(), proxyUrl, proxyTier } = options;
+        const {
+            id = nanoid(),
+            pageOptions,
+            browserPlugin = this._pickBrowserPlugin(),
+            proxyUrl,
+            proxyTier,
+            ignoreTlsErrors,
+        } = options;
 
         if (this.pages.has(id)) {
             throw new Error(`Page with ID: ${id} already exists.`);
@@ -451,10 +458,15 @@ export class BrowserPool<
             let browserController = this._pickBrowserWithFreeCapacity(browserPlugin, { proxyTier, proxyUrl });
 
             if (!browserController)
-                browserController = await this._launchBrowser(id, { browserPlugin, proxyTier, proxyUrl });
+                browserController = await this._launchBrowser(id, {
+                    browserPlugin,
+                    proxyTier,
+                    proxyUrl,
+                    ignoreTlsErrors,
+                });
             tryCancel();
 
-            return await this._createPageForBrowser(id, browserController, pageOptions, proxyUrl);
+            return await this._createPageForBrowser(id, browserController, pageOptions, proxyUrl, ignoreTlsErrors);
         });
     }
 
@@ -552,6 +564,7 @@ export class BrowserPool<
         browserController: BrowserControllerReturn,
         pageOptions: PageOptions = {} as PageOptions,
         proxyUrl?: string,
+        ignoreTlsErrors?: boolean,
     ) {
         // 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.
@@ -563,6 +576,13 @@ export class BrowserPool<
 
         if (finalPageOptions) {
             Object.assign(finalPageOptions, browserController.normalizeProxyOptions(proxyUrl, pageOptions));
+
+            if (ignoreTlsErrors) {
+                Object.assign(finalPageOptions, {
+                    ignoreHTTPSErrors: true,
+                    acceptInsecureCerts: true,
+                });
+            }
         }
 
         await this._executeHooks(this.prePageCreateHooks, pageId, browserController, finalPageOptions);
@@ -684,7 +704,7 @@ export class BrowserPool<
     }
 
     private async _launchBrowser(pageId: string, options: InternalLaunchBrowserOptions<BrowserPlugins[number]>) {
-        const { browserPlugin, launchOptions, proxyTier, proxyUrl } = options;
+        const { browserPlugin, launchOptions, proxyTier, proxyUrl, ignoreTlsErrors } = options;
 
         const browserController = browserPlugin.createController() as BrowserControllerReturn;
         this.startingBrowserControllers.add(browserController);
@@ -696,6 +716,16 @@ export class BrowserPool<
             proxyUrl,
         });
 
+        // Disable SSL verification for MITM proxies
+        if (ignoreTlsErrors) {
+            /**
+             * @see https://playwright.dev/docs/api/class-browser/#browser-new-context
+             * @see https://github.com/puppeteer/puppeteer/blob/main/docs/api.md
+             */
+            (launchContext.launchOptions as Record<string, unknown>).ignoreHTTPSErrors = true;
+            (launchContext.launchOptions as Record<string, unknown>).acceptInsecureCerts = true;
+        }
+
         try {
             // If the hooks or the launch fails, we need to delete the controller,
             // because otherwise it would be stuck in limbo without a browser.
@@ -882,6 +912,11 @@ export interface BrowserPoolNewPageOptions<PageOptions, BP extends BrowserPlugin
      * Proxy tier.
      */
     proxyTier?: number;
+    /**
+     * Disable TLS certificate verification for MITM proxies.
+     * Applied both when launching a new browser and when creating a page in an existing one.
+     */
+    ignoreTlsErrors?: boolean;
 }
 
 export interface BrowserPoolNewPageInNewBrowserOptions<PageOptions, BP extends BrowserPlugin> {
@@ -919,4 +954,5 @@ interface InternalLaunchBrowserOptions<BP extends BrowserPlugin> {
     launchOptions?: BP['launchOptions'];
     proxyTier?: number;
     proxyUrl?: string;
+    ignoreTlsErrors?: boolean;
 }

test/core/crawlers/browser_crawler.test.ts

@@ -821,7 +821,7 @@ describe('BrowserCrawler', () => {
                 { url: `${serverAddress}/?q=6` },
             ]);
 
-            expect(sessionUsageHistory).toEqual([0, 1, 2, 3, 4, 5]);
+            expect(sessionUsageHistory).toEqual([0, 0, 0, 0, 0, 0]);
         } finally {
             await localStorageEmulator.destroy();
         }