core(gather-runner): covert assertPageLoaded into soft failure

lighthouse-core/gather/gather-runner.js

@@ -138,14 +138,12 @@ class GatherRunner {
   }
 
   /**
-   * Throws an error if the original network request failed or wasn't found.
+   * Returns an error if the original network request failed or wasn't found.
    * @param {string} url The URL of the original requested page.
-   * @param {{online: boolean}} driver
    * @param {!Array<WebInspector.NetworkRequest>} networkRecords
+   * @return {?Error}
    */
-  static assertPageLoaded(url, driver, networkRecords) {
-    if (!driver.online) return;
-
+  static getPageLoadError(url, networkRecords) {
     const mainRecord = networkRecords.find(record => {
       // record.url is actual request url, so needs to be compared without any URL fragment.
       return URL.equalWithExcludedFragments(record.url, url);
@@ -162,7 +160,7 @@ class GatherRunner {
       log.error('GatherRunner', errorMessage, url);
       const error = new Error(`Unable to load page: ${errorMessage}`);
       error.code = 'PAGE_LOAD_ERROR';
-      throw error;
+      return error;
     }
   }
 
@@ -260,6 +258,7 @@ class GatherRunner {
     const passData = {};
 
     let pass = Promise.resolve();
+    let pageLoadError;
 
     if (config.recordTrace) {
       pass = pass.then(_ => {
@@ -280,9 +279,18 @@ class GatherRunner {
       log.log('status', status);
       const devtoolsLog = driver.endDevtoolsLog();
       const networkRecords = NetworkRecorder.recordsFromLogs(devtoolsLog);
-      GatherRunner.assertPageLoaded(options.url, driver, networkRecords);
       log.verbose('statusEnd', status);
 
+      pageLoadError = GatherRunner.getPageLoadError(options.url, networkRecords);
+      // If the driver was offline, a page load error is expected, so do not save it.
+      if (!driver.online) pageLoadError = null;
+
+      if (pageLoadError) {
+        gathererResults.LighthouseRunWarnings.push('Lighthouse was unable to reliably load the ' +
+          'page you requested. Make sure you are testing the correct URL and that the server is ' +
+          'properly responding to all requests.');
+      }
+
       // Expose devtoolsLog and networkRecords to gatherers
       passData.devtoolsLog = devtoolsLog;
       passData.networkRecords = networkRecords;
@@ -295,7 +303,9 @@ class GatherRunner {
       const status = `Retrieving: ${gatherer.name}`;
       return chain.then(_ => {
         log.log('status', status);
-        const artifactPromise = Promise.resolve().then(_ => gatherer.afterPass(options, passData));
+        const artifactPromise = pageLoadError ?
+          Promise.reject(pageLoadError) :
+          Promise.resolve().then(_ => gatherer.afterPass(options, passData));
         gathererResults[gatherer.name].push(artifactPromise);
         return GatherRunner.recoverOrThrow(artifactPromise);
       }).then(_ => {
@@ -319,8 +329,10 @@ class GatherRunner {
     const artifacts = {};
 
     // Nest LighthouseRunWarnings, if any, so they will be collected into artifact.
-    gathererResults.LighthouseRunWarnings = [gathererResults.LighthouseRunWarnings];
+    const uniqueWarnings = Array.from(new Set(gathererResults.LighthouseRunWarnings));
+    gathererResults.LighthouseRunWarnings = [uniqueWarnings];
 
+    const pageLoadFailures = [];
     return Object.keys(gathererResults).reduce((chain, gathererName) => {
       return chain.then(_ => {
         const phaseResultsPromises = gathererResults[gathererName];
@@ -336,9 +348,16 @@ class GatherRunner {
           // To reach this point, all errors are non-fatal, so return err to
           // runner to handle turning it into an error audit.
           artifacts[gathererName] = err;
+          // Track page load errors separately, so we can fail loudly if needed.
+          if (err.code === 'PAGE_LOAD_ERROR') pageLoadFailures.push(err);
         });
       });
     }, Promise.resolve()).then(_ => {
+      // Fail the run if more than 50% of all artifacts failed due to page load failure.
+      if (pageLoadFailures.length > Object.keys(artifacts).length * .5) {
+        throw pageLoadFailures[0];
+      }
+
       return artifacts;
     });
   }

lighthouse-core/test/gather/gather-runner-test.js

@@ -621,33 +621,31 @@ describe('GatherRunner', function() {
       /afterPass\(\) method/);
   });
 
-  describe('#assertPageLoaded', () => {
+  describe('#getPageLoadError', () => {
     it('passes when the page is loaded', () => {
       const url = 'http://the-page.com';
       const records = [{url}];
-      GatherRunner.assertPageLoaded(url, {online: true}, records);
+      assert.ok(!GatherRunner.getPageLoadError(url, records));
     });
 
     it('passes when the page is loaded, ignoring any fragment', () => {
       const url = 'http://example.com/#/page/list';
       const records = [{url: 'http://example.com'}];
-      GatherRunner.assertPageLoaded(url, {online: true}, records);
+      assert.ok(!GatherRunner.getPageLoadError(url, records));
     });
 
     it('throws when page fails to load', () => {
       const url = 'http://the-page.com';
       const records = [{url, failed: true, localizedFailDescription: 'foobar'}];
-      assert.throws(() => {
-        GatherRunner.assertPageLoaded(url, {online: true}, records);
-      }, /Unable.*foobar/);
+      const error = GatherRunner.getPageLoadError(url, records);
+      assert.ok(error && /Unable.*foobar/.test(error.message));
     });
 
     it('throws when page times out', () => {
       const url = 'http://the-page.com';
       const records = [];
-      assert.throws(() => {
-        GatherRunner.assertPageLoaded(url, {online: true}, records);
-      }, /Unable.*no document request/);
+      const error = GatherRunner.getPageLoadError(url, records);
+      assert.ok(error && /Unable.*no document request/.test(error.message));
     });
   });
 
@@ -901,13 +899,10 @@ describe('GatherRunner', function() {
         url,
         flags: {},
         config: new Config({}),
-      })
-        .then(_ => {
-          assert.ok(false);
-        }, error => {
-          assert.ok(true);
-          assert.ok(/net::ERR_NAME_NOT_RESOLVED/.test(error.message));
-        });
+      }).then(artifacts => {
+        assert.equal(artifacts.LighthouseRunWarnings.length, 1);
+        assert.ok(/unable.*load the page/.test(artifacts.LighthouseRunWarnings[0]));
+      });
     });
 
     it('resolves when domain name can\'t be resolved but is offline', () => {