GeezerVison is showing github.com — watch: fix --env-file-if-exists crashing on linux if the file is missing · nodejs/node@c58fe38
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -4784,6 +4784,9 @@ The `atime` and `mtime` arguments follow these rules: | 4784 | 4784 | <!-- YAML | 4785 | 4785 | added: v0.5.10 | 4786 | 4786 | changes: | | 4787 | + - version: REPLACEME | | 4788 | + pr-url: https://github.com/nodejs/node/pull/61870 | | 4789 | + description: Added `throwIfNoEntry` option. | 4787 | 4790 | - version: v19.1.0 | 4788 | 4791 | pr-url: https://github.com/nodejs/node/pull/45098 | 4789 | 4792 | description: Added recursive support for Linux, AIX and IBMi. | @@ -4812,6 +4815,8 @@ changes: | 4812 | 4815 | * `encoding` {string} Specifies the character encoding to be used for the | 4813 | 4816 | filename passed to the listener. **Default:** `'utf8'`. | 4814 | 4817 | * `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`. | 4815 | 4820 | * `ignore` {string|RegExp|Function|Array} Pattern(s) to ignore. Strings are | 4816 | 4821 | glob patterns (using [`minimatch`][]), RegExp patterns are tested against | 4817 | 4822 | the filename, and functions receive the filename and return `true` to | |
Collapse file Copy file name to clipboardExpand all lines: lib/fs.js +4-1Lines changed: 4 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -2512,6 +2512,7 @@ function appendFileSync(path, data, options) { | 2512 | 2512 | * recursive?: boolean; | 2513 | 2513 | * encoding?: string; | 2514 | 2514 | * signal?: AbortSignal; | | 2515 | + * throwIfNoEntry?: boolean; | 2515 | 2516 | * }} [options] | 2516 | 2517 | * @param {( | 2517 | 2518 | * eventType?: string, | @@ -2530,6 +2531,7 @@ function watch(filename, options, listener) { | 2530 | 2531 |
| 2531 | 2532 | if (options.persistent === undefined) options.persistent = true; | 2532 | 2533 | if (options.recursive === undefined) options.recursive = false; | | 2534 | + if (options.throwIfNoEntry === undefined) options.throwIfNoEntry = true; | 2533 | 2535 |
| 2534 | 2536 | let watcher; | 2535 | 2537 | const watchers = require('internal/fs/watchers'); | @@ -2547,7 +2549,8 @@ function watch(filename, options, listener) { | 2547 | 2549 | options.persistent, | 2548 | 2550 | options.recursive, | 2549 | 2551 | options.encoding, | 2550 | | - options.ignore); | | 2552 | + options.ignore, | | 2553 | + options.throwIfNoEntry); | 2551 | 2554 | } | 2552 | 2555 |
| 2553 | 2556 | if (listener) { | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/fs/recursive_watch.js +9-2Lines changed: 9 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -52,6 +52,7 @@ class FSWatcher extends EventEmitter { | 52 | 52 | assert(typeof options === 'object'); | 53 | 53 |
| 54 | 54 | const { persistent, recursive, signal, encoding, ignore } = options; | | 55 | + let { throwIfNoEntry } = options; | 55 | 56 |
| 56 | 57 | // TODO(anonrig): Add non-recursive support to non-native-watcher for IBMi & AIX support. | 57 | 58 | if (recursive != null) { | @@ -66,6 +67,12 @@ class FSWatcher extends EventEmitter { | 66 | 67 | validateAbortSignal(signal, 'options.signal'); | 67 | 68 | } | 68 | 69 |
| | 70 | + if (throwIfNoEntry != null) { | | 71 | + validateBoolean(throwIfNoEntry, 'options.throwIfNoEntry'); | | 72 | + } else { | | 73 | + throwIfNoEntry = true; | | 74 | + } | | 75 | + | 69 | 76 | if (encoding != null) { | 70 | 77 | // This is required since on macOS and Windows it throws ERR_INVALID_ARG_VALUE | 71 | 78 | if (typeof encoding !== 'string') { | @@ -76,7 +83,7 @@ class FSWatcher extends EventEmitter { | 76 | 83 | validateIgnoreOption(ignore, 'options.ignore'); | 77 | 84 | this.#ignoreMatcher = createIgnoreMatcher(ignore); | 78 | 85 |
| 79 | | - this.#options = { persistent, recursive, signal, encoding }; | | 86 | + this.#options = { persistent, recursive, signal, encoding, throwIfNoEntry }; | 80 | 87 | } | 81 | 88 |
| 82 | 89 | close() { | @@ -222,7 +229,7 @@ class FSWatcher extends EventEmitter { | 222 | 229 | this.#watchFolder(filename); | 223 | 230 | } | 224 | 231 | } catch (error) { | 225 | | - if (error.code === 'ENOENT') { | | 232 | + if (!this.#options.throwIfNoEntry && error.code === 'ENOENT') { | 226 | 233 | error.filename = filename; | 227 | 234 | throw error; | 228 | 235 | } | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/fs/watchers.js +7-2Lines changed: 7 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -35,7 +35,7 @@ const { | 35 | 35 | } = internalBinding('fs'); | 36 | 36 |
| 37 | 37 | const { FSEvent } = internalBinding('fs_event_wrap'); | 38 | | -const { UV_ENOSPC } = internalBinding('uv'); | | 38 | +const { UV_ENOSPC, UV_ENOENT } = internalBinding('uv'); | 39 | 39 | const { EventEmitter } = require('events'); | 40 | 40 |
| 41 | 41 | const { | @@ -293,7 +293,8 @@ FSWatcher.prototype[kFSWatchStart] = function(filename, | 293 | 293 | persistent, | 294 | 294 | recursive, | 295 | 295 | encoding, | 296 | | - ignore) { | | 296 | + ignore, | | 297 | + throwIfNoEntry = true) { | 297 | 298 | if (this._handle === null) { // closed | 298 | 299 | return; | 299 | 300 | } | @@ -313,6 +314,10 @@ FSWatcher.prototype[kFSWatchStart] = function(filename, | 313 | 314 | recursive, | 314 | 315 | encoding); | 315 | 316 | if (err) { | | 317 | + if (!throwIfNoEntry && err === UV_ENOENT) { | | 318 | + return; | | 319 | + } | | 320 | + | 316 | 321 | const error = new UVException({ | 317 | 322 | errno: err, | 318 | 323 | syscall: 'watch', | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/main/watch_mode.js +8-4Lines changed: 8 additions & 4 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -34,10 +34,8 @@ markBootstrapComplete(); | 34 | 34 |
| 35 | 35 | const kKillSignal = convertToValidSignal(getOptionValue('--watch-kill-signal')); | 36 | 36 | 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'); | 41 | 39 | const kWatchedPaths = ArrayPrototypeMap(getOptionValue('--watch-path'), (path) => resolve(path)); | 42 | 40 | const kPreserveOutput = getOptionValue('--watch-preserve-output'); | 43 | 41 | const kCommand = ArrayPrototypeSlice(process.argv, 1); | @@ -105,6 +103,10 @@ function start() { | 105 | 103 | if (kEnvFiles.length > 0) { | 106 | 104 | ArrayPrototypeForEach(kEnvFiles, (file) => watcher.filterFile(resolve(file))); | 107 | 105 | } | | 106 | + if (kOptionalEnvFiles.length > 0) { | | 107 | + ArrayPrototypeForEach(kOptionalEnvFiles, | | 108 | + (file) => watcher.filterFile(resolve(file), undefined, { allowMissing: true })); | | 109 | + } | 108 | 110 | child.once('exit', (code) => { | 109 | 111 | exited = true; | 110 | 112 | const waitingForChanges = 'Waiting for file changes before restarting...'; | @@ -160,6 +162,7 @@ async function stop(child) { | 160 | 162 | } | 161 | 163 |
| 162 | 164 | let restarting = false; | | 165 | + | 163 | 166 | async function restart(child) { | 164 | 167 | if (restarting) return; | 165 | 168 | restarting = true; | @@ -198,5 +201,6 @@ function signalHandler(signal) { | 198 | 201 | process.exit(exitCode ?? kNoFailure); | 199 | 202 | }; | 200 | 203 | } | | 204 | + | 201 | 205 | process.on('SIGTERM', signalHandler('SIGTERM')); | 202 | 206 | process.on('SIGINT', signalHandler('SIGINT')); |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/watch_mode/files_watcher.js +7-5Lines changed: 7 additions & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -110,11 +110,13 @@ class FilesWatcher extends EventEmitter { | 110 | 110 | return [...this.#watchers.keys()]; | 111 | 111 | } | 112 | 112 |
| 113 | | - watchPath(path, recursive = true) { | | 113 | + watchPath(path, recursive = true, options = kEmptyObject) { | 114 | 114 | if (this.#isPathWatched(path)) { | 115 | 115 | return; | 116 | 116 | } | 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 }); | 118 | 120 | watcher.on('change', (eventType, fileName) => { | 119 | 121 | // `fileName` can be `null` if it cannot be determined. See | 120 | 122 | // https://github.com/nodejs/node/pull/49891#issuecomment-1744673430. | @@ -126,14 +128,14 @@ class FilesWatcher extends EventEmitter { | 126 | 128 | } | 127 | 129 | } | 128 | 130 |
| 129 | | - filterFile(file, owner) { | | 131 | + filterFile(file, owner, options = kEmptyObject) { | 130 | 132 | if (!file) return; | 131 | 133 | if (supportsRecursiveWatching) { | 132 | | - this.watchPath(dirname(file)); | | 134 | + this.watchPath(dirname(file), true, options); | 133 | 135 | } else { | 134 | 136 | // Having multiple FSWatcher's seems to be slower | 135 | 137 | // than a single recursive FSWatcher | 136 | | - this.watchPath(file, false); | | 138 | + this.watchPath(file, false, options); | 137 | 139 | } | 138 | 140 | this.#filteredFiles.add(file); | 139 | 141 | if (owner) { | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-fs-watch-enoent.js +23Lines changed: 23 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -43,6 +43,29 @@ tmpdir.refresh(); | 43 | 43 | ); | 44 | 44 | } | 45 | 45 |
| | 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 | + | 46 | 69 | { | 47 | 70 | if (common.isMacOS || common.isWindows) { | 48 | 71 | const file = tmpdir.resolve('file-to-watch'); | |
Collapse file Copy file name to clipboardExpand all lines: test/sequential/test-watch-mode.mjs +21Lines changed: 21 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -277,6 +277,27 @@ describe('watch mode', { concurrency: !process.env.TEST_PARALLEL, timeout: 60_00 | 277 | 277 | } | 278 | 278 | }); | 279 | 279 |
| | 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 | + | 280 | 301 | it('should watch changes to a failing file', async () => { | 281 | 302 | const file = createTmpFile('throw new Error("fails");'); | 282 | 303 | const { stderr, stdout } = await runWriteSucceed({ | |
|