Repository: GoogleChrome/lighthouse
Pull request #12839 (2021-07-30 16:03:35)

core(byte-efficiency): mark n/a if no network records in timespan
This resolves most of the errors we were seeing for a timespan run after page load.

The solution is to mark all BE audits N/A if there were no network records in timespan mode. Conceptually, this makes some sense since ms savings are calculated from throughput in timespan mode. 

#### Possible improvement:

Not all BE audits require network records. It was `LoadSimulator`, used to calculate throughput savings for all BE audits, which requires network records. Instead of always returning N/A, we could get throughput from the settings when using DT or simulated throttling, but not provided throttling. Provided throttling would need to be N/A without network records in timespan mode.


Patch:
diff --git a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
index 4935e24f1..3b428d994 100644
--- a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
+++ b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
@@ -114,11 +114,25 @@ class UnusedBytes extends Audit {
       devtoolsLog,
       settings,
     };
-
     const networkRecords = await NetworkRecords.request(devtoolsLog, context);
+
+    // Requesting load simulator requires non-empty network records.
+    // Timespans are not guaranteed to have any network activity.
+    // There are no bytes to be saved if no bytes were downloaded, so mark N/A if empty.
+    if (!networkRecords.length && gatherContext.gatherMode === 'timespan') {
+      return {
+        score: 1,
+        notApplicable: true,
+      };
+    }
+
     const [result, graph, simulator] = await Promise.all([
       this.audit_(artifacts, networkRecords, context),
-      PageDependencyGraph.request({trace, devtoolsLog}, context),
+      // Page dependency graph is only used in navigation mode.
+      // TODO(FR-COMPAT): Use dependency graph in timespan mode.
+      gatherContext.gatherMode === 'navigation' ?
+        PageDependencyGraph.request({trace, devtoolsLog}, context) :
+        null,
       LoadSimulator.request(simulatorOptions, context),
     ]);
 
@@ -200,7 +214,7 @@ class UnusedBytes extends Audit {
 
   /**
    * @param {ByteEfficiencyProduct} result
-   * @param {Node} graph
+   * @param {Node|null} graph
    * @param {Simulator} simulator
    * @param {LH.Artifacts['GatherContext']} gatherContext
    * @return {LH.Audit.Product}
@@ -209,11 +223,16 @@ class UnusedBytes extends Audit {
     const results = result.items.sort((itemA, itemB) => itemB.wastedBytes - itemA.wastedBytes);
 
     const wastedBytes = results.reduce((sum, item) => sum + item.wastedBytes, 0);
-    const wastedMs = gatherContext.gatherMode === 'navigation' ?
-      this.computeWasteWithTTIGraph(results, graph, simulator, {
+
+    let wastedMs;
+    if (gatherContext.gatherMode === 'navigation') {
+      if (!graph) throw Error('Page dependency graph should always be computed in navigation mode');
+      wastedMs = this.computeWasteWithTTIGraph(results, graph, simulator, {
         providedWastedBytesByUrl: result.wastedBytesByUrl,
-      }) :
-      this.computeWastedMsWithThroughput(wastedBytes, simulator);
+      });
+    } else {
+      wastedMs = this.computeWastedMsWithThroughput(wastedBytes, simulator);
+    }
 
     let displayValue = result.displayValue || '';
     if (typeof result.displayValue === 'undefined' && wastedBytes) {
diff --git a/lighthouse-core/audits/network-rtt.js b/lighthouse-core/audits/network-rtt.js
index 7a789776f..3b35357b1 100644
--- a/lighthouse-core/audits/network-rtt.js
+++ b/lighthouse-core/audits/network-rtt.js
@@ -7,6 +7,7 @@
 
 const Audit = require('./audit.js');
 const i18n = require('../lib/i18n/i18n.js');
+const NetworkRecords = require('../computed/network-records.js');
 const NetworkAnalysisComputed = require('../computed/network-analysis.js');
 
 const UIStrings = {
@@ -41,6 +42,14 @@ class NetworkRTT extends Audit {
    */
   static async audit(artifacts, context) {
     const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
+    const records = await NetworkRecords.request(devtoolsLog, context);
+    if (!records.length) {
+      return {
+        score: 1,
+        notApplicable: true,
+      };
+    }
+
     const analysis = await NetworkAnalysisComputed.request(devtoolsLog, context);
 
     /** @type {number} */
diff --git a/lighthouse-core/audits/network-server-latency.js b/lighthouse-core/audits/network-server-latency.js
index 9fac7d8ae..f7a849778 100644
--- a/lighthouse-core/audits/network-server-latency.js
+++ b/lighthouse-core/audits/network-server-latency.js
@@ -7,6 +7,7 @@
 
 const Audit = require('./audit.js');
 const i18n = require('../lib/i18n/i18n.js');
+const NetworkRecords = require('../computed/network-records.js');
 const NetworkAnalysisComputed = require('../computed/network-analysis.js');
 
 const UIStrings = {
@@ -41,6 +42,14 @@ class NetworkServerLatency extends Audit {
    */
   static async audit(artifacts, context) {
     const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
+    const records = await NetworkRecords.request(devtoolsLog, context);
+    if (!records.length) {
+      return {
+        score: 1,
+        notApplicable: true,
+      };
+    }
+
     const analysis = await NetworkAnalysisComputed.request(devtoolsLog, context);
 
     /** @type {number} */


Test patch:
diff --git a/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js b/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
index e02554528..e29629bd5 100644
--- a/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
+++ b/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
@@ -148,6 +148,15 @@ describe('Byte efficiency base audit', () => {
     });
   });
 
+  it('should not throw on invalid graph in timespan mode', () => {
+    assert.doesNotThrow(() => {
+      ByteEfficiencyAudit.createAuditProduct({
+        headings: baseHeadings,
+        items: [{wastedBytes: 350, totalBytes: 700, wastedPercent: 50}],
+      }, null, simulator, {gatherMode: 'timespan'});
+    });
+  });
+
   it('should populate KiB', () => {
     const result = ByteEfficiencyAudit.createAuditProduct({
       headings: baseHeadings,
@@ -329,4 +338,30 @@ describe('Byte efficiency base audit', () => {
     const result = await MockAudit.audit(artifacts, {settings, computedCache});
     expect(result.details.overallSavingsMs).toBeCloseTo(914.2695);
   });
+
+  it('should return n/a if no network records in timespan mode', async () => {
+    class MockAudit extends ByteEfficiencyAudit {
+      static audit_(artifacts, records) {
+        return {
+          items: records,
+          headings: [],
+        };
+      }
+    }
+
+    const artifacts = {
+      GatherContext: {gatherMode: 'timespan'},
+      traces: {defaultPass: trace},
+      devtoolsLogs: {defaultPass: []},
+    };
+    const computedCache = new Map();
+
+    const modestThrottling = {rttMs: 150, throughputKbps: 1000, cpuSlowdownMultiplier: 2};
+    const settings = {throttlingMethod: 'simulate', throttling: modestThrottling};
+    const result = await MockAudit.audit(artifacts, {settings, computedCache});
+    expect(result).toEqual({
+      notApplicable: true,
+      score: 1,
+    });
+  });
 });
diff --git a/lighthouse-core/test/audits/network-rtt-test.js b/lighthouse-core/test/audits/network-rtt-test.js
index a470c4229..bbdcda171 100644
--- a/lighthouse-core/test/audits/network-rtt-test.js
+++ b/lighthouse-core/test/audits/network-rtt-test.js
@@ -32,4 +32,14 @@ describe('Network RTT audit', () => {
       },
     ]);
   });
+
+  it('should return n/a if no network records', async () => {
+    const artifacts = {devtoolsLogs: {defaultPass: []}};
+    const result = await NetworkRTT.audit(artifacts, {computedCache: new Map()});
+
+    expect(result).toEqual({
+      notApplicable: true,
+      score: 1,
+    });
+  });
 });
diff --git a/lighthouse-core/test/audits/network-server-latency-test.js b/lighthouse-core/test/audits/network-server-latency-test.js
index 059be9f4d..621b53112 100644
--- a/lighthouse-core/test/audits/network-server-latency-test.js
+++ b/lighthouse-core/test/audits/network-server-latency-test.js
@@ -36,4 +36,14 @@ describe('Network Server Latency audit', () => {
       },
     ]);
   });
+
+  it('should return n/a if no network records', async () => {
+    const artifacts = {devtoolsLogs: {defaultPass: []}};
+    const result = await ServerLatency.audit(artifacts, {computedCache: new Map()});
+
+    expect(result).toEqual({
+      notApplicable: true,
+      score: 1,
+    });
+  });
 });
diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
index 55c68b311..5d7a4979e 100644
--- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js
+++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
@@ -174,11 +174,11 @@ describe('Fraggle Rock API', () => {
       const {auditResults, erroredAudits, notApplicableAudits} = getAuditsBreakdown(result.lhr);
       expect(auditResults.length).toMatchInlineSnapshot(`50`);
 
-      expect(notApplicableAudits.length).toMatchInlineSnapshot(`9`);
+      expect(notApplicableAudits.length).toMatchInlineSnapshot(`22`);
       expect(notApplicableAudits.map(audit => audit.id)).toContain('server-response-time');
 
       // TODO(FR-COMPAT): Reduce this number by handling the error, making N/A, or removing timespan support.
-      expect(erroredAudits.length).toMatchInlineSnapshot(`14`);
+      expect(erroredAudits.length).toMatchInlineSnapshot(`1`);
     });
   });