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

misc: drop support for Node before 10.13 (LTS) #8117

Merged
merged 3 commits into from
Apr 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 1 addition & 5 deletions .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,8 @@ 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"
# platform: x86

build: off

Expand Down
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions docs/headless-chrome.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
10 changes: 6 additions & 4 deletions lighthouse-cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -99,7 +101,7 @@ function showRuntimeError(err) {
}

/**
* @param {LH.LighthouseError} err
* @param {LighthouseError} err
* @return {never}
*/
function handleError(err) {
Expand Down
9 changes: 4 additions & 5 deletions lighthouse-core/gather/connections/cri.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
});
Expand Down
5 changes: 2 additions & 3 deletions lighthouse-core/lib/asset-saver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

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

even though we can do this now, I think we like the one trace event per line format, so it made sense to drop the old comment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, we love it

* 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<string>}
*/
Expand Down
4 changes: 4 additions & 0 deletions lighthouse-core/lib/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
9 changes: 9 additions & 0 deletions lighthouse-core/lib/lh-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
};

Expand Down
12 changes: 9 additions & 3 deletions lighthouse-core/scripts/update-report-fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,24 @@
*/
'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
*/
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}`);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions proto/lighthouse-result.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**:

Expand Down
10 changes: 0 additions & 10 deletions types/externs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -202,10 +196,6 @@ declare global {
group: string;
}

export interface LighthouseError extends Error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

might have been nice to just export our class as this type so we could easily reference it as LH.LighthouseError still but I don't feel strongly

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I'd like to add this to our public types (when those actually exist :)

friendlyMessage?: string;
}

/**
* A record of DevTools Debugging Protocol events.
*/
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down