fix: drop network events once test runs to completion

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
--   repo: git@github.com:pre-commit/pre-commit-hooks
-    rev: v2.2.3
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.4.0
     hooks:
     -   id: check-case-conflict
     -   id: check-executables-have-shebangs

__tests__/plugins/network.test.ts

@@ -311,15 +311,7 @@ describe('network', () => {
       res.writeHead(200, {
         'Content-Type': 'text/html',
       });
-      res.end(`
-        <script>
-      var delayedPromise = new Promise(r => setTimeout(r, 300));
-      setTimeout(() => {
-        var x = new XMLHttpRequest();
-        x.open("POST", "chunked.txt");
-        x.send(null);
-      }, 0);
-      </script>`);
+      res.end('');
     });
     server.route('/chunked.txt', async (req, res) => {
       res.writeHead(200, {
@@ -330,11 +322,16 @@ describe('network', () => {
       // response is never ended
     });
 
-    driver.page.on('console', (msg: any) => {
-      console.log('console', msg.text());
-    });
     await driver.page.goto(server.PREFIX + '/index');
-    await driver.page.evaluate(() => (window as any).delayedPromise);
+    await Promise.all([
+      driver.page.evaluate(() => {
+        // hangs forever
+        const x = new XMLHttpRequest();
+        x.open('GET', 'chunked.txt', true);
+        x.send();
+      }, [server.PREFIX + 'chunked.txt']),
+      delay(1000),
+    ]);
     const netinfo = await network.stop();
     await Gatherer.stop();
     expect(netinfo.length).toBe(2);

src/core/gatherer.ts

@@ -72,6 +72,12 @@ export class Gatherer {
     const page = await context.newPage();
     const client = await context.newCDPSession(page);
     const request = await apiRequest.newContext({ ...playwrightOptions });
+
+    // Register sig int handler to close the browser
+    process.on('SIGINT', async () => {
+      await Gatherer.closeBrowser();
+      process.exit(130);
+    });
     return { browser: Gatherer.browser, context, page, client, request };
   }
 
@@ -108,6 +114,14 @@ export class Gatherer {
     }
   }
 
+  static async closeBrowser() {
+    log(`Gatherer: closing browser`);
+    if (Gatherer.browser) {
+      await Gatherer.browser.close();
+      Gatherer.browser = null;
+    }
+  }
+
   /**
    * Starts recording all events related to the v8 devtools protocol
    * https://chromedevtools.github.io/devtools-protocol/v8/
@@ -131,10 +145,6 @@ export class Gatherer {
   }
 
   static async stop() {
-    log(`Gatherer: closing browser`);
-    if (Gatherer.browser) {
-      await Gatherer.browser.close();
-      Gatherer.browser = null;
-    }
+    await Gatherer.closeBrowser();
   }
 }

src/core/index.ts

@@ -47,7 +47,7 @@ export const journey = wrapFnWithLocation(
     options: JourneyOptions | string,
     callback: JourneyCallback
   ) => {
-    log(`register journey: ${JSON.stringify(options)}`);
+    log(`Journey register: ${JSON.stringify(options)}`);
     if (typeof options === 'string') {
       options = { name: options, id: options };
     }
@@ -59,7 +59,7 @@ export const journey = wrapFnWithLocation(
 
 export const step = wrapFnWithLocation(
   (location: Location, name: string, callback: VoidCallback) => {
-    log(`register step: ${name}`);
+    log(`Step register: ${name}`);
     return runner.currentJourney?.addStep(name, callback, location);
   }
 );

src/core/logger.ts

@@ -41,5 +41,5 @@ export function log(msg) {
     msg = JSON.stringify(msg);
   }
   const time = dim(cyan(`at ${parseInt(String(now()))} ms `));
-  console.log(time + italic(grey(msg)));
+  process.stderr.write(time + italic(grey(msg)) + '\n');
 }

src/plugins/network.ts

@@ -180,31 +180,25 @@ export class NetworkManager {
           networkEntry.response.mimeType = contentType.split(';')[0];
       })
     );
+    this._addBarrier(
+      page,
+      response.serverAddr().then(server => {
+        networkEntry.response.remoteIPAddress = server?.ipAddress;
+        networkEntry.response.remotePort = server?.port;
+      })
+    );
+    this._addBarrier(
+      page,
+      response.securityDetails().then(details => {
+        if (details) networkEntry.response.securityDetails = details;
+      })
+    );
   }
 
   private async _onRequestCompleted(request: Request) {
     const networkEntry = this._findNetworkEntry(request);
     if (!networkEntry) return;
 
-    const page = request.frame().page();
-    const response = await request.response();
-    // response can be null if the request failed
-    if (response != null) {
-      this._addBarrier(
-        page,
-        response.serverAddr().then(server => {
-          networkEntry.response.remoteIPAddress = server?.ipAddress;
-          networkEntry.response.remotePort = server?.port;
-        })
-      );
-      this._addBarrier(
-        page,
-        response.securityDetails().then(details => {
-          if (details) networkEntry.response.securityDetails = details;
-        })
-      );
-    }
-
     networkEntry.loadEndTime = epochTimeInSeconds();
     const timing = request.timing();
     const { loadEndTime, requestSentTime } = networkEntry;
@@ -281,6 +275,7 @@ export class NetworkManager {
       total,
     };
 
+    const page = request.frame().page();
     // For aborted/failed requests sizes will not be present
     this._addBarrier(
       page,
@@ -301,7 +296,14 @@ export class NetworkManager {
   }
 
   async stop() {
-    await Promise.all(this._barrierPromises);
+    /**
+     * Waiting for all network events is error prone and might hang the tests
+     * from getting closed forever when there are upstream bugs in browsers or
+     * Playwright. So we log and drop these events once the test run is completed
+     */
+    if (this._barrierPromises.size > 0) {
+      log(`Plugins: dropping ${this._barrierPromises.size} network events}`);
+    }
     const context = this.driver.context;
     context.off('request', this._onRequest.bind(this));
     context.off('response', this._onResponse.bind(this));