diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 7bd665271e3f..1e1a8c7908c9 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -106,7 +106,7 @@ jobs: - run: sudo apt-get install xvfb - name: yarn smoke --fraggle-rock - run: xvfb-run --auto-servernum yarn smoke --debug --fraggle-rock -j=1 --retries=2 --invert-match pwa offline oopif errors-infinite-loop errors-expired-ssl perf-diagnostics-third-party + run: xvfb-run --auto-servernum yarn smoke --debug --fraggle-rock -j=1 --retries=2 --invert-match pwa offline oopif perf-diagnostics-third-party # Fail if any changes were written to source files. - run: git diff --exit-code diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index cdb21c1a9faf..d46cf09a4e66 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -290,12 +290,7 @@ function collateResults(localConsole, actual, expected) { } return [ - { - name: 'final url', - actual: actual.lhr.finalUrl, - expected: expected.lhr.finalUrl, - equal: actual.lhr.finalUrl === expected.lhr.finalUrl, - }, + makeComparison('final url', actual.lhr.finalUrl, expected.lhr.finalUrl), runtimeErrorAssertion, runWarningsAssertion, ...requestCountAssertion, diff --git a/lighthouse-cli/test/smokehouse/test-definitions/errors/error-expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/errors/error-expectations.js index a073f7c990d2..e5faf8ae1d61 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/errors/error-expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/errors/error-expectations.js @@ -31,9 +31,12 @@ const infiniteLoop = { artifacts: { PageLoadError: {code: 'PAGE_HUNG'}, devtoolsLogs: { - 'pageLoadError-defaultPass': NONEMPTY_ARRAY, + 'pageLoadError-defaultPass': {...NONEMPTY_ARRAY, _legacyOnly: true}, + 'pageLoadError-default': {...NONEMPTY_ARRAY, _fraggleRockOnly: true}, }, traces: { + // Fraggle Rock treats traces as regular artifacts which are not collected on error. + '_legacyOnly': true, 'pageLoadError-defaultPass': {traceEvents: NONEMPTY_ARRAY}, }, }, @@ -46,9 +49,12 @@ const infiniteLoop = { const expiredSsl = { lhr: { requestedUrl: 'https://expired.badssl.com', - finalUrl: 'https://expired.badssl.com/', + finalUrl: /(expired.badssl.com|chrome-error)/, runtimeError: {code: 'INSECURE_DOCUMENT_REQUEST'}, - runWarnings: ['The URL you have provided does not have a valid security certificate. net::ERR_CERT_DATE_INVALID'], + runWarnings: Object.defineProperty([ + /expired.badssl.*redirected to chrome-error:/, // This warning was not provided in legacy reports. + 'The URL you have provided does not have a valid security certificate. net::ERR_CERT_DATE_INVALID', + ], '_fraggleRockOnly', {value: true, enumerable: true}), audits: { 'first-contentful-paint': { scoreDisplayMode: 'error', @@ -59,9 +65,12 @@ const expiredSsl = { artifacts: { PageLoadError: {code: 'INSECURE_DOCUMENT_REQUEST'}, devtoolsLogs: { - 'pageLoadError-defaultPass': NONEMPTY_ARRAY, + 'pageLoadError-defaultPass': {...NONEMPTY_ARRAY, _legacyOnly: true}, + 'pageLoadError-default': {...NONEMPTY_ARRAY, _fraggleRockOnly: true}, }, traces: { + // Fraggle Rock treats traces as regular artifacts which are not collected on error. + '_legacyOnly': true, 'pageLoadError-defaultPass': {traceEvents: NONEMPTY_ARRAY}, }, }, diff --git a/lighthouse-core/fraggle-rock/gather/navigation-runner.js b/lighthouse-core/fraggle-rock/gather/navigation-runner.js index f01fcac9ca1f..ab5f4bbc33e7 100644 --- a/lighthouse-core/fraggle-rock/gather/navigation-runner.js +++ b/lighthouse-core/fraggle-rock/gather/navigation-runner.js @@ -105,9 +105,9 @@ async function _navigate(navigationContext) { /** * @param {NavigationContext} navigationContext * @param {PhaseState} phaseState - * @return {Promise | undefined>} + * @return {Promise<{devtoolsLog: LH.DevtoolsLog, records: Array} | undefined>} */ -async function _collectNetworkRecords(navigationContext, phaseState) { +async function _collectNetworkData(navigationContext, phaseState) { const devtoolsLogArtifactDefn = phaseState.artifactDefinitions.find( definition => definition.gatherer.instance.meta.symbol === DevtoolsLog.symbol ); @@ -119,7 +119,7 @@ async function _collectNetworkRecords(navigationContext, phaseState) { const devtoolsLog = await phaseState.artifactState.getArtifact[devtoolsLogArtifactId]; const records = await NetworkRecords.request(devtoolsLog, navigationContext); - return records; + return {devtoolsLog, records}; } /** @@ -137,12 +137,12 @@ async function _computeNavigationResult( ) { const {navigationError, finalUrl} = navigateResult; const warnings = [...setupResult.warnings, ...navigateResult.warnings]; - const networkRecords = await _collectNetworkRecords(navigationContext, phaseState); - const pageLoadError = networkRecords + const networkData = await _collectNetworkData(navigationContext, phaseState); + const pageLoadError = networkData ? getPageLoadError(navigationError, { url: finalUrl, loadFailureMode: navigationContext.navigation.loadFailureMode, - networkRecords, + networkRecords: networkData.records, }) : navigationError; @@ -151,10 +151,15 @@ async function _computeNavigationResult( const localizedMessage = i18n.getFormatted(pageLoadError.friendlyMessage, locale); log.error('NavigationRunner', localizedMessage, navigationContext.requestedUrl); + /** @type {Partial} */ + const artifacts = {}; + const pageLoadErrorId = `pageLoadError-${navigationContext.navigation.id}`; + if (networkData) artifacts.devtoolsLogs = {[pageLoadErrorId]: networkData.devtoolsLog}; + return { finalUrl, pageLoadError, - artifacts: {}, + artifacts, warnings: [...warnings, pageLoadError.friendlyMessage], }; } else { diff --git a/lighthouse-core/gather/driver/navigation.js b/lighthouse-core/gather/driver/navigation.js index 58b346b9db38..26e5ac22de0f 100644 --- a/lighthouse-core/gather/driver/navigation.js +++ b/lighthouse-core/gather/driver/navigation.js @@ -142,10 +142,7 @@ function getNavigationWarnings(navigation) { if (navigation.timedOut) warnings.push(str_(UIStrings.warningTimeout)); - if ( - !URL.equalWithExcludedFragments(requestedUrl, finalUrl) && - !finalUrl.startsWith('chrome-error://') - ) { + if (!URL.equalWithExcludedFragments(requestedUrl, finalUrl)) { warnings.push(str_(UIStrings.warningRedirected, { requested: requestedUrl, final: finalUrl, diff --git a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js index 5602f256eb4f..e17b4ce05bd2 100644 --- a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js @@ -336,7 +336,7 @@ describe('NavigationRunner', () => { const {artifacts, pageLoadError} = await run(navigation); expect(pageLoadError).toBeInstanceOf(LighthouseError); - expect(artifacts).toEqual({}); + expect(artifacts).toEqual({devtoolsLogs: {'pageLoadError-default': expect.any(Array)}}); }); it('cleans up throttling before getArtifact', async () => { diff --git a/lighthouse-core/test/gather/driver/navigation-test.js b/lighthouse-core/test/gather/driver/navigation-test.js index 69bcb02fd535..359d57e35c60 100644 --- a/lighthouse-core/test/gather/driver/navigation-test.js +++ b/lighthouse-core/test/gather/driver/navigation-test.js @@ -234,9 +234,9 @@ describe('.getNavigationWarnings()', () => { expect(warnings).toHaveLength(0); }); - it('does not add a url mismatch warning for failed navigations', () => { + it('adds a url mismatch warning for failed navigations', () => { const finalUrl = 'chrome-error://chromewebdata/'; const warnings = getNavigationWarnings({...normalNavigation, finalUrl}); - expect(warnings).toHaveLength(0); + expect(warnings).toHaveLength(1); }); }); diff --git a/types/smokehouse.d.ts b/types/smokehouse.d.ts index b6e5d840da02..39879f8df66f 100644 --- a/types/smokehouse.d.ts +++ b/types/smokehouse.d.ts @@ -13,7 +13,7 @@ declare global { interface ExpectedLHR { audits: Record; requestedUrl: string; - finalUrl: string; + finalUrl: string | RegExp; runWarnings?: Array; runtimeError?: { code?: any;