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

v15.5.0 release proposal #36597

Merged
merged 108 commits into from
Dec 22, 2020
Merged

v15.5.0 release proposal #36597

merged 108 commits into from
Dec 22, 2020

Conversation

targos
Copy link
Member

@targos targos commented Dec 22, 2020

2020-12-22, Version 15.5.0 (Current), @targos

Notable Changes

Extended support for AbortSignal in child_process and stream

The following APIs now support an AbortSignal in their options object:

  • child_process.spawn()

Calling .abort() on the corresponding AbortController is similar to calling .kill() on the child process except the error passed to the callback will be an AbortError:

const controller = new AbortController();
const { signal } = controller;
const grep = spawn('grep', ['ssh'], { signal });
grep.on('error', (err) => {
  // This will be called with err being an AbortError if the controller aborts
});
controller.abort(); // stops the process
  • new stream.Writable() and new stream.Readable()

Calling .abort() on the corresponding AbortController will behave the same way as calling .destroy(new AbortError()) on the stream:

const { Readable } = require('stream');
const controller = new AbortController();
const read = new Readable({
  read(size) {
    // ...
  },
  signal: controller.signal
});
// Later, abort the operation closing the stream
controller.abort();

Contributed by Benjamin Gruenbaum #36431, #36432.

BigInt support in querystring.stringify()

If querystring.stringify() is called with an object that contains BigInt values, they will now be serialized to their decimal representation instead of the empty string:

const querystring = require('querystring');
console.log(querystring.stringify({ bigint: 2n ** 64n }))
// Prints: bigint=18446744073709551616

Contributed by Darshan Sen #36499.

Additions to the C++ embedder APIs

A new IsolateSettingsFlag is available for those calling SetIsolateUpForNode(): SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK can be used to prevent Node.js from setting a custom callback to prepare stack traces.

Contributed by Shelley Vohr #36447.


Added node::GetEnvironmentIsolateData() and node::GetArrayBufferAllocator() to respectively get the current IsolateData* and, from it, the current Node.js ArrayBufferAllocator if there is one.

Contributed by Anna Henningsen #36441.

New core collaborator

With this release, we welcome a new Node.js core collaborator:

Commits

Semver-minor commits

  • [e449571230] - (SEMVER-MINOR) child_process: add signal support to spawn (Benjamin Gruenbaum) #36432
  • [25d7e90386] - (SEMVER-MINOR) http: use autoDestroy: true in incoming message (Daniele Belardi) #33035
  • [5481be8cbd] - (SEMVER-MINOR) lib: support BigInt in querystring.stringify (raisinten) #36499
  • [036ed1fafc] - (SEMVER-MINOR) src: add way to get IsolateData and allocator from Environment (Anna Henningsen) #36441
  • [e23309486b] - (SEMVER-MINOR) src: allow preventing SetPrepareStackTraceCallback (Shelley Vohr) #36447
  • [6ecbc1dcb3] - (SEMVER-MINOR) stream: support abortsignal in constructor (Benjamin Gruenbaum) #36431

Semver-patch commits

  • [1330995b80] - build,lib,test: change whitelist to allowlist (Michaël Zasso) #36406
  • [dc8d1a74a6] - deps: upgrade npm to 7.3.0 (Ruy Adorno) #36572
  • [b6a31f0a70] - deps: update archs files for OpenSSL-1.1.1i (Myles Borins) #36520
  • [5b49807c3f] - deps: re-enable OPENSSL_NO_QUIC guards (James M Snell) #36520
  • [309e2971a2] - deps: various quic patches from akamai/openssl (Todd Short) #36520
  • [27fb651cbc] - deps: upgrade openssl sources to 1.1.1i (Myles Borins) #36520
  • [1f43aadf90] - deps: update patch and docs for openssl update (Myles Borins) #36520
  • [752c94d202] - deps: fix npm doctor tests for pre-release node (nlf) #36543
  • [b0393fa2ed] - deps: upgrade npm to 7.2.0 (Myles Borins) #36543
  • [cb4652e91d] - deps: update to c-ares 1.17.1 (Danny Sonnenschein) #36207
  • [21fbcb6f81] - deps: V8: backport 4bf051d536a1 (Anna Henningsen) #36482
  • [30fe0ff681] - deps: upgrade npm to 7.1.2 (Darcy Clarke) #36487
  • [0baa610c3e] - deps: upgrade npm to 7.1.1 (Ruy Adorno) #36459
  • [5929b08851] - deps: upgrade npm to 7.1.0 (Ruy Adorno) #36395
  • [deaafd5788] - dns: refactor to use more primordials (Antoine du Hamel) #36314
  • [e30af7be33] - fs: refactor to use optional chaining (ZiJian Liu) #36524
  • [213dcd7930] - http: add test for incomingmessage destroy (Daniele Belardi) #33035
  • [36b4ddd382] - http: use standard args order in IncomingMEssage onError (Daniele Belardi) #33035
  • [60b5e696fc] - http: remove trailing space (Daniele Belardi) #33035
  • [f11a648d8e] - http: add comments in _http_incoming (Daniele Belardi) #33035
  • [4b81d79b58] - http: fix lint error in incoming message (Daniele Belardi) #33035
  • [397e31e25f] - http: reafactor incoming message destroy (Daniele Belardi) #33035
  • [9852ebca8d] - http: do not loop over prototype in Agent (Michaël Zasso) #36410
  • [e46a46a4cd] - inspector: refactor to use more primordials (Antoine du Hamel) #36356
  • [728f512c7d] - lib: make safe primordials safe to iterate (Antoine du Hamel) #36391
  • [f368d697cf] - Revert "perf_hooks: make PerformanceObserver an AsyncResource" (Nicolai Stange) #36343
  • [e2ced0d401] - perf_hooks: invoke performance_entry_callback via MakeSyncCallback() (Nicolai Stange) #36343
  • [7c903ec6c8] - repl: disable blocking completions by default (Anna Henningsen) #36564
  • [bbc0d14cd2] - src: use correct microtask queue for checkpoints (Anna Henningsen) #36581
  • [7efb3111e8] - src: remove unnecessary ToLocalChecked call (Daniel Bevenius) #36523
  • [68687d3419] - src: remove empty name check in node_env_var.cc (raisinten) #36133
  • [1b4984de98] - src: remove duplicate V macros in node_v8.cc (Daniel Bevenius) #36454
  • [5ff7f42e65] - src: use correct outer Context’s microtask queue (Anna Henningsen) #36482
  • [96c095f237] - src: guard against env != null in node_errors.cc (Anna Henningsen) #36414
  • [4f3d7bb417] - src: introduce convenience node::MakeSyncCallback() (Nicolai Stange) #36343
  • [e59788262c] - src: add typedef for CleanupHookCallback callback (Daniel Bevenius) #36442
  • [2a60e3b9df] - src: fix indentation in memory_tracker-inl.h (Daniel Bevenius) #36425
  • [210390f6fd] - src: remove identical V macro (Daniel Bevenius) #36427
  • [02afe586aa] - src: use using declarations consistently (Daniel Bevenius) #36365
  • [169406b7d7] - src: add missing context scopes (Anna Henningsen) #36413
  • [3f33d0bcda] - stream: fix pipe deadlock when starting with needDrain (Robert Nagy) #36563
  • [d8b5b9499c] - stream: accept iterable as a valid first argument (ZiJian Liu) #36479
  • [58319d5336] - tls: forward new SecureContext options (Alba Mendez) #36416
  • [fa40366276] - util: simplify constructor retrieval in inspect() (Rich Trott) #36466
  • [cc544dbfaa] - util: fix instanceof checks with null prototypes during inspection (Ruben Bridgewater) #36178
  • [13d6597b4b] - util: fix module prefixes during inspection (Ruben Bridgewater) #36178
  • [20ecc82569] - worker: fix broadcast channel SharedArrayBuffer passing (Anna Henningsen) #36501
  • [56fe9bae26] - worker: refactor MessagePort entanglement management (Anna Henningsen) #36345

Documentation commits

Other commits

targos and others added 30 commits December 13, 2020 22:18
Fixes: #36364

PR-URL: #36410
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Fixes: #36246

PR-URL: #36248
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: #36314
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
In the discussion of #35323
it was suggested that we should add some
additional context/clarification to the technical
values documented for the project.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #36201
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Make sure all the `wc` process stdout data is received before checking
its validity.

Fixes: #25988

PR-URL: #36366
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: #36368
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Add a test to check util.inspect()'s handling of a null
prototype-of-an-iterable-prototype. This covers a previously uncovered
code branch.

Refs: https://coverage.nodejs.org/coverage-0fd121e00c9d5987/lib/internal/util/inspect.js.html#L597

PR-URL: #36399
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Add scopes that ensure that the context associated with the
current Environment is always entered when working with it.

PR-URL: #36413
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
PR-URL: #36407
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
We added an `AbortError` with the same code and name as the web's as
part of the consensus in #36084 but
never actually documented the error in our error codes list.

This PR adds the error code.

PR-URL: #36319
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #36365
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
This commit removes one of the V macros in IsolateData::MemoryInfo as
they are identical as far as I can tell.

PR-URL: #36427
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
PR-URL: #36425
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
The timeout is insufficient in CI (failures on Raspberry Pi) and I can
reproduce locally (on macOS) with
`tools/test.py -j 96 --repeat 192 test-repl`.

Increase timeout drastically as it only is useful in an error
condition.

PR-URL: #36415
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: #36395
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
I would like to apply for the role of a triager in this project.
My motivation to become a triager is to help with the issues in
this repo and the help repo, as well as learn deeper internals
of Node.js, and to eventually become a collaborator!
I hearby declare that I have read and understood the Code of
Conduct of the project and will adhere to that.

PR-URL: #36404
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Update ESLint to 7.15.0

PR-URL: #36411
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
This rule is new in ESLint 7.15.0.

PR-URL: #36411
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
PR-URL: #36436
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Verify that if something different than Abortcontroller.signal is passed
to child_process.execFile(), ERR_INVALID_ARG_TYPE is thrown.

PR-URL: #36429
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
PR-URL: #36356
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: #36431
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #36475
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #36406
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
This commit adds a typedef for the callback parameter used in
CleanupHookCallback's constructor, AddCleanupHook, and
RemoveCleanupHook.

PR-URL: #36442
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
There are situations where one wants to invoke a JS callback's ->Call()
from C++ and in particular retain any existing async_context state, but
where it's not obvious that a plain ->Call() would be safe at the point
in question.

Such callsites usually resort to
node::MakeCallback(..., async_context{0, 0}), which unconditionally
pushes the async_context{0, 0} and takes the required provisions for the
->Call() itself such as triggering the tick after its return, if needed.

An example would be the PerformanceObserver invocation from
PerformanceEntry::Notify(): this can get called when coming from JS
through e.g. perf_hooks.performance.mark() and alike, but perhaps also
from nghttp2 (c.f. EmitStatistics() in node_http2.cc).

In the former case, a plain ->Call() would be safe and it would be
desirable to retain the current async_context so that
PerformanceObservers can access it resp. the associated
AsyncLocalStorage. However, in the second case the additional provisions
taken by node::MakeCallback() might potentially be strictly required.

So PerformanceEntry::Notify() bites the bullet and invokes the
PerformanceObservers through node::MakeCallback() unconditionally,
thereby always rendering any possibly preexisting async_context
inaccessible.

Introduce the convenience node::MakeSyncCallback() for such usecases,
which would basically forward to ->Call() if safe and to
node::MakeCallback(..., async_context{0, 0}) otherwise.

Co-Authored-By: ZauberNerd <[email protected]>

PR-URL: #36343
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
It's desirable to retain async_contexts active at callsites of
perf_hooks.performance.mark() and alike in the subsequent
PerformanceObserver invocations such that the latter can access e.g.
associated AsyncLocalStorage instances.

In working towards this goal replace the node::MakeCallback(...,
async_context{0, 0}) in PerformanceEntry::doNotify() by the new
node::MakeSyncCallback() introduced specifically for this purpose.

This change will retain the original async_context, if any, in
perf_hook's observersCallback() and thus, for the subsequent doNotify()
on unbuffered PerformanceObservers.

Co-Authored-By: ZauberNerd <[email protected]>

PR-URL: #36343
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
This reverts commit 009e418.

AFAIU the discussion at [1], PerformanceObserver had been made to
inherit from AsyncResource more or less as a band-aid in lack of a
better async_context candidate to invoke it in. In order to enable
access to AsyncLocalStores from PerformanceObservers invoked
synchronously through e.g. measure() or mark(), the current
async_context, if any, should be retained.

Note that this is a breaking change, but
- as has been commented at [1], PerformanceObserver being derived from
  AsyncResource is a "minor divergence from the spec" anyway,
- to my knowledge this is an internal implementation detail which has
  never been documented and
- I can't think of a good reason why existing PerformanceObserver
  implementations would possibly rely on it.

OTOH, it's probably worthwhile to not potentially invoke before() and
after() async_hooks for each and every PerformanceObserver notification.

[1] #18789

Co-Authored-By: ZauberNerd <[email protected]>

PR-URL: #36343
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
This test proves that the PerformanceObserver callback gets called with
the async context of the callsite of performance.mark()/measure() and
therefore AsyncLocalStorage can be used inside a PerformanceObserver.

PR: #36343

PR-URL: #36343
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Refs: #36473
Refs: GHSA-qqgx-2p2h-9c37

PR-URL: #36474
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@ExE-Boss
Copy link
Contributor

Do we want to include #36549 in this?

@addaleax
Copy link
Member

@ExE-Boss I’ve added that PR to the commit queue, so it should land in the next minutes, but I also don’t see any reason to rush it.

targos and others added 6 commits December 22, 2020 14:51
PR-URL: #36596
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
PR-URL: #36547
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
V8's `tools/dev/v8gen.py` does not like being passed an empty string
(`""`).

PR-URL: #36594
Refs: #36099
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
PR-URL: #36586
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Notable changes:

    child_process:
      * (SEMVER-MINOR) add signal support to spawn (Benjamin Gruenbaum) #36432
    doc:
      * add PoojaDurgad to collaborators (Pooja D P) #36511
    lib:
      * (SEMVER-MINOR) support BigInt in querystring.stringify (raisinten) #36499
    src:
      * (SEMVER-MINOR) add way to get IsolateData and allocator from Environment (Anna Henningsen) #36441
      * (SEMVER-MINOR) allow preventing SetPrepareStackTraceCallback (Shelley Vohr) #36447
    stream:
      * (SEMVER-MINOR) support abortsignal in constructor (Benjamin Gruenbaum) #36431

PR-URL: #36597
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 22, 2020

@targos
Copy link
Member Author

targos commented Dec 22, 2020

@richardlau done
@ExE-Boss sorry, it will be in the next release

@targos
Copy link
Member Author

targos commented Dec 22, 2020

Are there known issues with AIX CI? It failed twice for different reasons.

@richardlau
Copy link
Member

Are there known issues with AIX CI? It failed twice for different reasons.

I'm not aware of any.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 22, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/35042/ ✔️

@targos
Copy link
Member Author

targos commented Dec 22, 2020

It passed the retry 🤷

@targos
Copy link
Member Author

targos commented Dec 22, 2020

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

@targos targos merged commit f978628 into v15.x Dec 22, 2020
targos added a commit that referenced this pull request Dec 22, 2020
targos added a commit that referenced this pull request Dec 22, 2020
Notable changes:

    child_process:
      * (SEMVER-MINOR) add signal support to spawn (Benjamin Gruenbaum) #36432
    doc:
      * add PoojaDurgad to collaborators (Pooja D P) #36511
    lib:
      * (SEMVER-MINOR) support BigInt in querystring.stringify (raisinten) #36499
    src:
      * (SEMVER-MINOR) add way to get IsolateData and allocator from Environment (Anna Henningsen) #36441
      * (SEMVER-MINOR) allow preventing SetPrepareStackTraceCallback (Shelley Vohr) #36447
    stream:
      * (SEMVER-MINOR) support abortsignal in constructor (Benjamin Gruenbaum) #36431

PR-URL: #36597
targos added a commit to nodejs/nodejs.org that referenced this pull request Dec 22, 2020
@targos targos deleted the v15.5.0-proposal branch December 22, 2020 19:01
targos added a commit to nodejs/nodejs.org that referenced this pull request Dec 22, 2020
@targos targos added the release Issues and PRs related to Node.js releases. label Apr 12, 2021
@targos targos removed build Issues and PRs related to build files or the CI. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Issues and PRs related to Node.js releases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.