GoogleChrome · patrickhulce · Jul 24, 2019 · Jul 17, 2019 · Jul 17, 2019 · Jul 17, 2019
diff --git a/clients/lightrider/lightrider-entry.js b/clients/lightrider/lightrider-entry.js
@@ -9,6 +9,7 @@ const lighthouse = require('../../lighthouse-core/index.js');
 
 const LHError = require('../../lighthouse-core/lib/lh-error.js');
 const preprocessor = require('../../lighthouse-core/lib/proto-preprocessor.js');
+const assetSaver = require('../../lighthouse-core/lib/asset-saver.js')
 
 /** @type {Record<'mobile'|'desktop', LH.Config.Json>} */
 const LR_PRESETS = {
@@ -62,7 +63,9 @@ async function runLighthouseInLR(connection, url, flags, lrOpts) {
     }
 
     // pre process the LHR for proto
-    return JSON.stringify(preprocessor.processForProto(runnerResult.lhr));
+    const preprocessedLhr = preprocessor.processForProto(runnerResult.lhr);
+    // Properly serialize artifact errors for logging.
+    return JSON.stringify(preprocessedLhr, logAssets ? assetSaver.stringifyReplacer : undefined);
   } catch (err) {
     // If an error ruined the entire lighthouse run, attempt to return a meaningful error.
     let runtimeError;

diff --git a/lighthouse-core/lib/asset-saver.js b/lighthouse-core/lib/asset-saver.js
@@ -16,6 +16,7 @@ const rimraf = require('rimraf');
 const mkdirp = require('mkdirp');
 const NetworkAnalysisComputed = require('../computed/network-analysis.js');
 const LoadSimulatorComputed = require('../computed/load-simulator.js');
+const LHError = require('../lib/lh-error.js');
 
 const artifactsFilename = 'artifacts.json';
 const traceSuffix = '.trace.json';
@@ -42,9 +43,10 @@ function loadArtifacts(basePath) {
     throw new Error('No saved artifacts found at ' + basePath);
   }
 
-  // load artifacts.json
+  // load artifacts.json using a reviver to deserialize any LHErrors in artifacts.
+  const artifactsStr = fs.readFileSync(path.join(basePath, artifactsFilename), 'utf8');
   /** @type {LH.Artifacts} */
-  const artifacts = JSON.parse(fs.readFileSync(path.join(basePath, artifactsFilename), 'utf8'));
+  const artifacts = JSON.parse(artifactsStr, LHError.parseReviver);
 
   const filenames = fs.readdirSync(basePath);
 
@@ -73,6 +75,21 @@ function loadArtifacts(basePath) {
   return artifacts;
 }
 
+/**
+ * A replacer function for JSON.stingify of the artifacts. Used to serialize objects that
+ * JSON won't normally handle.
+ * @param {string} key
+ * @param {any} value
+ */
+function stringifyReplacer(key, value) {
+  // Currently only handle LHError and other Error types.
+  if (value instanceof Error) {
+    return LHError.stringifyReplacer(value);
+  }
+
+  return value;
+}
+
 /**
  * Save artifacts object mostly to single file located at basePath/artifacts.log.
  * Also save the traces & devtoolsLogs to their own files
@@ -100,8 +117,8 @@ async function saveArtifacts(artifacts, basePath) {
     fs.writeFileSync(`${basePath}/${passName}${devtoolsLogSuffix}`, log, 'utf8');
   }
 
-  // save everything else
-  const restArtifactsString = JSON.stringify(restArtifacts, null, 2);
+  // save everything else, using a replacer to serialize LHErrors in the artifacts.
+  const restArtifactsString = JSON.stringify(restArtifacts, stringifyReplacer, 2);
   fs.writeFileSync(`${basePath}/${artifactsFilename}`, restArtifactsString, 'utf8');
   log.log('Artifacts saved to disk in folder:', basePath);
   log.timeEnd(status);
@@ -271,4 +288,5 @@ module.exports = {
   prepareAssets,
   saveTrace,
   saveLanternNetworkData,
+  stringifyReplacer,
 };
diff --git a/lighthouse-core/lib/lh-error.js b/lighthouse-core/lib/lh-error.js
@@ -56,10 +56,17 @@ const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
  * @property {boolean} [lhrRuntimeError] True if it should appear in the top-level LHR.runtimeError property.
  */
 
+const LHERROR_SENTINEL = '__LighthouseErrorSentinel';
+const ERROR_SENTINEL = '__ErrorSentinel';
+/**
+ * @typedef {{sentinel: '__LighthouseErrorSentinel', code: string, stack?: string, [p: string]: string|undefined}} SerializedLighthouseError
+ * @typedef {{sentinel: '__ErrorSentinel', message: string, code?: string, stack?: string}} SerializedBaseError
+ */
+
 class LighthouseError extends Error {
   /**
    * @param {LighthouseErrorDefinition} errorDefinition
-   * @param {Record<string, string|boolean|undefined>=} properties
+   * @param {Record<string, string|undefined>=} properties
    */
   constructor(errorDefinition, properties) {
     super(errorDefinition.code);
@@ -96,6 +103,77 @@ class LighthouseError extends Error {
     const error = new Error(`Protocol error ${errMsg}`);
     return Object.assign(error, {protocolMethod: method, protocolError: protocolError.message});
   }
+
+  /**
+   * A JSON.stringify replacer to serialize LHErrors and (as a fallback) Errors.
+   * Returns a simplified version of the error object that can be reconstituted
+   * as a copy of the original error at parse time.
+   * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter
+   * @param {Error|LighthouseError} err
+   * @return {SerializedBaseError|SerializedLighthouseError}
+   */
+  static stringifyReplacer(err) {
+    if (err instanceof LighthouseError) {
+      // Remove class props so that remaining values were what was passed in as `properties`.
+      // eslint-disable-next-line no-unused-vars
+      const {name, code, message, friendlyMessage, lhrRuntimeError, stack, ...properties} = err;
+
+      return {
+        sentinel: LHERROR_SENTINEL,
+        code,
+        stack,
+        ...properties,
+      };
+    }
+
+    // Unexpected errors won't be LHErrors, but we want them serialized as well.
+    if (err instanceof Error) {
+      const {message, stack} = err;
+      // @ts-ignore - code can be helpful for e.g. node errors, so preserve it if it's present.
+      const code = err.code;
+      return {
+        sentinel: ERROR_SENTINEL,
+        message,
+        code,
+        stack,
+      };
+    }
+
+    throw new Error('Invalid value for LHError stringification');
+  }
+
+  /**
+   * A JSON.parse reviver. If any value passed in is a serialized Error or
+   * LHError, the error is recreated as the original object. Otherwise, the
+   * value is passed through unchanged.
+   * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse#Using_the_reviver_parameter
+   * @param {string} key
+   * @param {any} possibleError
+   * @return {any}
+   */
+  static parseReviver(key, possibleError) {
+    if (typeof possibleError === 'object' && possibleError !== null) {
+      if (possibleError.sentinel === LHERROR_SENTINEL) {
+        // Include sentinel in destructuring so it doesn't end up in `properties`.
+        // eslint-disable-next-line no-unused-vars
+        const {sentinel, code, stack, ...properties} = /** @type {SerializedLighthouseError} */ (possibleError);
+        const errorDefinition = LighthouseError.errors[/** @type {keyof typeof ERRORS} */ (code)];
+        const lhError = new LighthouseError(errorDefinition, properties);
+        lhError.stack = stack;
+
+        return lhError;
+      }
+
+      if (possibleError.sentinel === ERROR_SENTINEL) {
+        const {message, code, stack} = /** @type {SerializedBaseError} */ (possibleError);
+        const error = new Error(message);
+        Object.assign(error, {code, stack});
+        return error;
+      }
+    }
+
+    return possibleError;
+  }
 }
 
 const ERRORS = {
@@ -227,7 +305,7 @@ const ERRORS = {
   },
 
   /* Protocol timeout failures
-   * Requires an additional `icuProtocolMethod` field for translation.
+   * Requires an additional `protocolMethod` field for translation.
    */
   PROTOCOL_TIMEOUT: {
     code: 'PROTOCOL_TIMEOUT',

diff --git a/lighthouse-core/test/lib/asset-saver-test.js b/lighthouse-core/test/lib/asset-saver-test.js
@@ -9,6 +9,8 @@ const assetSaver = require('../../lib/asset-saver.js');
 const Metrics = require('../../lib/traces/pwmetrics-events.js');
 const assert = require('assert');
 const fs = require('fs');
+const rimraf = require('rimraf');
+const LHError = require('../../lib/lh-error.js');
 
 const traceEvents = require('../fixtures/traces/progressive-app.json');
 const dbwTrace = require('../results/artifacts/defaultPass.trace.json');
@@ -167,6 +169,68 @@ describe('asset-saver helper', () => {
     });
   });
 
+  describe('JSON serialization', () => {
+    const outputPath = __dirname + '/json-serialization-test-data/';
+
+    afterEach(() => {
+      rimraf.sync(outputPath);
+    });
+
+    it('round trips saved artifacts', async () => {
+      const artifactsPath = __dirname + '/../results/artifacts/';
+      const originalArtifacts = await assetSaver.loadArtifacts(artifactsPath);
+
+      await assetSaver.saveArtifacts(originalArtifacts, outputPath);
+      const roundTripArtifacts = await assetSaver.loadArtifacts(outputPath);
+      expect(roundTripArtifacts).toStrictEqual(originalArtifacts);
+    });
+
+    it('round trips artifacts with an Error member', async () => {
+      const error = new Error('Connection refused by server');
+      // test code to make sure e.g. Node errors get serialized well.
+      error.code = 'ECONNREFUSED';
+
+      const artifacts = {
+        traces: {},
+        devtoolsLogs: {},
+        ViewportDimensions: error,
+      };
+
+      await assetSaver.saveArtifacts(artifacts, outputPath);
+      const roundTripArtifacts = await assetSaver.loadArtifacts(outputPath);
+      expect(roundTripArtifacts).toStrictEqual(artifacts);
+
+      expect(roundTripArtifacts.ViewportDimensions).toBeInstanceOf(Error);
+      expect(roundTripArtifacts.ViewportDimensions.code).toEqual('ECONNREFUSED');
+      expect(roundTripArtifacts.ViewportDimensions.stack).toMatch(
+        /^Error: Connection refused by server.*test[\\/]lib[\\/]asset-saver-test\.js/s);
+    });
+
+    it('round trips artifacts with an LHError member', async () => {
+      // Use an LHError that has an ICU replacement.
+      const protocolMethod = 'Page.getFastness';
+      const lhError = new LHError(LHError.errors.PROTOCOL_TIMEOUT, {protocolMethod});
+
+      const artifacts = {
+        traces: {},
+        devtoolsLogs: {},
+        ScriptElements: lhError,
+      };
+
+      await assetSaver.saveArtifacts(artifacts, outputPath);
+      const roundTripArtifacts = await assetSaver.loadArtifacts(outputPath);
+      expect(roundTripArtifacts).toStrictEqual(artifacts);
+
+      expect(roundTripArtifacts.ScriptElements).toBeInstanceOf(LHError);
+      expect(roundTripArtifacts.ScriptElements.code).toEqual('PROTOCOL_TIMEOUT');
+      expect(roundTripArtifacts.ScriptElements.protocolMethod).toEqual(protocolMethod);
+      expect(roundTripArtifacts.ScriptElements.stack).toMatch(
+          /^LHError: PROTOCOL_TIMEOUT.*test[\\/]lib[\\/]asset-saver-test\.js/s);
+      expect(roundTripArtifacts.ScriptElements.friendlyMessage)
+        .toBeDisplayString(/\(Method: Page\.getFastness\)/);
+    });
+  });
+
   describe('saveLanternNetworkData', () => {
     const outputFilename = 'test-lantern-network-data.json';
 

diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js
@@ -330,32 +330,40 @@ describe('Runner', () => {
       });
     });
 
-    // TODO: need to support save/load of artifact errors.
-    // See https://github.com/GoogleChrome/lighthouse/issues/4984
-    it.skip('outputs an error audit result when required artifact was an Error', () => {
-      const errorMessage = 'blurst of times';
-      const artifactError = new Error(errorMessage);
+    it('outputs an error audit result when required artifact was an Error', async () => {
+      // Start with empty-artifacts.
+      const baseArtifacts = assetSaver.loadArtifacts(__dirname +
+          '/fixtures/artifacts/empty-artifacts/');
 
-      const url = 'https://example.com';
+      // Add error and save artifacts using assetSaver to serialize Error object.
+      const errorMessage = 'blurst of times';
+      const artifacts = {
+        ...baseArtifacts,
+        ViewportDimensions: new Error(errorMessage),
+        TestedAsMobileDevice: true,
+      };
+      const artifactsPath = '.tmp/test_artifacts';
+      const resolvedPath = path.resolve(process.cwd(), artifactsPath);
+      await assetSaver.saveArtifacts(artifacts, resolvedPath);
+
+      // Load artifacts via auditMode.
       const config = new Config({
+        settings: {
+          auditMode: resolvedPath,
+        },
         audits: [
+          // requires ViewportDimensions and TestedAsMobileDevice artifacts
           'content-width',
         ],
-
-        artifacts: {
-          // Error objects don't make it through the Config constructor due to
-          // JSON.stringify/parse step, so populate with test error below.
-          ViewportDimensions: null,
-        },
       });
-      config.artifacts.ViewportDimensions = artifactError;
 
-      return Runner.run({}, {url, config}).then(results => {
-        const auditResult = results.lhr.audits['content-width'];
-        assert.strictEqual(auditResult.score, null);
-        assert.strictEqual(auditResult.scoreDisplayMode, 'error');
-        assert.ok(auditResult.errorMessage.includes(errorMessage));
-      });
+      const results = await Runner.run({}, {config});
+      const auditResult = results.lhr.audits['content-width'];
+      assert.strictEqual(auditResult.score, null);
+      assert.strictEqual(auditResult.scoreDisplayMode, 'error');
+      assert.ok(auditResult.errorMessage.includes(errorMessage));
+
+      rimraf.sync(resolvedPath);
     });
 
     it('only passes the required artifacts to the audit', async () => {