Repository: GoogleChrome/lighthouse
Pull request #9828 (2019-10-11 20:45:32)

clients(lr): don't include html report assets in bundle
Lightrider doesn't support HTML output, so the entire assets module can be ignored.

7.01 MB -> 6.77 MB (3%) (the JS is much smaller now thanks to @wardpeet, so the impact is minimal)

patrickhulce: > 7.01 MB -> 6.77 MB (3%)

I'm sorry does this mean the lightrider bundle is **7** MB!? 😱 
connorjclark: locales are very heavy :(
connorjclark: it takes 80ms for node to parse the locales :P

@exterkamp and I agree it's not worth the complexity to split those off (like we are doing for devtools).

Patch:
diff --git a/.travis.yml b/.travis.yml
index be7f2b718..f5cf3ae9a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,7 +38,7 @@ script:
   - yarn test-clients
   - yarn test-viewer
   - yarn test-lantern
-  - yarn test-bundle
+  - yarn test-bundle || yarn test-bundle || yarn test-bundle
   - yarn i18n:checks
   - yarn dogfood-lhci
 before_cache:
diff --git a/build/build-bundle.js b/build/build-bundle.js
index a05679995..f1f32a3de 100644
--- a/build/build-bundle.js
+++ b/build/build-bundle.js
@@ -37,6 +37,8 @@ const locales = fs.readdirSync(__dirname + '/../lighthouse-core/lib/i18n/locales
 const isDevtools = file => path.basename(file).includes('devtools');
 /** @param {string} file */
 const isExtension = file => path.basename(file).includes('extension');
+/** @param {string} file */
+const isLightrider = file => path.basename(file).includes('lightrider');
 
 const BANNER = `// lighthouse, browserified. ${VERSION} (${COMMIT_HASH})\n`;
 const DEBUG = false; // true for sourcemaps
@@ -71,8 +73,9 @@ async function browserifyFile(entryPath, distPath) {
   // Don't include the desktop protocol connection.
   bundle.ignore(require.resolve('../lighthouse-core/gather/connections/cri.js'));
 
-  // Dont include the stringified report in DevTools.
-  if (isDevtools(entryPath)) {
+  // Don't include the stringified report in DevTools - see devtools-report-assets.js
+  // Don't include in Lightrider - HTML generation isn't supported, so report assets aren't needed.
+  if (isDevtools(entryPath) || isLightrider(entryPath)) {
     bundle.ignore(require.resolve('../lighthouse-core/report/html/html-report-assets.js'));
   }
 
diff --git a/build/build-lightrider-bundles.js b/build/build-lightrider-bundles.js
index 08cee17d7..0fa8c562a 100644
--- a/build/build-lightrider-bundles.js
+++ b/build/build-lightrider-bundles.js
@@ -10,6 +10,7 @@ const fs = require('fs');
 const path = require('path');
 const makeDir = require('make-dir');
 const bundleBuilder = require('./build-bundle.js');
+const {minifyFileTransform} = require('./build-utils.js');
 
 const distDir = path.join(__dirname, '..', 'dist', 'lightrider');
 const sourceDir = __dirname + '/../clients/lightrider';
@@ -37,7 +38,11 @@ function buildEntryPoint() {
 function buildReportGenerator() {
   browserify(generatorFilename, {standalone: 'ReportGenerator'})
     // Transform the fs.readFile etc into inline strings.
-    .transform('@wardpeet/brfs', {global: true, parserOpts: {ecmaVersion: 10}})
+    .transform('@wardpeet/brfs', {
+      readFileSyncTransform: minifyFileTransform,
+      global: true,
+      parserOpts: {ecmaVersion: 10},
+    })
     .bundle((err, src) => {
       if (err) throw err;
       fs.writeFileSync(bundleOutFile, src.toString());
diff --git a/build/build-utils.js b/build/build-utils.js
new file mode 100644
index 000000000..a8a4423b7
--- /dev/null
+++ b/build/build-utils.js
@@ -0,0 +1,37 @@
+/**
+ * @license Copyright 2019 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
+
+const stream = require('stream');
+const terser = require('terser');
+
+/**
+ * Minifies file which are read by fs.readFileSync (brfs)
+ *
+ * @param {string} file
+ */
+function minifyFileTransform(file) {
+  return new stream.Transform({
+    transform(chunk, enc, next) {
+      if (file.endsWith('.js')) {
+        const result = terser.minify(chunk.toString());
+        if (result.error) {
+          throw result.error;
+        }
+
+        this.push(result.code);
+      } else {
+        this.push(chunk);
+      }
+
+      next();
+    },
+  });
+}
+
+module.exports = {
+  minifyFileTransform,
+};
diff --git a/build/build-viewer.js b/build/build-viewer.js
index b8b490095..c3db40f4b 100644
--- a/build/build-viewer.js
+++ b/build/build-viewer.js
@@ -10,7 +10,6 @@ const path = require('path');
 const {promisify} = require('util');
 const readFileAsync = promisify(fs.readFile);
 const writeFileAsync = promisify(fs.writeFile);
-const stream = require('stream');
 
 const browserify = require('browserify');
 const cpy = require('cpy');
@@ -20,6 +19,7 @@ const lighthousePackage = require('../package.json');
 const makeDir = require('make-dir');
 const rimraf = require('rimraf');
 const terser = require('terser');
+const {minifyFileTransform} = require('./build-utils.js');
 
 const htmlReportAssets = require('../lighthouse-core/report/html/html-report-assets.js');
 const sourceDir = `${__dirname}/../lighthouse-viewer`;
@@ -99,30 +99,6 @@ async function html() {
   await safeWriteFileAsync(`${distDir}/index.html`, htmlSrc);
 }
 
-/**
- * Minifies file which are read by fs.readFileSync (brfs)
- *
- * @param {string} file
- */
-function minifyReadFileContent(file) {
-  return new stream.Transform({
-    transform(chunk, enc, next) {
-      if (file.endsWith('.js')) {
-        const result = terser.minify(chunk.toString());
-        if (result.error) {
-          throw result.error;
-        }
-
-        this.push(result.code);
-      } else {
-        this.push(chunk);
-      }
-
-      next();
-    },
-  });
-}
-
 /**
  * Combine multiple JS files into single viewer.js file.
  * @return {Promise<void>}
@@ -132,7 +108,7 @@ async function compileJs() {
   const generatorFilename = `${sourceDir}/../lighthouse-core/report/report-generator.js`;
   const generatorBrowserify = browserify(generatorFilename, {standalone: 'ReportGenerator'})
     .transform('@wardpeet/brfs', {
-      readFileSyncTransform: minifyReadFileContent,
+      readFileSyncTransform: minifyFileTransform,
     });
 
   /** @type {Promise<string>} */
diff --git a/lighthouse-core/audits/errors-in-console.js b/lighthouse-core/audits/errors-in-console.js
index 05cff456b..341204bff 100644
--- a/lighthouse-core/audits/errors-in-console.js
+++ b/lighthouse-core/audits/errors-in-console.js
@@ -28,6 +28,8 @@ const UIStrings = {
 
 const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
 
+/** @typedef {{ignoredPatterns?: Array<RegExp|string>}} AuditOptions */
+
 class ErrorLogs extends Audit {
   /**
    * @return {LH.Audit.Meta}
@@ -42,11 +44,41 @@ class ErrorLogs extends Audit {
     };
   }
 
+  /** @return {AuditOptions} */
+  static defaultOptions() {
+    return {};
+  }
+
+
+  /**
+   * @template {{description: string | undefined}} T
+   * @param {Array<T>} items
+   * @param {AuditOptions} options
+   * @return {Array<T>}
+   */
+  static filterAccordingToOptions(items, options) {
+    const {ignoredPatterns} = options;
+    if (!ignoredPatterns) return items;
+
+    return items.filter(({description}) => {
+      if (!description) return true;
+      for (const pattern of ignoredPatterns) {
+        if (pattern instanceof RegExp && pattern.test(description)) return false;
+        if (typeof pattern === 'string' && description.includes(pattern)) return false;
+      }
+
+      return true;
+    });
+  }
+
   /**
    * @param {LH.Artifacts} artifacts
+   * @param {LH.Audit.Context} context
    * @return {LH.Audit.Product}
    */
-  static audit(artifacts) {
+  static audit(artifacts, context) {
+    const auditOptions = /** @type {AuditOptions} */ (context.options);
+
     const consoleEntries = artifacts.ConsoleMessages;
     const runtimeExceptions = artifacts.RuntimeExceptions;
     /** @type {Array<{source: string, description: string|undefined, url: string|undefined}>} */
@@ -73,7 +105,10 @@ class ErrorLogs extends Audit {
         };
       });
 
-    const tableRows = consoleRows.concat(runtimeExRows);
+    const tableRows = ErrorLogs.filterAccordingToOptions(
+      consoleRows.concat(runtimeExRows),
+      auditOptions
+    ).sort((a, b) => (a.description || '').localeCompare(b.description || ''));
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index 7512c4570..7d45562a2 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -528,8 +528,12 @@ class Driver {
     this.setNextProtocolTimeout(timeout);
     const response = await this.sendCommand('Runtime.evaluate', evaluationParams);
     if (response.exceptionDetails) {
-      // An error occurred before we could even create a Promise, should be *very* rare
-      return Promise.reject(new Error(`Evaluation exception: ${response.exceptionDetails.text}`));
+      // An error occurred before we could even create a Promise, should be *very* rare.
+      // Also occurs when the expression is not valid JavaScript.
+      const errorMessage = response.exceptionDetails.exception ?
+        response.exceptionDetails.exception.description :
+        response.exceptionDetails.text;
+      return Promise.reject(new Error(`Evaluation exception: ${errorMessage}`));
     }
     // Protocol should always return a 'result' object, but it is sometimes undefined.  See #6026.
     if (response.result === undefined) {
diff --git a/package.json b/package.json
index f628bb17e..9696d3bbb 100644
--- a/package.json
+++ b/package.json
@@ -187,11 +187,15 @@
     },
     {
       "path": "./dist/viewer/src/viewer.js",
-      "maxSize": "76 kB"
+      "maxSize": "65 kB"
     },
     {
       "path": "./dist/lighthouse-dt-bundle.js",
       "maxSize": "400 kB"
+    },
+    {
+      "path": "./dist/lightrider/report-generator-bundle.js",
+      "maxSize": "50 kB"
     }
   ],
   "nyc": {
diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json
index 0df7875e3..588fc8007 100644
--- a/proto/sample_v2_round_trip.json
+++ b/proto/sample_v2_round_trip.json
@@ -600,6 +600,11 @@
                         "source": "other",
                         "url": "http://localhost:10200/dobetterweb/dbw_tester.html"
                     },
+                    {
+                        "description": "Error: An error\n    at http://localhost:10200/dobetterweb/dbw_tester.html:42:38",
+                        "source": "Runtime.exception",
+                        "url": "http://localhost:10200/dobetterweb/dbw_tester.html"
+                    },
                     {
                         "description": "Failed to load resource: the server responded with a status of 404 (Not Found)",
                         "source": "network",
@@ -614,11 +619,6 @@
                         "description": "Failed to load resource: the server responded with a status of 404 (Not Found)",
                         "source": "network",
                         "url": "http://localhost:10200/dobetterweb/unknown404.css?delay=200"
-                    },
-                    {
-                        "description": "Error: An error\n    at http://localhost:10200/dobetterweb/dbw_tester.html:42:38",
-                        "source": "Runtime.exception",
-                        "url": "http://localhost:10200/dobetterweb/dbw_tester.html"
                     }
                 ],
                 "type": "table"


Test patch:
diff --git a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
index 0ab54e3eb..3e98cbe37 100644
--- a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
+++ b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
@@ -56,6 +56,9 @@
 <!-- FAIL(errors-in-console): exception thrown -->
 <script type="text/javascript" id="error-time">throw new Error('A distinctive error');</script>
 
+<!-- PASS(errors-in-console): exception thrown but ignoredPatterns removes it -->
+<script type="text/javascript">throw new Error('An ignored error');</script>
+
 <!-- Add non-functional script to appear to be a Wordpress page for Stack Packs. -->
 <script type="example" src="fake-script-wp-includes"></script>
 
diff --git a/lighthouse-cli/test/smokehouse/dbw-config.js b/lighthouse-cli/test/smokehouse/dbw-config.js
index c4f87d835..468dfe344 100644
--- a/lighthouse-cli/test/smokehouse/dbw-config.js
+++ b/lighthouse-cli/test/smokehouse/dbw-config.js
@@ -22,4 +22,8 @@ module.exports = {
       'apple-touch-icon', // pull in apple touch icon to test `LinkElements`
     ],
   },
+  audits: [
+    // Test the `ignoredPatterns` audit option.
+    {path: 'errors-in-console', options: {ignoredPatterns: ['An ignored error']}},
+  ],
 };
diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
index 20a5f71d6..8a9d01213 100644
--- a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
+++ b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
@@ -184,16 +184,21 @@ const expectations = [
           score: 0,
           details: {
             items: [
-              {
-                source: 'network',
-                description: 'Failed to load resource: the server responded with a status of 404 (Not Found)',
-                url: 'http://localhost:10200/dobetterweb/unknown404.css?delay=200',
-              },
               {
                 source: 'other',
                 description: 'Application Cache Error event: Manifest fetch failed (404) http://localhost:10200/dobetterweb/clock.appcache',
                 url: 'http://localhost:10200/dobetterweb/dbw_tester.html',
               },
+              {
+                source: 'Runtime.exception',
+                description: /^Error: A distinctive error\s+at http:\/\/localhost:10200\/dobetterweb\/dbw_tester.html:\d+:\d+$/,
+                url: 'http://localhost:10200/dobetterweb/dbw_tester.html',
+              },
+              {
+                source: 'network',
+                description: 'Failed to load resource: the server responded with a status of 404 (Not Found)',
+                url: 'http://localhost:10200/dobetterweb/unknown404.css?delay=200',
+              },
               {
                 source: 'network',
                 description: 'Failed to load resource: the server responded with a status of 404 (Not Found)',
@@ -209,11 +214,6 @@ const expectations = [
                 description: 'Failed to load resource: the server responded with a status of 404 (Not Found)',
                 url: 'http://localhost:10200/dobetterweb/unknown404.css?delay=200',
               },
-              {
-                source: 'Runtime.exception',
-                description: /^Error: A distinctive error\s+at http:\/\/localhost:10200\/dobetterweb\/dbw_tester.html:\d+:\d+$/,
-                url: 'http://localhost:10200/dobetterweb/dbw_tester.html',
-              },
             ],
           },
         },
diff --git a/lighthouse-core/test/audits/errors-in-console-test.js b/lighthouse-core/test/audits/errors-in-console-test.js
index 482d52a0d..c9480cb1d 100644
--- a/lighthouse-core/test/audits/errors-in-console-test.js
+++ b/lighthouse-core/test/audits/errors-in-console-test.js
@@ -15,7 +15,7 @@ describe('Console error logs audit', () => {
     const auditResult = ErrorLogsAudit.audit({
       ConsoleMessages: [],
       RuntimeExceptions: [],
-    });
+    }, {options: {}});
     assert.equal(auditResult.numericValue, 0);
     assert.equal(auditResult.score, 1);
     assert.ok(!auditResult.displayValue, 0);
@@ -34,7 +34,7 @@ describe('Console error logs audit', () => {
         },
       ],
       RuntimeExceptions: [],
-    });
+    }, {options: {}});
     assert.equal(auditResult.numericValue, 0);
     assert.equal(auditResult.score, 1);
     assert.equal(auditResult.details.items.length, 0);
@@ -79,7 +79,7 @@ describe('Console error logs audit', () => {
           'executionContextId': 3,
         },
       }],
-    });
+    }, {options: {}});
 
     assert.equal(auditResult.numericValue, 3);
     assert.equal(auditResult.score, 0);
@@ -87,13 +87,13 @@ describe('Console error logs audit', () => {
     assert.equal(auditResult.details.items[0].url, 'http://www.example.com/favicon.ico');
     assert.equal(auditResult.details.items[0].description,
       'The server responded with a status of 404 (Not Found)');
-    assert.equal(auditResult.details.items[1].url, 'http://www.example.com/wsconnect.ws');
-    assert.equal(auditResult.details.items[1].description,
-      'WebSocket connection failed: Unexpected response code: 500');
-    assert.equal(auditResult.details.items[2].url,
+    assert.equal(auditResult.details.items[1].url,
       'http://example.com/fancybox.js');
-    assert.equal(auditResult.details.items[2].description,
+    assert.equal(auditResult.details.items[1].description,
       'TypeError: Cannot read property \'msie\' of undefined');
+    assert.equal(auditResult.details.items[2].url, 'http://www.example.com/wsconnect.ws');
+    assert.equal(auditResult.details.items[2].description,
+      'WebSocket connection failed: Unexpected response code: 500');
   });
 
   it('handle the case when some logs fields are undefined', () => {
@@ -106,7 +106,7 @@ describe('Console error logs audit', () => {
         },
       ],
       RuntimeExceptions: [],
-    });
+    }, {options: {}});
     assert.equal(auditResult.numericValue, 1);
     assert.equal(auditResult.score, 0);
     assert.equal(auditResult.details.items.length, 1);
@@ -137,7 +137,7 @@ describe('Console error logs audit', () => {
           'executionContextId': 3,
         },
       }],
-    });
+    }, {options: {}});
     assert.equal(auditResult.numericValue, 1);
     assert.equal(auditResult.score, 0);
     assert.equal(auditResult.details.items.length, 1);
@@ -145,4 +145,121 @@ describe('Console error logs audit', () => {
     assert.strictEqual(auditResult.details.items[0].description,
       'TypeError: Cannot read property \'msie\' of undefined');
   });
+
+  describe('options', () => {
+    it('does nothing with an empty pattern', () => {
+      const options = {ignoredPatterns: ''};
+      const result = ErrorLogsAudit.audit({
+        ConsoleMessages: [
+          {
+            entry: {
+              level: 'error',
+              source: 'network',
+              text: 'This is a simple error msg',
+            },
+          },
+        ],
+        RuntimeExceptions: [],
+      }, {options});
+
+      expect(result.score).toBe(0);
+      expect(result.details.items).toHaveLength(1);
+    });
+
+    it('does nothing with an empty description', () => {
+      const options = {ignoredPatterns: 'pattern'};
+      const result = ErrorLogsAudit.audit({
+        ConsoleMessages: [
+          {
+            entry: {
+              level: 'error',
+            },
+          },
+        ],
+        RuntimeExceptions: [],
+      }, {options});
+
+      expect(result.score).toBe(0);
+      expect(result.details.items).toHaveLength(1);
+    });
+
+    it('does nothing with an empty description', () => {
+      const options = {ignoredPatterns: 'pattern'};
+      const result = ErrorLogsAudit.audit({
+        ConsoleMessages: [
+          {
+            entry: {
+              level: 'error',
+            },
+          },
+        ],
+        RuntimeExceptions: [],
+      }, {options});
+
+      expect(result.score).toBe(0);
+      expect(result.details.items).toHaveLength(1);
+    });
+
+    it('filters console messages as a string', () => {
+      const options = {ignoredPatterns: ['simple']};
+      const result = ErrorLogsAudit.audit({
+        ConsoleMessages: [
+          {
+            entry: {
+              level: 'error',
+              source: 'network',
+              text: 'This is a simple error msg',
+            },
+          },
+        ],
+        RuntimeExceptions: [],
+      }, {options});
+
+      expect(result.score).toBe(1);
+      expect(result.details.items).toHaveLength(0);
+    });
+
+    it('filters console messages as a regex', () => {
+      const options = {ignoredPatterns: [/simple.*msg/]};
+      const result = ErrorLogsAudit.audit({
+        ConsoleMessages: [
+          {
+            entry: {
+              level: 'error',
+              source: 'network',
+              text: 'This is a simple error msg',
+            },
+          },
+        ],
+        RuntimeExceptions: [],
+      }, {options});
+
+      expect(result.score).toBe(1);
+      expect(result.details.items).toHaveLength(0);
+    });
+
+    it('filters exceptions with both regex and strings', () => {
+      const options = {ignoredPatterns: [/s.mple/i, 'really']};
+      const result = ErrorLogsAudit.audit({
+        ConsoleMessages: [],
+        RuntimeExceptions: [
+          {
+            exceptionDetails: {
+              url: 'http://example.com/url.js',
+              text: 'Simple Error: You messed up',
+            },
+          },
+          {
+            exceptionDetails: {
+              url: 'http://example.com/url.js',
+              text: 'Bad Error: You really messed up',
+            },
+          },
+        ],
+      }, {options});
+
+      expect(result.score).toBe(1);
+      expect(result.details.items).toHaveLength(0);
+    });
+  });
 });
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
index fe2621605..20f6dc01d 100644
--- a/lighthouse-core/test/results/sample_v2.json
+++ b/lighthouse-core/test/results/sample_v2.json
@@ -236,6 +236,11 @@
             "description": "Application Cache Error event: Manifest fetch failed (404) http://localhost:10200/dobetterweb/clock.appcache",
             "url": "http://localhost:10200/dobetterweb/dbw_tester.html"
           },
+          {
+            "source": "Runtime.exception",
+            "description": "Error: An error\n    at http://localhost:10200/dobetterweb/dbw_tester.html:42:38",
+            "url": "http://localhost:10200/dobetterweb/dbw_tester.html"
+          },
           {
             "source": "network",
             "description": "Failed to load resource: the server responded with a status of 404 (Not Found)",
@@ -250,11 +255,6 @@
             "source": "network",
             "description": "Failed to load resource: the server responded with a status of 404 (Not Found)",
             "url": "http://localhost:10200/dobetterweb/unknown404.css?delay=200"
-          },
-          {
-            "source": "Runtime.exception",
-            "description": "Error: An error\n    at http://localhost:10200/dobetterweb/dbw_tester.html:42:38",
-            "url": "http://localhost:10200/dobetterweb/dbw_tester.html"
           }
         ]
       }