Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i18n: add i18n for LighthouseError messages #6457

Merged
merged 27 commits into from
Dec 10, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d1be64d
Added i18n for runtime error messages.
exterkamp Nov 2, 2018
6799fab
Merge branch 'master' into i18n-errors
exterkamp Nov 2, 2018
3db8628
Formatting on substitutes.
exterkamp Nov 2, 2018
b9c4b4e
Removed redundant 'Method'.
exterkamp Nov 2, 2018
9b6062b
All backticks for variable interpolation highlighting
exterkamp Nov 2, 2018
b0d369f
Thanks linter!
exterkamp Nov 2, 2018
1481b08
Added substitution check to PROTOCOL_TIMEOUT.
exterkamp Nov 2, 2018
cf8e569
Using test-utils toBeDisplayString.
exterkamp Nov 2, 2018
df29d17
Added descriptive ICU names from feedback.
exterkamp Nov 5, 2018
020056a
Added Method back into test for protocol timeout.
exterkamp Nov 5, 2018
56e3daf
Removed unnecessary var.
exterkamp Nov 5, 2018
e764e2d
Fixed i18n of errors and added test to make sure it works!
exterkamp Nov 5, 2018
7a65d02
Better gather runner test comparison.
exterkamp Nov 5, 2018
a6cd8bc
Linter caught me slippin'
exterkamp Nov 5, 2018
02765e4
Moved icu args to properties.
exterkamp Nov 14, 2018
a991732
Merge branch 'master' into i18n-errors
exterkamp Nov 28, 2018
879c333
Added early exit. Fixed i18n punc. formatting.
exterkamp Nov 28, 2018
345d3e4
Collected new strings, and fixed sec-msg test.
exterkamp Nov 28, 2018
9a87b04
Formatting fix.
exterkamp Nov 28, 2018
9beb615
Fixed LR test.
exterkamp Nov 28, 2018
bdfcc4e
Removed backticks, and added localized test.
exterkamp Nov 28, 2018
c56c83f
I was led astray.
exterkamp Nov 28, 2018
9857760
Update lighthouse-core/test/runner-test.js
patrickhulce Nov 29, 2018
d1e1943
Switched up string templates.
exterkamp Nov 29, 2018
c6cfb3b
Merge branch 'i18n-errors' of github.com:GoogleChrome/lighthouse into…
exterkamp Nov 29, 2018
14ceeb2
Apply suggestions from code review
paulirish Dec 7, 2018
972ddb5
Updated strings and test.
exterkamp Dec 7, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,9 @@ class Driver {
}
return new Promise(async (resolve, reject) => {
const asyncTimeout = setTimeout((_ => {
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
const err = new LHError(LHError.errors.PROTOCOL_TIMEOUT);
err.message += ` Method: ${method}`;
const errorDef = {...LHError.errors.PROTOCOL_TIMEOUT};
errorDef.substitute = `${method}`;
const err = new LHError(errorDef);
reject(err);
}), timeout);
try {
Expand Down
6 changes: 3 additions & 3 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ class GatherRunner {
errorDef = LHError.errors.NO_DOCUMENT_REQUEST;
} else if (mainRecord.failed) {
errorDef = {...LHError.errors.FAILED_DOCUMENT_REQUEST};
errorDef.message += ` ${mainRecord.localizedFailDescription}.`;
errorDef.substitute = `${mainRecord.localizedFailDescription}.`;
} else if (mainRecord.hasErrorStatusCode()) {
errorDef = {...LHError.errors.ERRORED_DOCUMENT_REQUEST};
errorDef.message += ` Status code: ${mainRecord.statusCode}.`;
errorDef.substitute = `${mainRecord.statusCode}.`;
}

if (errorDef) {
Expand All @@ -173,7 +173,7 @@ class GatherRunner {
const insecureDescriptions = explanations
.filter(exp => exp.securityState === 'insecure')
.map(exp => exp.description);
errorDef.message += ` ${insecureDescriptions.join(' ')}`;
errorDef.substitute = `${insecureDescriptions.join(' ')}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this join seems like it could be a problem. Are these descriptions localized by chrome?

throw new LHError(errorDef);
}
}
Expand Down
44 changes: 44 additions & 0 deletions lighthouse-core/lib/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,50 @@
"message": "{timeInMs, number, seconds} s",
"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.",
"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.",
"description": "Error message explaining that the Lighthouse run was not able to collect screenshots through Chrome."
},
"lighthouse-core/lib/lh-error.js | internalChromeError": {
"message": "An internal Chrome error occurred. Please restart Chrome and try re-running Lighthouse.",
"description": "Error message explaining that Chrome has encountered an error during the Lighthouse run, and that Chrome should be restarted."
},
"lighthouse-core/lib/lh-error.js | pageLoadFailed": {
"message": "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.",
"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 | pageLoadFailedInsecure": {
"message": "The URL you have provided does not have valid security credentials. {substitute}",
"description": "Error message explaining that the credentials included in the Lighthouse run were invalid, so the URL cannot be accessed."
},
"lighthouse-core/lib/lh-error.js | pageLoadFailedWithDetails": {
"message": "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. Detailed Error: {substitute}",
"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 | pageLoadFailedWithStatusCode": {
"message": "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. Status Code: {substitute}",
"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.",
"description": "Error message explaining that the page loaded too slowly to perform a Lighthouse run."
},
"lighthouse-core/lib/lh-error.js | protocolTimeout": {
"message": "Waiting for DevTools protocol response has exceeded the allotted time. Method: {substitute}",
"description": "Error message explaining that the Chrome Devtools protocol has exceeded the maximum timeout allowed."
},
"lighthouse-core/lib/lh-error.js | requestContentTimeout": {
"message": "Fetching resource content has exceeded the allotted time",
"description": "Error message explaining that fetching the resources of the webpage has taken longer than the maximum time."
},
"lighthouse-core/lib/lh-error.js | urlInvalid": {
"message": "The URL you have provided appears to be invalid.",
"description": "Error message explaining that the provided URL Lighthouse points to is not valid, and cannot be loaded."
},
"lighthouse-core/report/html/renderer/util.js | auditGroupExpandTooltip": {
"message": "Show audits",
"description": "The tooltip text on an expandable chevron icon. Clicking the icon expands a section to reveal a list of audit results that was hidden by default."
Expand Down
85 changes: 60 additions & 25 deletions lighthouse-core/lib/lh-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,41 @@
*/
'use strict';

const strings = require('./strings');
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.`,
/** Error message explaining that the network trace was not able to be recorded for the Lighthouse run. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe for some of these descriptions we should include for the translators the distinction of whether it was something weird that retrying should fix, or something that the user needs to fix and try again? I'm thinking a stock explanation.

This is an error message that was because of certain user conditions they must fix before retrying.
vs.
This is an error message that was because of irregularities with Lighthouse that should be fixed by trying again.

just spitballin'

badTraceRecording: 'Something went wrong with recording the trace over your page load. Please run Lighthouse again.',
/** 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.',
/** 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.',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
/** Error message explaining that Lighthouse could not load the requested URL and the steps that might be taken to fix the unreliability. */
pageLoadFailedWithStatusCode: '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. Status Code: {substitute}',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
/** Error message explaining that Lighthouse could not load the requested URL and the steps that might be taken to fix the unreliability. */
pageLoadFailedWithDetails: '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. Detailed Error: {substitute}',
/** Error message explaining that the credentials included in the Lighthouse run were invalid, so the URL cannot be accessed. */
pageLoadFailedInsecure: 'The URL you have provided does not have valid security credentials. {substitute}',
/** Error message explaining that Chrome has encountered an error during the Lighthouse run, and that Chrome should be restarted. */
internalChromeError: 'An internal Chrome error occurred. Please restart Chrome and try re-running Lighthouse.',
/** Error message explaining that fetching the resources of the webpage has taken longer than the maximum time. */
requestContentTimeout: 'Fetching resource content has exceeded the allotted time',
/** Error message explaining that the provided URL Lighthouse points to is not valid, and cannot be loaded. */
urlInvalid: 'The URL you have provided appears to be invalid.',
/** Error message explaining that the Chrome Devtools protocol has exceeded the maximum timeout allowed. */
protocolTimeout: 'Waiting for DevTools protocol response has exceeded the allotted time. Method: {substitute}',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

/**
* @typedef LighthouseErrorDefinition
* @property {string} code
* @property {string} message
* @property {string} [substitute]
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
* @property {RegExp} [pattern]
* @property {boolean} [lhrRuntimeError] True if it should appear in the top-level LHR.runtimeError property.
*/
Expand All @@ -24,7 +53,9 @@ class LighthouseError extends Error {
super(errorDefinition.code);
this.name = 'LHError';
this.code = errorDefinition.code;
this.friendlyMessage = errorDefinition.message;
this.friendlyMessage = (errorDefinition.substitute) ?
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
str_(errorDefinition.message, {'substitute': errorDefinition.substitute}) :
str_(errorDefinition.message);
this.lhrRuntimeError = !!errorDefinition.lhrRuntimeError;
if (properties) Object.assign(this, properties);

Expand Down Expand Up @@ -70,116 +101,120 @@ const ERRORS = {
// Screenshot/speedline errors
NO_SPEEDLINE_FRAMES: {
code: 'NO_SPEEDLINE_FRAMES',
message: strings.didntCollectScreenshots,
message: UIStrings.didntCollectScreenshots,
lhrRuntimeError: true,
},
SPEEDINDEX_OF_ZERO: {
code: 'SPEEDINDEX_OF_ZERO',
message: strings.didntCollectScreenshots,
message: UIStrings.didntCollectScreenshots,
lhrRuntimeError: true,
},
NO_SCREENSHOTS: {
code: 'NO_SCREENSHOTS',
message: strings.didntCollectScreenshots,
message: UIStrings.didntCollectScreenshots,
lhrRuntimeError: true,
},
INVALID_SPEEDLINE: {
code: 'INVALID_SPEEDLINE',
message: strings.didntCollectScreenshots,
message: UIStrings.didntCollectScreenshots,
lhrRuntimeError: true,
},

// Trace parsing errors
NO_TRACING_STARTED: {
code: 'NO_TRACING_STARTED',
message: strings.badTraceRecording,
message: UIStrings.badTraceRecording,
lhrRuntimeError: true,
},
NO_NAVSTART: {
code: 'NO_NAVSTART',
message: strings.badTraceRecording,
message: UIStrings.badTraceRecording,
lhrRuntimeError: true,
},
NO_FCP: {
code: 'NO_FCP',
message: strings.badTraceRecording,
message: UIStrings.badTraceRecording,
lhrRuntimeError: true,
},
NO_DCL: {
code: 'NO_DCL',
message: strings.badTraceRecording,
message: UIStrings.badTraceRecording,
lhrRuntimeError: true,
},
NO_FMP: {
code: 'NO_FMP',
message: strings.badTraceRecording,
message: UIStrings.badTraceRecording,
},

// TTI calculation failures
FMP_TOO_LATE_FOR_FCPUI: {code: 'FMP_TOO_LATE_FOR_FCPUI', message: strings.pageLoadTookTooLong},
NO_FCPUI_IDLE_PERIOD: {code: 'NO_FCPUI_IDLE_PERIOD', message: strings.pageLoadTookTooLong},
NO_TTI_CPU_IDLE_PERIOD: {code: 'NO_TTI_CPU_IDLE_PERIOD', message: strings.pageLoadTookTooLong},
FMP_TOO_LATE_FOR_FCPUI: {code: 'FMP_TOO_LATE_FOR_FCPUI', message: UIStrings.pageLoadTookTooLong},
NO_FCPUI_IDLE_PERIOD: {code: 'NO_FCPUI_IDLE_PERIOD', message: UIStrings.pageLoadTookTooLong},
NO_TTI_CPU_IDLE_PERIOD: {code: 'NO_TTI_CPU_IDLE_PERIOD', message: UIStrings.pageLoadTookTooLong},
NO_TTI_NETWORK_IDLE_PERIOD: {
code: 'NO_TTI_NETWORK_IDLE_PERIOD',
message: strings.pageLoadTookTooLong,
message: UIStrings.pageLoadTookTooLong,
},

// Page load failures
NO_DOCUMENT_REQUEST: {
code: 'NO_DOCUMENT_REQUEST',
message: strings.pageLoadFailed,
message: UIStrings.pageLoadFailed,
lhrRuntimeError: true,
},
/* Used when DevTools reports loading failed. Usually an internal (Chrome) issue. */
FAILED_DOCUMENT_REQUEST: {
code: 'FAILED_DOCUMENT_REQUEST',
message: strings.pageLoadFailed,
message: UIStrings.pageLoadFailedWithDetails,
lhrRuntimeError: true,
substitute: 'No Failure Description Loaded.',
},
/* Used when status code is 4xx or 5xx. */
ERRORED_DOCUMENT_REQUEST: {
code: 'ERRORED_DOCUMENT_REQUEST',
message: strings.pageLoadFailed,
message: UIStrings.pageLoadFailedWithStatusCode,
lhrRuntimeError: true,
substitute: 'No Error Code Loaded.',
},
/* Used when security error prevents page load. */
INSECURE_DOCUMENT_REQUEST: {
code: 'INSECURE_DOCUMENT_REQUEST',
message: strings.pageLoadFailedInsecure,
message: UIStrings.pageLoadFailedInsecure,
lhrRuntimeError: true,
substitute: 'No Security Descriptions Loaded.',
},

// Protocol internal failures
TRACING_ALREADY_STARTED: {
code: 'TRACING_ALREADY_STARTED',
message: strings.internalChromeError,
message: UIStrings.internalChromeError,
pattern: /Tracing.*started/,
lhrRuntimeError: true,
},
PARSING_PROBLEM: {
code: 'PARSING_PROBLEM',
message: strings.internalChromeError,
message: UIStrings.internalChromeError,
pattern: /Parsing problem/,
lhrRuntimeError: true,
},
READ_FAILED: {
code: 'READ_FAILED',
message: strings.internalChromeError,
message: UIStrings.internalChromeError,
pattern: /Read failed/,
lhrRuntimeError: true,
},

// URL parsing failures
INVALID_URL: {
code: 'INVALID_URL',
message: strings.urlInvalid,
message: UIStrings.urlInvalid,
},

// Protocol timeout failures
PROTOCOL_TIMEOUT: {
code: 'PROTOCOL_TIMEOUT',
message: strings.protocolTimeout,
message: UIStrings.protocolTimeout,
lhrRuntimeError: true,
substitute: 'No Method Loaded.',
},

// Hey! When adding a new error type, update lighthouse-result.proto too.
Expand All @@ -190,4 +225,4 @@ LighthouseError.errors = ERRORS;
LighthouseError.NO_ERROR = 'NO_ERROR';
LighthouseError.UNKNOWN_ERROR = 'UNKNOWN_ERROR';
module.exports = LighthouseError;

module.exports.UIStrings = UIStrings;
19 changes: 0 additions & 19 deletions lighthouse-core/lib/strings.js

This file was deleted.

4 changes: 3 additions & 1 deletion lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class Runner {
* @return {Promise<LH.RunnerResult|undefined>}
*/
static async run(connection, runOpts) {
const settings = runOpts.config.settings;
try {
const settings = runOpts.config.settings;
const runnerStatus = {msg: 'Runner setup', id: 'lh:runner:run'};
log.time(runnerStatus, 'verbose');

Expand Down Expand Up @@ -157,6 +157,8 @@ class Runner {

return {lhr, artifacts, report};
} catch (err) {
// i18n error strings
i18n.replaceIcuMessageInstanceIds(err, settings.locale);
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
await Sentry.captureException(err, {level: 'fatal'});
throw err;
}
Expand Down
13 changes: 10 additions & 3 deletions lighthouse-core/test/gather/gather-runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const assert = require('assert');
const Config = require('../../config/config');
const unresolvedPerfLog = require('./../fixtures/unresolved-perflog.json');
const NetworkRequest = require('../../lib/network-request.js');
const i18n = require('../../lib/i18n/i18n');

class TestGatherer extends Gatherer {
constructor() {
Expand Down Expand Up @@ -646,7 +647,8 @@ describe('GatherRunner', function() {
const error = GatherRunner.getPageLoadError(url, [mainRecord]);
assert.equal(error.message, 'FAILED_DOCUMENT_REQUEST');
assert.equal(error.code, 'FAILED_DOCUMENT_REQUEST');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
i18n.replaceIcuMessageInstanceIds(error, 'en-US');
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
assert.ok(/^Lighthouse was unable to reliably load.*foobar/.test(error.friendlyMessage));
});

it('fails when page times out', () => {
Expand All @@ -655,6 +657,7 @@ describe('GatherRunner', function() {
const error = GatherRunner.getPageLoadError(url, records);
assert.equal(error.message, 'NO_DOCUMENT_REQUEST');
assert.equal(error.code, 'NO_DOCUMENT_REQUEST');
i18n.replaceIcuMessageInstanceIds(error, 'en-US');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
});

Expand All @@ -666,7 +669,8 @@ describe('GatherRunner', function() {
const error = GatherRunner.getPageLoadError(url, [mainRecord]);
assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
i18n.replaceIcuMessageInstanceIds(error, 'en-US');
assert.ok(/^Lighthouse was unable to reliably load.*404/.test(error.friendlyMessage));
});

it('fails when page returns with a 500', () => {
Expand All @@ -677,7 +681,8 @@ describe('GatherRunner', function() {
const error = GatherRunner.getPageLoadError(url, [mainRecord]);
assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST');
assert.ok(/^Lighthouse was unable to reliably load/.test(error.friendlyMessage));
i18n.replaceIcuMessageInstanceIds(error, 'en-US');
assert.ok(/^Lighthouse was unable to reliably load.*500/.test(error.friendlyMessage));
});
});

Expand Down Expand Up @@ -713,6 +718,7 @@ describe('GatherRunner', function() {
} catch (err) {
assert.equal(err.message, 'INSECURE_DOCUMENT_REQUEST');
assert.equal(err.code, 'INSECURE_DOCUMENT_REQUEST');
i18n.replaceIcuMessageInstanceIds(err, 'en-US');
/* eslint-disable-next-line max-len */
assert.equal(err.friendlyMessage, 'The URL you have provided does not have valid security credentials. reason 1. reason 2.');
}
Expand Down Expand Up @@ -1059,6 +1065,7 @@ describe('GatherRunner', function() {
config: new Config({}),
}).then(artifacts => {
assert.equal(artifacts.LighthouseRunWarnings.length, 1);
i18n.replaceIcuMessageInstanceIds(artifacts.LighthouseRunWarnings, 'en-US');
assert.ok(/unable.*load the page/.test(artifacts.LighthouseRunWarnings[0]));
});
});
Expand Down