test: puppeteer.connect with using disconnects the browser (#12371)

2024-06-14 14:02:48 +00:00 · 2024-05-02 09:22:06 +02:00 · 2024-05-02 09:22:06 +02:00 · dfb31f3e92
commit dfb31f3e92
parent b5aab04cbc
10 changed files with 47 additions and 39 deletions
--- a/package.json
+++ b/package.json
@ -145,6 +145,7 @@
    "@typescript-eslint/eslint-plugin": "7.7.1",
    "@typescript-eslint/parser": "7.7.1",
    "esbuild": "0.20.2",
+    "eslint": "8.57.0",
    "eslint-config-prettier": "9.1.0",
    "eslint-import-resolver-typescript": "3.6.1",
    "eslint-plugin-import": "2.29.1",
@ -153,7 +154,6 @@
    "eslint-plugin-rulesdir": "0.2.2",
    "eslint-plugin-tsdoc": "0.2.17",
    "eslint-plugin-unused-imports": "3.1.0",
-    "eslint": "8.57.0",
    "execa": "8.0.1",
    "expect": "29.7.0",
    "gts": "5.3.0",
--- a/packages/puppeteer-core/src/api/Browser.ts
+++ b/packages/puppeteer-core/src/api/Browser.ts
@ -424,13 +424,19 @@ export abstract class Browser extends EventEmitter<BrowserEvents> {

  /** @internal */
  [disposeSymbol](): void {
+    if (this.process()) {
      return void this.close().catch(debugError);
    }
+    return void this.disconnect().catch(debugError);
+  }

  /** @internal */
  [asyncDisposeSymbol](): Promise<void> {
+    if (this.process()) {
      return this.close();
    }
+    return this.disconnect();
+  }

  /**
   * @internal
--- a/packages/testserver/src/index.ts
+++ b/packages/testserver/src/index.ts
@ -197,7 +197,7 @@ export class TestServer {
    this.#requestSubscribers.clear();
    for (const request of this.#requests.values()) {
      if (!request.writableEnded) {
-        request.end();
+        request.destroy();
      }
    }
    this.#requests.clear();
--- a/test/src/browser.spec.ts
+++ b/test/src/browser.spec.ts
@ -57,12 +57,11 @@ describe('Browser specs', function () {
      });

      const browserWSEndpoint = browser.wsEndpoint();
-      const remoteBrowser = await puppeteer.connect({
+      using remoteBrowser = await puppeteer.connect({
        browserWSEndpoint,
        protocol: browser.protocol,
      });
      expect(remoteBrowser.process()).toBe(null);
-      await remoteBrowser.disconnect();
    });
    it('should keep connected after the last page is closed', async () => {
      const {browser, close} = await launch({}, {createContext: false});
@ -90,7 +89,7 @@ describe('Browser specs', function () {
      });

      const browserWSEndpoint = browser.wsEndpoint();
-      const newBrowser = await puppeteer.connect({
+      using newBrowser = await puppeteer.connect({
        browserWSEndpoint,
        protocol: browser.protocol,
      });
--- a/test/src/browsercontext.spec.ts
+++ b/test/src/browsercontext.spec.ts
@ -222,13 +222,12 @@ describe('BrowserContext', function () {
    const context = await browser.createBrowserContext();
    try {
      expect(browser.browserContexts()).toHaveLength(2);
-      const remoteBrowser = await puppeteer.connect({
+      using remoteBrowser = await puppeteer.connect({
        browserWSEndpoint: browser.wsEndpoint(),
        protocol: browser.protocol,
      });
      const contexts = remoteBrowser.browserContexts();
      expect(contexts).toHaveLength(2);
-      await remoteBrowser.disconnect();
    } finally {
      await context.close();
    }
--- a/test/src/cdp/devtools.spec.ts
+++ b/test/src/cdp/devtools.spec.ts
@ -50,7 +50,7 @@ describe('DevTools', function () {

    const browserWSEndpoint = originalBrowser.wsEndpoint();

-    const browser = await puppeteer.connect({
+    using browser = await puppeteer.connect({
      browserWSEndpoint,
      _isPageTarget(target) {
        return (
@ -75,7 +75,7 @@ describe('DevTools', function () {

    const browserWSEndpoint = originalBrowser.wsEndpoint();

-    const browser = await puppeteer.connect({
+    using browser = await puppeteer.connect({
      browserWSEndpoint,
    });
    const devtoolsPageTarget = await browser.waitForTarget(target => {
--- a/test/src/chromiumonly.spec.ts
+++ b/test/src/chromiumonly.spec.ts
@ -21,7 +21,7 @@ describe('Chromium-Specific Launcher tests', function () {
      try {
        const browserURL = 'http://127.0.0.1:21222';

-        const browser1 = await puppeteer.connect({browserURL});
+        using browser1 = await puppeteer.connect({browserURL});
        const page1 = await browser1.newPage();
        expect(
          await page1.evaluate(() => {
@ -30,7 +30,7 @@ describe('Chromium-Specific Launcher tests', function () {
        ).toBe(56);
        await browser1.disconnect();

-        const browser2 = await puppeteer.connect({
+        using browser2 = await puppeteer.connect({
          browserURL: browserURL + '/',
        });
        const page2 = await browser2.newPage();
@ -39,7 +39,6 @@ describe('Chromium-Specific Launcher tests', function () {
            return 8 * 7;
          })
        ).toBe(56);
-        await browser2.disconnect();
      } finally {
        await close();
      }
--- a/test/src/fixtures.spec.ts
+++ b/test/src/fixtures.spec.ts
@ -101,7 +101,7 @@ describe('Fixtures', function () {
          wsEndPointCallback(output.substring(0, output.indexOf('\n')));
        }
      });
-      const browser = await puppeteer.connect({
+      using browser = await puppeteer.connect({
        browserWSEndpoint: await wsEndPointPromise,
      });
      const promises = [
--- a/test/src/launcher.spec.ts
+++ b/test/src/launcher.spec.ts
@ -31,13 +31,13 @@ describe('Launcher specs', function () {
        const {browser, close, puppeteer, server} = await launch({});
        server.setRoute('/one-style.css', () => {});
        try {
-          const remote = await puppeteer.connect({
+          using remote = await puppeteer.connect({
            browserWSEndpoint: browser.wsEndpoint(),
            protocol: browser.protocol,
          });
          const page = await remote.newPage();
          const navigationPromise = page
-            .goto(server.PREFIX + '/one-style.html', {timeout: 60000})
+            .goto(server.PREFIX + '/one-style.html', {timeout: 60_000})
            .catch(error_ => {
              return error_;
            });
@ -61,13 +61,13 @@ describe('Launcher specs', function () {
        const {browser, close, server, puppeteer} = await launch({});
        server.setRoute('/empty.html', () => {});
        try {
-          const remote = await puppeteer.connect({
+          using remote = await puppeteer.connect({
            browserWSEndpoint: browser.wsEndpoint(),
            protocol: browser.protocol,
          });
          const page = await remote.newPage();
          const watchdog = page
-            .waitForSelector('div', {timeout: 60000})
+            .waitForSelector('div', {timeout: 60_000})
            .catch(error_ => {
              return error_;
            });
@ -83,7 +83,7 @@ describe('Launcher specs', function () {
      it('should terminate network waiters', async () => {
        const {browser, close, server, puppeteer} = await launch({});
        try {
-          const remote = await puppeteer.connect({
+          using remote = await puppeteer.connect({
            browserWSEndpoint: browser.wsEndpoint(),
            protocol: browser.protocol,
          });
@ -119,9 +119,9 @@ describe('Launcher specs', function () {

      it('can launch multiple instances without node warnings', async () => {
        const instances = [];
-        let warning = null;
+        let warning: Error | undefined;
        const warningHandler: NodeJS.WarningListener = w => {
-          return (warning = w);
+          warning = w;
        };
        process.on('warning', warningHandler);
        process.setMaxListeners(1);
@ -138,7 +138,7 @@ describe('Launcher specs', function () {
          process.setMaxListeners(10);
        }
        process.off('warning', warningHandler);
-        expect(warning).toBe(null);
+        expect(warning?.stack).toBe(undefined);
      });
      it('should have default url when launching browser', async function () {
        const {browser, close} = await launch({}, {createContext: false});
@ -536,11 +536,10 @@ describe('Launcher specs', function () {
        }
      });
      it('should pass the timeout parameter to browser.waitForTarget', async () => {
-        const options = {
-          timeout: 1,
-        };
        let error!: Error;
-        await launch(options).catch(error_ => {
+        await launch({
+          timeout: 1,
+        }).catch(error_ => {
          return (error = error_);
        });
        expect(error).toBeInstanceOf(TimeoutError);
@ -660,7 +659,7 @@ describe('Launcher specs', function () {
      it('should be able to connect multiple times to the same browser', async () => {
        const {puppeteer, browser, close} = await launch({});
        try {
-          const otherBrowser = await puppeteer.connect({
+          using otherBrowser = await puppeteer.connect({
            browserWSEndpoint: browser.wsEndpoint(),
            protocol: browser.protocol,
          });
@ -685,7 +684,7 @@ describe('Launcher specs', function () {
      it('should be able to close remote browser', async () => {
        const {puppeteer, browser, close} = await launch({});
        try {
-          const remoteBrowser = await puppeteer.connect({
+          using remoteBrowser = await puppeteer.connect({
            browserWSEndpoint: browser.wsEndpoint(),
            protocol: browser.protocol,
          });
@ -707,7 +706,7 @@ describe('Launcher specs', function () {
              return page.close();
            })
          );
-          const remoteBrowser = await puppeteer.connect({
+          using remoteBrowser = await puppeteer.connect({
            browserWSEndpoint: browser.wsEndpoint(),
            protocol: browser.protocol,
          });
@ -729,7 +728,7 @@ describe('Launcher specs', function () {

        try {
          const browserWSEndpoint = browser.wsEndpoint();
-          const remoteBrowser = await puppeteer.connect({
+          using remoteBrowser = await puppeteer.connect({
            browserWSEndpoint,
            ignoreHTTPSErrors: true,
            protocol: browser.protocol,
@ -746,7 +745,6 @@ describe('Launcher specs', function () {
            .replace('v', ' ');
          expect(response!.securityDetails()!.protocol()).toBe(protocol);
          await page.close();
-          await remoteBrowser.close();
        } finally {
          await close();
        }
@ -791,7 +789,7 @@ describe('Launcher specs', function () {
          const page2 = await browser.newPage();
          await page2.goto(server.EMPTY_PAGE + '?should-be-ignored');

-          const remoteBrowser = await puppeteer.connect({
+          using remoteBrowser = await puppeteer.connect({
            browserWSEndpoint,
            targetFilter: target => {
              return !target.url().includes('should-be-ignored');
@ -819,6 +817,8 @@ describe('Launcher specs', function () {
      });
      it('should be able to reconnect to a disconnected browser', async () => {
        const {puppeteer, server, browser, close} = await launch({});
+        // Connection is closed on the original one
+        let remoteClose!: () => Promise<void>;
        try {
          const browserWSEndpoint = browser.wsEndpoint();
          const page = await browser.newPage();
@ -829,6 +829,8 @@ describe('Launcher specs', function () {
            browserWSEndpoint,
            protocol: browser.protocol,
          });
+          remoteClose = remoteBrowser.close.bind(remoteBrowser);
+          console.log(remoteClose);
          const pages = await remoteBrowser.pages();
          const restoredPage = pages.find(page => {
            return page.url() === server.PREFIX + '/frames/nested-frames.html';
@ -845,8 +847,8 @@ describe('Launcher specs', function () {
              return 7 * 8;
            })
          ).toBe(56);
-          await remoteBrowser.close();
        } finally {
+          await remoteClose();
          await close();
        }
      });
@ -855,7 +857,7 @@ describe('Launcher specs', function () {
        const {puppeteer, browser: browserOne, close} = await launch({});

        try {
-          const browserTwo = await puppeteer.connect({
+          using browserTwo = await puppeteer.connect({
            browserWSEndpoint: browserOne.wsEndpoint(),
            protocol: browserOne.protocol,
          });
@ -889,6 +891,8 @@ describe('Launcher specs', function () {
          browser: browserOne,
          close,
        } = await launch({});
+        // Connection is closed on the original one
+        let remoteClose!: () => Promise<void>;
        try {
          const browserWSEndpoint = browserOne.wsEndpoint();
          const pageOne = await browserOne.newPage();
@ -899,6 +903,7 @@ describe('Launcher specs', function () {
            browserWSEndpoint,
            protocol: browserOne.protocol,
          });
+          remoteClose = browserTwo.close.bind(browserTwo);
          const pages = await browserTwo.pages();
          const pageTwo = pages.find(page => {
            return page.url() === server.EMPTY_PAGE;
@ -907,8 +912,8 @@ describe('Launcher specs', function () {
          using _ = await pageTwo.waitForSelector('body', {
            timeout: 10000,
          });
-          await browserTwo.close();
        } finally {
+          await remoteClose();
          await close();
        }
      });
@ -993,11 +998,11 @@ describe('Launcher specs', function () {
      const {puppeteer, browser, close} = await launch({});
      try {
        const browserWSEndpoint = browser.wsEndpoint();
-        const remoteBrowser1 = await puppeteer.connect({
+        using remoteBrowser1 = await puppeteer.connect({
          browserWSEndpoint,
          protocol: browser.protocol,
        });
-        const remoteBrowser2 = await puppeteer.connect({
+        using remoteBrowser2 = await puppeteer.connect({
          browserWSEndpoint,
          protocol: browser.protocol,
        });
--- a/test/src/oopif.spec.ts
+++ b/test/src/oopif.spec.ts
@ -404,7 +404,7 @@ describe('OOPIF', function () {
    expect(page.frames()).toHaveLength(2);

    const browserURL = 'http://127.0.0.1:21222';
-    const browser1 = await puppeteer.connect({browserURL});
+    using browser1 = await puppeteer.connect({browserURL});
    const target = await browser1.waitForTarget(target => {
      return target.url().endsWith('dynamic-oopif.html');
    });