Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(driver): recover from PROTOCOL_TIMEOUT in void calls #11508

Closed
wants to merge 1 commit into from
Closed

core(driver): recover from PROTOCOL_TIMEOUT in void calls #11508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 40 additions & 18 deletions lighthouse-core/gather/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,28 @@ class Driver {
return this.sendCommandToSession(method, undefined, ...params);
}

/**
* Version of sendCommand that tolerates PROTOCOL_TIMEOUTs if the browser is still
* responding and the caller didn't need to use the return value.
* @template {keyof LH.CrdpCommands} C
* @param {C} method
* @param {LH.CrdpCommands[C]['paramsType']} params
* @return {Promise<void>}
*/
async sendVoidCommand(method, ...params) {
try {
await this.sendCommandToSession(method, undefined, ...params);
} catch (err) {
if (err.code !== LHError.errors.PROTOCOL_TIMEOUT.code) throw err;
if (typeof err.protocolMethod !== 'string') throw err;

const isHung = await this.isPageHung();
if (!isHung) return;

throw new LHError(LHError.errors.CHROME_NOT_RESPONDING, {protocolMethod: err.protocolMethod});
}
}

/**
* Call protocol methods.
* @private
Expand Down Expand Up @@ -999,8 +1021,8 @@ class Driver {
log.warn('Driver', 'Timed out waiting for page load. Checking if page is hung...');
if (await this.isPageHung()) {
log.warn('Driver', 'Page appears to be hung, killing JavaScript...');
await this.sendCommand('Emulation.setScriptExecutionDisabled', {value: true});
await this.sendCommand('Runtime.terminateExecution');
await this.sendVoidCommand('Emulation.setScriptExecutionDisabled', {value: true});
await this.sendVoidCommand('Runtime.terminateExecution');
throw new LHError(LHError.errors.PAGE_HUNG);
}

Expand Down Expand Up @@ -1036,7 +1058,7 @@ class Driver {
// Reset back to empty
this._monitoredUrlNavigations = [];

return this.sendCommand('Network.enable');
return this.sendVoidCommand('Network.enable');
}

/**
Expand Down Expand Up @@ -1117,16 +1139,16 @@ class Driver {
await this._clearIsolatedContextId();

// Enable auto-attaching to subtargets so we receive iframe information
await this.sendCommand('Target.setAutoAttach', {
await this.sendVoidCommand('Target.setAutoAttach', {
flatten: true,
autoAttach: true,
// Pause targets on startup so we don't miss anything
waitForDebuggerOnStart: true,
});

await this.sendCommand('Page.enable');
await this.sendCommand('Page.setLifecycleEventsEnabled', {enabled: true});
await this.sendCommand('Emulation.setScriptExecutionDisabled', {value: disableJS});
await this.sendVoidCommand('Page.enable');
await this.sendVoidCommand('Page.setLifecycleEventsEnabled', {enabled: true});
await this.sendVoidCommand('Emulation.setScriptExecutionDisabled', {value: disableJS});
// No timeout needed for Page.navigate. See https://github.com/GoogleChrome/lighthouse/pull/6413.
const waitforPageNavigateCmd = this._innerSendCommand('Page.navigate', undefined, {url});

Expand Down Expand Up @@ -1310,7 +1332,7 @@ class Driver {
}

// Enable Page domain to wait for Page.loadEventFired
return this.sendCommand('Page.enable')
return this.sendVoidCommand('Page.enable')
.then(_ => this.sendCommand('Tracing.start', {
categories: uniqueCategories.join(','),
options: 'sampling-frequency=10000', // 1000 is default and too slow.
Expand Down Expand Up @@ -1374,9 +1396,9 @@ class Driver {
* @return {Promise<void>}
*/
async enableAsyncStacks() {
await this.sendCommand('Debugger.enable');
await this.sendCommand('Debugger.setSkipAllPauses', {skip: true});
await this.sendCommand('Debugger.setAsyncCallStackDepth', {maxDepth: 8});
await this.sendVoidCommand('Debugger.enable');
await this.sendVoidCommand('Debugger.setSkipAllPauses', {skip: true});
await this.sendVoidCommand('Debugger.setAsyncCallStackDepth', {maxDepth: 8});
}

/**
Expand Down Expand Up @@ -1413,7 +1435,7 @@ class Driver {
* @return {Promise<void>}
*/
async goOffline() {
await this.sendCommand('Network.enable');
await this.sendVoidCommand('Network.enable');
await emulation.goOffline(this);
this.online = false;
}
Expand All @@ -1437,10 +1459,10 @@ class Driver {
log.time(status);

// Wipe entire disk cache
await this.sendCommand('Network.clearBrowserCache');
await this.sendVoidCommand('Network.clearBrowserCache');
// Toggle 'Disable Cache' to evict the memory cache
await this.sendCommand('Network.setCacheDisabled', {cacheDisabled: true});
await this.sendCommand('Network.setCacheDisabled', {cacheDisabled: false});
await this.sendVoidCommand('Network.setCacheDisabled', {cacheDisabled: true});
await this.sendVoidCommand('Network.setCacheDisabled', {cacheDisabled: false});

log.timeEnd(status);
}
Expand Down Expand Up @@ -1482,7 +1504,7 @@ class Driver {
this.setNextProtocolTimeout(5000);

try {
await this.sendCommand('Storage.clearDataForOrigin', {
await this.sendVoidCommand('Storage.clearDataForOrigin', {
origin: origin,
storageTypes: typesToClear,
});
Expand Down Expand Up @@ -1565,7 +1587,7 @@ class Driver {
* @return {Promise<void>}
*/
blockUrlPatterns(urls) {
return this.sendCommand('Network.setBlockedURLs', {urls})
return this.sendVoidCommand('Network.setBlockedURLs', {urls})
.catch(err => {
// TODO(COMPAT): remove this handler once m59 hits stable
if (!/wasn't found/.test(err.message)) {
Expand All @@ -1589,7 +1611,7 @@ class Driver {
}).catch(err => log.warn('Driver', err));
});

await this.sendCommand('Page.enable');
await this.sendVoidCommand('Page.enable');
}
}

Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/gather/fetcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Fetcher {
if (this._enabled) return;

this._enabled = true;
await this.driver.sendCommand('Fetch.enable', {
await this.driver.sendVoidCommand('Fetch.enable', {
patterns: [{requestStage: 'Request'}, {requestStage: 'Response'}],
});
await this.driver.on('Fetch.requestPaused', this._onRequestPaused);
Expand All @@ -56,7 +56,7 @@ class Fetcher {

this._enabled = false;
await this.driver.off('Fetch.requestPaused', this._onRequestPaused);
await this.driver.sendCommand('Fetch.disable');
await this.driver.sendVoidCommand('Fetch.disable');
this._onRequestPausedHandlers.clear();
}

Expand Down
6 changes: 3 additions & 3 deletions lighthouse-core/gather/gatherers/anchor-elements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ class AnchorElements extends Gatherer {

/** @type {LH.Artifacts['AnchorElements']} */
const anchors = await driver.evaluateAsync(expression, {useIsolation: true});
await driver.sendCommand('DOM.enable');
await driver.sendVoidCommand('DOM.enable');

// DOM.getDocument is necessary for pushNodesByBackendIdsToFrontend to properly retrieve nodeIds if the `DOM` domain was enabled before this gatherer, invoke it to be safe.
await driver.sendCommand('DOM.getDocument', {depth: -1, pierce: true});
await driver.sendVoidCommand('DOM.getDocument', {depth: -1, pierce: true});
const anchorsWithEventListeners = anchors.map(async anchor => {
const listeners = await getEventListeners(driver, anchor.devtoolsNodePath);

Expand All @@ -118,7 +118,7 @@ class AnchorElements extends Gatherer {
});

const result = await Promise.all(anchorsWithEventListeners);
await driver.sendCommand('DOM.disable');
await driver.sendVoidCommand('DOM.disable');
return result;
}
}
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/gather/gatherers/console-messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ class ConsoleMessages extends Gatherer {
async beforePass(passContext) {
const driver = passContext.driver;
driver.on('Log.entryAdded', this._onConsoleEntryAdded);
await driver.sendCommand('Log.enable');
await driver.sendCommand('Log.startViolationsReport', {
await driver.sendVoidCommand('Log.enable');
await driver.sendVoidCommand('Log.startViolationsReport', {
config: [{name: 'discouragedAPIUse', threshold: -1}],
});
}
Expand All @@ -44,9 +44,9 @@ class ConsoleMessages extends Gatherer {
* @return {Promise<LH.Artifacts['ConsoleMessages']>}
*/
async afterPass(passContext) {
await passContext.driver.sendCommand('Log.stopViolationsReport');
await passContext.driver.sendVoidCommand('Log.stopViolationsReport');
await passContext.driver.off('Log.entryAdded', this._onConsoleEntryAdded);
await passContext.driver.sendCommand('Log.disable');
await passContext.driver.sendVoidCommand('Log.disable');
return this._logEntries;
}
}
Expand Down
10 changes: 5 additions & 5 deletions lighthouse-core/gather/gatherers/css-usage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ class CSSUsage extends Gatherer {
const onStylesheetAdded = sheet => stylesheets.push(sheet);
driver.on('CSS.styleSheetAdded', onStylesheetAdded);

await driver.sendCommand('DOM.enable');
await driver.sendCommand('CSS.enable');
await driver.sendCommand('CSS.startRuleUsageTracking');
await driver.sendVoidCommand('DOM.enable');
await driver.sendVoidCommand('CSS.enable');
await driver.sendVoidCommand('CSS.startRuleUsageTracking');
await driver.evaluateAsync('getComputedStyle(document.body)');
driver.off('CSS.styleSheetAdded', onStylesheetAdded);

Expand All @@ -43,8 +43,8 @@ class CSSUsage extends Gatherer {
const styleSheetInfo = await Promise.all(promises);

const ruleUsageResponse = await driver.sendCommand('CSS.stopRuleUsageTracking');
await driver.sendCommand('CSS.disable');
await driver.sendCommand('DOM.disable');
await driver.sendVoidCommand('CSS.disable');
await driver.sendVoidCommand('DOM.disable');

const dedupedStylesheets = new Map(styleSheetInfo.map(sheet => {
return /** @type {[string, LH.Artifacts.CSSStyleSheetInfo]} */ ([sheet.content, sheet]);
Expand Down
16 changes: 15 additions & 1 deletion lighthouse-core/lib/lh-error.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ const UIStrings = {
* @example {Network.enable} protocolMethod
* */
protocolTimeout: 'Waiting for DevTools protocol response has exceeded the allotted time. (Method: {protocolMethod})',
/**
* @description Error message explaining that Chrome has stopped responding to protocol messages.
* @example {Network.enable} protocolMethod
* */
chromeNotResponding: 'Waiting for DevTools protocol response has exceeded the allotted time. (Method: {protocolMethod})',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could tweak this obviously, I have half a mind to just tweak both of these to say "Chrome stopped responding, Lighthouse couldn't really do anything about it"

/** Error message explaining that the requested page could not be resolved by the DNS server. */
dnsFailure: 'DNS servers could not resolve the provided domain.',
/** Error message explaining that Lighthouse couldn't complete because the page has stopped responding to its instructions. */
Expand Down Expand Up @@ -360,14 +365,23 @@ const ERRORS = {
message: UIStrings.urlInvalid,
},

/* Protocol timeout failures
/* Protocol timeout failures for a single specific command.
* Most instances of this error are recoverable.
* Requires an additional `protocolMethod` field for translation.
*/
PROTOCOL_TIMEOUT: {
code: 'PROTOCOL_TIMEOUT',
message: UIStrings.protocolTimeout,
lhrRuntimeError: true,
},
/* Protocol timeout failures that mean Chrome has hung and there's nothing Lighthouse can do to recover.
* Requires an additional `protocolMethod` field for translation.
*/
CHROME_NOT_RESPONDING: {
code: 'CHROME_NOT_RESPONDING',
message: UIStrings.protocolTimeout,
lhrRuntimeError: true,
},

// DNS failure on main document (no resolution, timed out, etc)
DNS_FAILURE: {
Expand Down