diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 7444607e62b7..c49e512218f3 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -259,6 +259,7 @@ class Driver { } /** + * timeout is used for the next call to 'sendCommand'. * NOTE: This can eventually be replaced when TypeScript * resolves https://github.com/Microsoft/TypeScript/issues/5453. * @param {number} timeout @@ -268,7 +269,7 @@ class Driver { } /** - * Call protocol methods. + * Call protocol methods, with a timeout. * To configure the timeout for the next call, use 'setNextProtocolTimeout'. * @template {keyof LH.CrdpCommands} C * @param {C} method @@ -278,30 +279,42 @@ class Driver { sendCommand(method, ...params) { const timeout = this._nextProtocolTimeout; this._nextProtocolTimeout = DEFAULT_PROTOCOL_TIMEOUT; - const domainCommand = /^(\w+)\.(enable|disable)$/.exec(method); - if (domainCommand) { - const enable = domainCommand[2] === 'enable'; - if (!this._shouldToggleDomain(domainCommand[1], enable)) { - return Promise.resolve(); - } - } return new Promise(async (resolve, reject) => { - const asyncTimeout = setTimeout((_ => { + const asyncTimeout = setTimeout((() => { const err = new LHError(LHError.errors.PROTOCOL_TIMEOUT); err.message += ` Method: ${method}`; reject(err); }), timeout); try { - const result = await this._connection.sendCommand(method, ...params); - clearTimeout(asyncTimeout); + const result = await this._innerSendCommand(method, ...params); resolve(result); } catch (err) { - clearTimeout(asyncTimeout); reject(err); + } finally { + clearTimeout(asyncTimeout); } }); } + /** + * Call protocol methods. + * @private + * @template {keyof LH.CrdpCommands} C + * @param {C} method + * @param {LH.CrdpCommands[C]['paramsType']} params + * @return {Promise} + */ + _innerSendCommand(method, ...params) { + const domainCommand = /^(\w+)\.(enable|disable)$/.exec(method); + if (domainCommand) { + const enable = domainCommand[2] === 'enable'; + if (!this._shouldToggleDomain(domainCommand[1], enable)) { + return Promise.resolve(); + } + } + return this._connection.sendCommand(method, ...params); + } + /** * Returns whether a domain is currently enabled. * @param {string} domain @@ -882,8 +895,8 @@ class Driver { // happen _after_ onload: https://crbug.com/768961 this.sendCommand('Page.enable'); this.sendCommand('Emulation.setScriptExecutionDisabled', {value: disableJS}); - this.setNextProtocolTimeout(30 * 1000); - this.sendCommand('Page.navigate', {url}); + // No timeout needed for Page.navigate. See #6413. + this._innerSendCommand('Page.navigate', {url}); if (waitForNavigated) { await this._waitForFrameNavigated(); diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 419ddd76005f..a68266a4bfcf 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -240,6 +240,32 @@ describe('Browser Driver', () => { }); }); + it('.sendCommand timesout when commands take too long', async () => { + class SlowConnection extends EventEmitter { + connect() { + return Promise.resolve(); + } + disconnect() { + return Promise.resolve(); + } + sendCommand() { + return new Promise(resolve => setTimeout(resolve, 15)); + } + } + const slowConnection = new SlowConnection(); + const driver = new Driver(slowConnection); + driver.setNextProtocolTimeout(25); + await driver.sendCommand('Page.enable'); + + driver.setNextProtocolTimeout(5); + try { + await driver.sendCommand('Page.disable'); + assert.fail('expected driver.sendCommand to timeout'); + } catch (err) { + assert.equal(err.code, 'PROTOCOL_TIMEOUT'); + } + }); + it('will request default traceCategories', () => { return driverStub.beginTrace().then(() => { const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');