fix: BrowserCrawler closes ctx.page before errorHandler runs on navig… (#3655)

Co-authored-by: nikitachapovskii-dev <nikita.chapovskii@apify.com>
Co-authored-by: Jan Buchar <jan.buchar@apify.com>

Author: 3 people

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

@@ -686,16 +686,18 @@ export abstract class BrowserCrawler<
     }
 
     /**
-     * Marks session bad in case of navigation timeout.
+     * Marks session bad on navigation timeout, and stops in-flight page loading on any navigation error.
      */
     protected async _handleNavigationTimeout(crawlingContext: Context, error: Error): Promise<void> {
-        const { session } = crawlingContext;
+        const { session, page } = crawlingContext;
 
         if (error && error.constructor.name === 'TimeoutError') {
             handleRequestTimeout({ session, errorMessage: error.message });
         }
 
-        await crawlingContext.page.close();
+        // Fire-and-forget: no user code will run on this page after a failed navigation.
+        // Swallow rejections: the page may already be detached.
+        void page.evaluate(() => window.stop()).catch(() => {});
     }
 
     /**

packages/browser-pool/src/abstract-classes/browser-plugin.ts

@@ -42,6 +42,7 @@ export interface CommonBrowser {
 export interface CommonPage {
     close(...args: unknown[]): Promise<unknown>;
     url(): string | Promise<string>;
+    evaluate(pageFunction: ((...args: any[]) => unknown) | string, ...args: unknown[]): Promise<unknown>;
 }
 
 export interface BrowserPluginOptions<LibraryOptions> {

test/core/crawlers/browser_crawler.test.ts

@@ -291,6 +291,43 @@ describe('BrowserCrawler', () => {
         }
     });
 
+    test.concurrent('errorHandler has open page after non-timeout navigation error', async () => {
+        const localStorageEmulator = new MemoryStorageEmulator();
+        await localStorageEmulator.init();
+        const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
+
+        try {
+            const requestList = await RequestList.open({
+                sources: [{ url: `${serverAddress}/?q=1` }],
+            });
+
+            const pageClosedStates: boolean[] = [];
+
+            const browserCrawler = new (class extends BrowserCrawlerTest {
+                protected override async _navigationHandler(): Promise<HTTPResponse | null | undefined> {
+                    throw new Error('net::ERR_NAME_NOT_RESOLVED');
+                }
+            })({
+                browserPoolOptions: {
+                    browserPlugins: [puppeteerPlugin],
+                },
+                requestList,
+                requestHandler: async () => {},
+                maxRequestRetries: 1,
+                errorHandler: async (ctx) => {
+                    pageClosedStates.push(ctx.page.isClosed());
+                },
+            });
+
+            await browserCrawler.run();
+
+            expect(pageClosedStates).toHaveLength(1);
+            expect(pageClosedStates[0]).toBe(false);
+        } finally {
+            await localStorageEmulator.destroy();
+        }
+    });
+
     test.concurrent('should correctly track request.state', async () => {
         const localStorageEmulator = new MemoryStorageEmulator();
         await localStorageEmulator.init();