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


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

Add type checking for options.hostname / options.host
Maintains the use of `undefined` and `null` for default `localhost`,
but other falsy values (like `false`, `0` and `NaN`) are rejected.

PR-URL: https://github.com/nodejs/node/pull/12494
Ref: https://github.com/nodejs/node/issues/12488


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

remove typeError constructor and replace with regex string
to match typeError message

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Removed common.PORT from test-cluster-message,
test-cluster-server-restart-none, test-cluster-server-restart-rr
and test-cluster-shared-handle-bind-error to eliminate the
possibility that a dynamic port used in another test will collide
with common.PORT.

PR-URL: https://github.com/nodejs/node/pull/12584
Ref: https://github.com/nodejs/node/issues/12376


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

With _debugger.js removed, _debug_agent.js appears to be unnecessary.

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


Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

https://github.com/nodejs/node/issues/5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

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


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: Gibson Fahnestock <gibfahn@gmail.com>

This got accidentally added when landing original commit

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


Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>

This option prevents ESlint from unnecessary searching
in parent folders. It also protects ESlint rules
from accidental rewriting by a config in a parent folder.

PR-URL: https://github.com/nodejs/node/pull/12570
Fixes: https://github.com/nodejs/node/issues/12566


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

https://github.com/nodejs/node/pull/11863 adds _O_RDWR to node.cc
which is defined in fcntl.h. This adds this include directly to
node.cc.

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>

This does not use the internal/errors.js module because the error
in question may actually be *caused* by an attempt to load
internal/errors.js. This error should only be encountered in the
case of a bug within Node.js itself.

PR-URL: https://github.com/nodejs/node/pull/11298
Ref: https://github.com/nodejs/node/issues/11273


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/12612
Fixes: https://github.com/nodejs/node/issues/12565


Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>

I've added a deprecation notice as the functions are public, but not
sure if this is correct or the format of the deprecation notice.

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


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Remove common.PORT from test-cluster-dgram-1 and
test-cluster-dgram-2, in order to eliminate the
possibility of port collision.

PR-URL: https://github.com/nodejs/node/pull/12487
Ref: https://github.com/nodejs/node/issues/12376


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

- Can now link to 'Commit Guidelines' from pull requests
- Breaks up commit requirements and recommendations

PR-URL: https://github.com/nodejs/node/pull/11732
Refs: https://github.com/nodejs/node/pull/11723#issuecomment-284556146


Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

This commit removes a TODO regarding the removal of uv errno. errno
is currently used and cannot be removed so removing the comment to
avoid any confusion.

PR-URL: https://github.com/nodejs/node/pull/12536
Ref: https://github.com/nodejs/node/issues/4641


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

* Use the more efficient module.exports = {} approach
* Eliminate some uses of arguments

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


Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>

When a possible listener leak is detected, a warning is
emitted. This commit updates an existing test to verify that the
warning is only emitted once.

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


Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Commit 8d386ed7 stopped the
Event Emitter implementation from storing arrays containing a
single listener. This change left a section of code in
removeListener() as unreachable. This commit removes the
unreachable code.

Refs: https://github.com/nodejs/node/pull/12043
PR-URL: https://github.com/nodejs/node/pull/12501


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

The CI seems to do a 3way merge so it is possible
that even though the CI passed, the existing git am command
may fail.  Add text to suggest how to handle this
by adding the --3way option.

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

test-stream-duplex.js uses transform streams instead of the
duplex stream base class. This leads to some code not being
covered in the Duplex constructor.

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

The breakOnSigint option follows different code paths, depending
on the number of listeners for SIGINT. This commit updates the
existing test to vary the number of SIGINT handlers.

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


Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

ninja on Windows chokes if libs are named .Lib (Capital L)

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


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

V8's interpreter performs stack checks both at the call site and at the
function entry. A recursive function could therefore trigger stack
overflow at two different source locations. Instead of recursion, call
JSON.stringify on a deeply nested array.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

HGlobalValueNumberingPhase::CollectSideEffectsOnPathsToDominatedBlock()
used to self-recurse before this commit, causing stack overflows on
systems with small stack sizes.  Make it non-recursive by storing
intermediate results in a heap-allocated list.

Fixes: https://github.com/nodejs/node/issues/11991
PR-URL: https://github.com/nodejs/node/pull/12460


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>

The file no longer works after the removal of the --debug/--debug-brk
switches in commit 47f8f746 ("src: remove support for --debug".)

This commit also removes several tests that still referenced the
old debugger but were either unit-testing its internals or passing
for the wrong reason (like expecting an operation to fail, which
it did because the debugger is gone.)

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


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

Overlooked in commit 47f8f746 ("src: remove support for --debug") from
earlier this month.

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


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

Since Busybox ps does not support -p switch, using ps -o and grep
instead to get the process title and then check it.

PR-URL: https://github.com/nodejs/node/pull/12413
Fixes: https://github.com/nodejs/node/issues/12399


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>

sequential/test-benchmark-child-process is still failing sometimes flaky
on Windows in CI. Mark it as flaky in sequential.status until it gets
sorted.

PR-URL: https://github.com/nodejs/node/pull/12561
Ref: https://github.com/nodejs/node/issues/12560


Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

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


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

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


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

* Assign codes to the handful of errors reported by
  internal/process/*.js
* Include documentation for the new error codes
* Improve error messages
* Improve test coverage for process.nextTick

PR-URL: https://github.com/nodejs/node/pull/11294
Ref: https://github.com/nodejs/node/issues/11273


Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.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.

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>

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


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

On Windows when REPL history file has the hidden attribute node will
fail when trying to open it in 'w' mode. This changes the mode to
'r+'. The file is guaranteed to exists because of earlier open call
with 'a+'.

Fixes: https://github.com/nodejs/node/issues/5261
PR-URL: https://github.com/nodejs/node/pull/12207


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

Currently when building --without-ssl there is a failure due to a change
made by me in commit 3cf88a45 ("test:
add --use-bundled-ca to tls-cnnic-whitelist") which added a
'--use-bundled-ca' flag to that test. But when building --without-ssl
that flag will be invalid and an error (bad option)  will be reported.

This commit filters tests that specify the --use-bundled-ca or
--use-openssl-ca flags so that they are skipped when configured
--without-ssl.

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


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

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


Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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


Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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


Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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


Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>