core(gather-runner): include error status codes in pageLoadError (#6051)

GoogleChrome · Sep 18, 2018 · d73c715 · d73c715
1 parent 18e847b
commit d73c715
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 12 deletions.
diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -78,8 +78,8 @@ module.exports = [
     },
   },
   {
-    requestedUrl: BASE_URL + 'seo-failure-cases.html?status_code=403&' + failureHeaders,
-    finalUrl: BASE_URL + 'seo-failure-cases.html?status_code=403&' + failureHeaders,
+    requestedUrl: BASE_URL + 'seo-failure-cases.html?' + failureHeaders,
+    finalUrl: BASE_URL + 'seo-failure-cases.html?' + failureHeaders,
     audits: {
       'viewport': {
         score: 0,
@@ -91,8 +91,7 @@ module.exports = [
         score: 0,
       },
       'http-status-code': {
-        score: 0,
-        displayValue: '403',
+        score: 1,
       },
       'font-size': {
         rawValue: false,
@@ -137,4 +136,42 @@ module.exports = [
       },
     },
   },
+  {
+    // Note: most scores are null (audit error) because the page 403ed.
+    requestedUrl: BASE_URL + 'seo-failure-cases.html?status_code=403',
+    finalUrl: BASE_URL + 'seo-failure-cases.html?status_code=403',
+    audits: {
+      'http-status-code': {
+        score: 0,
+        displayValue: '403',
+      },
+      'viewport': {
+        score: null,
+      },
+      'document-title': {
+        score: null,
+      },
+      'meta-description': {
+        score: null,
+      },
+      'font-size': {
+        score: null,
+      },
+      'link-text': {
+        score: null,
+      },
+      'is-crawlable': {
+        score: null,
+      },
+      'hreflang': {
+        score: null,
+      },
+      'plugins': {
+        score: null,
+      },
+      'canonical': {
+        score: null,
+      },
+    },
+  },
 ];
diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
@@ -163,6 +163,9 @@ class GatherRunner {
     } else if (mainRecord.failed) {
       errorCode = LHError.errors.FAILED_DOCUMENT_REQUEST;
       errorReason = mainRecord.localizedFailDescription;
+    } else if (mainRecord.hasErrorStatusCode()) {
+      errorCode = LHError.errors.ERRORED_DOCUMENT_REQUEST;
+      errorReason = `Status code: ${mainRecord.statusCode}`;
     }
 
     if (errorCode) {

diff --git a/lighthouse-core/lib/lh-error.js b/lighthouse-core/lib/lh-error.js
@@ -139,11 +139,18 @@ const ERRORS = {
     message: strings.pageLoadFailed,
     lhrRuntimeError: true,
   },
+  /* Used when DevTools reports loading failed. Usually an internal (Chrome) issue. */
   FAILED_DOCUMENT_REQUEST: {
     code: 'FAILED_DOCUMENT_REQUEST',
     message: strings.pageLoadFailed,
     lhrRuntimeError: true,
   },
+  /* Used when status code is 4xx or 5xx. */
+  ERRORED_DOCUMENT_REQUEST: {
+    code: 'ERRORED_DOCUMENT_REQUEST',
+    message: strings.pageLoadFailed,
+    lhrRuntimeError: true,
+  },
 
   // Protocol internal failures
   TRACING_ALREADY_STARTED: {

diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js
@@ -105,6 +105,13 @@ module.exports = class NetworkRequest {
     this.isLinkPreload = false;
   }
 
+  /**
+   * @return {boolean}
+   */
+  hasErrorStatusCode() {
+    return this.statusCode >= 400;
+  }
+
   /**
    * @param {NetworkRequest} initiator
    */

diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
@@ -12,6 +12,7 @@ const GatherRunner = require('../../gather/gather-runner');
 const assert = require('assert');
 const Config = require('../../config/config');
 const unresolvedPerfLog = require('./../fixtures/unresolved-perflog.json');
+const NetworkRequest = require('../../lib/network-request.js');
 
 class TestGatherer extends Gatherer {
   constructor() {
@@ -606,31 +607,56 @@ describe('GatherRunner', function() {
   describe('#getPageLoadError', () => {
     it('passes when the page is loaded', () => {
       const url = 'http://the-page.com';
-      const records = [{url}];
-      assert.ok(!GatherRunner.getPageLoadError(url, records));
+      const mainRecord = new NetworkRequest();
+      mainRecord.url = url;
+      assert.ok(!GatherRunner.getPageLoadError(url, [mainRecord]));
     });
 
     it('passes when the page is loaded, ignoring any fragment', () => {
       const url = 'http://example.com/#/page/list';
-      const records = [{url: 'http://example.com'}];
-      assert.ok(!GatherRunner.getPageLoadError(url, records));
+      const mainRecord = new NetworkRequest();
+      mainRecord.url = 'http://example.com';
+      assert.ok(!GatherRunner.getPageLoadError(url, [mainRecord]));
     });
 
-    it('throws when page fails to load', () => {
+    it('fails when page fails to load', () => {
       const url = 'http://the-page.com';
-      const records = [{url, failed: true, localizedFailDescription: 'foobar'}];
-      const error = GatherRunner.getPageLoadError(url, records);
+      const mainRecord = new NetworkRequest();
+      mainRecord.url = url;
+      mainRecord.failed = true;
+      mainRecord.localizedFailDescription = 'foobar';
+      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
       assert.equal(error.message, 'FAILED_DOCUMENT_REQUEST');
       assert.ok(/Your page failed to load/.test(error.friendlyMessage));
     });
 
-    it('throws when page times out', () => {
+    it('fails when page times out', () => {
       const url = 'http://the-page.com';
       const records = [];
       const error = GatherRunner.getPageLoadError(url, records);
       assert.equal(error.message, 'NO_DOCUMENT_REQUEST');
       assert.ok(/Your page failed to load/.test(error.friendlyMessage));
     });
+
+    it('fails when page returns with a 404', () => {
+      const url = 'http://the-page.com';
+      const mainRecord = new NetworkRequest();
+      mainRecord.url = url;
+      mainRecord.statusCode = 404;
+      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
+      assert.ok(/Your page failed to load/.test(error.friendlyMessage));
+    });
+
+    it('fails when page returns with a 500', () => {
+      const url = 'http://the-page.com';
+      const mainRecord = new NetworkRequest();
+      mainRecord.url = url;
+      mainRecord.statusCode = 500;
+      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
+      assert.ok(/Your page failed to load/.test(error.friendlyMessage));
+    });
   });
 
   describe('artifact collection', () => {