Repository: elastic/synthetics (ts)

Issue:
DNS failures not represented correctly
There was a [suggestion](https://github.com/elastic/synthetics/issues/175#issuecomment-755012081) that this may have been fixed, however we still don’t appear to correctly represent DNS failures in the waterfall chart.

There are two different cases of where the failure can occur, and each behaves slightly differently:
* Where the root request fails (e.g. `await page.goto("http://www.thisisaninvaliddomain.com")`), this shows nothing in the waterfall chart
![image](https://user-images.githubusercontent.com/70908889/112473901-d7b3d680-8d66-11eb-91f3-0933856f92be.png)
* Where a non root network request results in a DNS failure, in which case we show a row/entry on the waterfall chart, but don’t show any DNS timing data for it (i.e. how long it took for the DNS lookup to take place, and fail)
![image](https://user-images.githubusercontent.com/70908889/112474590-bc959680-8d67-11eb-9348-4153a924a137.png)




Pull request:
fix: handle aborted/inflight network requests
+ fix #246 #190
+ We now properly calculate the request timings 
   - When the remote host connection cannot be made (Network failure)
   - When the response is never received for a given request
   - When the request in inflight is aborted 
   
+ Added test cases for all the scenarios and tested the confirmed the timing with chrome devtools. 

When the request is  queued and the request got aborted due to browser being explicited closed or the connection cannot be established - The timing data point would look like this explaining that the given request is stalled / blocked for the whole time frame. 
```
{
  blocked: 2.885,
  queueing: -1,
  proxy: -1,
  dns: -1,
  ssl: -1,
  connect: -1,
  send: -1,
  wait: -1,
  receive: -1,
  total: 2.885
}
```

Matching chrome devtools behaviour 
<img width="1200" alt="Screen Shot 2021-04-06 at 16 58 23" src="https://user-images.githubusercontent.com/3902525/113791814-c09cbd80-9744-11eb-9974-2b20a7ae8901.png">


Patch:
diff --git a/src/plugins/network.ts b/src/plugins/network.ts
index d092517..9eef885 100644
--- a/src/plugins/network.ts
+++ b/src/plugins/network.ts
@@ -31,8 +31,9 @@ import { getTimestamp } from '../helpers';
 
 export class NetworkManager {
   private _browser: BrowserInfo;
+  private _InflightRequestMap = new Map<string, NetworkInfo>();
+  results: Array<NetworkInfo> = [];
   _currentStep: Partial<Step> = null;
-  waterfallMap = new Map<string, NetworkInfo>();
 
   async start(client: CDPSession) {
     const { product } = await client.send('Browser.getVersion');
@@ -57,7 +58,7 @@ export class NetworkManager {
   }
 
   _findNetworkRecord(requestId: string) {
-    return this.waterfallMap.get(requestId);
+    return this._InflightRequestMap.get(requestId);
   }
 
   _onRequestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) {
@@ -96,14 +97,14 @@ export class NetworkManager {
        * Rewrite the map with new redirect id to not reset
        * the redirect request with original request
        */
-      this.waterfallMap.delete(requestId);
-      this.waterfallMap.set(
+      this._InflightRequestMap.delete(requestId);
+      this._InflightRequestMap.set(
         `redirect:${timestamp}:${requestId}`,
         redirectedRecord
       );
     }
 
-    this.waterfallMap.set(requestId, {
+    this._InflightRequestMap.set(requestId, {
       browser: this._browser,
       step: this._currentStep,
       timestamp: getTimestamp(),
@@ -166,28 +167,36 @@ export class NetworkManager {
 
   _onLoadingFinished(event: Protocol.Network.loadingFinishedPayload) {
     const { requestId, timestamp, encodedDataLength } = event;
-    const record = this._findNetworkRecord(requestId);
-    if (!record) {
-      return;
-    }
-    if (encodedDataLength >= 0) {
-      record.transferSize = encodedDataLength;
-      record.response &&
-        (record.response.encodedDataLength = encodedDataLength);
-    }
-    record.loadEndTime = timestamp;
-    record.timings = this.calculateTimings(record);
+    this._requestCompleted(requestId, timestamp, encodedDataLength);
   }
 
   _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) {
     const { requestId, timestamp } = event;
+    this._requestCompleted(requestId, timestamp);
+  }
+
+  _requestCompleted(
+    requestId: string,
+    endTime: number,
+    encodedDataLength?: number
+  ) {
     const record = this._findNetworkRecord(requestId);
     if (!record) {
       return;
     }
-    record.loadEndTime = timestamp;
+    if (encodedDataLength >= 0) {
+      record.transferSize = encodedDataLength;
+    }
+    record.loadEndTime = endTime;
     record.timings = this.calculateTimings(record);
+    this.results.push(record);
+    /**
+     * When there is DNS failure, browsers keeps retrying requests which
+     * might result in incorrect timings so we get remove them to measure only once
+     */
+    this._InflightRequestMap.delete(requestId);
   }
+
   /**
    * Account for missing response received event and also adjust the
    * response received event based on when first byte event was recorded
@@ -218,12 +227,6 @@ export class NetworkManager {
    * https://github.com/ChromeDevTools/devtools-frontend/blob/7f5478d8ceb7586f23f4073ab5c2085dac1ec26a/front_end/network/RequestTimingView.js#L98-L193
    */
   calculateTimings(record: NetworkInfo) {
-    const {
-      response,
-      requestSentTime,
-      loadEndTime,
-      responseReceivedTime,
-    } = record;
     const result: NetworkInfo['timings'] = {
       blocked: -1,
       queueing: -1,
@@ -236,9 +239,17 @@ export class NetworkManager {
       receive: -1,
       total: -1,
     };
-    if (response == null) {
-      return result;
-    }
+    const { requestSentTime, loadEndTime, responseReceivedTime } = record;
+
+    /**
+     * Handle when request failed and no responseReceived event was
+     * fired for that particular request
+     * Eg: connection refused by remote host
+     */
+    const response = record.response || {
+      timing: null,
+      fromServiceWorker: false,
+    };
 
     const toMilliseconds = (time: number) => (time === -1 ? -1 : time * 1000);
     const calculateDiffInMs = (
@@ -292,6 +303,11 @@ export class NetworkManager {
       calculateDiffInMs('total', start, end);
       calculateDiffInMs('blocked', start, middle);
       calculateDiffInMs('receive', middle, end);
+      // Check if the request is blocked/stalled for the whole timeframe and
+      // calculate timings appropriately for that request
+      if (!isFinite(result['blocked'])) {
+        result['blocked'] = result['total'];
+      }
       return result;
     }
     calculateDiffInMs(
@@ -338,6 +354,6 @@ export class NetworkManager {
   }
 
   stop() {
-    return [...this.waterfallMap.values()];
+    return this.results;
   }
 }


Test patch:
diff --git a/__tests__/plugins/network.test.ts b/__tests__/plugins/network.test.ts
index 94c2f85..b3fa368 100644
--- a/__tests__/plugins/network.test.ts
+++ b/__tests__/plugins/network.test.ts
@@ -37,6 +37,8 @@ describe('network', () => {
     await server.close();
   });
 
+  const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
+
   it('should capture network info', async () => {
     const driver = await Gatherer.setupDriver({ wsEndpoint });
     const network = new NetworkManager();
@@ -99,6 +101,69 @@ describe('network', () => {
     await Gatherer.dispose(driver);
   });
 
+  it('timings for aborted requests', async () => {
+    const driver = await Gatherer.setupDriver({ wsEndpoint });
+    const network = new NetworkManager();
+    await network.start(driver.client);
+
+    const delayTime = 20;
+    server.route('/delay100', async (req, res) => {
+      await delay(delayTime);
+      res.destroy();
+    });
+    server.route('/index', async (_, res) => {
+      res.setHeader('content-type', 'text/html');
+      res.end(`<script src=${server.PREFIX}/delay100 />`);
+    });
+
+    await driver.page.goto(server.PREFIX + '/index');
+    await Gatherer.dispose(driver);
+    const netinfo = await network.stop();
+    expect(netinfo.length).toBe(2);
+    expect(netinfo[1]).toMatchObject({
+      url: `${server.PREFIX}/delay100`,
+      status: 0,
+      response: null,
+      timings: expect.any(Object),
+    });
+    expect(netinfo[1].timings.total).toBeGreaterThan(delayTime);
+    expect(netinfo[1].timings.total).toEqual(netinfo[1].timings.blocked);
+  });
+
+  it('timings for chunked response', async () => {
+    const driver = await Gatherer.setupDriver({ wsEndpoint });
+    const network = new NetworkManager();
+    await network.start(driver.client);
+
+    const delayTime = 100;
+    server.route('/chunked', async (req, res) => {
+      res.writeHead(200, {
+        'content-type': 'application/javascript',
+      });
+      res.write('a');
+      await delay(delayTime);
+      res.write('b');
+      await delay(delayTime);
+      return res.end('c');
+    });
+    server.route('/index', async (_, res) => {
+      res.setHeader('content-type', 'text/html');
+      res.end(`<script src=${server.PREFIX}/chunked />`);
+    });
+
+    await driver.page.goto(server.PREFIX + '/index');
+    await Gatherer.dispose(driver);
+    const netinfo = await network.stop();
+    expect(netinfo.length).toBe(2);
+    expect(netinfo[1]).toMatchObject({
+      url: `${server.PREFIX}/chunked`,
+      status: 200,
+      response: expect.any(Object),
+      timings: expect.any(Object),
+    });
+    expect(netinfo[1].timings.total).toBeGreaterThan(delayTime);
+  });
+
   describe('waterfall timing calculation', () => {
     const getEvent = () => {
       return {