From 8a718117472a9aa544914c8d3f3df243878b102b Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 9 Apr 2019 14:28:10 -0700 Subject: [PATCH 1/3] misc: drop support for Node before 10.13 (LTS) --- .appveyor.yml | 4 ++-- .travis.yml | 2 -- docs/headless-chrome.md | 8 ++++---- lighthouse-core/lib/asset-saver.js | 5 ++--- package.json | 2 +- readme.md | 2 +- yarn.lock | 6 +++--- 7 files changed, 13 insertions(+), 16 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 71902d735df3..116243c38932 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -9,11 +9,11 @@ branches: environment: fast_finish: true matrix: - - nodejs_version: "8" + - nodejs_version: "10" platform: x86 # other Node versions are skipped, as appveyor only allows 1 concurrent job # and we want appveyor finishing ASAP. see #2382 - # - nodejs_version: "9" + # - nodejs_version: "11" # platform: x86 build: off diff --git a/.travis.yml b/.travis.yml index 506451877ea9..a69b1aa6dc3e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,9 +5,7 @@ branches: - master matrix: include: - - node_js: "8" - node_js: "10" - if: head_branch IS blank AND branch = master - node_js: "11" if: head_branch IS blank AND branch = master dist: trusty diff --git a/docs/headless-chrome.md b/docs/headless-chrome.md index 1c6ed4f75204..1537e17e2ae7 100644 --- a/docs/headless-chrome.md +++ b/docs/headless-chrome.md @@ -5,8 +5,8 @@ Setup: ```sh -# Lighthouse requires Node 8 LTS (8.9) or later. -curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash - &&\ +# Lighthouse requires Node 10 LTS (10.13) or later. +curl -sL https://deb.nodesource.com/setup_10.x | sudo -E bash - &&\ sudo apt-get install -y nodejs npm # get chromium (stable) @@ -27,8 +27,8 @@ lighthouse --chrome-flags="--headless" https://github.com Alternatively, you can run full Chrome + xvfb instead of headless mode. These steps worked on Debian Jessie: ```sh -# get node 8 -curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash - +# get node 10 +curl -sL https://deb.nodesource.com/setup_10.x | sudo -E bash - sudo apt-get install -y nodejs npm # get chromium (stable) and Xvfb diff --git a/lighthouse-core/lib/asset-saver.js b/lighthouse-core/lib/asset-saver.js index 75cdf628da0d..c0ba8a1172d2 100644 --- a/lighthouse-core/lib/asset-saver.js +++ b/lighthouse-core/lib/asset-saver.js @@ -139,9 +139,8 @@ async function prepareAssets(artifacts, audits) { } /** - * Generates a JSON representation of traceData line-by-line to avoid OOM due to very large traces. - * COMPAT: As of Node 9, JSON.parse/stringify can handle 256MB+ strings. Once we drop support for - * Node 8, we can 'revert' PR #2593. See https://stackoverflow.com/a/47781288/89484 + * Generates a JSON representation of traceData line-by-line for a nicer printed + * version with one trace event per line. * @param {LH.Trace} traceData * @return {IterableIterator} */ diff --git a/package.json b/package.json index 5386e3cd2ab5..ea368674ac54 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "chrome-debug": "./lighthouse-core/scripts/manual-chrome-launcher.js" }, "engines": { - "node": ">=8.10" + "node": ">=10.13" }, "scripts": { "build-all": "npm-run-posix-or-windows build-all:task", diff --git a/readme.md b/readme.md index 77b8512269d0..c394015e1184 100644 --- a/readme.md +++ b/readme.md @@ -24,7 +24,7 @@ The Chrome extension was available prior to Lighthouse being available in Chrome 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._ +_Lighthouse requires Node 10 LTS (10.13) or later._ **Installation**: diff --git a/yarn.lock b/yarn.lock index 182622b84614..0bfb3ddbe134 100644 --- a/yarn.lock +++ b/yarn.lock @@ -556,9 +556,9 @@ integrity sha1-fyrX7FX5FEgvybHsS7GuYCjUYGY= "@types/node@*", "@types/node@^9.3.0": - version "8.9.5" - resolved "https://registry.yarnpkg.com/@types/node/-/node-8.9.5.tgz#162b864bc70be077e6db212b322754917929e976" - integrity sha512-jRHfWsvyMtXdbhnz5CVHxaBgnV6duZnPlQuRSo/dm/GnmikNcmZhxIES4E9OZjUmQ8C+HCl4KJux+cXN/ErGDQ== + version "10.14.0" + resolved "https://registry.yarnpkg.com/@types/node/-/node-10.14.0.tgz#1c297530428c6f4e0a0a3222f5b44745669aa9f7" + integrity sha512-1UhSMMDix7bVdUeqtZERQQyJr3QuFoN5X5APtpIooGkumE3crPaeq7UgFeJNjGD8yCQ8od8PzRkgptR5+x327Q== "@types/opn@^3.0.28": version "3.0.28" From a5fce57424749a37657c07af518602cf7255599d Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 9 Apr 2019 15:37:20 -0700 Subject: [PATCH 2/3] fix error.code type errors --- .appveyor.yml | 4 ---- lighthouse-cli/run.js | 10 ++++++---- lighthouse-core/gather/connections/cri.js | 9 ++++----- lighthouse-core/lib/lh-error.js | 9 +++++++++ lighthouse-core/scripts/update-report-fixtures.js | 12 +++++++++--- proto/lighthouse-result.proto | 2 ++ types/externs.d.ts | 10 ---------- 7 files changed, 30 insertions(+), 26 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 116243c38932..71fa74955d9f 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -11,10 +11,6 @@ environment: matrix: - nodejs_version: "10" platform: x86 - # other Node versions are skipped, as appveyor only allows 1 concurrent job - # and we want appveyor finishing ASAP. see #2382 - # - nodejs_version: "11" - # platform: x86 build: off diff --git a/lighthouse-cli/run.js b/lighthouse-cli/run.js index 10cbf8f6effb..357b19046a89 100644 --- a/lighthouse-cli/run.js +++ b/lighthouse-cli/run.js @@ -20,6 +20,8 @@ const assetSaver = require('../lighthouse-core/lib/asset-saver.js'); const opn = require('opn'); +/** @typedef {import('../lighthouse-core/lib/lh-error.js')} LighthouseError */ + const _RUNTIME_ERROR_CODE = 1; const _PROTOCOL_TIMEOUT_EXIT_CODE = 67; const _PAGE_HUNG_EXIT_CODE = 68; @@ -74,20 +76,20 @@ function showProtocolTimeoutError() { return process.exit(_PROTOCOL_TIMEOUT_EXIT_CODE); } -/** @param {LH.LighthouseError} err @return {never} */ +/** @param {LighthouseError} err @return {never} */ function showPageHungError(err) { console.error('Page hung:', err.friendlyMessage); return process.exit(_PAGE_HUNG_EXIT_CODE); } -/** @param {LH.LighthouseError} err @return {never} */ +/** @param {LighthouseError} err @return {never} */ function showInsecureDocumentRequestError(err) { console.error('Insecure document request:', err.friendlyMessage); return process.exit(_INSECURE_DOCUMENT_REQUEST_EXIT_CODE); } /** - * @param {LH.LighthouseError} err + * @param {LighthouseError} err * @return {never} */ function showRuntimeError(err) { @@ -99,7 +101,7 @@ function showRuntimeError(err) { } /** - * @param {LH.LighthouseError} err + * @param {LighthouseError} err * @return {never} */ function handleError(err) { diff --git a/lighthouse-core/gather/connections/cri.js b/lighthouse-core/gather/connections/cri.js index f0b2713d1e36..8b47e11b9e3b 100644 --- a/lighthouse-core/gather/connections/cri.js +++ b/lighthouse-core/gather/connections/cri.js @@ -9,6 +9,7 @@ const Connection = require('./connection.js'); const WebSocket = require('ws'); const http = require('http'); const log = require('lighthouse-logger'); +const LighthouseError = require('../../lib/lh-error.js'); const DEFAULT_HOSTNAME = 'localhost'; const CONNECT_TIMEOUT = 10000; @@ -80,7 +81,6 @@ class CriConnection extends Connection { * @private */ _runJsonCommand(command) { - // TODO(bckenny): can base type on command once conditional types land in TS return new Promise((resolve, reject) => { const request = http.get({ hostname: this.hostname, @@ -113,16 +113,15 @@ class CriConnection extends Connection { // After aborting, we expect an ECONNRESET error. Ignore. request.on('error', err => { + // @ts-ignore - not in @types yet. See https://nodejs.org/api/errors.html#errors_class_systemerror. if (err.code !== 'ECONNRESET') { throw err; } }); - // TODO: Replace this with an LHError on next major version bump // Reject on error with code specifically indicating timeout in connection setup. - const err = new Error('Timeout waiting for initial Debugger Protocol connection.'); - err.code = 'CRI_TIMEOUT'; - log.error('CriConnection', err.message); + const err = new LighthouseError(LighthouseError.errors.CRI_TIMEOUT); + log.error('CriConnection', err.friendlyMessage); reject(err); }); }); diff --git a/lighthouse-core/lib/lh-error.js b/lighthouse-core/lib/lh-error.js index d1ef25f2c99e..a349667fbc03 100644 --- a/lighthouse-core/lib/lh-error.js +++ b/lighthouse-core/lib/lh-error.js @@ -35,6 +35,8 @@ const UIStrings = { 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.', + /** Error message explaining that Lighthouse timed out while waiting for the initial connection to the Chrome Devtools protocol. */ + criTimeout: 'Timeout waiting for initial Debugger Protocol connection.', }; const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); @@ -227,6 +229,13 @@ const ERRORS = { lhrRuntimeError: true, }, + /** A timeout in the initial connection to the debugger protocol. */ + CRI_TIMEOUT: { + code: 'CRI_TIMEOUT', + message: UIStrings.criTimeout, + lhrRuntimeError: true, + }, + // Hey! When adding a new error type, update lighthouse-result.proto too. }; diff --git a/lighthouse-core/scripts/update-report-fixtures.js b/lighthouse-core/scripts/update-report-fixtures.js index ec1ed590e770..8aabdc464b6f 100644 --- a/lighthouse-core/scripts/update-report-fixtures.js +++ b/lighthouse-core/scripts/update-report-fixtures.js @@ -5,10 +5,12 @@ */ 'use strict'; -const cli = require('../../lighthouse-cli/run'); +const cli = require('../../lighthouse-cli/run.js'); const cliFlags = require('../../lighthouse-cli/cli-flags.js'); -const {server} = require('../../lighthouse-cli/test/fixtures/static-server'); +const {server} = require('../../lighthouse-cli/test/fixtures/static-server.js'); + +/** @typedef {import('net').AddressInfo} AddressInfo */ /** * Update the report artifacts @@ -16,7 +18,11 @@ const {server} = require('../../lighthouse-cli/test/fixtures/static-server'); async function update() { // get an available port server.listen(0, 'localhost'); - const port = await new Promise(res => server.on('listening', () => res(server.address().port))); + const port = await new Promise(res => server.on('listening', () => { + // Not a pipe or a domain socket, so will not be a string. See https://nodejs.org/api/net.html#net_server_address. + const address = /** @type {AddressInfo} */ (server.address()); + res(address.port); + })); const url = `http://localhost:${port}/dobetterweb/dbw_tester.html`; const flags = cliFlags.getFlags(`--gather-mode=lighthouse-core/test/results/artifacts ${url}`); diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto index ce8ba59567d4..d0783a76cc64 100644 --- a/proto/lighthouse-result.proto +++ b/proto/lighthouse-result.proto @@ -51,6 +51,8 @@ enum LighthouseError { PAGE_HUNG = 18; // DNS failure on main document (no resolution, timed out, etc) DNS_FAILURE = 19; + // A timeout in the initial connection to the debugger protocol. + CRI_TIMEOUT: 20; } // The overarching Lighthouse Response object (LHR) diff --git a/types/externs.d.ts b/types/externs.d.ts index a708bd8fe2d4..ba302b5fd32e 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -8,12 +8,6 @@ import _Crdp from 'devtools-protocol/types/protocol'; import _CrdpMappings from 'devtools-protocol/types/protocol-mapping' declare global { - // Augment global Error type to include node's optional `code` property - // see https://nodejs.org/api/errors.html#errors_error_code - interface Error { - code?: string; - } - // Augment Intl to include // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/getCanonicalLocales namespace Intl { @@ -202,10 +196,6 @@ declare global { group: string; } - export interface LighthouseError extends Error { - friendlyMessage?: string; - } - /** * A record of DevTools Debugging Protocol events. */ From 6426ba0dcd7ed76a96961b43bfddf22e7deb293a Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 9 Apr 2019 16:16:15 -0700 Subject: [PATCH 3/3] not a JS object --- lighthouse-core/lib/i18n/en-US.json | 4 ++++ proto/lighthouse-result.proto | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/lib/i18n/en-US.json b/lighthouse-core/lib/i18n/en-US.json index dd8d1eebef06..9eecf480202d 100644 --- a/lighthouse-core/lib/i18n/en-US.json +++ b/lighthouse-core/lib/i18n/en-US.json @@ -1211,6 +1211,10 @@ "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 | criTimeout": { + "message": "Timeout waiting for initial Debugger Protocol connection.", + "description": "Error message explaining that Lighthouse timed out while waiting for the initial connection to the Chrome Devtools protocol." + }, "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. ({errorCode})", "description": "Error message explaining that the Lighthouse run was not able to collect screenshots through Chrome." diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto index d0783a76cc64..df1a1543766b 100644 --- a/proto/lighthouse-result.proto +++ b/proto/lighthouse-result.proto @@ -52,7 +52,7 @@ enum LighthouseError { // DNS failure on main document (no resolution, timed out, etc) DNS_FAILURE = 19; // A timeout in the initial connection to the debugger protocol. - CRI_TIMEOUT: 20; + CRI_TIMEOUT = 20; } // The overarching Lighthouse Response object (LHR)