Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 457fb55

Browse filesBrowse files
efekrskladuh95
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 3af44ee commit 457fb55
Copy full SHA for 457fb55

8 files changed

+84-14Lines changed: 84 additions & 14 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎doc/api/fs.md‎

Copy file name to clipboardExpand all lines: doc/api/fs.md
+5Lines changed: 5 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -5059,6 +5059,9 @@ The `atime` and `mtime` arguments follow these rules:
50595059
<!-- YAML
50605060
added: v0.5.10
50615061
changes:
5062+
- version: REPLACEME
5063+
pr-url: https://github.com/nodejs/node/pull/61870
5064+
description: Added `throwIfNoEntry` option.
50625065
- version: v19.1.0
50635066
pr-url: https://github.com/nodejs/node/pull/45098
50645067
description: Added recursive support for Linux, AIX and IBMi.
@@ -5087,6 +5090,8 @@ changes:
50875090
* `encoding` {string} Specifies the character encoding to be used for the
50885091
filename passed to the listener. **Default:** `'utf8'`.
50895092
* `signal` {AbortSignal} allows closing the watcher with an AbortSignal.
5093+
* `throwIfNoEntry` {boolean} Indicates whether an exception should be thrown when the
5094+
path does not exist. **Default:** `true`.
50905095
* `ignore` {string|RegExp|Function|Array} Pattern(s) to ignore. Strings are
50915096
glob patterns (using [`minimatch`][]), RegExp patterns are tested against
50925097
the filename, and functions receive the filename and return `true` to
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2496,6 +2496,7 @@ function appendFileSync(path, data, options) {
24962496
* recursive?: boolean;
24972497
* encoding?: string;
24982498
* signal?: AbortSignal;
2499+
* throwIfNoEntry?: boolean;
24992500
* }} [options]
25002501
* @param {(
25012502
* eventType?: string,
@@ -2514,6 +2515,7 @@ function watch(filename, options, listener) {
25142515

25152516
if (options.persistent === undefined) options.persistent = true;
25162517
if (options.recursive === undefined) options.recursive = false;
2518+
if (options.throwIfNoEntry === undefined) options.throwIfNoEntry = true;
25172519

25182520
let watcher;
25192521
const watchers = require('internal/fs/watchers');
@@ -2531,7 +2533,8 @@ function watch(filename, options, listener) {
25312533
options.persistent,
25322534
options.recursive,
25332535
options.encoding,
2534-
options.ignore);
2536+
options.ignore,
2537+
options.throwIfNoEntry);
25352538
}
25362539

25372540
if (listener) {
Collapse file

‎lib/internal/fs/recursive_watch.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/recursive_watch.js
+9-2Lines 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
}
Collapse file

‎lib/internal/fs/watchers.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/watchers.js
+7-2Lines 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',
Collapse file

‎lib/internal/main/watch_mode.js‎

Copy file name to clipboardExpand all lines: lib/internal/main/watch_mode.js
+8-4Lines 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'));
Collapse file

‎lib/internal/watch_mode/files_watcher.js‎

Copy file name to clipboardExpand all lines: lib/internal/watch_mode/files_watcher.js
+7-5Lines 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) {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-fs-watch-enoent.js
+23Lines 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');
Collapse file

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

Copy file name to clipboardExpand all lines: test/sequential/test-watch-mode.mjs
+21Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.