With the removal of `GetHostByNameWrap` in the previous commit, there
is only one remaining call site. Inlining it there lets us simplify
the logic.
PR-URL: https://github.com/nodejs/node/pull/17860
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
It was a wrapper for `ares_gethostbyname()` that I'm unsure about if
it was ever exposed at the binding layer, let alone the public API.
PR-URL: https://github.com/nodejs/node/pull/17860
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
* Remove fluff text and get to the point: Report security flaws to
security@nodejs.org. Please do not disclose security flaws publicly
until they have been handled by the security team.
* Fix somewhat confusing paragraph that says there are no "hard
and fast rules" but then uses _must_ in the context of a "general
rule". Easiest solution seems to be to change _must_ to _should_.
* Minor style change (_you will_ instead of _you'll_)
PR-URL: https://github.com/nodejs/node/pull/17929
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This removes some wordy phrases (notably "it is important to note
that").
PR-URL: https://github.com/nodejs/node/pull/17891
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
It is not possible for `process.arch` (which comes from V8's
`target_arch`) to be `x86`.
Also updates `process.arch` to have the same information.
PR-URL: https://github.com/nodejs/node/pull/17899
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Add a padding strategy option that makes a best attempt to ensure
that total frame length for DATA and HEADERS frames are aligned
on multiples of 8-bytes.
PR-URL: https://github.com/nodejs/node/pull/17938
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Add new properties to `Http2Session` to identify alpnProtocol,
and indicator about whether the session is TLS or not, and
initial support for origin set (preparinng for `ORIGIN` frame
support and the client-side `Pool` implementation.
The `originSet` is the set of origins for which an `Http2Session`
may be considered authoritative. Per the `ORIGIN` frame spec,
the originSet is only valid on TLS connections, so this is only
exposed when using a `TLSSocket`.
PR-URL: https://github.com/nodejs/node/pull/17935
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Test test-require-deps-deprecation.js was failing when user already had
node installed with acorn in require.resolve range.
Modified test to acknowledge the possibility and throw only if acorn is
found in the deps directory.
PR-URL: https://github.com/nodejs/node/pull/17848
Fixes: https://github.com/nodejs/node/issues/17148
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Remove redundant "blocking it or stopping it" as blocking and stopping
are the same thing in this case.
* Make another sentence less wordy. Fix incorrect verb conjugation.
Break into two clear sentences.
PR-URL: https://github.com/nodejs/node/pull/17931
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Remove incorrect usage of "in which". The sentence is better and shorter
without it anyway. Replace incorrect "with" with "in".
PR-URL: https://github.com/nodejs/node/pull/17930
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
PR-URL: https://github.com/nodejs/node/pull/17924
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@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/17924
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
While tools should be used insofar as possible, that information
does not belong in the style guide. Move to onboarding doc.
PR-URL: https://github.com/nodejs/node/pull/17866
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The `default` test suite in `test.py` includes `async-hooks` and
`known_issues`. Our current setup results in those test suites being run
twice during each CI run. Remove the duplication.
PR-URL: https://github.com/nodejs/node/pull/17912
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Current alt text results in "GitHub badge badge". Change to intended use
of alt text so that it reads "GitHub First-time contributor badge". (The
alt text is supposed to replace the image in the event that the image
cannot be rendered.)
PR-URL: https://github.com/nodejs/node/pull/17922
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Strictly limit the number of concurrent streams based on the
current setting of the MAX_CONCURRENT_STREAMS setting
PR-URL: https://github.com/nodejs/node/pull/16766
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md
PR-URL: https://github.com/nodejs/node/pull/17902
Fixes: https://github.com/nodejs/node/issues/16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
If an error is encountered during the processing of Immediates, schedule
the remaining queue to finish after all error handling code runs (if the
process is still alive to do so). The new changes make the Immediates
error handling behaviour entirely deterministic and predictable, as the
full queue will be flushed on each Immediates cycle, regardless of
whether an error is encountered or not.
Currently this processing is scheduled for nextTick which can yield
unpredictable results as the nextTick might happen as early as close
callbacks phase or as late as after the next event loop turns Immediates
all fully processed. The latter can result in two full cycles of
Immediates processing during one even loop turn.
The current implementation also doesn't differentiate between Immediates
scheduled for the current queue run or the next one, so Immediates that
were scheduled for the next turn of the event loop, will process
alongside the ones that were scheduled for the current turn.
PR-URL: https://github.com/nodejs/node/pull/17879
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17939
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17939
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17937
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/17904
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
PR-URL: https://github.com/nodejs/node/pull/17923
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
With domains implemented over AsyncHooks, it's no longer
necessary to explicitly enter and exit the domain.
PR-URL: https://github.com/nodejs/node/pull/17880
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17877
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Currently, when configured --without-ssl test-repl-tab-complete fails
with the following error:
assert.js:43
throw new errors.AssertionError(obj);
^
AssertionError [ERR_ASSERTION]: [ [], 'lexicalL' ] deepStrictEqual []
at testRepl.complete.common.mustCall
(node/test/parallel/test-repl-tab-complete.js:549:14)
at /node/test/common/index.js:530:15
at completionGroupsLoaded (repl.js:1204:5)
at REPLServer.complete (repl.js:1090:11)
at REPLServer.completer (repl.js:450:14)
at REPLServer.complete (repl.js:919:18)
at __dirname.forEach (parallel/test-repl-tab-complete.js:548:14)
at Array.forEach (<anonymous>)
at Object.<anonymous> (parallel/test-repl-tab-complete.js:545:29)
at Module._compile (module.js:660:30)
This commit attempts to fix this test but I'm not sure if this is a
proper fix as I'm not familiar with the repl code base yet.
PR-URL: https://github.com/nodejs/node/pull/17867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently, when configured --without-ssl tests that use
process.binding('crypto') fail with the following error:
=== release test-accessor-properties ===
Path: parallel/test-accessor-properties
node/test/parallel/test-accessor-properties.js:16
const crypto = process.binding('crypto');
^
Error: No such module: crypto
at Object.<anonymous> (test-accessor-properties.js:16:24)
at Module._compile (module.js:660:30)
at Object.Module._extensions..js (module.js:671:10)
at Module.load (module.js:577:32)
at tryModuleLoad (module.js:517:12)
at Function.Module._load (module.js:509:3)
at Function.Module.runMain (module.js:701:10)
at startup (bootstrap_node.js:194:16)
at bootstrap_node.js:645:3
This commit adds a hasCrypto check.
PR-URL: https://github.com/nodejs/node/pull/17867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently, when configuring --without-ssl any tests that use
process.binding('crypto') will not report a lint warning. This is
because the eslint check only generates a warning when using require.
This commit adds a check for using binding in addition to require.
PR-URL: https://github.com/nodejs/node/pull/17867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The usage of ERR_INVALID_ARG_TYPE in _errnoException
is a little inappropriate. This change is to improve it.
PR-URL: https://github.com/nodejs/node/pull/17626
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
A sort-of follow-up to https://github.com/nodejs/node/pull/17704, this
removes the last internal use of enroll().
PR-URL: https://github.com/nodejs/node/pull/17800
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Remove the custom formatter that was added in commit 4fb27d4 ("intl: Add
more versions from ICU"). It's not necessary anymore (and may not have
been necessary at all) and prevents proper coloring in the REPL.
PR-URL: https://github.com/nodejs/node/pull/17861
Fixes: https://github.com/nodejs/node/issues/17086
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Documenting the best way to imitate the old behavior saves time for people
migrating from older versions. (E.g. for unexpected ECONNRESET)
It isn't immediately obvious if earlier nodejs versions behaved the same
way as nodejs 8 does with keepAliveTimeout = 0.
From 0aa7ef5950, it seems like they behave
the same way.
Related to issues such as #13391 that show up when migrating to node 8
PR-URL: https://github.com/nodejs/node/pull/17660
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
