i18n: add i18n for LighthouseError messages #6457
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,40 @@ | |
| */ | ||
| 'use strict'; | ||
|
|
||
| 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. */ | ||
|
There was a problem hiding this comment. 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.
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: {statusCode}.', | ||
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: {errorDetails}.', | ||
exterkamp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** 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. {securityMessages}.', | ||
exterkamp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** 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: {protocolMethod}.', | ||
exterkamp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** Error message explaining that the requested page could not be resolved by the DNS server. */ | ||
| dnsFailure: 'DNS servers could not resolve the provided domain.', | ||
| /** Error message explaining that Lighthouse couldn't complete because the page has stopped responding to its instructions. */ | ||
| pageLoadFailedHung: 'Lighthouse was unable to reliably load the URL you requested because the page stopped responding.', | ||
| }; | ||
|
|
||
| const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); | ||
|
|
||
|
|
||
| /** | ||
| * @typedef LighthouseErrorDefinition | ||
|
|
@@ -24,7 +57,7 @@ class LighthouseError extends Error { | |
| super(errorDefinition.code); | ||
| this.name = 'LHError'; | ||
| this.code = errorDefinition.code; | ||
| this.friendlyMessage = str_(errorDefinition.message, properties); | ||
|
|
||
| this.lhrRuntimeError = !!errorDefinition.lhrRuntimeError; | ||
| if (properties) Object.assign(this, properties); | ||
|
|
||
|
|
@@ -70,128 +103,136 @@ const ERRORS = { | |
| // Screenshot/speedline errors | ||
| NO_SPEEDLINE_FRAMES: { | ||
| code: 'NO_SPEEDLINE_FRAMES', | ||
| message: UIStrings.didntCollectScreenshots, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| SPEEDINDEX_OF_ZERO: { | ||
| code: 'SPEEDINDEX_OF_ZERO', | ||
| message: UIStrings.didntCollectScreenshots, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| NO_SCREENSHOTS: { | ||
| code: 'NO_SCREENSHOTS', | ||
| message: UIStrings.didntCollectScreenshots, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| INVALID_SPEEDLINE: { | ||
| code: 'INVALID_SPEEDLINE', | ||
| message: UIStrings.didntCollectScreenshots, | ||
| lhrRuntimeError: true, | ||
| }, | ||
|
|
||
| // Trace parsing errors | ||
| NO_TRACING_STARTED: { | ||
| code: 'NO_TRACING_STARTED', | ||
| message: UIStrings.badTraceRecording, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| NO_NAVSTART: { | ||
| code: 'NO_NAVSTART', | ||
| message: UIStrings.badTraceRecording, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| NO_FCP: { | ||
| code: 'NO_FCP', | ||
| message: UIStrings.badTraceRecording, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| NO_DCL: { | ||
| code: 'NO_DCL', | ||
| message: UIStrings.badTraceRecording, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| NO_FMP: { | ||
| code: 'NO_FMP', | ||
| message: UIStrings.badTraceRecording, | ||
| }, | ||
|
|
||
| // TTI calculation failures | ||
| 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: UIStrings.pageLoadTookTooLong, | ||
| }, | ||
|
|
||
| // Page load failures | ||
| NO_DOCUMENT_REQUEST: { | ||
| code: 'NO_DOCUMENT_REQUEST', | ||
| message: UIStrings.pageLoadFailed, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| /* Used when DevTools reports loading failed. Usually an internal (Chrome) issue. | ||
| * Requries an additional `errorDetails` field for translation. | ||
| */ | ||
| FAILED_DOCUMENT_REQUEST: { | ||
| code: 'FAILED_DOCUMENT_REQUEST', | ||
| message: UIStrings.pageLoadFailedWithDetails, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| /* Used when status code is 4xx or 5xx. | ||
| * Requires an additional `statusCode` field for translation. | ||
| */ | ||
| ERRORED_DOCUMENT_REQUEST: { | ||
| code: 'ERRORED_DOCUMENT_REQUEST', | ||
| message: UIStrings.pageLoadFailedWithStatusCode, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| /* Used when security error prevents page load. | ||
| * Requires an additional `securityMessages` field for translation. | ||
|
There was a problem hiding this comment. I feel like we're going to forget to update these if they ever change, but I'm not sure what to do about it There was a problem hiding this comment. Well the current plan is that if we change them, then there will be a ton of errors thrown from str_ because it is missing the required substitution fields. But I'm not sure that's a long term strategy. There was a problem hiding this comment. we could maybe figure out something with typescript, but I can't think of a way without keeping a manual list of error codes that will require an extra field. @exterkamp would you want to do that? or wait on it. main issue with
is we need to make sure error cases are getting hit in unit tests and that the unit tests aren't testing for any throw, since |
||
| */ | ||
| INSECURE_DOCUMENT_REQUEST: { | ||
| code: 'INSECURE_DOCUMENT_REQUEST', | ||
| message: UIStrings.pageLoadFailedInsecure, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| /* Used when the page stopped responding and did not finish loading. */ | ||
| PAGE_HUNG: { | ||
| code: 'PAGE_HUNG', | ||
| message: UIStrings.pageLoadFailedHung, | ||
| lhrRuntimeError: true, | ||
| }, | ||
|
|
||
| // Protocol internal failures | ||
| TRACING_ALREADY_STARTED: { | ||
| code: 'TRACING_ALREADY_STARTED', | ||
| message: UIStrings.internalChromeError, | ||
| pattern: /Tracing.*started/, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| PARSING_PROBLEM: { | ||
| code: 'PARSING_PROBLEM', | ||
| message: UIStrings.internalChromeError, | ||
| pattern: /Parsing problem/, | ||
| lhrRuntimeError: true, | ||
| }, | ||
| READ_FAILED: { | ||
| code: 'READ_FAILED', | ||
| message: UIStrings.internalChromeError, | ||
| pattern: /Read failed/, | ||
| lhrRuntimeError: true, | ||
| }, | ||
|
|
||
| // URL parsing failures | ||
| INVALID_URL: { | ||
| code: 'INVALID_URL', | ||
| message: UIStrings.urlInvalid, | ||
| }, | ||
|
|
||
| /* Protocol timeout failures | ||
| * Requires an additional `icuProtocolMethod` field for translation. | ||
| */ | ||
| PROTOCOL_TIMEOUT: { | ||
| code: 'PROTOCOL_TIMEOUT', | ||
| message: UIStrings.protocolTimeout, | ||
| lhrRuntimeError: true, | ||
| }, | ||
|
|
||
| // DNS failure on main document (no resolution, timed out, etc) | ||
| DNS_FAILURE: { | ||
| code: 'DNS_FAILURE', | ||
| message: UIStrings.dnsFailure, | ||
| lhrRuntimeError: true, | ||
| }, | ||
|
|
||
|
|
@@ -203,4 +244,4 @@ LighthouseError.errors = ERRORS; | |
| LighthouseError.NO_ERROR = 'NO_ERROR'; | ||
| LighthouseError.UNKNOWN_ERROR = 'UNKNOWN_ERROR'; | ||
| module.exports = LighthouseError; | ||
| module.exports.UIStrings = UIStrings; | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens with the
parsedResult.runtimeError.message.includescheck below? Are they expected to be localized by that point? Can we assert that? (and can we check that theerrorDetailsreplacement worked)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is what the includes checks, that the runtime message in the end is the same as the orig friendly msg post a round of translation since it is outside of the Runner.run which translates errors before it exits. It uses the includes instead of equal b/c LR appends the msg to the friendly msg which may not be desired? It's the first I'm really seeing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had noticed that
runLighthouseInLRdoesn't run i18n on its generated output, hence the question, but the newerr.friendlyMessage = i18n.getFormatted(err.friendlyMessage, settings.locale);line inrunner.jstranslates the errorfriendlyMessagebefore it gets back torunLighthouseInLR, so localization is good to go!