Streams were recently updated to emit their own close event. The
Http2Stream was an exception because it included the close argument
with the close event. Refactor that to use the built in close.
PR-URL: https://github.com/nodejs/node/pull/19451
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/19481
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Assign a code to a user-facing error.
Turn other internal-only errors to checks.
PR-URL: https://github.com/nodejs/node/pull/19387
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Put the "who to cc" information in the COLLABORATOR_GUIDE. The
onboarding-extras doc is a bit of miscellaneous collection. Rather than
stashing things in a junk drawer doc, put them where they are relevant.
PR-URL: https://github.com/nodejs/node/pull/19460
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/19458
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Remove bulleted list formatting. Only check email/name for new
contributors. Change `full name` to `name` and remove italics to avoid
incorrect implication that we need a "legal name".
PR-URL: https://github.com/nodejs/node/pull/19458
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
PR-URL: https://github.com/nodejs/node/pull/19458
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
This re-writes the test in C by dropping std::vector<napi_value> in
favour of a C99 variable length array, and by dropping the anonymous
namespace in favour of static function declarations.
PR-URL: https://github.com/nodejs/node/pull/19448
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
When using cluster and --inspect as cli argument it is correctly
handled and each worker will use a different port, this was
fixed by #13619. But when env var NODE_OPTIONS="--inspect"
is set this logic doesn't apply and the workers will fail as they
try to attach to the same port.
Fixes: https://github.com/nodejs/node/issues/19026
PR-URL: https://github.com/nodejs/node/pull/19165
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
This adds a rule that forbids the use of native Error constructors in
the `lib` directory. This is to encourage use of the `internal/errors`
mechanism. The rule is disabled for errors that are not created with
the `internal/errors` module but are still assigned an error code.
PR-URL: https://github.com/nodejs/node/pull/19373
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
After this commit, all errors thrown from JS code in lib have an error
code.
PR-URL: https://github.com/nodejs/node/pull/19373
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Also consistently import the `common` module where possible.
PR-URL: https://github.com/nodejs/node/pull/19409
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Currently the writable side of the socket is closed as soon as `UV_EOF`
is read regardless of the state of the socket. This allows the handle
to be closed before `'end'` is emitted and thus `'close'` can be
emitted before `'end'` if the socket is paused.
This commit prevents the handle from being closed until `'end'` is
emitted ensuring the correct order of events.
PR-URL: https://github.com/nodejs/node/pull/19241
Fixes: https://github.com/nodejs/node/issues/19166
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Wait for the sockets to be connected before closing them and remove
unneeded `setTimeout()`.
PR-URL: https://github.com/nodejs/node/pull/19241
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
createCipher and createDecipher are cryptographically weak, can cause
severe security issues when used improperly and are unsupported in FIPS
mode.
PR-URL: https://github.com/nodejs/node/pull/19343
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Place `writable.destroy()`, `readable.destroy()`,
and `readable.read()` in the proper ABC order.
PR-URL: https://github.com/nodejs/node/pull/19501
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This also updates some parts of the onboarding. It is mainly about
how to handle pull requests, how and when to start a CI, when to add
the `ready` label and some other minor changes.
PR-URL: https://github.com/nodejs/node/pull/19116
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Chen Gang <gangc.cxy@foxmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Glen Keane <glenkeane.94@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The first parameter to the callback is `err`. Fix that. Expand example
to demonstrate an actual write to disk.
PR-URL: https://github.com/nodejs/node/pull/19379
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Add function to trigger and uncaught exception.
Useful if an async callback throws an exception with
no way to recover.
PR-URL: https://github.com/nodejs/node/pull/19337
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
The test uses both `assert()` and `assert.ok()`. Use just `assert.ok()`.
Remove a comment that does not appear to match the code and does not
seem to explain much beyond the bare code itself.
PR-URL: https://github.com/nodejs/node/pull/19404
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit introduces a copyHooks function that can be used by
storeActiveHooks and restoreActiveHooks to remove some code duplication.
PR-URL: https://github.com/nodejs/node/pull/19391
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Commit 870229e66529309dfea932c52d718ddc2d734966 ("src: Add ABORT
macro") replaced the abort call with the abort macro in util-inl.h.
This commit removes the include as it is not needed anymore.
PR-URL: https://github.com/nodejs/node/pull/19427
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit adds proper error handling to net.connect() when
a custom lookup() function returns an invalid address family.
PR-URL: https://github.com/nodejs/node/pull/19415
Fixes: https://github.com/nodejs/node/issues/19407
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
The test is unreliable on some Windows platforms in its current form.
Make it more robust by using `setInterval()` to repeat the flooding
until an error is triggered.
PR-URL: https://github.com/nodejs/node/pull/19395
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Place the test_make_callback async_hooks-related test into its own file.
PR-URL: https://github.com/nodejs/node/pull/19392
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
- Check if the watcher is active in JS land before
invoking the binding, act as a noop if the state of
the watcher does not match the expectation. This
avoids firing 'stop' when the watcher is already
stopped.
- Update comments, validate more arguments and
the type of the handle.
- Handle the errors from uv_fs_poll_start
- Create an `IsActive` helper method on StatWatcher
PR-URL: https://github.com/nodejs/node/pull/19345
Refs: https://github.com/nodejs/node/pull/19089
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
- Remove ERR_FS_WATCHER_ALREADY_STARTED and
ERR_FS_WATCHER_NOT_STARTED because those two situations should
result in noop instead of errors for consistency with the
documented behavior of fs.watchFile.
This partially reverts https://github.com/nodejs/node/pull/19089
- Update comments about this behavior.
Refs: https://github.com/nodejs/node/pull/19089
PR-URL: https://github.com/nodejs/node/pull/19345
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
In the async case, if the buffer was copied instead of being moved
then the buf will not get deleted after the request is done.
This was introduced when the FSReqWrap:: Ownership was
removed in 4b9ba9b, and ReleaseEarly was no longer called upon
destruction of FSReqWrap.
Create a custom Init function so we can use the MaybeStackBuffer in
the FSReqBase to copy the string in the async case. The data will
then get destructed when the FSReqBase is destructed.
Fixes: https://github.com/nodejs/node/issues/19356
PR-URL: https://github.com/nodejs/node/pull/19357
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/19429
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Rename the tests appropriately alongside mentioning the subsystem.
Also, make a few basic changes to make sure the tests conform to the
standard test structure.
- Rename test-regress-GH-io-1068 to test-tty-stdin-end
- Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror
- Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd
- Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop
- Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror
- Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault
PR-URL: https://github.com/nodejs/node/pull/19332
Fixes: https://github.com/nodejs/node/issues/19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Use of a MaybeStackBuffer was just silly. Fix a long standing todo
Reduce code duplication a bit.
PR-URL: https://github.com/nodejs/node/pull/19400
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Most of the inlines were leftovers from a much older design
iteration and are largely pointless or counter productive.
PR-URL: https://github.com/nodejs/node/pull/19400
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/19353
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/19390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit removes the builtin async_wrap module from
lib/async_hooks.js.
The motivation for this is that lib/async_hooks.js requires
lib/internal/async_hooks which also binds async_wrap. Instead of
lib/async_hooks.js also binding async_wrap it now only has to require
the internal async_hooks and access it's exports.
There might be a very good reason for doing it the current way but the
reason is not obvious to me. Hopefully someone can shed some light on
this.
PR-URL: https://github.com/nodejs/node/pull/19368
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Currently, there is an AsyncWrap constructor that is only used by
PromiseWrap. This constructor has a body which is very similar
to the other AsyncWrap constructor.
This commit suggests updating the private constructor that is used
by PromiseWrap and also have the second constructor delegate to this
one to avoid the code duplication.
PR-URL: https://github.com/nodejs/node/pull/19366
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
