Enable `space-unary-ops` in `.eslintrc`. This prohibits things like:
i ++ // use `i++` instead
typeof(foo) // use `typeof foo` or `typeof (foo)` instead
Ref: https://github.com/nodejs/node/pull/4772#discussion_r51732299
PR-URL: https://github.com/nodejs/node/pull/5063
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/5047
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4999
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
This fixes some variable redeclarations in pummel tests and, in the
process, gets `test-stream-pipe-multi.js` to run again. It had been
failing to run.
PR-URL: https://github.com/nodejs/node/pull/4998
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4997
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4993
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4994
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
I'm a fan of small changesets, but even I'm getting a little annoyed at
me for opening all these PRs weeding out variable redeclarations. So I'm
bundling a bunch of small changes here.
PR-URL: https://github.com/nodejs/node/pull/4992
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4991
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4990
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4989
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4987
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4985
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4984
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Instead of using the same session over and over, evict it when the
socket emits error. This could be used as a mitigation of #3692, until
OpenSSL fix will be merged/released.
See: https://github.com/nodejs/node/issues/3692
PR-URL: https://github.com/nodejs/node/pull/4982
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
It can happen that the extra response is to be sent in a different chunk
from the rest of the data. At this moment, the client might have already
closed the socket causing an `ECONNRESET` error.
PR-URL: https://github.com/nodejs/node/pull/4979
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
A handful of child process tests had variables declared multiple times
in the same scope using `var`. This change scopes those declarations.
PR-URL: https://github.com/nodejs/node/pull/4944
Reviewed-By: Michaël Zasso <mic.besace@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>
Many variables in the buffer tests are redeclared. Change them so that
they are scoped appropriately.
PR-URL: https://github.com/nodejs/node/pull/4893
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
It can happen that the server-side socket is destroyed before the
client-side socket has gracefully closed, thus causing a 'ECONNRESET'
error in this socket. To solve this, also close gracefully in the server
side too.
PR-URL: https://github.com/nodejs/node/pull/4888
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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>
isLegalPort can be used in more places than just net.js. This change
moves it to a new internal net module in preparation for using it in
the dns module.
PR-URL: https://github.com/nodejs/node/pull/4882
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
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-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>
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>
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>
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>
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>
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>
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>
* 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>
From time to time this test is failing in OS X because at least one of
the connections takes quite a long time (around 5 seconds) causing some
of the timers may fire before the test exited. To solve this, wait for
all the connections to be established before setting the timeouts and
unrefing the sockets.
PR-URL: https://github.com/nodejs/node/pull/4772
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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>
This change adds a new event handler to the `error` event of the socket
after it has been used by the http_client.
The purpose of this change is to catch errors on *keep alived*
connections from idle sockets that otherwise will cause an uncaugh error
event on the application.
Fix: #3595
PR-URL: https://github.com/nodejs/node/pull/4482
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Helps in implementation of #6204, where some options passed to
`createSecurePair()` are ignored before this patch.
These options are very helpful if someone wants to pass
`options.servername` or `options.SNICallback` to securepair.
PR-URL: https://github.com/nodejs/node/pull/2441
Reviewed-By: Fedor Indutny <fedor@indutny.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, wait in the client-side
socket for the 'end' event before closing it.
PR-URL: https://github.com/nodejs/node/pull/4043
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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>
Rich Trott
Brian White
Fedor Indutny
Santiago Gimeno
Alan Cohen
Evan Lucas
Roman Reiss
Drew Folta
cjihrig
Vladimir Kurchatkin
José F. Romaniello
Коренберг Марк
Michaël Zasso
Myles Borins