diff --git a/cli/test/smokehouse/config/exclusions.js b/cli/test/smokehouse/config/exclusions.js index 730f7267f183..0ff3b3ec707c 100644 --- a/cli/test/smokehouse/config/exclusions.js +++ b/cli/test/smokehouse/config/exclusions.js @@ -26,6 +26,8 @@ const exclusions = { 'metrics-tricky-tti', 'metrics-tricky-tti-late-fcp', 'screenshot', // Disabled because of differences that need further investigation 'byte-efficiency', 'byte-gzip', 'perf-preload', + // Disabled because a renderer crash also breaks devtools frontend + 'crash', ], }; diff --git a/cli/test/smokehouse/core-tests.js b/cli/test/smokehouse/core-tests.js index ad59c85c23c7..0d48a428d3f9 100644 --- a/cli/test/smokehouse/core-tests.js +++ b/cli/test/smokehouse/core-tests.js @@ -7,6 +7,7 @@ import a11y from './test-definitions/a11y.js'; import byteEfficiency from './test-definitions/byte-efficiency.js'; import byteGzip from './test-definitions/byte-gzip.js'; +import crash from './test-definitions/crash.js'; import cspAllowAll from './test-definitions/csp-allow-all.js'; import cspBlockAll from './test-definitions/csp-block-all.js'; import dbw from './test-definitions/dobetterweb.js'; @@ -65,6 +66,7 @@ const smokeTests = [ a11y, byteEfficiency, byteGzip, + crash, cspAllowAll, cspBlockAll, dbw, diff --git a/cli/test/smokehouse/test-definitions/crash.js b/cli/test/smokehouse/test-definitions/crash.js new file mode 100644 index 000000000000..c790c40138fc --- /dev/null +++ b/cli/test/smokehouse/test-definitions/crash.js @@ -0,0 +1,41 @@ +/** + * @license + * Copyright 2024 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +const config = { + extends: 'lighthouse:default', + settings: { + maxWaitForLoad: 5000, + onlyAudits: [ + 'first-contentful-paint', + ], + }, +}; + +/** + * @type {Smokehouse.ExpectedRunnerResult} + */ +const expectations = { + lhr: { + requestedUrl: 'chrome://crash', + finalDisplayedUrl: 'about:blank', + runtimeError: {code: 'TARGET_CRASHED'}, + runWarnings: [ + 'Browser tab has unexpectedly crashed.', + ], + audits: { + 'first-contentful-paint': { + scoreDisplayMode: 'error', + errorMessage: 'Browser tab has unexpectedly crashed.', + }, + }, + }, +}; + +export default { + id: 'crash', + expectations, + config, +}; diff --git a/core/gather/driver.js b/core/gather/driver.js index 73a9ba7cf78f..fd25c98e21d7 100644 --- a/core/gather/driver.js +++ b/core/gather/driver.js @@ -8,6 +8,7 @@ import log from 'lighthouse-logger'; import {ExecutionContext} from './driver/execution-context.js'; import {TargetManager} from './driver/target-manager.js'; +import {LighthouseError} from '../lib/lh-error.js'; import {Fetcher} from './fetcher.js'; import {NetworkMonitor} from './driver/network-monitor.js'; @@ -47,6 +48,12 @@ class Driver { this._fetcher = undefined; this.defaultSession = throwingSession; + + // Poor man's Promise.withResolvers() + /** @param {Error} _ */ + let rej = _ => {}; + const promise = /** @type {Promise} */ (new Promise((_, theRej) => rej = theRej)); + this.fatalRejection = {promise, rej}; } /** @return {LH.Gatherer.Driver['executionContext']} */ @@ -86,11 +93,30 @@ class Driver { this._networkMonitor = new NetworkMonitor(this._targetManager); await this._networkMonitor.enable(); this.defaultSession = this._targetManager.rootSession(); + this.listenForCrashes(); this._executionContext = new ExecutionContext(this.defaultSession); this._fetcher = new Fetcher(this.defaultSession); log.timeEnd(status); } + /** + * If the target crashes, we can't continue gathering. + * + * FWIW, if the target unexpectedly detaches (eg the user closed the tab), pptr will + * catch that and reject into our this._cdpSession.send, which we'll alrady handle appropriately + * @return {void} + */ + listenForCrashes() { + this.defaultSession.on('Inspector.targetCrashed', async _ => { + log.error('Driver', 'Inspector.targetCrashed'); + // Manually detach so no more CDP traffic is attempted. + // Don't await, else our rejection will be a 'Target closed' protocol error on cross-talk CDP calls. + void this.defaultSession.dispose(); + this.fatalRejection.rej(new LighthouseError(LighthouseError.errors.TARGET_CRASHED)); + }); + } + + /** @return {Promise} */ async disconnect() { if (this.defaultSession === throwingSession) return; diff --git a/core/gather/driver/navigation.js b/core/gather/driver/navigation.js index 4ea0cd99d236..9518246c38b6 100644 --- a/core/gather/driver/navigation.js +++ b/core/gather/driver/navigation.js @@ -122,7 +122,10 @@ async function gotoURL(driver, requestor, options) { throw new Error('Cannot wait for FCP without waiting for page load'); } - const waitConditions = await Promise.all(waitConditionPromises); + const waitConditions = await Promise.race([ + driver.fatalRejection.promise, + Promise.all(waitConditionPromises), + ]); const timedOut = waitConditions.some(condition => condition.timedOut); const navigationUrls = await networkMonitor.getNavigationUrls(); diff --git a/core/gather/driver/prepare.js b/core/gather/driver/prepare.js index 3d0510614e35..4f3c96d90ccc 100644 --- a/core/gather/driver/prepare.js +++ b/core/gather/driver/prepare.js @@ -45,7 +45,7 @@ async function enableAsyncStacks(session) { await enable(); return async () => { - await session.sendCommand('Debugger.disable'); + await session.sendCommandAndIgnore('Debugger.disable'); session.off('Debugger.paused', onDebuggerPaused); session.off('Page.frameNavigated', onFrameNavigated); }; diff --git a/core/gather/driver/target-manager.js b/core/gather/driver/target-manager.js index 6872df807e80..76d2d89cdd10 100644 --- a/core/gather/driver/target-manager.js +++ b/core/gather/driver/target-manager.js @@ -266,8 +266,9 @@ class TargetManager extends ProtocolEventEmitter { cdpSession.off('sessionattached', this._onSessionAttached); } - await this._rootCdpSession.send('Page.disable'); - await this._rootCdpSession.send('Runtime.disable'); + // Ignore failures on these in case the tab has crashed. + await this._rootCdpSession.send('Page.disable').catch(_ => {}); + await this._rootCdpSession.send('Runtime.disable').catch(_ => {}); this._enabled = false; this._targetIdToTargets = new Map(); diff --git a/core/gather/navigation-runner.js b/core/gather/navigation-runner.js index 3f862231eb89..dcd79e97b208 100644 --- a/core/gather/navigation-runner.js +++ b/core/gather/navigation-runner.js @@ -92,7 +92,9 @@ async function _navigate(navigationContext) { return {requestedUrl, mainDocumentUrl, navigationError: undefined}; } catch (err) { if (!(err instanceof LighthouseError)) throw err; - if (err.code !== 'NO_FCP' && err.code !== 'PAGE_HUNG') throw err; + if (err.code !== 'NO_FCP' && err.code !== 'PAGE_HUNG' && err.code !== 'TARGET_CRASHED') { + throw err; + } if (typeof requestor !== 'string') throw err; // TODO: Make the urls optional here so we don't need to throw an error with a callback requestor. diff --git a/core/gather/session.js b/core/gather/session.js index b01e1d68bd0a..25f3c830b25b 100644 --- a/core/gather/session.js +++ b/core/gather/session.js @@ -100,6 +100,7 @@ class ProtocolSession extends CrdpEventEmitter { /** @type {NodeJS.Timer|undefined} */ let timeout; const timeoutPromise = new Promise((resolve, reject) => { + // Unexpected setTimeout invocation to preserve the error stack. https://github.com/GoogleChrome/lighthouse/issues/13332 // eslint-disable-next-line max-len timeout = setTimeout(reject, timeoutMs, new LighthouseError(LighthouseError.errors.PROTOCOL_TIMEOUT, { protocolMethod: method, @@ -139,7 +140,7 @@ class ProtocolSession extends CrdpEventEmitter { async dispose() { // @ts-expect-error Puppeteer expects the handler params to be type `unknown` this._cdpSession.off('*', this._handleProtocolEvent); - await this._cdpSession.detach(); + await this._cdpSession.detach().catch(e => log.verbose('session', 'detach failed', e.message)); } } diff --git a/core/lib/emulation.js b/core/lib/emulation.js index 54a6f17a9eca..ec1f521661d7 100644 --- a/core/lib/emulation.js +++ b/core/lib/emulation.js @@ -128,7 +128,7 @@ function enableNetworkThrottling(session, throttlingSettings) { * @return {Promise} */ function clearNetworkThrottling(session) { - return session.sendCommand('Network.emulateNetworkConditions', NO_THROTTLING_METRICS); + return session.sendCommandAndIgnore('Network.emulateNetworkConditions', NO_THROTTLING_METRICS); } /** @@ -146,7 +146,7 @@ function enableCPUThrottling(session, throttlingSettings) { * @return {Promise} */ function clearCPUThrottling(session) { - return session.sendCommand('Emulation.setCPUThrottlingRate', NO_CPU_THROTTLE_METRICS); + return session.sendCommandAndIgnore('Emulation.setCPUThrottlingRate', NO_CPU_THROTTLE_METRICS); } export { diff --git a/core/lib/lh-error.js b/core/lib/lh-error.js index cc77e2fc0759..065acb9fe8e4 100644 --- a/core/lib/lh-error.js +++ b/core/lib/lh-error.js @@ -91,6 +91,9 @@ const UIStrings = { * @example {Largest Contentful Paint} featureName * */ oldChromeDoesNotSupportFeature: 'This version of Chrome is too old to support \'{featureName}\'. Use a newer version to see full results.', + + /** Error message explaining that the browser tab that Lighthouse is inspecting has crashed. */ + targetCrashed: 'Browser tab has unexpectedly crashed.', }; const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings); @@ -423,6 +426,13 @@ const ERRORS = { message: UIStrings.erroredRequiredArtifact, }, + /** The page has crashed and will no longer respond to 99% of CDP commmands. */ + TARGET_CRASHED: { + code: 'TARGET_CRASHED', + message: UIStrings.targetCrashed, + lhrRuntimeError: true, + }, + // Hey! When adding a new error type, update lighthouse-result.proto too. // Only necessary for runtime errors, which come from artifacts or pageLoadErrors. }; diff --git a/core/lib/navigation-error.js b/core/lib/navigation-error.js index 1db66cab7034..69d4480fe5c0 100644 --- a/core/lib/navigation-error.js +++ b/core/lib/navigation-error.js @@ -150,6 +150,9 @@ function getPageLoadError(navigationError, context) { let finalRecord; if (mainRecord) { finalRecord = NetworkAnalyzer.resolveRedirects(mainRecord); + } else { + // We have no network requests to process, use the navError + return navigationError; } if (finalRecord?.mimeType === XHTML_MIME_TYPE) { diff --git a/core/test/gather/mock-driver.js b/core/test/gather/mock-driver.js index 6ef25211ce99..1290ff09b34d 100644 --- a/core/test/gather/mock-driver.js +++ b/core/test/gather/mock-driver.js @@ -158,6 +158,13 @@ function createMockDriver() { const context = createMockExecutionContext(); const targetManager = createMockTargetManager(session); + // The `fatalRejection` + /** @param {Error} _ */ + let rej = _ => {}; + const promise = new Promise((_, theRej) => { + rej = theRej; + }); + return { _page: page, _executionContext: context, @@ -172,6 +179,8 @@ function createMockDriver() { fetchResource: fnAny(), }, networkMonitor: new NetworkMonitor(targetManager.asTargetManager()), + listenForCrashes: fnAny(), + fatalRejection: {promise, rej}, /** @return {Driver} */ asDriver() { diff --git a/core/test/gather/session-test.js b/core/test/gather/session-test.js index a0c63c68686b..b69818917ce8 100644 --- a/core/test/gather/session-test.js +++ b/core/test/gather/session-test.js @@ -104,6 +104,7 @@ describe('ProtocolSession', () => { describe('.dispose', () => { it('should detach from the session', async () => { const detach = fnAny(); + detach.mockResolvedValue(undefined); class MockCdpSession extends EventEmitter { constructor() { super(); diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto index dea6e9eb2391..e10919048331 100644 --- a/proto/lighthouse-result.proto +++ b/proto/lighthouse-result.proto @@ -61,6 +61,10 @@ enum LighthouseError { NOT_HTML = 21; // The trace did not contain a ResourceSendRequest event. NO_RESOURCE_REQUEST = 22; + // Used when any Chrome interstitial error prevents page load. + CHROME_INTERSTITIAL_ERROR = 23; + // The page has crashed and will no longer respond to 99% of CDP commmands. + TARGET_CRASHED = 24; } // The overarching Lighthouse Response object (LHR) diff --git a/shared/localization/locales/en-US.json b/shared/localization/locales/en-US.json index b25846193b90..cadd758a1e28 100644 --- a/shared/localization/locales/en-US.json +++ b/shared/localization/locales/en-US.json @@ -2471,6 +2471,9 @@ "core/lib/lh-error.js | requestContentTimeout": { "message": "Fetching resource content has exceeded the allotted time" }, + "core/lib/lh-error.js | targetCrashed": { + "message": "Browser tab has unexpectedly crashed." + }, "core/lib/lh-error.js | urlInvalid": { "message": "The URL you have provided appears to be invalid." }, diff --git a/shared/localization/locales/en-XL.json b/shared/localization/locales/en-XL.json index 9968976789c7..50cc70f4c5ff 100644 --- a/shared/localization/locales/en-XL.json +++ b/shared/localization/locales/en-XL.json @@ -2471,6 +2471,9 @@ "core/lib/lh-error.js | requestContentTimeout": { "message": "F̂ét̂ćĥín̂ǵ r̂éŝóûŕĉé ĉón̂t́êńt̂ h́âś êx́ĉéêd́êd́ t̂h́ê ál̂ĺôt́t̂éd̂ t́îḿê" }, + "core/lib/lh-error.js | targetCrashed": { + "message": "B̂ŕôẃŝér̂ t́âb́ ĥáŝ ún̂éx̂ṕêćt̂éd̂ĺŷ ćr̂áŝh́êd́." + }, "core/lib/lh-error.js | urlInvalid": { "message": "T̂h́ê ÚR̂Ĺ ŷóû h́âv́ê ṕr̂óv̂íd̂éd̂ áp̂ṕêár̂ś t̂ó b̂é îńv̂ál̂íd̂." }, diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts index c5e6796a1aad..39fde8e239be 100644 --- a/types/gatherer.d.ts +++ b/types/gatherer.d.ts @@ -49,6 +49,8 @@ declare module Gatherer { off(event: 'protocolevent', callback: (payload: Protocol.RawEventMessage) => void): void }; networkMonitor: NetworkMonitor; + listenForCrashes: (() => void); + fatalRejection: {promise: Promise, rej: (reason: Error) => void} } interface Context {