Removes the requirement to use `--trace-events-enabled` to enable
trace events. Tracing is enabled automatically if there are any
enabled categories.
Adds a new `trace_events` module with an API for enabling/disabling
trace events at runtime without a command line flag.
```js
const trace_events = require('trace_events');
const categories = [ 'node.perf', 'node.async_hooks' ];
const tracing = trace_events.createTracing({ categories });
tracing.enable();
// do stuff
tracing.disable();
```
Multiple `Tracing` objects may exist and be enabled at any point
in time. The enabled trace event categories is the union of all
enabled `Tracing` objects and the `--trace-event-categories`
flag.
PR-URL: https://github.com/nodejs/node/pull/19803
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Rather than an option, introduce a method and an event...
```js
server.on('stream', (stream) => {
stream.respond(undefined, { waitForTrailers: true });
stream.on('wantTrailers', () => {
stream.sendTrailers({ abc: 'xyz'});
});
stream.end('hello world');
});
```
This is a breaking change in the API such that the prior
`options.getTrailers` is no longer supported at all.
Ordinarily this would be semver-major and require a
deprecation but the http2 stuff is still experimental.
PR-URL: https://github.com/nodejs/node/pull/19959
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This improves the error messages for:
- assert.notDeepStrictEqual
- assert.deepStrictEqual
- assert.notStrictEqual
- assert.strictEqual
Those will now always use the same error message as used in the
strict mode.
PR-URL: https://github.com/nodejs/node/pull/19467
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This makes sure the input arguments get validated so implementation
errors will be caught early. It also improves a couple of error
messages by providing more detailed information and fixes errors
detected by the new functionality. Besides that a error type got
simplified and tests got refactored.
PR-URL: https://github.com/nodejs/node/pull/19924
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
One of the biggest downsides to the `assert.throws` API is that it
does not check for the error message in case that is used as second
argument. It will instead be used in case no error is thrown.
This improves the situation by checking the actual error message
against the provided one and throws an error in case they are
identical. It is very unlikely that the user wants to use that error
message as information instead of checking against that message.
PR-URL: https://github.com/nodejs/node/pull/19867
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit improves the SystemError messages by allowing user
to combine a custom message and the libuv error message. Also
since we now prefer use subclasses to construct the errors instead
of using `new errors.SystemError()` directly, this removes
the behavior of assigning a default error code `ERR_SYSTEM_ERROR`
to SystemError and requires the user to directly use the
`ERR_SYSTEM_ERROR` class to construct errors instead.
Also merges `makeNodeError` into the SystemError class definition
since that's the only place the function gets used and it seems
unnecessary to introduce another level of inheritance. SystemError
now directly inherits from Error instead of an intermmediate Error
class that inherits from Error.
Class hierarchy before this patch:
ERR_SOCKET_BUFFER_SIZE -> Error (use message formatted by SystemError)
ERR_SYSTEM_ERROR -> NodeError (temp) -> Error
After:
ERR_SOCKET_BUFFER_SIZE -> SystemError -> Error
ERR_TTY_INIT_FAILED -> SystemError -> Error
ERR_SYSTEM_ERROR -> SystemError -> Error
Error messages before this patch:
```
const dgram = require('dgram');
const socket = dgram.createSocket('udp4');
socket.setRecvBufferSize(8192);
// Error [ERR_SOCKET_BUFFER_SIZE]: Could not get or set buffer
// size: Error [ERR_SYSTEM_ERROR]: bad file descriptor:
// EBADF [uv_recv_buffer_size]
// at bufferSize (dgram.js:191:11)
// at Socket.setRecvBufferSize (dgram.js:689:3)
const tty = require('tty');
new tty.WriteStream(1 << 30);
// Error [ERR_SYSTEM_ERROR]: invalid argument: EINVAL [uv_tty_init]
// at new WriteStream (tty.js:84:11)
```
After:
```
const dgram = require('dgram');
const socket = dgram.createSocket('udp4');
socket.setRecvBufferSize(8192);
// SystemError [ERR_SOCKET_BUFFER_SIZE]: Could not get or set buffer
// size: uv_recv_buffer_size returned EBADF (bad file descriptor)
// at bufferSize (dgram.js:191:11)
// at Socket.setRecvBufferSize (dgram.js:689:3)
const tty = require('tty');
new tty.WriteStream(1 << 30);
// SystemError [ERR_TTY_INIT_FAILED]: TTY initialization failed:
// uv_tty_init returned EINVAL (invalid argument)
// at new WriteStream (tty.js:84:11)
```
PR-URL: https://github.com/nodejs/node/pull/19514
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The color output was broken at some point and that was not detected
because it was not tested for properly so far. This makes sure the
colors work again and it adds a regression test as well.
PR-URL: https://github.com/nodejs/node/pull/19464
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This refactors a couple tests to have upper case first characters
in comments and to use `input` instead of `i`.
It also adds a few TODOs and rewrites a few lines to use default
arguments and to prevent function recreation when unnecessary.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This makes a effort to make sure all of these errors will actually
also show the received input.
On top of that it refactors a few tests for better maintainability.
It will also change the returned type to always be a simple typeof
instead of special handling null.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This adds a rule that forbids the use of native Error constructors in
the `lib` directory. This is to encourage use of the `internal/errors`
mechanism. The rule is disabled for errors that are not created with
the `internal/errors` module but are still assigned an error code.
PR-URL: https://github.com/nodejs/node/pull/19373
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Requiring `respondWithFile()` to only work with regular files
is an artificial restriction on Node’s side and has become unnecessary.
Offsets or lengths cannot be specified for those files,
but that is an inherent property of other file types.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
- Add an accessor property `initialized `to FSEventWrap to
check the state of the handle from the JS land
- Introduce ERR_FS_WATCHER_ALREADY_STARTED so calling start()
on a watcher that is already started will throw instead of
doing nothing silently.
- Introduce ERR_FS_WATCHER_NOT_STARTED so calling close()
on a watcher that is already closed will throw instead of
doing nothing silently.
- Validate the filename passed to fs.watch()
- Assert that the handle in the watcher are instances of
FSEvent instead of relying on the illegal invocation error
from the VM.
- Add more assertions in FSEventWrap methods now that we check
`initialized` and the filename in JS land before invoking
the binding.
- Use uvException instead of errornoException to create
the errors with the error numbers from libuv to make them
consistent with other errors in fs.
TODO:
- Improve fs.watchFile() the same way this patch improves fs.watch()
- It seems possible to fire both rename and change event from libuv
together now that we can check if the handle is closed via
`initialized` in JS land.
PR-URL: https://github.com/nodejs/node/pull/19089
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This improves error handling for streams in a few ways.
1. It ensures that no user defined methods (_read, _write, ...) are run
after .destroy has been called.
2. It introduces an explicit error to tell the user if they are write to
write, etc to the stream after it has been destroyed.
3. It makes streams always emit close as the last thing after they have
been destroyed
4. Changes the default _destroy to not gracefully end streams.
It also updates net, http2, zlib and fs to the new error handling.
PR-URL: https://github.com/nodejs/node/pull/18438
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The binding writeBuffer has been changed in
https://github.com/nodejs/node/pull/19041 and it now requires
the last argument to be a context object. makeSyncWrite
was not updated accordingly, resulting assertions on Windows.
This patch fixes the usage of writeBuffer there.
Also fix errors.uvException() so error.message are no longer
enumerable, this fixes the deepStrictEqual assertion on the
error object in test-stdout-close-catch.
PR-URL: https://github.com/nodejs/node/pull/19103
Refs: https://github.com/nodejs/node/pull/19041
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This updates all internal errors to the new error type. While doing
so it removes unused errors.
A few errors currently seem to have the wrong type. To identify them
later, comments were added next to the error type.
PR-URL: https://github.com/nodejs/node/pull/18857
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This implements a function based system. Instead of passing in the
error code as first argument, the error code itself is a error class.
It already contains the correct error type, so while adding a new
error no one has to think about the error type anymore. In case a
single error code has more than one error type, the error class has
properties for the non default error types. Those can be used as
fallback.
This prevents typos, makes the implementation easier and it is less
verbose when writing the code for a new error.
The implementation itself does not interfere with the old
implementation. So the old and the new system can co-exist and it is
possible to slowly migrate the old ones to the new system.
PR-URL: https://github.com/nodejs/node/pull/18857
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Right now it is possible to get an AssertionError from input that has
the customInspect function set to always return the same value.
That way the error message is actually misleading because the output
is going to look the same. This fixes it by deactivating the custom
inspect function.
PR-URL: https://github.com/nodejs/node/pull/18611
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit moves error creation helpers scattered around
under lib/ into lib/internal/errors.js in the hope of being clearer
about the differences of errors that we throw into the user land.
- Move util._errnoException and util._exceptionWithHostPort
into internal/errors.js and simplify their logic so it's
clearer what the properties these helpers create.
- Move the errnoException helper in dns.js to internal/errors.js
into internal/errors.js and rename it to dnsException. Simplify
it's logic so it no longer calls errnoException and skips
the unnecessary argument checks.
PR-URL: https://github.com/nodejs/node/pull/18546
Reviewed-By: James M Snell <jasnell@gmail.com>
James M Snell