Repository: GoogleChrome/lighthouse
Pull request #6812 (2018-12-14 19:42:00)

i18n: localize audit error strings
<!-- Thank you for submitting a pull request! -->

**Summary**
Audit errors were adding the message code to the end of the errorMessage with parens which was destroying the ICU replacement.  This adds `defaultProperties` to easier facilitate mutliple LHErrors using the same UIString to replace and use their own defaults that are constant per error.

**Related Issues/PRs**
fixes: #6804 


exterkamp: Okay, flip flopped again.  I was about to split out all the speedline UIStrings, and then noticed all the pageLoadTookTooLong UIStrings which would also need to be split up afaik.  So this seemed like it was getting ridiculous, so @brendankenny and I came to the compromise of providing the `code` of the Error to the ICU replacement by default, this seems to make sense since the code is always enumerated, and is all the replacements needed anyway at this point.

Thoughts?

Patch:
diff --git a/docs/configuration.md b/docs/configuration.md
index d3bd38659..d54eb3aeb 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -207,5 +207,5 @@ The best examples are the ones Lighthouse uses itself! There are several referen
 * [lighthouse-core/config/default-config.js](https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/config/default-config.js)
 * [lighthouse-core/config/perf-config.js](https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/config/perf-config.js)
 * [docs/recipes/custom-audit/custom-config.js](https://github.com/GoogleChrome/lighthouse/blob/master/docs/recipes/custom-audit/custom-config.js)
-* [pwmetrics](https://github.com/paulirish/pwmetrics/blob/master/lib/lh-config.ts)
+* [pwmetrics](https://github.com/paulirish/pwmetrics/blob/v4.1.1/lib/perf-config.ts)
 
diff --git a/lighthouse-core/lib/i18n/en-US.json b/lighthouse-core/lib/i18n/en-US.json
index d2888845e..5e4218ce6 100644
--- a/lighthouse-core/lib/i18n/en-US.json
+++ b/lighthouse-core/lib/i18n/en-US.json
@@ -928,11 +928,11 @@
     "description": "Used to show the duration in seconds that something lasted. The {timeInMs} placeholder will be replaced with the time duration, shown in seconds (e.g. 5.2 s)"
   },
   "lighthouse-core/lib/lh-error.js | badTraceRecording": {
-    "message": "Something went wrong with recording the trace over your page load. Please run Lighthouse again.",
+    "message": "Something went wrong with recording the trace over your page load. Please run Lighthouse again. ({errorCode})",
     "description": "Error message explaining that the network trace was not able to be recorded for the Lighthouse run."
   },
   "lighthouse-core/lib/lh-error.js | didntCollectScreenshots": {
-    "message": "Chrome didn't collect any screenshots during the page load. Please make sure there is content visible on the page, and then try re-running Lighthouse.",
+    "message": "Chrome didn't collect any screenshots during the page load. Please make sure there is content visible on the page, and then try re-running Lighthouse. ({errorCode})",
     "description": "Error message explaining that the Lighthouse run was not able to collect screenshots through Chrome."
   },
   "lighthouse-core/lib/lh-error.js | dnsFailure": {
@@ -964,7 +964,7 @@
     "description": "Error message explaining that Lighthouse could not load the requested URL and the steps that might be taken to fix the unreliability."
   },
   "lighthouse-core/lib/lh-error.js | pageLoadTookTooLong": {
-    "message": "Your page took too long to load. Please follow the opportunities in the report to reduce your page load time, and then try re-running Lighthouse.",
+    "message": "Your page took too long to load. Please follow the opportunities in the report to reduce your page load time, and then try re-running Lighthouse. ({errorCode})",
     "description": "Error message explaining that the page loaded too slowly to perform a Lighthouse run."
   },
   "lighthouse-core/lib/lh-error.js | protocolTimeout": {
diff --git a/lighthouse-core/lib/lh-error.js b/lighthouse-core/lib/lh-error.js
index c83015e3f..df1b3f406 100644
--- a/lighthouse-core/lib/lh-error.js
+++ b/lighthouse-core/lib/lh-error.js
@@ -10,11 +10,11 @@ const i18n = require('./i18n/i18n.js');
 /* eslint-disable max-len */
 const UIStrings = {
   /** Error message explaining that the Lighthouse run was not able to collect screenshots through Chrome.*/
-  didntCollectScreenshots: `Chrome didn't collect any screenshots during the page load. Please make sure there is content visible on the page, and then try re-running Lighthouse.`,
+  didntCollectScreenshots: `Chrome didn't collect any screenshots during the page load. Please make sure there is content visible on the page, and then try re-running Lighthouse. ({errorCode})`,
   /** Error message explaining that the network trace was not able to be recorded for the Lighthouse run. */
-  badTraceRecording: 'Something went wrong with recording the trace over your page load. Please run Lighthouse again.',
+  badTraceRecording: 'Something went wrong with recording the trace over your page load. Please run Lighthouse again. ({errorCode})',
   /** Error message explaining that the page loaded too slowly to perform a Lighthouse run.  */
-  pageLoadTookTooLong: 'Your page took too long to load. Please follow the opportunities in the report to reduce your page load time, and then try re-running Lighthouse.',
+  pageLoadTookTooLong: 'Your page took too long to load. Please follow the opportunities in the report to reduce your page load time, and then try re-running Lighthouse. ({errorCode})',
   /** Error message explaining that Lighthouse could not load the requested URL and the steps that might be taken to fix the unreliability. */
   pageLoadFailed: 'Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests.',
   /** Error message explaining that Lighthouse could not load the requested URL and the steps that might be taken to fix the unreliability. */
@@ -57,23 +57,14 @@ class LighthouseError extends Error {
     super(errorDefinition.code);
     this.name = 'LHError';
     this.code = errorDefinition.code;
-    this.friendlyMessage = str_(errorDefinition.message, properties);
+    // Insert the i18n reference with errorCode and all additional ICU replacement properties.
+    this.friendlyMessage = str_(errorDefinition.message, {errorCode: this.code, ...properties});
     this.lhrRuntimeError = !!errorDefinition.lhrRuntimeError;
     if (properties) Object.assign(this, properties);
 
     Error.captureStackTrace(this, LighthouseError);
   }
 
-  /**
-   * @param {LighthouseError} err
-   * @return {LighthouseError}
-   */
-  static fromLighthouseError(err) {
-    const {code, friendlyMessage: message, ...rest} = err;
-    // Note: {...rest} convinces tsc 3.1 that it's assignable to a Record.
-    return new LighthouseError({code, message}, {...rest});
-  }
-
   /**
    * @param {string} method
    * @param {{message: string, data?: string|undefined}} protocolError
diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js
index a77a5a8a6..db7c06575 100644
--- a/lighthouse-core/runner.js
+++ b/lighthouse-core/runner.js
@@ -321,9 +321,7 @@ class Runner {
 
       Sentry.captureException(err, {tags: {audit: audit.meta.id}, level: 'error'});
       // Errors become error audit result.
-      const errorMessage = err.friendlyMessage ?
-        `${err.friendlyMessage} (${err.message})` :
-        `Audit error: ${err.message}`;
+      const errorMessage = err.friendlyMessage ? err.friendlyMessage : err.message;
       auditResult = Audit.generateErrorAuditResult(audit, errorMessage);
     }
 
diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json
index f19089ebc..5f901a4f9 100644
--- a/proto/sample_v2_round_trip.json
+++ b/proto/sample_v2_round_trip.json
@@ -969,7 +969,7 @@
         }, 
         "is-crawlable": {
             "description": "Search engines are unable to include your pages in search results if they don't have permission to crawl them. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/indexing).", 
-            "errorMessage": "Audit error: Required RobotsTxt gatherer did not run.", 
+            "errorMessage": "Required RobotsTxt gatherer did not run.", 
             "id": "is-crawlable", 
             "score": null, 
             "scoreDisplayMode": "error", 
@@ -1821,7 +1821,7 @@
         }, 
         "robots-txt": {
             "description": "If your robots.txt file is malformed, crawlers may not be able to understand how you want your website to be crawled or indexed.", 
-            "errorMessage": "Audit error: Required RobotsTxt gatherer did not run.", 
+            "errorMessage": "Required RobotsTxt gatherer did not run.", 
             "id": "robots-txt", 
             "score": null, 
             "scoreDisplayMode": "error", 
diff --git a/readme.md b/readme.md
index 21604b8b5..c9882b27f 100644
--- a/readme.md
+++ b/readme.md
@@ -14,12 +14,16 @@ Lighthouse is integrated directly into the Chrome Developer Tools, under the "Au
 
 ## Using the Chrome extension
 
+The Chrome extension was available prior to Lighthouse being available in Chrome Developer Tools, and offers similar functionality.
+
 **Installation**: [install the extension](https://chrome.google.com/webstore/detail/lighthouse/blipmdconlkpinefehnmjammfjpmpbjk) from the Chrome Web Store.
 
 **Run it**: follow the [extension quick-start guide](https://developers.google.com/web/tools/lighthouse/#extension).
 
 ## Using the Node CLI
 
+The Node CLI provides the most flexibility in how Lighthouse runs can be configured and reported. Users who want more advanced usage, or want to run Lighthouse in an automated fashion should use the Node CLI.
+
 _Lighthouse requires Node 8 LTS (8.9) or later._
 
 **Installation**:


Test patch:
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
index 71ef29e28..05a2a3b3d 100644
--- a/lighthouse-core/test/results/sample_v2.json
+++ b/lighthouse-core/test/results/sample_v2.json
@@ -2669,7 +2669,7 @@
       "score": null,
       "scoreDisplayMode": "error",
       "rawValue": null,
-      "errorMessage": "Audit error: Required RobotsTxt gatherer did not run."
+      "errorMessage": "Required RobotsTxt gatherer did not run."
     },
     "robots-txt": {
       "id": "robots-txt",
@@ -2678,7 +2678,7 @@
       "score": null,
       "scoreDisplayMode": "error",
       "rawValue": null,
-      "errorMessage": "Audit error: Required RobotsTxt gatherer did not run."
+      "errorMessage": "Required RobotsTxt gatherer did not run."
     },
     "hreflang": {
       "id": "hreflang",
diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js
index d3683f316..8774274e2 100644
--- a/lighthouse-core/test/runner-test.js
+++ b/lighthouse-core/test/runner-test.js
@@ -606,7 +606,8 @@ describe('Runner', () => {
     assert.ok(lhr.audits['test-audit'].errorMessage.includes(NO_FCP.code));
     // And it bubbled up to the runtimeError.
     assert.strictEqual(lhr.runtimeError.code, NO_FCP.code);
-    assert.ok(lhr.runtimeError.message.includes(NO_FCP.message));
+    expect(lhr.runtimeError.message)
+      .toBeDisplayString(/Something .*\(NO_FCP\)/);
   });
 
   it('localized errors thrown from driver', async () => {