core: add DevtoolsLogError and TraceError artifacts (#15311)

GoogleChrome · Aug 2, 2023 · 67699c5 · 67699c5
1 parent e5d1b9c
commit 67699c5
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 30 deletions.
diff --git a/cli/test/smokehouse/test-definitions/errors-expired-ssl.js b/cli/test/smokehouse/test-definitions/errors-expired-ssl.js
@@ -40,18 +40,14 @@ const expectations = {
     audits: {
       'first-contentful-paint': {
         scoreDisplayMode: 'error',
-        errorMessage: 'Required traces gatherer did not run.',
+        errorMessage: 'The URL you have provided does not have a valid security certificate. net::ERR_CERT_DATE_INVALID',
       },
     },
   },
   artifacts: {
     PageLoadError: {code: 'INSECURE_DOCUMENT_REQUEST'},
-    devtoolsLogs: {
-      'pageLoadError-default': {...NONEMPTY_ARRAY},
-    },
-    traces: {
-      'pageLoadError-default': {traceEvents: NONEMPTY_ARRAY},
-    },
+    DevtoolsLogError: NONEMPTY_ARRAY,
+    TraceError: {traceEvents: NONEMPTY_ARRAY},
   },
 };
 

diff --git a/cli/test/smokehouse/test-definitions/errors-iframe-expired-ssl.js b/cli/test/smokehouse/test-definitions/errors-iframe-expired-ssl.js
@@ -43,12 +43,8 @@ const expectations = {
     },
   },
   artifacts: {
-    devtoolsLogs: {
-      defaultPass: NONEMPTY_ARRAY,
-    },
-    traces: {
-      defaultPass: {traceEvents: NONEMPTY_ARRAY},
-    },
+    DevtoolsLog: NONEMPTY_ARRAY,
+    Trace: {traceEvents: NONEMPTY_ARRAY},
   },
 };
 

diff --git a/cli/test/smokehouse/test-definitions/errors-infinite-loop.js b/cli/test/smokehouse/test-definitions/errors-infinite-loop.js
@@ -37,18 +37,14 @@ const expectations = {
     audits: {
       'first-contentful-paint': {
         scoreDisplayMode: 'error',
-        errorMessage: 'Required traces gatherer did not run.',
+        errorMessage: 'Lighthouse was unable to reliably load the URL you requested because the page stopped responding.',
       },
     },
   },
   artifacts: {
     PageLoadError: {code: 'PAGE_HUNG'},
-    devtoolsLogs: {
-      'pageLoadError-default': {...NONEMPTY_ARRAY},
-    },
-    traces: {
-      'pageLoadError-default': {traceEvents: NONEMPTY_ARRAY},
-    },
+    DevtoolsLogError: NONEMPTY_ARRAY,
+    TraceError: {traceEvents: NONEMPTY_ARRAY},
   },
 };
 

diff --git a/core/config/constants.js b/core/config/constants.js
@@ -129,7 +129,7 @@ const defaultSettings = {
 
 /** @type {Required<LH.Config.NavigationJson>} */
 const defaultNavigationConfig = {
-  id: 'default',
+  id: 'defaultPass',
   loadFailureMode: 'fatal',
   disableThrottling: false,
   disableStorageReset: false,

diff --git a/core/gather/navigation-runner.js b/core/gather/navigation-runner.js
@@ -188,8 +188,14 @@ async function _computeNavigationResult(
     /** @type {Partial<LH.GathererArtifacts>} */
     const artifacts = {};
     const pageLoadErrorId = `pageLoadError-${navigationContext.navigation.id}`;
-    if (debugData.devtoolsLog) artifacts.devtoolsLogs = {[pageLoadErrorId]: debugData.devtoolsLog};
-    if (debugData.trace) artifacts.traces = {[pageLoadErrorId]: debugData.trace};
+    if (debugData.devtoolsLog) {
+      artifacts.DevtoolsLogError = debugData.devtoolsLog;
+      artifacts.devtoolsLogs = {[pageLoadErrorId]: debugData.devtoolsLog};
+    }
+    if (debugData.trace) {
+      artifacts.TraceError = debugData.trace;
+      artifacts.traces = {[pageLoadErrorId]: debugData.trace};
+    }
 
     return {
       pageLoadError,

diff --git a/core/lib/asset-saver.js b/core/lib/asset-saver.js
@@ -62,6 +62,9 @@ function loadArtifacts(basePath) {
     if (passName === 'defaultPass') {
       artifacts.DevtoolsLog = devtoolsLog;
     }
+    if (passName === 'pageLoadError-defaultPass') {
+      artifacts.DevtoolsLogError = devtoolsLog;
+    }
   });
 
   // load traces
@@ -74,6 +77,9 @@ function loadArtifacts(basePath) {
     if (passName === 'defaultPass') {
       artifacts.Trace = artifacts.traces[passName];
     }
+    if (passName === 'pageLoadError-defaultPass') {
+      artifacts.TraceError = artifacts.traces[passName];
+    }
   });
 
   if (Array.isArray(artifacts.Timing)) {

diff --git a/core/runner.js b/core/runner.js
@@ -367,6 +367,8 @@ class Runner {
 
     let auditResult;
     try {
+      if (artifacts.PageLoadError) throw artifacts.PageLoadError;
+
       // Return an early error if an artifact required for the audit is missing or an error.
       for (const artifactName of audit.meta.requiredArtifacts) {
         const noArtifact = artifacts[artifactName] === undefined;

diff --git a/core/test/config/config-test.js b/core/test/config/config-test.js
@@ -253,9 +253,10 @@ describe('Config', () => {
 
       expect(resolvedConfig).toMatchObject({
         artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
-        navigations: [
-          {id: 'default', artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}]},
-        ],
+        navigations: [{
+          id: 'defaultPass',
+          artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
+        }],
       });
     });
 
@@ -269,7 +270,7 @@ describe('Config', () => {
       expect(resolvedConfig).toMatchObject({
         navigations: [
           {
-            id: 'default',
+            id: 'defaultPass',
             blankPage: 'about:blank',
             artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
             loadFailureMode: 'fatal',
@@ -366,7 +367,7 @@ describe('Config', () => {
           {id: 'Accessibility'},
         ],
         navigations: [
-          {id: 'default', artifacts: [{id: 'Accessibility'}]},
+          {id: 'defaultPass', artifacts: [{id: 'Accessibility'}]},
         ],
       });
     });

diff --git a/core/test/gather/navigation-runner-test.js b/core/test/gather/navigation-runner-test.js
@@ -472,8 +472,10 @@ describe('NavigationRunner', () => {
       const {artifacts, pageLoadError} = await run(navigation);
       expect(pageLoadError).toBeInstanceOf(LighthouseError);
       expect(artifacts).toEqual({
-        devtoolsLogs: {'pageLoadError-default': expect.any(Array)},
-        traces: {'pageLoadError-default': {traceEvents: []}},
+        DevtoolsLogError: expect.any(Array),
+        TraceError: {traceEvents: []},
+        devtoolsLogs: {'pageLoadError-defaultPass': expect.any(Array)},
+        traces: {'pageLoadError-defaultPass': {traceEvents: []}},
       });
     });
 

diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts
@@ -108,6 +108,8 @@ export interface GathererArtifacts extends PublicGathererArtifacts {
   CSSUsage: {rules: Crdp.CSS.RuleUsage[], stylesheets: Artifacts.CSSStyleSheetInfo[]};
   /** The primary log of devtools protocol activity. */
   DevtoolsLog: DevtoolsLog;
+  /** The log of devtools protocol activity if there was a page load error and Chrome navigated to a `chrome-error://` page. */
+  DevtoolsLogError: DevtoolsLog;
   /** Information on the document's doctype(or null if not present), specifically the name, publicId, and systemId.
       All properties default to an empty string if not present */
   Doctype: Artifacts.Doctype | null;
@@ -152,6 +154,8 @@ export interface GathererArtifacts extends PublicGathererArtifacts {
   TapTargets: Artifacts.TapTarget[];
   /** The primary trace taken over the entire run. */
   Trace: Trace;
+  /** The trace if there was a page load error and Chrome navigated to a `chrome-error://` page. */
+  TraceError: Trace;
   /** Elements associated with metrics (ie: Largest Contentful Paint element). */
   TraceElements: Artifacts.TraceElement[];
   /** Parsed version of the page's Web App Manifest, or null if none found. */