This commit applies new arrow function linting rules across the
codebase. As it turns out, the only offenders were in the test
directory.
PR-URL: https://github.com/nodejs/node/pull/4813
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Do not swallow error details when reporting UV_EPROTO asynchronously,
and when creating artificial errors.
Fix: #3692
PR-URL: https://github.com/nodejs/node/pull/4885
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Testing this wasn't really useful, besides Object.observe is going to be
deprecated.
Also this test fails with Chakra (#4765) for obvious reason.
PR-URL: https://github.com/nodejs/node/pull/4769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
`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>
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>
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>
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>
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>
* 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>
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>
Fix a regression introduced in commit 89f056b ("node: improve
performance of hrtime()") where the nanosecond field sometimes
had a negative value when calculating the difference between two
timestamps.
Fixes: https://github.com/nodejs/node/issues/4751
PR-URL: https://github.com/nodejs/node/pull/4757
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Previously, test-cluster-disconnect-suicide-race had two issues:
* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...
* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."
This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.
Advantages of this approach include:
* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.
PR-URL: https://github.com/nodejs/node/pull/4739
Ref: https://github.com/nodejs/node/pull/4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously, test-cluster-disconnect-leak had two issues:
* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...
* Non-determinism: The test *seems* to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."
This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the `disconnect`
event will fire reliably for a single worker. So we check for that and
the test still fails when the fix is not in the code base and succeeds
when it is.
Advantages of this approach include:
* The test runs much faster.
* The test now works on Windows. The previous version skipped Windows.
* The test should be reliable on any new platform regardless of CPU and
RAM.
Ref: https://github.com/nodejs/node/pull/4674
PR-URL: https://github.com/nodejs/node/pull/4736
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Make the byteLength work correctly when input is Buffer.
e.g:
```js
// The incomplete unicode string
Buffer.byteLength(new Buffer([0xe4, 0xb8, 0xad, 0xe6, 0x96]))
```
The old output: 9
The new output: 5
PR-URL: https://github.com/nodejs/node/pull/4738
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Provide means to inspect information about the separate heap spaces
via a callable API. This is helpful to analyze memory issues.
Fixes: https://github.com/nodejs/node/issues/2079
PR-URL: https://github.com/nodejs/node/pull/4463
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
In some conditions it can happen that the client-side socket is
destroyed before the server-side socket has gracefully closed, thus
causing a 'ECONNRESET' error in this socket. To solve this, also close
gracefully in the client side.
PR-URL: https://github.com/nodejs/node/pull/3966
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If one were to set NODE_REPL_HISTORY to a string that contains only a
space (" "), then the history file would be created with that name
which can cause problems are certain systems.
PR-URL: https://github.com/nodejs/node/pull/4539
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Add a bytesRead property for readable is
useful in some use cases.
When user want know how many bytes read of
readable, need to caculate it in userland.
If encoding is specificed, get the value is
very slowly.
PR-URL: https://github.com/nodejs/node/pull/4372
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Clean up OpenSSL error stack in `ECDH::Initialize`, some curves have
faulty implementations that are leaving dangling errors after
initializing the curve.
Fix: #4686
PR-URL: https://github.com/nodejs/node/pull/4689
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
There is no guarantee that the `suicide` property of a worker in the
master process is going to be set when the `disconnect` and `exit`
events are emitted.
To fix it, wait for the ACK of the suicide message from the master
before disconnecting the worker. Also, there's no need to send the
suicide message from the worker if the disconnection has been
initiated in the master.
Add `test-cluster-disconnect-suicide-race` that forks a lot of workers
to consistently reproduce the issue this patch tries to solve.
Modify `test-regress-GH-3238` so it checks both the `kill` and
`disconnect` cases. Also take into account that the `disconnect` event
may be received after the `exit` event.
PR-URL: https://github.com/nodejs/node/pull/4349
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Clear domains stack __even if no domain error handler is set__ so that
code running in the process' uncaughtException handler, or any code that
may be executed when an error is thrown and not caught and that is not
the domain's error handler, doesn't run in the context of the domain
within which the error was thrown.
PR: #4659
PR-URL: https://github.com/nodejs/node/pull/4659
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In test-cluster-worker-wait-server-close, remove unneeded 1-second delay
and refactor to eliminate flakiness on FreeBSD.
PR-URL: https://github.com/nodejs/node/pull/4616
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Prior to this commit, the test was flaky because it was
executing the majority of its logic in a function called from
the client and multiple events on the server. This commit
simplifies the test by separating the server's connection and
listening events, and isolating the client logic.
Refs: https://github.com/nodejs/node/pull/4476
Refs: https://github.com/nodejs/node/pull/4644
PR-URL: https://github.com/nodejs/node/pull/4650
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
A 50ms timeout results in a race condition. Instead, enforce expected
order through callbacks. This has the side effect of speeding up the
test in most situations.
Ref: https://github.com/nodejs/node/pull/4476
PR-URL: https://github.com/nodejs/node/pull/4637
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Now returns a 417 error status or allows for an event listener on
the `checkExpectation` event. Before we were ignoring requests that
had misspelled `100-continue` values for expect headers.
This is a quick port of the work done here:
https://github.com/nodejs/node-v0.x-archive/pull/7132 by alFReD-NSH
with surrounding discussion here:
https://github.com/nodejs/node-v0.x-archive/issues/4651
Also updates all the instances of the deprecated
EventEmitter.listenerCount to the current self.listenerCount. Most
of these were in the new code ported over but there was another
legacy instance.
Refs: #2403
PR-URL: https://github.com/nodejs/node/pull/4501
Reviewed-By: James M Snell <jasnell@gmail.com>
Reduce the number of stat() system calls that require() makes by caching
the results more aggressively.
To avoid unbounded growth without implementing a LRU cache, scope the
cache to the lifetime of the first call to require(). Recursive calls
(i.e. require() calls in the included code) transparently profit from
the cache.
The benchmarked application is the loopback-sample-app[0] and it sees
the number of stat calls at start-up go down by 40%, from 4736 to 2810.
[0] https://github.com/strongloop/loopback-sample-app
PR-URL: https://github.com/nodejs/node/pull/4575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
It's not guaranteed that the socket data is received in the same chunk
as the upgrade response. Listen for the `data` event to make sure all
the data is received.
PR-URL: https://github.com/nodejs/node/pull/4602
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
It's not guaranteed that the socket data is received in the same chunk
as the upgrade response. Listen for the `data` event to make sure all
the data is received.
PR-URL: https://github.com/nodejs/node/pull/4520
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`fork` is imported twice in a row. Remove duplication.
PR-URL: https://github.com/nodejs/node/pull/4634
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Add support to fs.createWriteStream and fs.createWriteStream for an autoClose
option that behaves similarly to the autoClose option supported by
fs.createReadStream and fs.ReadStream.
When an instance of fs.createWriteStream created with autoClose === false finishes,
it is not destroyed. Its underlying fd is not closed and it is the
responsibility of the user to close it.
PR-URL: https://github.com/nodejs/node/pull/3679
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
cjihrig
Fedor Indutny
Vladimir Kurchatkin
Rich Trott
Roman Reiss
Drew Folta
Evan Lucas
Trevor Norris
Tom Gallacher
Ali Ijaz Sheikh
José Moreira
Ben Noordhuis
Jackson Tian
Ben Ripkens
Santiago Gimeno
Michaël Zasso
Dave
Santiago Gimeno
Julien Gilli
Daniel Sellers
Saquib