review

Author: nikitachapovskii-dev

packages/actor-scraper/web-scraper/src/internals/crawler_setup.ts

@@ -225,40 +225,35 @@ export class CrawlerSetup implements CrawlerSetupOptions {
         this.initPromise = this._initializeAsync();
     }
 
-    // DevToolsServer must be started only once per actor process
-    private static _devToolsStartPromise: Promise<void> | null = null;
-    private static _devToolsServer: any | null = null;
-    private static _devToolsExitHookRegistered = false;
+    private static devToolsStartPromise: Promise<DevToolsServer | null> | null =
+        null;
 
-    private static async _startDevToolsServerOnce(): Promise<void> {
-        if (this._devToolsStartPromise) return this._devToolsStartPromise;
+    private static async getDevToolsServer(): Promise<DevToolsServer | null> {
+        if (this.devToolsStartPromise) return this.devToolsStartPromise;
 
-        this._devToolsStartPromise = (async () => {
+        this.devToolsStartPromise = (async () => {
             const url = process.env.ACTOR_WEB_SERVER_URL;
             const portRaw = process.env.ACTOR_WEB_SERVER_PORT;
-
-            if (!url || !portRaw) return;
+            if (!url || !portRaw) return null;
 
             const port = Number(portRaw);
-            if (!Number.isFinite(port)) return;
+            if (!Number.isFinite(port)) return null;
 
-            this._devToolsServer = new DevToolsServer({
+            const server = new DevToolsServer({
                 containerHost: new URL(url).host,
-                devToolsServerPort: process.env.ACTOR_WEB_SERVER_PORT,
+                devToolsServerPort: port,
                 chromeRemoteDebuggingPort: CHROME_DEBUGGER_PORT,
             });
 
-            await this._devToolsServer.start();
+            await server.start();
 
-            if (!this._devToolsExitHookRegistered) {
-                this._devToolsExitHookRegistered = true;
-                Actor.on('exit', () => {
-                    this._devToolsServer?.stop?.();
-                });
-            }
+            Actor.on('exit', () => {
+                server.stop?.();
+            });
+            return server;
         })();
 
-        return this._devToolsStartPromise;
+        return this.devToolsStartPromise;
     }
 
     private async _initializeAsync() {
@@ -348,8 +343,7 @@ export class CrawlerSetup implements CrawlerSetupOptions {
                 preLaunchHooks: [
                     async () => {
                         if (!this.isDevRun) return;
-                        // eslint-disable-next-line no-underscore-dangle
-                        await CrawlerSetup._startDevToolsServerOnce();
+                        await CrawlerSetup.getDevToolsServer();
                     },
                 ],
             },

packages/actor-scraper/web-scraper/test/bundle.browser.test.ts

@@ -1,15 +1,5 @@
 import { launchPuppeteer } from '@crawlee/puppeteer';
 import type { Browser, Page } from 'puppeteer';
-import {
-    afterAll,
-    afterEach,
-    beforeAll,
-    beforeEach,
-    describe,
-    expect,
-    it,
-    vi,
-} from 'vitest';
 
 import { createBundle } from '../src/internals/bundle.browser';
 
@@ -109,40 +99,4 @@ describe('Bundle', () => {
             });
         });
     });
-
-    describe('DevToolsServer (regression)', () => {
-        it('starts only once even if preLaunchHook calls it multiple times', async () => {
-            const oldEnv = process.env;
-            process.env = { ...oldEnv };
-
-            process.env.ACTOR_WEB_SERVER_URL = 'http://127.0.0.1:4321';
-            process.env.ACTOR_WEB_SERVER_PORT = '4321';
-
-            const startMock = vi.fn(async () => {});
-            const stopMock = vi.fn(() => {});
-            const DevToolsCtorMock = vi.fn(() => ({
-                start: startMock,
-                stop: stopMock,
-            }));
-
-            vi.resetModules();
-            vi.doMock('devtools-server', () => ({ default: DevToolsCtorMock }));
-            vi.doMock('apify', () => ({ Actor: { on: vi.fn() } }));
-
-            const mod = await import('../src/internals/crawler_setup.ts');
-            const { CrawlerSetup } = mod as any;
-
-            await Promise.all(
-                Array.from({ length: 10 }, () =>
-                    // eslint-disable-next-line no-underscore-dangle
-                    CrawlerSetup._startDevToolsServerOnce(),
-                ),
-            );
-
-            expect(DevToolsCtorMock).toHaveBeenCalledTimes(1);
-            expect(startMock).toHaveBeenCalledTimes(1);
-
-            process.env = oldEnv;
-        });
-    });
 });

packages/actor-scraper/web-scraper/test/devtools-server.regression.test.ts

@@ -0,0 +1,52 @@
+import { describe, expect, it, vi } from 'vitest';
+
+describe('DevToolsServer (regression)', () => {
+    it('starts only once even if preLaunchHook calls it multiple times', async () => {
+        const oldEnv = process.env;
+        const actorOnMock = vi.fn();
+
+        const startMock = vi.fn(async () => {});
+        const stopMock = vi.fn(() => {});
+        const DevToolsCtorMock = vi.fn(() => ({
+            start: startMock,
+            stop: stopMock,
+        }));
+
+        try {
+            process.env = { ...oldEnv };
+            process.env.ACTOR_WEB_SERVER_URL = 'http://127.0.0.1:4321';
+            process.env.ACTOR_WEB_SERVER_PORT = '4321';
+
+            vi.resetModules();
+            vi.doMock('devtools-server', () => ({ default: DevToolsCtorMock }));
+            vi.doMock('apify', () => ({ Actor: { on: actorOnMock } }));
+
+            const mod = await import(
+                new URL('../src/internals/crawler_setup.ts', import.meta.url)
+                    .href
+            );
+            const { CrawlerSetup } = mod as any;
+
+            CrawlerSetup.devToolsServerPromise = null;
+
+            const fn =
+                CrawlerSetup.getDevToolsServer ??
+                CrawlerSetup.startDevToolsServerOnce;
+
+            await Promise.all(
+                Array.from({ length: 10 }, () => fn.call(CrawlerSetup)),
+            );
+
+            expect(DevToolsCtorMock).toHaveBeenCalledTimes(1);
+            expect(startMock).toHaveBeenCalledTimes(1);
+            expect(actorOnMock).toHaveBeenCalledTimes(1);
+            expect(actorOnMock).toHaveBeenCalledWith(
+                'exit',
+                expect.any(Function),
+            );
+        } finally {
+            process.env = oldEnv;
+            vi.resetModules();
+        }
+    });
+});