FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

watch: fix --env-file-if-exists crashing on linux if the file is missing · nodejs/node@c58fe38 · GitHub

/ node Public

Commit c58fe38

Browse files
efekrskl
authored and
aduh95
committed
watch: fix --env-file-if-exists crashing on linux if the file is missing
PR-URL: #61870 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent 3c96ae1 commit c58fe38

8 files changed

Lines changed: 84 additions & 14 deletions

File tree

‎doc/api/fs.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4784,6 +4784,9 @@ The `atime` and `mtime` arguments follow these rules:
47844784
<!-- YAML
47854785
added: v0.5.10
47864786
changes:
4787+
- version: REPLACEME
4788+
pr-url: https://github.com/nodejs/node/pull/61870
4789+
description: Added `throwIfNoEntry` option.
47874790
- version: v19.1.0
47884791
pr-url: https://github.com/nodejs/node/pull/45098
47894792
description: Added recursive support for Linux, AIX and IBMi.
@@ -4812,6 +4815,8 @@ changes:
48124815
* `encoding` {string} Specifies the character encoding to be used for the
48134816
filename passed to the listener. **Default:** `'utf8'`.
48144817
* `signal` {AbortSignal} allows closing the watcher with an AbortSignal.
4818+
* `throwIfNoEntry` {boolean} Indicates whether an exception should be thrown when the
4819+
path does not exist. **Default:** `true`.
48154820
* `ignore` {string|RegExp|Function|Array} Pattern(s) to ignore. Strings are
48164821
glob patterns (using [`minimatch`][]), RegExp patterns are tested against
48174822
the filename, and functions receive the filename and return `true` to

‎lib/fs.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2512,6 +2512,7 @@ function appendFileSync(path, data, options) {
25122512
* recursive?: boolean;
25132513
* encoding?: string;
25142514
* signal?: AbortSignal;
2515+
* throwIfNoEntry?: boolean;
25152516
* }} [options]
25162517
* @param {(
25172518
* eventType?: string,
@@ -2530,6 +2531,7 @@ function watch(filename, options, listener) {
25302531

25312532
if (options.persistent === undefined) options.persistent = true;
25322533
if (options.recursive === undefined) options.recursive = false;
2534+
if (options.throwIfNoEntry === undefined) options.throwIfNoEntry = true;
25332535

25342536
let watcher;
25352537
const watchers = require('internal/fs/watchers');
@@ -2547,7 +2549,8 @@ function watch(filename, options, listener) {
25472549
options.persistent,
25482550
options.recursive,
25492551
options.encoding,
2550-
options.ignore);
2552+
options.ignore,
2553+
options.throwIfNoEntry);
25512554
}
25522555

25532556
if (listener) {

‎lib/internal/fs/recursive_watch.js‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class FSWatcher extends EventEmitter {
5252
assert(typeof options === 'object');
5353

5454
const { persistent, recursive, signal, encoding, ignore } = options;
55+
let { throwIfNoEntry } = options;
5556

5657
// TODO(anonrig): Add non-recursive support to non-native-watcher for IBMi & AIX support.
5758
if (recursive != null) {
@@ -66,6 +67,12 @@ class FSWatcher extends EventEmitter {
6667
validateAbortSignal(signal, 'options.signal');
6768
}
6869

70+
if (throwIfNoEntry != null) {
71+
validateBoolean(throwIfNoEntry, 'options.throwIfNoEntry');
72+
} else {
73+
throwIfNoEntry = true;
74+
}
75+
6976
if (encoding != null) {
7077
// This is required since on macOS and Windows it throws ERR_INVALID_ARG_VALUE
7178
if (typeof encoding !== 'string') {
@@ -76,7 +83,7 @@ class FSWatcher extends EventEmitter {
7683
validateIgnoreOption(ignore, 'options.ignore');
7784
this.#ignoreMatcher = createIgnoreMatcher(ignore);
7885

79-
this.#options = { persistent, recursive, signal, encoding };
86+
this.#options = { persistent, recursive, signal, encoding, throwIfNoEntry };
8087
}
8188

8289
close() {
@@ -222,7 +229,7 @@ class FSWatcher extends EventEmitter {
222229
this.#watchFolder(filename);
223230
}
224231
} catch (error) {
225-
if (error.code === 'ENOENT') {
232+
if (!this.#options.throwIfNoEntry && error.code === 'ENOENT') {
226233
error.filename = filename;
227234
throw error;
228235
}

‎lib/internal/fs/watchers.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const {
3535
} = internalBinding('fs');
3636

3737
const { FSEvent } = internalBinding('fs_event_wrap');
38-
const { UV_ENOSPC } = internalBinding('uv');
38+
const { UV_ENOSPC, UV_ENOENT } = internalBinding('uv');
3939
const { EventEmitter } = require('events');
4040

4141
const {
@@ -293,7 +293,8 @@ FSWatcher.prototype[kFSWatchStart] = function(filename,
293293
persistent,
294294
recursive,
295295
encoding,
296-
ignore) {
296+
ignore,
297+
throwIfNoEntry = true) {
297298
if (this._handle === null) { // closed
298299
return;
299300
}
@@ -313,6 +314,10 @@ FSWatcher.prototype[kFSWatchStart] = function(filename,
313314
recursive,
314315
encoding);
315316
if (err) {
317+
if (!throwIfNoEntry && err === UV_ENOENT) {
318+
return;
319+
}
320+
316321
const error = new UVException({
317322
errno: err,
318323
syscall: 'watch',

‎lib/internal/main/watch_mode.js‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,8 @@ markBootstrapComplete();
3434

3535
const kKillSignal = convertToValidSignal(getOptionValue('--watch-kill-signal'));
3636
const kShouldFilterModules = getOptionValue('--watch-path').length === 0;
37-
const kEnvFiles = [
38-
...getOptionValue('--env-file'),
39-
...getOptionValue('--env-file-if-exists'),
40-
];
37+
const kEnvFiles = getOptionValue('--env-file');
38+
const kOptionalEnvFiles = getOptionValue('--env-file-if-exists');
4139
const kWatchedPaths = ArrayPrototypeMap(getOptionValue('--watch-path'), (path) => resolve(path));
4240
const kPreserveOutput = getOptionValue('--watch-preserve-output');
4341
const kCommand = ArrayPrototypeSlice(process.argv, 1);
@@ -105,6 +103,10 @@ function start() {
105103
if (kEnvFiles.length > 0) {
106104
ArrayPrototypeForEach(kEnvFiles, (file) => watcher.filterFile(resolve(file)));
107105
}
106+
if (kOptionalEnvFiles.length > 0) {
107+
ArrayPrototypeForEach(kOptionalEnvFiles,
108+
(file) => watcher.filterFile(resolve(file), undefined, { allowMissing: true }));
109+
}
108110
child.once('exit', (code) => {
109111
exited = true;
110112
const waitingForChanges = 'Waiting for file changes before restarting...';
@@ -160,6 +162,7 @@ async function stop(child) {
160162
}
161163

162164
let restarting = false;
165+
163166
async function restart(child) {
164167
if (restarting) return;
165168
restarting = true;
@@ -198,5 +201,6 @@ function signalHandler(signal) {
198201
process.exit(exitCode ?? kNoFailure);
199202
};
200203
}
204+
201205
process.on('SIGTERM', signalHandler('SIGTERM'));
202206
process.on('SIGINT', signalHandler('SIGINT'));

‎lib/internal/watch_mode/files_watcher.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,13 @@ class FilesWatcher extends EventEmitter {
110110
return [...this.#watchers.keys()];
111111
}
112112

113-
watchPath(path, recursive = true) {
113+
watchPath(path, recursive = true, options = kEmptyObject) {
114114
if (this.#isPathWatched(path)) {
115115
return;
116116
}
117-
const watcher = watch(path, { recursive, signal: this.#signal });
117+
const { allowMissing = false } = options;
118+
119+
const watcher = watch(path, { recursive, signal: this.#signal, throwIfNoEntry: !allowMissing });
118120
watcher.on('change', (eventType, fileName) => {
119121
// `fileName` can be `null` if it cannot be determined. See
120122
// https://github.com/nodejs/node/pull/49891#issuecomment-1744673430.
@@ -126,14 +128,14 @@ class FilesWatcher extends EventEmitter {
126128
}
127129
}
128130

129-
filterFile(file, owner) {
131+
filterFile(file, owner, options = kEmptyObject) {
130132
if (!file) return;
131133
if (supportsRecursiveWatching) {
132-
this.watchPath(dirname(file));
134+
this.watchPath(dirname(file), true, options);
133135
} else {
134136
// Having multiple FSWatcher's seems to be slower
135137
// than a single recursive FSWatcher
136-
this.watchPath(file, false);
138+
this.watchPath(file, false, options);
137139
}
138140
this.#filteredFiles.add(file);
139141
if (owner) {

‎test/parallel/test-fs-watch-enoent.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,29 @@ tmpdir.refresh();
4343
);
4444
}
4545

46+
{
47+
assert.throws(
48+
() => fs.watch(nonexistentFile, { throwIfNoEntry: true }, common.mustNotCall()),
49+
{
50+
path: nonexistentFile,
51+
filename: nonexistentFile,
52+
code: /^(ENOENT|ENODEV)$/,
53+
},
54+
);
55+
}
56+
57+
{
58+
if (common.isAIX) {
59+
assert.throws(
60+
() => fs.watch(nonexistentFile, { throwIfNoEntry: false }, common.mustNotCall()),
61+
{ code: 'ENODEV' },
62+
);
63+
} else {
64+
const watcher = fs.watch(nonexistentFile, { throwIfNoEntry: false }, common.mustNotCall());
65+
watcher.close();
66+
}
67+
}
68+
4669
{
4770
if (common.isMacOS || common.isWindows) {
4871
const file = tmpdir.resolve('file-to-watch');

‎test/sequential/test-watch-mode.mjs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,27 @@ describe('watch mode', { concurrency: !process.env.TEST_PARALLEL, timeout: 60_00
277277
}
278278
});
279279

280+
it('should not crash when --env-file-if-exists points to a missing file', async () => {
281+
const envKey = `TEST_ENV_${Date.now()}`;
282+
const jsFile = createTmpFile(`console.log('ENV: ' + process.env.${envKey});`);
283+
const missingEnvFile = path.join(tmpdir.path, `missing-${Date.now()}.env`);
284+
const { done, restart } = runInBackground({
285+
args: ['--watch-path', tmpdir.path, `--env-file-if-exists=${missingEnvFile}`, jsFile],
286+
});
287+
288+
try {
289+
const { stderr, stdout } = await restart();
290+
291+
assert.doesNotMatch(stderr, /ENOENT: no such file or directory, watch/);
292+
assert.deepStrictEqual(stdout, [
293+
'ENV: undefined',
294+
`Completed running ${inspect(jsFile)}. Waiting for file changes before restarting...`,
295+
]);
296+
} finally {
297+
await done();
298+
}
299+
});
300+
280301
it('should watch changes to a failing file', async () => {
281302
const file = createTmpFile('throw new Error("fails");');
282303
const { stderr, stdout } = await runWriteSucceed({

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL