Previously, the err passed to the callback of fs.open() was not checked.

PR-URL: https://github.com/nodejs/node/pull/18681


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Ref: https://github.com/nodejs/node/issues/18643

PR-URL: https://github.com/nodejs/node/pull/18677
Refs: https://github.com/nodejs/node/issues/18643


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

PR-URL: https://github.com/nodejs/node/pull/18665


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Missing the length argument in napi_create_function.

PR-URL: https://github.com/nodejs/node/pull/18661


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

PR-URL: https://github.com/nodejs/node/pull/18685


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18644


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18637


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Previously, fs.readdirSync calls the function returned by
env->push_values_to_array_function() in batch and check the returned
Maybe right away in C++, which can lead to assertions if the call stack
already reaches the maximum size. This patch fixes that by returning
early the call fails so the stack overflow error will be properly
thrown into JS land.

PR-URL: https://github.com/nodejs/node/pull/18647
Fixes: https://github.com/nodejs/node/issues/18645


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>

Replace var for let or const.

PR-URL: https://github.com/nodejs/node/pull/18649


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

The specific issues raised by Coverity are:

** CID 182716:  Control flow issues  (DEADCODE)
/src/node_file.cc: 1192
>>> CID 182716:  Control flow issues  (DEADCODE)
>>> Execution cannot reach this statement:
    "args->GetReturnValue().Set(...".

** CID 182715:  Uninitialized members  (UNINIT_CTOR)
/src/node_file.h: 29
>>> CID 182715:  Uninitialized members  (UNINIT_CTOR)
>>> Non-static class member "syscall_" is not initialized in this
    constructor nor in any functions that it calls.

PR-URL: https://github.com/nodejs/node/pull/18629


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18581


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Previously, we performed casts that are considered undefined behavior.
Instead, just define `ssize_t` for nghttp2 the same way we define it
for the rest of Node.

Also, remove a TODO comment that would probably also be *technically*
correct but shouldn’t matter as long as nobody is complaining.

PR-URL: https://github.com/nodejs/node/pull/18565


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Implement string decoder in C++. The perks are a decent speed boost
(for decoding, whereas creation show some performance degradation),
that this can now be used more easily to add native decoding support
to C++ streams and (arguably) more readable variable names.

PR-URL: https://github.com/nodejs/node/pull/18537


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Tests relying on synchronous timing have been migrated to use events.

PR-URL: https://github.com/nodejs/node/pull/17828


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

Adds the remaining options from tls.createSecureContext() to the
string generated by Agent#getName(). This allows https.request() to
accept the options and generate unique sockets appropriately.

PR-URL: https://github.com/nodejs/node/pull/16402


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

In ICU 61.x, icu4c will no longer put its declarations in the global namespace.
Everything will be in the "icu::" namespace (or icu_60:: in the linker).
Prepare for this.
https://ssl.icu-project.org/trac/ticket/13460

This adds puctiations to the comments, uses a capital letters for
the first character, removes a few obsolete comments and switches
to assert.ok when suitable.

It also moves all `assert.deepEqual()` and `assert.deepStrictEqual()`
tests to the appropriate file.

PR-URL: https://github.com/nodejs/node/pull/18610


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18683


Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

The code coverage in `root/internal/vm/Module.js` lacked test coverage
for the url options paramter. The test adds a check to ensure error
is thrown.

PR-URL: https://github.com/nodejs/node/pull/18664


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

The emit{Before,After} APIs in AsyncResource are problematic.

* emit{Before,After} are named to suggest that the only thing they do
  is emit the before and after hooks. However, they in fact, mutate
  the current execution context.
* They must be properly nested. Failure to do so by user code leads
  to catastrophic (unrecoverable) exceptions. It is very easy for the
  users to forget that they must be using a try/finally block around
  the code that must be surrounded by these operations. Even the
  example provided in the official docs makes this mistake. Failing
  to use a finally can lead to a catastrophic crash if the callback
  ends up throwing.

This change provides a safer `runInAsyncScope` API as an alternative
and deprecates emit{Before,After}.

PR-URL: https://github.com/nodejs/node/pull/18513


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

Instead of using kOnTimeout index to track a special list
processing function, just pass in a function to C++ at
startup that executes all handles and determines which
function to call.

This change improves the performance of unpooled timeouts
by roughly 20%, as well as makes the unref/ref processing
easier to follow.

PR-URL: https://github.com/nodejs/node/pull/18582


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>

Add optional Http2ServerRequest and Http2ServerResponse options
to createServer and createSecureServer. Allows custom req & res
classes that extend the default ones to be used without
overriding the prototype.

PR-URL: https://github.com/nodejs/node/pull/15560


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Currently doc building doesn't support ES-style default params in
function definitions which causes an error.

PR-URL: https://github.com/nodejs/node/pull/18678


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gus Caplan <me@gus.host>

PR-URL: https://github.com/nodejs/node/pull/18679
Fixes: https://github.com/nodejs/node/issues/18544


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18670


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

The current stack trace thrown in case `assert.throws(fn, object)`
is used did not filter the stack trace. This fixes it.

PR-URL: https://github.com/nodejs/node/pull/18595


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

Users of MakeCallback that adds the domain property to carry context,
should start using the async_context variant of MakeCallback or the
AsyncResource class.

PR-URL: https://github.com/nodejs/node/pull/17417


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: https://github.com/nodejs/node/pull/18617


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18617


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18651
Refs: https://github.com/nodejs/node/pull/8281


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/18651
Refs: https://github.com/nodejs/node/pull/12223


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

For tests that use anonymous namespaces, some tagged the close
of the namespace with 'namespace' while others used
'anonymous namespace'. It was suggested I should use
'anonymous namespace' in a recent PR review so make all of the
tests consistent with this.

PR-URL: https://github.com/nodejs/node/pull/18583


Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Due to extensive reliance on timings and the fs module, this test
is currently inherently flaky. Refactor it to simply use setImmediate
and only one busy loop.

PR-URL: https://github.com/nodejs/node/pull/18567


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Unused since 34b535f4.

PR-URL: https://github.com/nodejs/node/pull/18568


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Remove the various **Note:** prefixes throughout the docs.

PR-URL: https://github.com/nodejs/node/pull/18592


Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

The current output uses JSON.stringify to escape the config values.
This switches to util.inspect to have a better readable output.

PR-URL: https://github.com/nodejs/node/pull/18597


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

The original test case hides the underlying cause by using
`PassThrough`. This change adds a test case for the underlying cause.
This makes it clearer and easier to be understood.

Refs: https://github.com/nodejs/node/pull/18372

PR-URL: https://github.com/nodejs/node/pull/18575


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Remove unnecessary condition from timeout & interval clearing.

PR-URL: https://github.com/nodejs/node/pull/18579


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

When async hooks integration for Timers was introduced, it was
not included in the code for unref'd or subsequently ref'd
timers which means those timers only have Timerwrap hooks.

PR-URL: https://github.com/nodejs/node/pull/18579


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

It's possible for user-code to flip an existing timeout to
be an interval during its execution, in which case the
current code would crash due to start being undefined. Fix
this by providing a default start value within rearm.

PR-URL: https://github.com/nodejs/node/pull/18579


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>