adding test coverage for napi_cancel_async_work based
on coverage report

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12575


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

We chose to document this in the docs as there are
different possible behaviours.  Adding this test to validate
that all vm implementations do it the same way.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12633


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

N-API is somewhat strict about blocking calls to many APIs while there
is a pending exception. The NAPI_PREAMBLE macro at the beginning of
many API implementations checks for a pending exception. However, a
subset of the APIs (which don't call back into JavaScript) still need
to work while in a pending-exception state. This changes the reference
APIs (equivalent to v8::Persistent) and handle scope APIs so that they
can be used for cleanup up while an exception is pending.

We may decide to similarly enable a few other APIs later, (which would
be a non-breaking change) but we know at least these are needed now
to unblock some specific scenarios.

Fixes: https://github.com/nodejs/abi-stable-node/issues/122
Fixes: https://github.com/nodejs/abi-stable-node/issues/228
Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12524


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>

We left out null-checks for many of the parameters passed to our APIs.
In particular, arguments of type `napi_value` were often accepted
without a null-check, even though they should never be null.

Additionally, many APIs simply returned `napi_ok` on success. This
leaves in place an error that may have occurred in a previous N-API
call. Others (those which perform `NAPI_PREAMBLE(env)` at the top)
OTOH explicitly clear the last error before proceeding. With this
modification all APIs explicitly clear the last error on success.

Fixes: https://github.com/nodejs/abi-stable-node/issues/227
Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12539


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

This API doesn't serve much purpose, and is only likely to cause
confusion and bugs. The intention was that this would return the
number of characters in a string independent of encoding, but
that's not generally useful. In almost all cases, one of the
encoding-specific napi_get_value_string_* APIs is more correct.
(Pass a null buffer if only the encoded length is desired.)

Anyway the current implementation of napi_get_value_string_length()
is technically wrong: it returns the number of 2-byte code units of
the UTF-16 encoding, but there are actually some characters that
are encoded as two UTF-16 code units.

Note the JavaScript String.prototype.length property returns the
number of UTF-16 code units, which may be different from the number
of characters. So, getting the true character count is not common
with JavaScript, and is probably best left to specialized
internationalization libraries.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12496
Fixes: https://github.com/nodejs/abi-stable-node/issues/226


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

This reverts commit 70b51c8c.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12475


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Improved test coverage for napi_make_callback by porting the
existing addons/make_callback test to n-api

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12409


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Coverity was reporting _request.work_req as not being initialized.
Add memset to ensure all of _request is initialized.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12365


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

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12368


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>

Missed in ca786c37. This does
not actually affect the outcome because returning `nullptr` or
`this` from a constructor has the same effect.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12318


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

This adds RegExp or error constructor arguments to the remaining places
where it is missing in preparation for the commit that will enforce the
presence of at least two arguments.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12270


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Add basic tests for handle scopes as code coverage
reports that we are not covering these with the existing
tests.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12327


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

`napi_env__` was declared as a struct in one place and a class in
another.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12333


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Based on the async methods we had in abi-stable-node before the napi
feature landed in node/master. Changed this set of APIs to handle
error cases and removed a lot of the extra methods we had for setting
all the pieces of napi_work opting instead to pass all of those as
arguments to napi_create_async_work as none of those parameters are
optional except for the complete callback, anyway.

Renamed the napi_work struct to napi_async_work and replace the
struct itself with a class which can better encapsulate the object
lifetime and uv_work_t that we're trying to wrap anyway.

Added a napi_async_callback type for the async helper callbacks
instead of taking raw function pointers and make this callback take a
napi_env parameter as well as the void* data it was already taking.

Call the complete handler for the async work item with a napi_status
code translated from the uvlib error code.

The execute callback is required for napi_create_async_work, though
complete callback is still optional.

Also added some async unit tests for addons-napi based on the
addons/async_hello_world test.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12250


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>

Change `napi_callback` to return `napi_value` directly instead of
requiring `napi_set_return_value`.

When we invoke the callback, we will check the return value and
call `SetReturnValue` ourselves. If the callback returns `NULL`,
we don't set the return value in v8 which would have the same
effect as previously if the callback didn't call
`napi_set_return_value`. Seems to be a more natural way
to handle return values from callbacks. As a consequence,
remove `napi_set_return_value`.

Add a `napi_value` to `napi_property_descriptor` to support string
values which couldn't be passed in the `utf8name` parameter or
symbols as property names. Class names, however, cannot be symbols
so this `napi_value` must be a string type in that case.

Remove all of the `napi_callback_info` helpers except for
`napi_get_cb_info` and make all the parameters to
`napi_get_cb_info` optional except for argc.

Update all the test collateral according to these changes.
Also add `test/addons-napi/common.h` to house some common macros
for wrapping N-API calls and error handling.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12248


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Initialize status to napi_generic_failure and only check it after
having made an actual N-API call.

This fixes up 8fbace16.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12283
Ref: https://github.com/nodejs/node/pull/12279


Reviewed-By: Refael Ackermann <refack@gmail.com>

This improves the performance of napi_instanceof() by retrieving
Symbol.hasInstance from the global object once and then storing a
persistent reference to it in the env.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12246


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

The napi_property_attributes enum used names and values from
v8::PropertyAttribute, but those negative flag names were outdated
along with the default behavior of a property being writable,
enumerable, and configurable unless otherwise specified. To match the
ES5 standard property descriptor those attributes should be positive
flags and should default to false unless otherwise specified.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12240
Fixes: https://github.com/nodejs/abi-stable-node/issues/221


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

1. We define struct napi_env__ to include the isolate, the last
exception, and the info about the last error.

2. We instantiate one struct napi_env__ during module registration and
we pass it into the FunctionCallbackInfo for all subsequent entries into
N-API when we create functions/accessors/finalizers.

Once module unloading will be supported we shall have to delete the
napi_env we create during module init.

There is a clear separation between public and private API wrt. env:

1. Public APIs assert that env is not nullptr as their first action.

2. Private APIs need not validate env. They assume it's not nullptr.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12195
Fixes: https://github.com/nodejs/abi-stable-node/issues/198


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Coverity was complaining that finalizer was being
leaked in this method, however it should be
freed when the buffer is finalized so I believe
the message is invalid.

Add the required comments to suppress the warning.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12192


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>

The v8 n-api implementation had been depending on a one-to-one
relationship between v8 and n-api v8 property attributes.
Remove this dependency and fix coverity scan issue
165845.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/12191


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.

A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.

The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.

The API is defined by the methods in "src/node_api.h" and
"src/node_api_types.h". This is the best
starting point to review the API surface. More documentation will f...

This PR makes sure the object emitted as the 'unpipe'
event in the destination stream is not shared between
destination, as it would be muted.

Refs: https://github.com/nodejs/node/pull/12746
PR-URL: https://github.com/nodejs/node/pull/18266


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Fix the uncommon situation when a readable stream is piped twice into
the same destination stream, and then unpiped once.

Previously, the `unpipe` event handlers weren’t able to tell whether
they were corresponding to the “right” conceptual pipe that was being
removed; this fixes this by adding a counter to the `unpipe` event
handler and only removing a single piping destination at most.

Fixes: https://github.com/nodejs/node/issues/12718
PR-URL: https://github.com/nodejs/node/pull/12746


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

Fixes: https://github.com/nodejs/node/issues/18302

Backport-PR-URL: https://github.com/nodejs/node/pull/19639
PR-URL: https://github.com/nodejs/node/pull/18357


Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

* Replace 2 hyphens (--) by spaced m-dashes (—) as per STYLE_GUIDE.md.
* Space infix operators.
* Unify quotes in inline code spans (use only single quotes).
* Unify `* Returns:` (eliminate deviations).
* Dedupe spaces.

Backport-PR-URL: https://github.com/nodejs/node/pull/19761
PR-URL: https://github.com/nodejs/node/pull/19743


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Currently `Agent.prototype.createConnection()` is called uncoditionally
if the `socketPath` option is used. This throws an error if no agent is
used, preventing, for example, the `socketPath` and `createConnection`
options to be used together.

This commit fixes the issue by falling back to the `createConnection`
option or `net.createConnection()`.

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


Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Chen Gang <gangc.cxy@foxmail.com>

Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

In a conversation Fedor requested that this PR be made. They plan
to continue working on core as a Collaborator. It is this committers
belief that if Fedor would like to join the TSC again in the future
there is no reason that could not be made possible.

Thanks for all the hard work!

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


Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.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: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Tobia...

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


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>

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/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>

Add reference to strategic initiatives on core README.md
in order to ensure broarder visibility.

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


Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

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


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

Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: https://github.com/nodejs/node/pull/17977
Fixes: https://github.com/nodejs/node/issues/17970

,
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

This test was added over six years ago, and the behavior
seems to have changed since then. When the test was originally
written, common.mustCall() did not exist. The only assertion
in this test was not actually executing. Instead of an 'error'
event being emitted as expected, an 'abort' event was emitted.

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@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>

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>