diff --git a/lighthouse-core/gather/connections/connection.js b/lighthouse-core/gather/connections/connection.js index 36b7396f2474..fefa60f677f6 100644 --- a/lighthouse-core/gather/connections/connection.js +++ b/lighthouse-core/gather/connections/connection.js @@ -95,13 +95,16 @@ class Connection { const callback = this._callbacks.get(object.id); this._callbacks.delete(object.id); - if (object.error) { - return this._handleRawError(object, callback); - } - log.formatProtocol('method <= browser OK', + // handleRawError returns or throws synchronously; wrap to put into promise chain. + return callback.resolve(Promise.resolve().then(_ => { + if (object.error) { + return this.handleRawError(object.error, callback.method); + } + + log.formatProtocol('method <= browser OK', {method: callback.method, params: object.result}, 'verbose'); - callback.resolve(object.result); - return; + return object.result; + })); } log.formatProtocol('<= event', {method: object.method, params: object.params}, 'verbose'); @@ -109,19 +112,24 @@ class Connection { } /** - * @param {{error: {message: string}}} object - * @param {{reject: function(*), method: string}} callback - * @private + * Handles error responses from the protocol, absorbing errors we don't care + * about and throwing on the rest. + * + * Currently the only error ignored is from defensive calls of `DOM.disable` + * when already disabled. + * @param {{message: string}} error + * @param {string} method Protocol method that received the error response. + * @throws {Error} + * @protected */ - _handleRawError(object, callback) { - // We proactively disable a few domains. Ignore any errors - if (object.error.message && object.error.message.includes('DOM agent hasn\'t been enabled')) { - callback.resolve(); + handleRawError(error, method) { + // We proactively disable the DOM domain. Ignore any errors. + if (error.message && error.message.includes('DOM agent hasn\'t been enabled')) { return; } - log.formatProtocol('method <= browser ERR', - {method: callback.method}, 'error'); - callback.reject(new Error(`Raw Protocol (${callback.method}) ${object.error.message}`)); + + log.formatProtocol('method <= browser ERR', {method}, 'error'); + throw new Error(`Protocol error (${method}): ${error.message}`); } /** diff --git a/lighthouse-core/gather/connections/extension.js b/lighthouse-core/gather/connections/extension.js index ba8a48dea2d1..f4eebf99498b 100644 --- a/lighthouse-core/gather/connections/extension.js +++ b/lighthouse-core/gather/connections/extension.js @@ -99,7 +99,7 @@ class ExtensionConnection extends Connection { /** * @override - * @param {!string} method + * @param {!string} command * @param {!Object} params * @return {!Promise} */ @@ -107,17 +107,26 @@ class ExtensionConnection extends Connection { return new Promise((resolve, reject) => { log.formatProtocol('method => browser', {method: command, params: params}, 'verbose'); if (!this._tabId) { - log.error('No tabId set for sendCommand'); + log.error('ExtensionConnection', 'No tabId set for sendCommand'); } + chrome.debugger.sendCommand({tabId: this._tabId}, command, params, result => { if (chrome.runtime.lastError) { - log.formatProtocol('method <= browser ERR', {method: command, params: result}, 'error'); - return reject(chrome.runtime.lastError); - } - - if (result.wasThrown) { - log.formatProtocol('method <= browser ERR', {method: command, params: result}, 'error'); - return reject(result.exceptionDetails); + // The error from the extension has a `message` property that is the + // stringified version of the actual protocol error object. + const message = chrome.runtime.lastError.message; + let error; + try { + error = JSON.parse(message); + } catch (e) {} + error = error || {message: 'Unknown debugger protocol error.'}; + + // handleRawError returns or throws synchronously, so try/catch awkwardly. + try { + return resolve(this.handleRawError(error, command)); + } catch (err) { + return reject(err); + } } log.formatProtocol('method <= browser OK', {method: command, params: result}, 'verbose'); diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 0290e32cd90c..8b8c28685111 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -155,7 +155,10 @@ class Driver { } else { resolve(result.result.value); } - }).catch(reject); + }).catch(err => { + clearTimeout(asyncTimeout); + reject(err); + }); }); } @@ -475,11 +478,7 @@ class Driver { // Disable any domains that could interfere or add overhead to the trace return this.sendCommand('Debugger.disable') .then(_ => this.sendCommand('CSS.disable')) - .then(_ => { - return this.sendCommand('DOM.disable') - // If it wasn't already enabled, it will throw; ignore these. See #861 - .catch(_ => {}); - }) + .then(_ => this.sendCommand('DOM.disable')) // Enable Page domain to wait for Page.loadEventFired .then(_ => this.sendCommand('Page.enable')) .then(_ => this.sendCommand('Tracing.start', tracingOpts));