The existing implementation created a state that if the assert failed
we got an error message without the values of hadError.
Removed the default error message and added a comment explaining the
assert.
PR-URL: https://github.com/nodejs/node/pull/22069
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Use `setImmediate()` instead of `setTimeout()` to improve robustness of
test-http2-session-timeout.
Fixes: https://github.com/nodejs/node/issues/20628
PR-URL: https://github.com/nodejs/node/pull/22026
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Add missing link at the top.
PR-URL: https://github.com/nodejs/node/pull/22072
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
`CryptoStream` was removed via 9301b8a9c6.
PR-URL: https://github.com/nodejs/node/pull/22033
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/21971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Given that FSReqPromise does not inherit from FSReqWrap, FSReqWrap
should be renamed FSReqCallback to better describe what it does.
First of a few upcoming `fs` refactorings :)
PR-URL: https://github.com/nodejs/node/pull/21971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
There’s no reason to have a separate addon just for
testing GC anymore.
PR-URL: https://github.com/nodejs/node/pull/22001
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Concurrent writes to the same fd are generally not ideal,
since it’s not generally guaranteed that data from those
writes will end up on disk in the right order.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Close existing file descriptors before overriding
the field with new ones.
Also, use `nullptr` as the loop for these synchronous
operations, since they do not run on the same thread
as the `uv_run()` call for the previously used loop.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Run the initialization for the file trace writer’s `uv_async_t`s
on the same thread as `uv_run()` for their loop to avoid race
conditions.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Clean up resources when tearing down the tracing agent.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Otherwise this would have crashed the process.
In particular, checking the return value of an libuv call against `-1`
was invalid to begin with, as libuv uses it to propagate the
error code.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
The agent code is supposed to manage multiple writers/clients.
Adding the default writer into the mix breaks that encapsulation.
Instead, manage default options through a separate "virtual"
default client handle, and keep the file writer management
all to the actual users.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Avoid unnecessary operations, add a bit of documentation,
and turn the `ClientHandle` smart pointer alias into a real
class.
This should allow de-coupling the unnecessary combination of
a single specific `file_writer` from `Agent`.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
A forward declaration suffices and means that the tracing
agent header and its dependencies don’t need to be included
in nearly every Node.js file.
PR-URL: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
This is the first step in a long process of deprecating
`process.binding()` and replacing it with `internalBinding()`.
Eventually, once we have replaced internal uses of
`process.binding()` with `internalBinding()`, we can escalate
to a runtime deprecation and eventual end-of-life.
PR-URL: https://github.com/nodejs/node/pull/22004
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This helps a lot with debugging failing benchmark tests,
which would otherwise just print an assertion for the
exit code (something like `+1 -0`, which yields almost no
information about a failure).
PR-URL: https://github.com/nodejs/node/pull/21860
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node/pull/21929
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Improve assertions in test-child-process-execsync by removing unneeded
third arguments and replacing equal checks with assert.ok() where
appropriate.
PR-URL: https://github.com/nodejs/node/pull/22016
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
At the last TC39 meeting, a new type of Module Records backed by
JavaScript source called Dynamic Module Records was discussed, and
it is now at Stage 1. Regardless of whether that proposal makes it
all the way into the spec, SourceTextModule is indeed a more
descriptive and accurate name for what this class represents.
PR-URL: https://github.com/nodejs/node/pull/22007
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Removes a couple of unused/empty functions inside of `tools/test.py`
PR-URL: https://github.com/nodejs/node/pull/22010
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Has the same behavior as `ParallelTestConfiguration`
PR-URL: https://github.com/nodejs/node/pull/22008
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
As noted by @richardlau, `Makefile` and `vcbuild.bat` no longer need to
be updated since they run the `default` suite.
PR-URL: https://github.com/nodejs/node/pull/22009
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
To increase test coverage for fs/promises, add tests for
methods chown(), filehandle.chown() and lchown().
PR-URL: https://github.com/nodejs/node/pull/20574
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Refs: https://github.com/nodejs/node/pull/21079#discussion_r194251183
PR-URL: https://github.com/nodejs/node/pull/21982
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Use a UTF-8 encoded string for naming interfaces in unix, instead of
using Latin-1, as had been done on windows previously.
PR-URL: https://github.com/nodejs/node/pull/21926
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use v8::NewStringType::kInternalized instead of
v8::NewStringType::kNormal in different calls to v8::String::NewFromUtf8
in node.h and node.cc wherever it makes sense, to save the GC some work
which would largely be redundant.
PR-URL: https://github.com/nodejs/node/pull/21926
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove all calls to deprecated v8 functions (here: String::NewFromUtf8) inside
the code (src directory only).
PR-URL: https://github.com/nodejs/node/pull/21926
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: https://github.com/nodejs/node/issues/12531
PR-URL: https://github.com/nodejs/node/pull/21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Update cpplint.py to check for inline headers when the corresponding
header is already included.
PR-URL: https://github.com/nodejs/node/pull/21521
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The `Http2Session#request()` method internally listens to the "connect"
event if the session has not yet established a connection so that the
actual request can be sent after the connection has been established.
This commit removes the event listener after it runs and carries out
the request and is no longer needed. In practice this shouldn't affect
the behavior of the session object since the "connect" event fires only
once anyway, but removing the listener releases its references. The
rest of this class subscribes to the "connect" event with `once`
instead of `on` as well.
Tested by adding a new test that ensures `Http2Session#request()` is
called before the connection is established, indicated by a "connect"
listener that is run. The test also ensures all "connect" listeners are
removed after the connection is established.
PR-URL: https://github.com/nodejs/node/pull/21916
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/21817
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is the last use of the remark *module*. tools/remark-cli and
tools/remark-preset-lint-node remain.
PR-URL: https://github.com/nodejs/node/pull/21978
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit makes all previously private APIs available via
getters, setters, and wrapper functions.
PR-URL: https://github.com/nodejs/node/pull/21923
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
_createSocketHandle() is used internally by the cluster module.
This commit makes it internal only API.
PR-URL: https://github.com/nodejs/node/pull/21923
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
dgram sockets have a fair number of exposed private properties.
This commit hides them all behind a single symbol property.
PR-URL: https://github.com/nodejs/node/pull/21923
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
These methods are private APIs of dgram sockets, but do not
need to be exposed via the Socket prototype.
PR-URL: https://github.com/nodejs/node/pull/21923
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
