Skip to content

Commit c58fe38

Browse files
efekrskladuh95
authored andcommitted
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
letwatcher;
25352537
constwatchers=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(typeofoptions==='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(typeofencoding!=='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
throwerror;
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
consterror=newUVException({
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
constkKillSignal=convertToValidSignal(getOptionValue('--watch-kill-signal'));
3636
constkShouldFilterModules=getOptionValue('--watch-path').length===0;
37-
constkEnvFiles=[
38-
...getOptionValue('--env-file'),
39-
...getOptionValue('--env-file-if-exists'),
40-
];
37+
constkEnvFiles=getOptionValue('--env-file');
38+
constkOptionalEnvFiles=getOptionValue('--env-file-if-exists');
4139
constkWatchedPaths=ArrayPrototypeMap(getOptionValue('--watch-path'),(path)=>resolve(path));
4240
constkPreserveOutput=getOptionValue('--watch-preserve-output');
4341
constkCommand=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
constwaitingForChanges='Waiting for file changes before restarting...';
@@ -160,6 +162,7 @@ async function stop(child) {
160162
}
161163

162164
letrestarting=false;
165+
163166
asyncfunctionrestart(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-
constwatcher=watch(path,{ recursive,signal: this.#signal });
117+
const{ allowMissing =false}=options;
118+
119+
constwatcher=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+
constwatcher=fs.watch(nonexistentFile,{throwIfNoEntry: false},common.mustNotCall());
65+
watcher.close();
66+
}
67+
}
68+
4669
{
4770
if(common.isMacOS||common.isWindows){
4871
constfile=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+
constenvKey=`TEST_ENV_${Date.now()}`;
282+
constjsFile=createTmpFile(`console.log('ENV: ' + process.env.${envKey});`);
283+
constmissingEnvFile=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 }=awaitrestart();
290+
291+
assert.doesNotMatch(stderr,/ENOENT:nosuchfileordirectory,watch/);
292+
assert.deepStrictEqual(stdout,[
293+
'ENV: undefined',
294+
`Completed running ${inspect(jsFile)}. Waiting for file changes before restarting...`,
295+
]);
296+
}finally{
297+
awaitdone();
298+
}
299+
});
300+
280301
it('should watch changes to a failing file',async()=>{
281302
constfile=createTmpFile('throw new Error("fails");');
282303
const{ stderr, stdout }=awaitrunWriteSucceed({

0 commit comments

Comments
 (0)