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.

Sign up for GitHub

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

Jump to bottom

v12.17.0 release proposal #33197

Merged
merged 349 commits into from May 26, 2020
Merged

v12.17.0 release proposal #33197

merged 349 commits into from May 26, 2020

Conversation

targos
Copy link
Member

@targos targos commented May 1, 2020 •
edited

2020-05-26, Version 12.17.0 'Erbium' (LTS), @targos

Notable Changes

ECMAScript Modules - --experimental-modules flag removal

As of Node.js 12.17.0, the --experimental-modules flag is no longer necessary
to use ECMAScript modules (ESM). However, the ESM implementation in Node.js
remains experimental. As per our stability index: “The feature is not subject
to Semantic Versioning rules. Non-backward compatible changes or removal may
occur in any future release.” Users should be cautious when using the feature
in production environments.

Unlike Node.js 14, using ESM will still emit a runtime experimental warning,
either when a module is used a the application's entrypoint or the first time
dynamic import() is called.

Please keep in mind that the implementation of ESM in Node.js differs from the
developer experience you might be familiar with. Most transpilation workflows
support features such as named exports from CommonJS module imports, optional
file extensions or JSON modules that the Node.js ESM implementation does not
support. It is highly likely that modules from transpiled environments will
require a certain degree of refactoring to work in Node.js. It is worth
mentioning that many of our design decisions were made with two primary goals.
Spec compliance and Web Compatibility. It is our belief that the current
implementation offers a future proof model to authoring ESM modules that paves
the path to Universal JavaScript. Please read more in our documentation.

The ESM implementation in Node.js is still experimental but we do believe that
we are getting very close to being able to call ESM in Node.js “stable”.
Removing the flag is a huge step in that direction.

We expect to remove the warning Node.js 12 later this year, possibly in late
October, when Node.js 14 will become LTS.

AsyncLocalStorage API (experimental)

The AsyncLocalStorage class has been introduced in the Async Hooks module.

This API allows keeping a context across asynchronous operations. For instance,
if a sequence id is stored within an instance of AsyncLocalStorage for each
HTTP request entering in a server, it will be possible to retrieve this id
without having access the current HTTP request:

const http = require('http');
const { AsyncLocalStorage } = require('async_hooks');

const asyncLocalStorage = new AsyncLocalStorage();

function logWithId(msg) {
  const id = asyncLocalStorage.getStore();
  console.log(`${id !== undefined ? id : '-'}: `, msg);
}

let idSeq = 0;
http.createServer((req, res) => {
  asyncLocalStorage.run(idSeq++, () => {
    logWithId('start');
    // Imagine any chain of async operations here.
    setImmediate(() => {
      logWithId('finish');
      res.end();
    });
  });
}).listen(8080);

In this example, the logWithId function will always know what the current
request id is, even when there are multiple requests in parallel.

What can this API be used for

Use cases of this API include:

  • Logging
  • User identification
  • Performance tracking
  • Error tracking and handling
  • Much more!

Note: This API is still experimental and some methods might change in future releases of Node.js

Contributed by Vladimir de Turckheim - #26540.

REPL previews

If further input is predicable, a suggestion is inserted as preview.

The REPL now supports previews similar to the Chrome DevTools console. An input
suggestion is inserted as preview in case further input is predicable. The
suggestion may be accepted by either pressing <TAB> or <RIGHT> at the end of
the input.
On top of that, output is previewed when entering variable names or function
calls that have no side effect.

image
image

Check the preview in action
and try it out on your own. Just access the REPL on your terminal by starting
the Node.js executable without any further command.

Contributed by Ruben Bridgewater - #30907, #30811.

REPL reverse-i-search

The REPL supports bi-directional reverse-i-search similar to
ZSH. It is triggered with <ctrl> + R
to search backwards and <ctrl> + S to search forwards.

Entries are accepted as soon as any button is pressed that doesn't correspond
with the reverse search. Cancelling is possible by pressing escape or
<ctrl> + C.

Changing the direction immediately searches for the next entry in the expected
direction from the current position on.

image

Reverse-i-search in action.

Contributed by Ruben Bridgewater - #31006.

REPL substring-based search

It is now possible to access former history entries very fast by writing the
first characters of the formerly entered code you are looking for. Then push
<UP> or <DOWN> to go through the history entries that start with those
characters.

It works similar to the Fish Shell substring-based
history search.

Contributed by Ruben Bridgewater - #31112.

Error monitoring

Monitoring error events

It is now possible to monitor 'error' events on an EventEmitter without
consuming the emitted error by installing a listener using the symbol
EventEmitter.errorMonitor:

const myEmitter = new MyEmitter();

myEmitter.on(EventEmitter.errorMonitor, (err) => {
  MyMonitoringTool.log(err);
});

myEmitter.emit('error', new Error('whoops!'));
// Still throws and crashes Node.js

Contributed by Gerhard Stoebich - #30932.

Monitoring uncaught exceptions

It is now possible to monitor 'uncaughtException' events without overriding
the default behavior that exits the process by installing an
'uncaughtExceptionMonitor' listener:

process.on('uncaughtExceptionMonitor', (err, origin) => {
  MyMonitoringTool.logSync(err, origin);
});

// Intentionally cause an exception, but do not catch it.
nonexistentFunc();
// Still crashes Node.js

Contributed by Gerhard Stoebich - #31257.

File system APIs

New function: fs.readv

This new function (along with its sync and promisified versions) takes an array
of ArrayBufferView elements and will write the data it reads sequentially to
the buffers.

Contributed by Sk Sajidul Kadir - #32356.

Optional parameters in fs.read

A new overload is available for fs.read (along with its sync and promisified
versions), which allows to optionally pass any of the offset, length and
position parameters.

Contributed by Lucas Holmquist - #31402.

Console groupIndentation option

The Console constructor (require('console').Console) now supports different group indentations.

This is useful in case you want different grouping width than 2 spaces.

const { Console } = require('console');
const customConsole = new Console({
  stdout: process.stdout,
  stderr: process.stderr,
  groupIndentation: 10
});

customConsole.log('foo');
// 'foo'
customConsole.group();
customConsole.log('foo');
//           'foo'

Contributed by rickyes - #32964.

maxStringLength option for util.inspect()

It is now possible to limit the length of strings while inspecting objects.
This is possible by passing through the maxStringLength option similar to:

const { inspect } = require('util');

const string = inspect(['a'.repeat(1e8)], { maxStringLength: 10 });

console.log(string);
// "[ 'aaaaaaaaaa'... 99999990 more characters ]"

Contributed by rosaxny - #32392.

Stable N-API release 6

The following N-API features are now stable as part of the N-API 6 release:

Stable diagnostic reports

The Diagnostic Report
feature is now stable and supports a new --report-compact flag to write the
reports in a compact, single-line JSON format, more easily consumable by log
processing systems than the default multi-line format designed for human
consumption.

Increase of the default server headers timeout

The default value of server.headersTimeout for http and https servers was
increased from 40000 to 60000 (60 seconds). This to accomodate for systems
like AWS ELB that have a timeout of 60 seconds.

Contributed by Tim Costa - #30071.

Other changes

  • cli:
    • Added a --trace-sigint CLI flag that will print the current execution
      stack on SIGINT (legendecas) #29207.
  • crypto:
    • Various crypto APIs now support Diffie-Hellman secrets (Tobias Nießen) #31178.
  • dns:
    • Added the dns.ALL flag, that can be passed to dns.lookup() with dns.V4MAPPED
      to return resolved IPv6 addresses as well as IPv4 mapped IPv6 addresses (murgatroid99) #32183.
  • module
    • Added a new experimental API to interact with Source Map V3 data (Benjamin Coe) #31132.
  • worker:
    • Added support for passing a transferList along with workerData to the
      Worker constructor (Juan José Arboleda) #32278.

bnoordhuis and others added 30 commits April 28, 2020 16:00
@bnoordhuis @targos
test: show child stderr output in largepages test
4aaf407 
The test starts child processes. A recent change is suspected of causing
flaky crashes on one of the alpine buildbots but we can't know for sure
because the test hides the child's stderr.

Refs: #31547 (comment)

PR-URL: #31612
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax @targos
worker: add ability to take heap snapshot from parent thread
ab8f38b 
PR-URL: #31569
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@fanatid @targos
perf_hooks: add property flags to GCPerformanceEntry
9eeee0d 
PR-URL: #29547
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@BridgeAR @targos
test: improve logged errors
a4ee930 
To indicate which lines are test lines and which from Node.js core,
it's good to rely on `util.inspect()` while inspecting errors.

The stack was accessed directly instead in multiple cases and logging
that does not provide as much information as using `util.inspect()`.

PR-URL: #31425
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yihong Wang <yh.wang@ibm.com>
@addaleax @targos
test: fix flaky test-trace-sigint-on-idle
2fc72ca 
Previously, the test could fail on slow machines because the
child process was still in the process of starting up after
one second, and not yet idle.

To resolve this:
- Wait for a message from the child process indicating that it
  had started.
- Wait some time after that, but make it platform-dependent to
  account for timing differences.
- Remove the timer in the child process.

PR-URL: #31645
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@addaleax @targos
src: do not unnecessarily re-assign uv handle data
38de40a 
a555be2 re-assigned `async_.data` to indicate success
or failure of the constructor. As the `HandleWrap` implementation
uses that field to access the `HandleWrap` instance from the
libuv handle, this introduced two issues:

- It implicitly assumed that casting
  `MessagePort*` → `void*` → `HandleWrap*` would be valid.
- It made the `HandleWrap::OnClose()` function fail with a
  `nullptr` dereference if the constructor did fail.

In particular, the second issue made
test/parallel/test-worker-cleanexit-with-moduleload.js` crash at
least once in CI.

Since re-assigning `async_.data` isn’t actually necessary here
(only a leftover from earlier versions of that commit), fix this by
using a local variable instead, and add a `CHECK` that provides better
error messages for this type of issue in the future.

Refs: #31605

PR-URL: #31696
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @targos
Revert "benchmark: remove special test entries"
ef80c02 
This reverts commit 357230f.

Refs: #31396
PR-URL: #31722
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax @targos
Revert "benchmark: refactor helper into a class"
cf2ca11 
This reverts commit b70741e.

Refs: #31396
PR-URL: #31722
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR @targos
test: fix flaky parallel/test-repl-history-navigation test
8a044cb 
Two scenarios should be tested:

1. The completion is triggered and the result is printed before the
   next invocation.
2. The completion is triggered multiple times right after each other
   without waiting for the result. In that case only the last result
   should be printed.

The first scenario did not need a timeout while the latter did not
need a timeout for the second invocation.

PR-URL: #31708
Fixes: #31094
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@joyeecheung @targos
vm: lazily initialize primordials for vm contexts
02de66a 
Lazily initialize primordials when cross-context support for
builtins is needed to fix the performance regression in context
creation.

PR-URL: #31738
Fixes: #29842
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
@simllll @targos
tls: expose SSL_export_keying_material
6348fae 
Fixes: #31802

PR-URL: #31814
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@danbev @targos
test: add secp224k1 check in crypto-dh-stateless
d1d22fa 
This commit adds a check to test-crypto-dh-statless to avoid an error if
the curve secp224k1 is not present. This could occur if node was
configured with shared-openssl.

The use case for this was building on RHEL 8.1 which only has the
following curves:
$ openssl ecparam -list_curves
secp224r1 : NIST/SECG curve over a 224 bit prime field
secp256k1 : SECG curve over a 256 bit prime field
secp384r1 : NIST/SECG curve over a 384 bit prime field
secp521r1 : NIST/SECG curve over a 521 bit prime field
prime256v1: X9.62/SECG curve over a 256 bit prime field

PR-URL: #31715
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig @targos
build: add missing comma in node.gyp
e765d59 
This commit adds a missing comma for consistency with the
surrounding lines.

PR-URL: #31959
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
@bcoe @targos
module: port source map sort logic from chromium
b62db59 
Digging in to the delta between V8's source map library, and chromium's
the most significant difference that jumped out at me was that we were
failing to sort generated columns. Since negative offsets are not
restricted in the spec, this can lead to bugs.

fixes: #31286

PR-URL: #31927
Fixes: #31286
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@tniessen @targos
crypto: simplify exportKeyingMaterial
0847bc3 
PR-URL: #31922
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@jasnell @targos
src: improve handling of internal field counting
c979aea 
Change suggested by bnoordhuis.

Improve handing of internal field counting by using enums.
Helps protect against future possible breakage if field
indexes are ever changed or added to.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #31960
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@addaleax @targos
dgram: make UDPWrap more reusable
322a998 
Allow using the handle more directly for I/O in other parts of
the codebase.

Originally landed in the QUIC repo

Original review metadata:

```
  PR-URL: nodejs/quic#165
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #31871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jasnell @targos
lib: move isLegalPort to validators, refactor
434ca87 
isLegalPort was used multiple places in the same way -- to validate
the port and throw if necessary. Moved into internal/validators.

PR-URL: #31851
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@jasnell @targos
src: introduce node_sockaddr
83c47b6 
Introduce the SocketAddress utility class. The QUIC implementation
makes extensive use of this for handling of socket addresses. It
was separated out to make it generically reusable throughout core

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #32070
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
@cjihrig @targos
wasi: add returnOnExit option
5c81b8d 
This commit adds a WASI option allowing the __wasi_proc_exit()
function to return an exit code instead of forcefully terminating
the process.

PR-URL: #32101
Fixes: #32093
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@juanarbol @targos
src: create a getter for kernel version
394487e 
Backport-PR-URL: #32166
PR-URL: #31732
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@legendecas @targos
src: add missing namespace using statements in node_watchdog.h
154da1f 
Although these `using`s can derived from other header files, it will
be better to be self-contained.

PR-URL: #32117
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@HarshithaKP @targos
src: handle NULL env scenario
51a3456 
Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc.

PR-URL: #31899
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@BridgeAR @targos
benchmark: refactor helper into a class
011e3de 
This reverts commit 5b0308c.

PR-URL: #31755
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR @targos
benchmark: add test and all options and improve errors"
7139788 
This reverts commit 4671d55 and
contains a fix to the issue raised for the revert.

PR-URL: #31755
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR @targos
test: refactor all benchmark tests to use the new test option
5a8e54b 
This reverts commit aa0a01b.

PR-URL: #31755
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR @targos
benchmark: remove special test entries
b4a6351 
This reverts commit 7cfbc9f.

PR-URL: #31755
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott @targos
doc: remove personal pronoun usage in addons.md
c98ba95 
Per our style guide, avoid personal pronouns (I, you, we, etc.) in
reference documentation.

PR-URL: #32142
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@Trott @targos
doc: remove personal pronoun usage in errors.md
9a7f892 
Per our style guide, avoid personal pronouns (I, you, we, etc.) in
reference documentation.

PR-URL: #32142
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@Trott @targos
doc: remove personal pronoun usage in fs.md
97689e0 
Per our style guide, avoid personal pronouns (I, you, we, etc.) in
reference documentation.

PR-URL: #32142
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@targos
Copy link
Member Author

targos commented May 25, 2020

/cc @nodejs/platform-aix

There's an issue with CI, can you please take a look?

https://ci.nodejs.org/job/node-test-commit-aix/30702/nodes=aix71-ppc64/console

16:34:38 if [ -d doc/api/assets ]; then cp -r doc/api/assets out/doc/api; fi;
16:34:39 if [ -x /home/iojs/build/workspace/node-test-commit-aix/nodes/aix71-ppc64/./node ] && [ -e /home/iojs/build/workspace/node-test-commit-aix/nodes/aix71-ppc64/./node ]; then /home/iojs/build/workspace/node-test-commit-aix/nodes/aix71-ppc64/./node  tools/doc/versions.js out/previous-doc-versions.json; elif [ -x `which node` ] && [ -e `which node` ] && [ `which node` ]; then `which node`  tools/doc/versions.js out/previous-doc-versions.json; else echo "No available node, cannot run \"node  tools/doc/versions.js out/previous-doc-versions.json\""; exit 1; fi;
16:35:09 Error: socket hang up
16:35:09     at connResetException (internal/errors.js:609:14)
16:35:09     at TLSSocket.socketCloseListener (_http_client.js:400:25)
16:35:09     at TLSSocket.emit (events.js:327:22)
16:35:09     at net.js:674:12
16:35:09     at TCP.done (_tls_wrap.js:566:7) {
16:35:09   code: 'ECONNRESET'
16:35:09 }
16:35:09 gmake[2]: *** [Makefile:794: out/previous-doc-versions.json] Error 1
16:35:09 gmake[1]: *** [Makefile:752: doc-only] Error 2
16:35:09 gmake: *** [Makefile:587: run-ci] Error 2

@targos
Copy link
Member Author

targos commented May 25, 2020

Running AIX CI on v12.x to check if that works: https://ci.nodejs.org/job/node-test-commit-aix/30710/

@targos
Copy link
Member Author

targos commented May 25, 2020

@nodejs/releasers what do you suggest here? This seems to introduce a bug on AIX but I have no idea what could be the cause...

@addaleax
Copy link
Member

@targos I can’t do it tonight because it’s a bit late for that, but would it be worth for somebody to take an AIX CI machine and bisect on that to see where it’s coming from? I don’t know what other path forward there is

@richardlau
Copy link
Member

I can take a look in the morning.

@targos
Copy link
Member Author

targos commented May 26, 2020 •
edited

@richardlau thanks! should I still start to bisect?

Edit: started.

Bisecting: 174 revisions left to test after this (roughly 8 steps)
[c572218] benchmark: use let instead of var in util - https://ci.nodejs.org/job/node-test-commit-aix/30712/

$ git bisect good
Bisecting: 87 revisions left to test after this (roughly 7 steps)
[d883024] http2: wait for secureConnect before initializing - https://ci.nodejs.org/job/node-test-commit-aix/30715/

$ git bisect good
Bisecting: 43 revisions left to test after this (roughly 6 steps)
[e1fe0b6] wasi: rename __wasi_unstable_reactor_start() - https://ci.nodejs.org/job/node-test-commit-aix/30717/ (red but not same error)

$ git bisect good
Bisecting: 21 revisions left to test after this (roughly 5 steps)
[9570644] lib: cosmetic change to builtinLibs list for maintainability - https://ci.nodejs.org/job/node-test-commit-aix/30718/

$ git bisect good
Bisecting: 10 revisions left to test after this (roughly 4 steps)
[f127ae3] doc: clarify when not to run CI on docs - https://ci.nodejs.org/job/node-test-commit-aix/30719/

$ git bisect good
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[a3decf5] http: simplify sending header - https://ci.nodejs.org/job/node-test-commit-aix/30720/

$ git bisect good
Bisecting: 2 revisions left to test after this (roughly 2 steps)
[2d76ae7] deps: update to ICU 67.1 - https://ci.nodejs.org/job/node-test-commit-aix/30721/

$ git bisect good
Bisecting: 0 revisions left to test after this (roughly 1 step)
[ebd9090] http: disable headersTimeout check when set to zero - https://ci.nodejs.org/job/node-test-commit-aix/30724/

$ git bisect good
cd4ae7c is the first bad commit
commit cd4ae7c - https://ci.nodejs.org/job/node-test-commit-aix/30725/

@richardlau
Copy link
Member

richardlau commented May 26, 2020 •
edited

@richardlau thanks! should I still start to bisect?

👋 Thanks for starting the bisect. FWIW I've also started bisecting (using git bisect run ...) on an AIX system I have access to inside IBM.

Edit: Unfortunately it looks like I can't reproduce the failure on the system I've been testing on.

@targos
Copy link
Member Author

targos commented May 26, 2020

@richardlau I'm really puzzled now. My bisect session points to the release commit!

@addaleax
Copy link
Member

addaleax commented May 26, 2020 •
edited

@targos I’m assuming the test is failing always? Or is it failing in a flaky way?

@richardlau
Copy link
Member

This is looking like some sort of infrastructure issue where the AIX CI machine doesn't appear to be able to download https://raw.githubusercontent.com/ URLs.

e.g.

Running AIX CI on v12.x to check if that works: https://ci.nodejs.org/job/node-test-commit-aix/30710/

(Note this build passed)

...
+ curl https://raw.githubusercontent.com/nodejs/build/master/jenkins/scripts/node-test-commit-pre.sh
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0+ bash -xe

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
...
  0     0    0     0    0     0      0      0 --:--:--  0:01:14 --:--:--     0
curl: (28) Failed to connect to raw.githubusercontent.com port 443: A remote host did not respond within the timeout period.
...
if [ -x /home/iojs/build/workspace/node-test-commit-aix/nodes/aix71-ppc64/./node ] && [ -e /home/iojs/build/workspace/node-test-commit-aix/nodes/aix71-ppc64/./node ]; then /home/iojs/build/workspace/node-test-commit-aix/nodes/aix71-ppc64/./node  tools/doc/versions.js out/previous-doc-versions.json; elif [ -x `which node` ] && [ -e `which node` ] && [ `which node` ]; then `which node`  tools/doc/versions.js out/previous-doc-versions.json; else echo "No available node, cannot run \"node  tools/doc/versions.js out/previous-doc-versions.json\""; exit 1; fi;
Unable to retrieve https://raw.githubusercontent.com/nodejs/node/master/CHANGELOG.md. Falling back to /home/iojs/build/workspace/node-test-commit-aix/nodes/aix71-ppc64/CHANGELOG.md.
...

The tools/doc/versions.js tool falls back to the local copy of the CHANGELOG.md for non-release commits (which is why only the release commit build failed) to accommodate users with poor/firewalled Internet connections.

cc @nodejs/platform-aix

@targos
Copy link
Member Author

targos commented May 26, 2020

It failed 3 times in a row before I started bisecting.
Here's a new run: https://ci.nodejs.org/job/node-test-commit-aix/30725/

@targos
Copy link
Member Author

targos commented May 26, 2020

OK, then can I build and promote the release?

@richardlau
Copy link
Member

OK, then can I build and promote the release?

I'd be okay with it but I can't remember if the release build attempts to build the docs (in which case it's likely to fail if the release AIX machine has the same issue as the two test ones).

@targos
Copy link
Member Author

targos commented May 26, 2020

Release: https://ci-release.nodejs.org/job/iojs+release/6061/

@targos
Copy link
Member Author

targos commented May 26, 2020

AIX build succeeded

@richardlau
Copy link
Member

@nodejs/platform-aix We do need to address the issue in #33197 (comment) but given all the bisect AIX CI's passed I'm okay with going ahead with the v12.17.0 release today.

@targos targos mentioned this pull request May 26, 2020
Closed
@targos targos merged commit cd4ae7c into v12.x May 26, 2020
targos added a commit that referenced this pull request May 26, 2020
@targos
Working on v12.17.1
dc4b125 
PR-URL: #33197
targos added a commit that referenced this pull request May 26, 2020
@targos
2020-05-26, Version 12.17.0 'Erbium' (LTS)
dcc112b 
Notable changes:

* ECMAScript Modules - `--experimental-modules` flag removal
* AsyncLocalStorage API (experimental)
* REPL previews
* REPL reverse-i-search
* REPL substring-based search
* Error monitoring
  * Monitoring `error` events
  * Monitoring uncaught exceptions
* File system APIs
  * New function: `fs.readv`
  * Optional parameters in `fs.read`
* Console `groupIndentation` option
* `maxStringLength` option for `util.inspect()`
* Stable N-API release 6
* Stable diagnostic reports
* Increase of the default server headers timeout
* New `--trace-sigint` CLI flag
* Various crypto APIs now support Diffie-Hellman secrets
* Added support for the `dns.ALL` flag in `dns.lookup()`
* Added a new experimental API to interact with Source Map V3 data
* Added support for passing a `transferList` along with `workerData` to
  the `Worker` constructor

PR-URL: #33197
targos added a commit to nodejs/nodejs.org that referenced this pull request May 26, 2020
@targos
Blog: v12.17.0 release post
de24289 
Refs: nodejs/node#33197
@targos targos mentioned this pull request May 26, 2020
Merged
@targos targos deleted the v12.17.0-proposal branch May 26, 2020 14:07
targos added a commit to nodejs/nodejs.org that referenced this pull request May 26, 2020
@targos
Blog: v12.17.0 release post (#3194)
e2aa129 
Refs: nodejs/node#33197
@blakeembrey
Copy link
Contributor

blakeembrey commented Jun 11, 2020 •
edited

It looks like #33209 should have been back ported with d883024, I'm seeing a behavior with http2 clients hanging on node.js 12.17 and 12.18.

@mshustov mshustov mentioned this pull request Aug 22, 2020
Closed
@targos targos added release Issues and PRs related to Node.js releases. and removed build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. labels Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@codebytere codebytere codebytere approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@Flarna Flarna Flarna approved these changes

Assignees
No one assigned
Labels
release Issues and PRs related to Node.js releases.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet