This will be a start to generalize all argument validation
errors. As currently we throw ARG/OPT, OUT_OF_RANGE, and other more
specific errors.
The OPT errors didn't bring much to the errors as it's just another
variant of ARG error which is sometimes more confusing (some of our code
used OPT errors to denote just argument validation errors presumably
because of similarity of OPT to 'option' and not 'options-object')
and they don't specify the name of the options object where the invalid
value is located. Much better approach would be to just specify path
to the invalid value in the name of the value as it is done in this PR
(i.e. 'options.format', 'options.publicKey.type' etc)
Also since this decreases a variety of errors we have it'd be easier to
reuse validation code across the codebase.
Refs: https://github.com/nodejs/node/pull/31251
Refs: https://github.com/nodejs/node/pull/34070#discussion_r467251009
Signed-off-by: Denys Otrishko <shishugi@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/34682
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Because of the timing of qlog events emitted by ngtcp2, it
becomes difficult to handle those as events on the QuicSession
object because the final qlog entry is not emitted until the
ngtcp2_conn is freed, which can occur when the object is being
garbage collected (meaning, we a: can't call out to javascript
and b: don't have an object we can use to emit the event).
This refactors it into a QLogStream object that allows the
qlog data to be piped out using a separate Readable stream.
PR-URL: https://github.com/nodejs/node/pull/34160
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The way the preferred address mechanism is supposed to work
is this: A server might be exposed via multiple network
interfaces / addresses. The preferred address is the one that
clients should use. If a client uses one of the non-preferred
addresses to initially connect to the server, the server will
include the preferred address in it's initial transport params
back to the client over the original connection. The client
then can make a choice: it can either choose to ignore the
advertised preferred address and continue using the original,
or it can transition the in-flight connection to the preferred
address without having to restart the connection. In the latter
case, the connection will start making use of the preferred
address but it might not do so immediately.
To test this mechanism properly, we should have our server
configured on multiple endpoints with one of those marked
as the preferred. The connection should start on one and preceed
uninterrupted to completion. If the preferred address policy
is "accept", the client will accept and transition to the servers
preferred address transparently, without interupting the flow.
The current test is deficient because the server is only listening
on a single address which is also advertised as the preferred.
While the client should get the advertisement, we're not actually
making use of the preferred address advertisement and nothing on
the connection changes
The ipv6 version of the test is also hanging on some platforms and
it's not entirely clear why yet.
PR-URL: https://github.com/nodejs/node/pull/33934
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Extracted from the QUIC PR. This adds a utility used to
deterministically test UDP traffic. It is currently only
used by the experimental QUIC implementation. Separated
out on request to make review easier.
PR-URL: https://github.com/nodejs/node/pull/33380
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
When the user tries to activate the inspector that is already active
on a different port and host, we previously just silently reset
the port and host stored in the Environment without actually doing
anything for that to be effective. After this patch, we throw
an error telling the user to close the active inspector before invoking
`inspector.open()` again.
PR-URL: https://github.com/nodejs/node/pull/33015
Fixes: https://github.com/nodejs/node/issues/33012
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.
Example failure (note that `foo` does not appear in test-process-title
but in test-process-title-cli):
not ok 1727 parallel/test-process-title
---
duration_ms: 0.156
severity: fail
exitcode: 1
stack: |-
assert.js:103
throw new AssertionError(obj);
^
AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected
+ 'foo'
- 'd:\\a\\node\\node\\out\\Release\\node.exe'
at Object.<anonymous> (d:\a\node\node\test\parallel\test-process-title.js:22:1)
at Module._compile (internal/modules/cjs/loader.js:1176:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1196:10)
at Module.load (internal/modules/cjs/loader.js:1040:32)
at Function.Module._load (internal/modules/cjs/loader.js:929:14)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
at internal/main/run_main_module.js:17:47 {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: 'foo',
expected: 'd:\\a\\node\\node\\out\\Release\\node.exe',
operator: 'strictEqual'
}
...
(from https://github.com/nodejs/node/runs/628144750?check_suite_focus=true)
PR-URL: https://github.com/nodejs/node/pull/33150
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
If the bug this test is intented to catch is reintroduced, or if
5aac4c42da is effectively reverted, many
(50+) tests time out, rendering this test redundant and unnecessary.
in particular, the following timer tests catch an effective revert of
5aac4c42da:
not ok 21 parallel/test-timers-api-refs
not ok 22 parallel/test-timers-args
not ok 23 parallel/test-timers-destroyed
not ok 25 parallel/test-timers-nested
not ok 26 parallel/test-timers-interval-throw
not ok 28 parallel/test-timers-non-integer-delay
not ok 32 parallel/test-timers-ordering
not ok 33 parallel/test-timers-refresh
not ok 34 parallel/test-timers-refresh-in-callback
not ok 35 parallel/test-timers-reset-process-domain-on-throw
not ok 40 parallel/test-timers-timeout-to-interval
not ok 41 parallel/test-timers-uncaught-exception
not ok 42 parallel/test-timers-timeout-with-non-integer
not ok 43 parallel/test-timers-unenroll-unref-interval
not ok 44 parallel/test-timers-unref
not ok 45 parallel/test-timers-unref-active
not ok 46 parallel/test-timers-unrefd-interval-still-fires
not ok 47 parallel/test-timers-unrefed-in-callback
not ok 48 parallel/test-timers-user-call
not ok 49 parallel/test-timers-zero-timeout
Refs: https://github.com/nodejs/node/issues/21781
PR-URL: https://github.com/nodejs/node/pull/32870
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Under peculiar system load conditions, the profiler thread
does not get enough CPU slices to perform the sampling.
Improve the interaction between worker and parent thread
by performing a large disc read, which is a better blend of
CPU and I/O bound work, than earlier versions.
This produces x10 more samples than the existing one,
in 10 iterations, as opposed to 1024.
Also capture worker error situations to improve debugging
Refs: https://github.com/nodejs/node/issues/26401#issuecomment-597438516
PR-URL: https://github.com/nodejs/node/pull/32190
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
`test-inspector-multisession-ws` and `test-inspector-break-when-eval`
will be affected by an upstream bug when we upgrade V8 to 8.1. The bug
is caused when the Inspector sets a pause at the start of a function
compiled with `CompileFunctionInContext`, but that function hasn't been
executed yet.
On both tests, this issue is triggered by pausing while in C++ executing
LookupAndCompile, which is called by requiring internal modules while
running `console.log`. To eliminate this issue in both tests, we add an
extra `console.log` to ensure we only pause we required all internal
modules we need. On `test-inspector-break-when-eval`, we also need to
start the child process with `--inspect-brk` instead of `--inspect` to
ensure the test is predictable (this test would occasianlly fail on
slower machines, when console.log doesn't run fast enough to finish
after emitting `Runtime.consoleAPICalled` and before the parent process
sending `Runtime.evaluate` message.
Ref: https://bugs.chromium.org/p/v8/issues/detail?id=10287
PR-URL: https://github.com/nodejs/node/pull/32234
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
OpenSSL s_server accepts port 0 as an indicator to use an open port
provided by the operating system. Use that instead of common.PORT in the
test.
Remove 500ms delay added in 8e461673c4.
Hopefully the race condition in OpenSSL s_server has been fixed and/or
the change to port 0 means that the server is listening by the time
the ACCEPT text is printed and the setTimeout() is no longer necessary.
PR-URL: https://github.com/nodejs/node/pull/32024
Reviewed-By: Ben Coe <bencoe@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
