core(driver): add timeout to getRequestContent (#4718)

lighthouse-core/gather/driver.js

@@ -9,6 +9,7 @@
 const NetworkRecorder = require('../lib/network-recorder');
 const emulation = require('../lib/emulation');
 const Element = require('../lib/element');
+const LHError = require('../lib/errors');
 const EventEmitter = require('events').EventEmitter;
 const URL = require('../lib/url-shim');
 const TraceParser = require('../lib/traces/trace-parser');
@@ -714,13 +715,25 @@ class Driver {
   /**
    * Return the body of the response with the given ID.
    * @param {string} requestId
+   * @param {number|undefined} timeout
    * @return {string}
    */
-  getRequestContent(requestId) {
-    return this.sendCommand('Network.getResponseBody', {
-      requestId,
-    // Ignoring result.base64Encoded, which indicates if body is already encoded
-    }).then(result => result.body);
+  getRequestContent(requestId, timeout = 1000) {
+    return new Promise((resolve, reject) => {
+      // If this takes more than 1s, reject the Promise.
+      // Why? Encoding issues can lead to hanging getResponseBody calls: https://github.com/GoogleChrome/lighthouse/pull/4718
+      const err = new LHError(LHError.errors.REQUEST_CONTENT_TIMEOUT);
+      const asyncTimeout = setTimeout((_ => reject(err)), timeout);
+
+      this.sendCommand('Network.getResponseBody', {requestId}).then(result => {
+        clearTimeout(asyncTimeout);
+        // Ignoring result.base64Encoded, which indicates if body is already encoded
+        resolve(result.body);
+      }).catch(e => {
+        clearTimeout(asyncTimeout);
+        reject(e);
+      });
+    });
   }
 
   /**

lighthouse-core/lib/errors.js

@@ -88,6 +88,9 @@ const ERRORS = {
   TRACING_ALREADY_STARTED: {message: strings.internalChromeError, pattern: /Tracing.*started/},
   PARSING_PROBLEM: {message: strings.internalChromeError, pattern: /Parsing problem/},
   READ_FAILED: {message: strings.internalChromeError, pattern: /Read failed/},
+
+  // Protocol timeout failures
+  REQUEST_CONTENT_TIMEOUT: {message: strings.requestContentTimeout},
 };
 
 Object.keys(ERRORS).forEach(code => ERRORS[code].code = code);

lighthouse-core/lib/strings.js

@@ -12,4 +12,5 @@ module.exports = {
   pageLoadTookTooLong: `Your page took too long to load. Please follow the opportunities in the report to reduce your page load time, and then try re-running Lighthouse.`,
   pageLoadFailed: `Your page failed to load. Verify that the URL is valid and re-run Lighthouse.`,
   internalChromeError: `An internal Chrome error occurred. Please restart Chrome and try re-running Lighthouse.`,
+  requestContentTimeout: 'Fetching resource content has exceeded the allotted time',
 };

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

@@ -17,6 +17,7 @@ const connection = new Connection();
 const driverStub = new Driver(connection);
 
 const redirectDevtoolsLog = require('../fixtures/wikipedia-redirect.devtoolslog.json');
+const MAX_WAIT_FOR_PROTOCOL = 20;
 
 function createOnceStub(events) {
   return (eventName, cb) => {
@@ -75,6 +76,8 @@ connection.sendCommand = function(command, params) {
       return Promise.resolve({frameTree: {frame: {id: 1}}});
     case 'Page.createIsolatedWorld':
       return Promise.resolve({executionContextId: 1});
+    case 'Network.getResponseBody':
+      return new Promise(res => setTimeout(res, MAX_WAIT_FOR_PROTOCOL + 20));
     case 'Page.enable':
     case 'Tracing.start':
     case 'ServiceWorker.enable':
@@ -138,6 +141,14 @@ describe('Browser Driver', () => {
     });
   });
 
+  it('throws if getRequestContent takes too long', () => {
+    return driverStub.getRequestContent(0, MAX_WAIT_FOR_PROTOCOL).then(_ => {
+      assert.ok(false, 'long-running getRequestContent supposed to reject');
+    }, e => {
+      assert.equal(e.code, 'REQUEST_CONTENT_TIMEOUT');
+    });
+  });
+
   it('evaluates an expression', () => {
     return driverStub.evaluateAsync('120 + 3').then(value => {
       assert.deepEqual(value, 123);