PR-URL: https://github.com/nodejs/node/pull/4244
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
`test-child-process-fork-net2.js` has a switch statement with 6 cases.
Each case uses `child.send()`, passing an object for the callback.
`child.send()` ignores the callback because it is not a function.
Removing the unused argument.
PR-URL: https://github.com/nodejs/node/pull/4870
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
Timer race results in some flakiness on slower devices in CI. Remove
unneeded setTimeout() and replace booleans with common.mustCall().
PR-URL: https://github.com/nodejs/node/pull/4793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit adds a shell option, to spawn() and spawnSync(). This
option allows child processes to be spawned with or without a
shell. The option also allows a custom shell to be defined, for
compatibility with exec()'s shell option.
Fixes: https://github.com/nodejs/node/issues/1009
PR-URL: https://github.com/nodejs/node/pull/4598
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
`test-assert.js` redeclares a variable with `var`. This change converts
it to a `const` declaration and wraps it in a standalone block to scope
it to just the test that uses it.
PR-URL: https://github.com/nodejs/node/pull/4854
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
`lib/_tls_common.js` had instances of `for` loops that defined variables
with `var` such that they were re-declared in the same scope. This
change scopes those variables with `let` so that they are not
re-declared.
PR-URL: https://github.com/nodejs/node/pull/4853
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: https://github.com/nodejs/node/pull/4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
1. The release team's names were not in the sorted order and
2. the old list of releasers' names were not in bold
This patch fixes both of these issues
PR-URL: https://github.com/nodejs/node/pull/4876
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: https://github.com/nodejs/node/pull/4863
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
The vm module's displayErrors option attaches error arrow
messages as a hidden property. Later, core JavaScript code
can optionally decorate the error stack with the arrow message.
However, when user code catches an error, it has no way to
access the arrow message. This commit changes the behavior of
displayErrors to mean "decorate the error stack if an error
occurs."
Fixes: https://github.com/nodejs/node/issues/4835
PR-URL: https://github.com/nodejs/node/pull/4874
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
When running the tests if `NODE_TEST_DIR` is set to a device different
than the location of the test files (where this repo is checked out),
then the parallel/test-fs-link.js test will fail with
`EXDEV: cross-device link not permitted`. The code works fine (and is in
fact throwing an error as desired) but the test fails.
This commit first creates the "source" file in the same directory as the
"destination" (where the hardlink will be created).
PR-URL: https://github.com/nodejs/node/pull/4861
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
test-net-settimeout is unnecessarily complex. This change simplifies it.
PR-URL: https://github.com/nodejs/node/pull/4799
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Previously, port was assumed to be a number and would cause an abort in
cares_wrap. This change throws a TypeError if port is not a number
before we actually hit C++.
Fixes: https://github.com/nodejs/node/issues/4837
PR-URL: https://github.com/nodejs/node/pull/4839
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
1. correct code type in addons.markdown
2. add missed code type in crypto.markdown
PR-URL: https://github.com/nodejs/node/pull/4858
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
`lib/_stream_readable.js` contained three instances of `var`
declarations occurring twice in the same scope. Refactored to `const` or
`let` as appropriate.
PR-URL: https://github.com/nodejs/node/pull/4816
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Introduce `cachedData`/`produceCachedData` options for `v8.Script`.
Could be used to consume/produce V8's code cache for speeding up
compilation of known code.
PR-URL: https://github.com/nodejs/node/pull/4777
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In the docs we typically check for errors and surface them. This
is IMO a good idea and good practice. This PR adds a check for
errors in three places in the `net` docs where it was missing.
PR-URL: https://github.com/nodejs/node/pull/4834
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>>
The description for `statusMessage` was accidentally moved under the
heading for `http.IncomingMessage.socket`. This commit puts it back
in the correct place.
PR-URL: https://github.com/nodejs/node/pull/4822
Fixes: https://github.com/nodejs/node/issues/4558
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
- Hide the scrollbar on the TOC on all browsers. It was never the
intention for it to be visible with the scroll indication in place.
A wrapper element with 20px padding was added to accommodate for
hopefully all scrollbar widths as well as to avoid overflowing
content.
- Fixed the scroll indication gradient on Safari, which was caused by
the wrong from-color, which now matches the to-color.
- Fixed a issue in old IE where the TOC didn't render on the correct
position through setting `left: 0` and `top: 0` on it.
PR-URL: https://github.com/nodejs/node/pull/4748
Reviewed-By: James M Snell <jasnell@gmail.com>
In a few places the code was refactored to use `maybeCallback` which
always returns a function. Checking for `if (callback)` always returns
true anyway.
PR-URL: https://github.com/nodejs/node/pull/4795
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
This commit fixes an issue with the Node.js command prompt
on Windows where the PATH environment variable would not be
set correctly if the existing PATH contained an '&'.
Fixes: https://github.com/nodejs/node/issues/4802
PR-URL: https://github.com/nodejs/node/pull/4804
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4805
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4806
Reviewed-By: Stephan Belanger <admin@stephenbelanger.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
If the needle contains an extended latin-1 character then using
String::Utf8Length() will be too large and the search will return early.
Instead use String::Length() when encoding is BINARY.
PR-URL: https://github.com/nodejs/node/pull/4803
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently the signature is indexOf(val[, byteOffset[, encoding]])
Instead allow indexOf(val[, byteOffset][, encoding])
so that byteOffset does not need to be passed.
PR-URL: https://github.com/nodejs/node/pull/4803
Reviewed-By: James M Snell <jasnell@gmail.com>
Call `obj.postSend` in error case of `process.send()`. The
`net.Socket`'s handle should not be leaked.
Note that there are two callbacks invoked on handles
when they are sent to the child process over IPC pipes.
These callbacks are specified by `handleConversion` object, and
during send two of them are invoked:
* `send`
* `postSend`
Now for `net.Socket` in particular, `postSend` performs clean up by
closing the actual uv handle. However this clean up will not happen in
one of the branches. This pull request aims to fix this.
PR-URL: https://github.com/nodejs/node/pull/4752
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-http-exit-delay has a flaky history. Examination of the bug it was
written to find suggests that the test should be removed.
* The test is trying to find a delay of up to 1 second, but the delay
may also be much smaller than that. So the test will not catch the bug
all the time. It is therefore flaky when the bug exists.
* Experience has shown that the test is flaky as well when the bug is
absent in the code because it can sometimes take slower devices (such as
the Raspberry Pi devices that we have in continuous integration) more
than the allowed one second to run. Increasing the timeout for those
devices will make the test pass but will also mean that the test isn't
really testing anything because the delay it is trying to catch was a
delay of up to one second.
I don't think this is an appropriate test to run once on CI. If there is
to be a test for the issue in question, it should be a benchmark test
that is run a large number of times. We don't really have such tests in
CI yet
I would argue that this test is actively a problem. It does not reliably
catch the issue it is supposed to catch, nor can it likely be made to do
so. (To do so would likely require converting it to a benchmarking test
as previously described. We don't run those in CI, at least not at this
time.)
Because this test will have both false positives and false negatives,
especially on the slower devices, it contributes to a culture of
dismissing failed tests. It does not reliably identify an issue nor does
it reliably pass on a working code base. This test should be removed.
Ref: https://github.com/nodejs/node/pull/4277
PR-URL: https://github.com/nodejs/node/pull/4786
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
As per #3292, this PR introduces a deprecation notice about removing
the 'default digest' overload which currently defaults to the soon
to be defunct SHA1 digest.
Instead it should be left up to the documentation and implementor to
suggest a suitable digest function.
Ref: https://github.com/nodejs/node/pull/3292
PR-URL: https://github.com/nodejs/node/pull/4047
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Original commit message:
[tools] Make gen-postmortem-metadata.py more reliable
Instead of basing matches off of whitespace, walk the
inheritance chain and include any classes that inherit
from Object.
R=machenbach@chromium.org,jkummerow@chromium.org
NOTRY=true
Review URL: https://codereview.chromium.org/1435643002
Cr-Commit-Position: refs/heads/master@{#31964}
This adds some missing classes to postmortem info like
JSMap and JSSet.
Ref: https://github.com/nodejs/node/pull/3792
PR-URL: https://github.com/nodejs/node/pull/4106
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: rvagg - Rod Vagg <rod@vagg.org>
Original commit message:
This commit adds some postmortem data that is otherwise unavailable.
I have discovered need in those values when writing:
https://github.com/indutny/llnode
BUG=
Review URL: https://codereview.chromium.org/1436473002
Cr-Commit-Position: refs/heads/master@{#31947}
This postmortem information is useful for both object inspection, and
function's context variables inspection.
Ref: https://github.com/nodejs/node/pull/3779
PR-URL: https://github.com/nodejs/node/pull/4106
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: rvagg - Rod Vagg <rod@vagg.org>
Parsers hold a reference to the socket associated with the request
through onParserExecute. This must be removed when the parser is
freed so that the socket can be garbage collected when destroyed.
Regression introduced in commit 59b91f1 ("http_parser: consume
StreamBase instance").
PR-URL: https://github.com/nodejs/node/pull/4773
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
* Exchange 20 millisecond timers for setImmediate().
* Do not attempt to unlink path that will have been guaranteed to be
removed by `common.refreshTmpDir()`
* Do not swallow errors thrown by failed creation of needed test
subdirectory. If that happens, we want to know about it.
* Use `common.isSunOS` in one place where it is applicable
PR-URL: https://github.com/nodejs/node/pull/4776
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Added a listening property into net.Server.prototype indicating
if the server is listening or not for connections.
Other Server constructors that rely on net.Server should also
gain access to this property.
Also included tests for net and http subsystems.
PR-URL: https://github.com/nodejs/node/pull/4743
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Notable changes:
* Fix regression in debugger and profiler functionality
PR-URL: https://github.com/nodejs/node/pull/4788
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Event emitters support symbols as event names. The process object
assumes that the event name is a string, and examines the first
three characters to check for signals. This causes an exception
if the event name is a symbol. This commit ensures that the
event name is a string before trying to slice() it.
PR-URL: https://github.com/nodejs/node/pull/4798
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
This changes the code blocks from 4-space indentation to ``` fences for
better syntax highlighting and future linting support. Minor On-the-fly
changes for typos and highlight breaking markdown have been made.
JSON-Style objects have been changed so their closing bracket is
on the same line as the opening one.
Known issues:
* Not every JSON / object notation has been improved. Should
make another run for this.
* Some example functions break hightlighting due to various
combinations of brackets. However changing them means leaving
the code style.
Fixes: https://github.com/nodejs/node/issues/4726
PR-URL: https://github.com/nodejs/node/pull/4733
Reviewed-By: Roman Reiss <me@silverwind.io>
If the VARIATION variable is present, then make binary will produce archives
named node-$(FULLVERSION)-$(PLATFORM)-$(ARCH)-$(VARIATION).
PR-URL: https://github.com/nodejs/node/pull/4631
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Sakthipriyan Vairamani
Bryan English
Rich Trott
cjihrig
Dmitriy Lazarev
Jimb Esser
Myles Borins
Roman Reiss
Drew Folta
Evan Lucas
Alexander Makarenko
Jackson Tian
Fedor Indutny
Benjamin Gruenbaum
Jeff Harris
Brian White
Robert Jefe Lindstaedt
Glen Arrowsmith
Kat Marchán
Karl Skomski
Trevor Norris
Tom Gallacher
Ali Ijaz Sheikh
evan.lucas
Tom Atkinson
José Moreira
Myles Borins
Stefan Budeanu