The `flag` and `mode` options were not being validated correctly.
Signed-off-by: James M Snell <jasnell@gmail.com>
Fixes: https://github.com/nodejs/node/issues/37430
PR-URL: https://github.com/nodejs/node/pull/37480
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This completely refactors the `expectsError` behavior: so far it's
almost identical to `assert.throws(fn, object)` in case it was used
with a function as first argument. It had a magical property check
that allowed to verify a functions `type` in case `type` was passed
used in the validation object. This pattern is now completely removed
and `assert.throws()` should be used instead.
The main intent for `common.expectsError()` is to verify error cases
for callback based APIs. This is now more flexible by accepting all
validation possibilites that `assert.throws()` accepts as well. No
magical properties exist anymore. This reduces surprising behavior
for developers who are not used to the Node.js core code base.
This has the side effect that `common` is used significantly less
frequent.
PR-URL: https://github.com/nodejs/node/pull/31092
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
ERR_INVALID_ARG_TYPE is the most common error used throughout the
code base. This improves the error message by providing more details
to the user and by indicating more precisely which values are allowed
ones and which ones are not.
It adds the actual input to the error message in case it's a primitive.
If it's a class instance, it'll print the class name instead of
"object" and "falsy" or similar entries are not named "type" anymore.
PR-URL: https://github.com/nodejs/node/pull/29675
Reviewed-By: Rich Trott <rtrott@gmail.com>
This updates all Node.js errors by removing the `code` being part
of the `name` property. Instead, the name is just changed once on
instantiation, the stack is accessed to create the stack as expected
and then the `name` property is set back to it's original form.
PR-URL: https://github.com/nodejs/node/pull/26738
Fixes: https://github.com/nodejs/node/issues/26669
Fixes: https://github.com/nodejs/node/issues/20253
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
- Introduce the `validateAndMaskMode` validator that
validates `mode_t` arguments and mask them with 0o777
if they are 32-bit unsigned integer or octal string
to be more consistent with POSIX APIs.
- Use the validator in fs APIs and process.umask for
consistency.
- Add tests for 32-bit unsigned modes larger than 0o777.
PR-URL: https://github.com/nodejs/node/pull/20636
Fixes: https://github.com/nodejs/node/issues/20498
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Right now there are multiple cases where the validated entry would
not be returned or a wrong error is thrown. This fixes both cases.
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>
`test-fs-chmod` modifies the permissions on files in `test/fixtures`.
This change has the test use the temp directory instead. One of the
fixture files is not used by any other test, so it has been deleted.
I took this opportunity to remove `console.log()` statements from the
test.
PR-URL: https://github.com/nodejs/node/pull/14926
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
To avoid '[DEP0013] DeprecationWarning:
Calling an asynchronous function without callback is deprecated.'
PR-URL: https://github.com/nodejs/node/pull/12795
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Manually fix issues that eslint --fix couldn't do automatically.
PR-URL: https://github.com/nodejs/node/pull/10685
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
* use const and let instead of var
* use common.mustCall to control functions executions
* use assert.strictEqual instead of assert.equal
* use assert.ifError to handle errors
* use arrow functions
* remove unnecessary variables
PR-URL: https://github.com/nodejs/node/pull/10440
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In preparation for a lint rule that disallows empty lines at the end of
a file, remove such lines from a number of test files.
Refs: https://github.com/nodejs/node/issues/8918
PR-URL: https://github.com/nodejs/node/pull/8920
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This reverts commit 9359de9dd2.
Original Commit Message:
The "fs" module has two functions called `maybeCallback` and
`makeCallback`, as of now.
The `maybeCallback` creates a default function to report errors, if the
parameter passed is not a function object. Basically, if the callback
is omitted in some cases, this function is used to create a default
callback function.
The `makeCallback`, OTOH, creates a default function only if the
parameter passed is `undefined`, and if it is not a function object it
will throw an `Error`.
This patch removes the `maybeCallback` function and makes the callback
function argument mandatory for all the async functions.
PR-URL: https://github.com/nodejs/node/pull/7168
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/7846
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
The "fs" module has two functions called `maybeCallback` and
`makeCallback`, as of now.
The `maybeCallback` creates a default function to report errors, if the
parameter passed is not a function object. Basically, if the callback
is omitted in some cases, this function is used to create a default
callback function.
The `makeCallback`, OTOH, creates a default function only if the
parameter passed is `undefined`, and if it is not a function object it
will throw an `Error`.
This patch removes the `maybeCallback` function and makes the callback
function argument mandatory for all the async functions.
PR-URL: https://github.com/nodejs/node/pull/7168
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
In the tests, we use "process.platform === 'win32'" in some places.
This patch replaces them with the "common.isWindows" for consistency.
PR-URL: https://github.com/nodejs/io.js/pull/2269
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Expose `common.refreshTmpDir()` and only call it
for tests that use common.tmpDir or common.PIPE.
A positive side effect is the removal of a code
smell where child processes were detected by the
presence of `.send()`. Now each process can decide
for itself if it needs to refresh tmpDir.
PR-URL: https://github.com/nodejs/io.js/pull/1954
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Enable linting for the test directory. A number of changes was made so
all tests conform the current rules used by lib and src directories. The
only exception for tests is that unreachable (dead) code is allowed.
test-fs-non-number-arguments-throw had to be excluded from the changes
because of a weird issue on Windows CI.
PR-URL: https://github.com/nodejs/io.js/pull/1721
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
