diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index 85d84f9dd8f3..cb2269425506 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -157,8 +157,11 @@ class FSWatcher extends EventEmitter { } // Report an entry that vanished between being listed and being watched so - // the caller can discard its stale bookkeeping. - #watch(file, onChange) { + // the caller can discard its stale bookkeeping. Other failures propagate + // out of fs.watch() during the initial scan. Later, nothing could catch + // them, so they are reported through 'error' and the entry is discarded as + // well; a later event about it picks it up again. + #watch(file, onChange, initial) { if (this.#closed) { return false; } @@ -174,7 +177,11 @@ class FSWatcher extends EventEmitter { (file !== this.#rootPath || !this.#options.throwIfNoEntry)) { return false; } - throw err; + if (initial) { + throw err; + } + this.emit('error', err); + return false; } this.#watchers.set(file, watcher); return true; @@ -184,7 +191,7 @@ class FSWatcher extends EventEmitter { // yet (emitting 'rename' for them unless this is the initial scan); // #addEntry() descends into subdirectories. #scanFolder(folder, initial) { - if (!this.#watch(folder, (eventType, filename) => this.#onFolderEvent(folder, filename))) { + if (!this.#watch(folder, (eventType, filename) => this.#onFolderEvent(folder, filename), initial)) { this.#forget(folder); return; } @@ -230,18 +237,21 @@ class FSWatcher extends EventEmitter { // The link target is watched so that changes behind the link surface // as a 'rename' of the link, as they always have on this code path. this.#symbolicLinks.add(file); - if (!this.#watch(file, () => this.#emit('rename', file))) { + if (!this.#watch(file, () => this.#emit('rename', file), initial)) { this.#forget(file); } } else if (entry.isDirectory()) { this.#scanFolder(file, initial); } else if (!kDirectoryWatchReportsEntries) { - if (!this.#watch(file, () => this.#onEntryEvent(file))) { + if (!this.#watch(file, () => this.#onEntryEvent(file), initial)) { this.#forget(file); } } } + // This and #onEntryEvent() run from fs.watch() callbacks, where nothing can + // catch an exception, so failures other than an entry being gone are + // reported through 'error', as #scanFolder() does for readdir(). #onFolderEvent(folder, filename) { if (this.#closed) { return; @@ -249,7 +259,14 @@ class FSWatcher extends EventEmitter { const { lstatSync, statSync } = lazyLoadFsSync(); if (!kDirectoryWatchReportsEntries || filename == null) { // All that is known is that something about `folder` changed. - if (statSync(folder, { throwIfNoEntry: false }) === undefined) { + let stats; + try { + stats = statSync(folder, { throwIfNoEntry: false }); + } catch (error) { + this.emit('error', error); + return; + } + if (stats === undefined) { this.#emit('rename', folder); this.#forget(folder); } else { @@ -265,12 +282,31 @@ class FSWatcher extends EventEmitter { if (this.#ignoreMatcher?.(pathRelative(this.#rootPath, file))) { return; } - const entry = lstatSync(file, { throwIfNoEntry: false }); + let entry; + try { + entry = lstatSync(file, { throwIfNoEntry: false }); + } catch (error) { + // Like statSync(), take ENOTDIR (`folder` has been replaced by a + // non-directory) to mean that the entry is gone. + if (error.code !== 'ENOTDIR') { + this.emit('error', error); + return; + } + } if (entry !== undefined) { this.#addEntry(file, entry, false); - } else if (folder === this.#rootPath && statSync(folder, { throwIfNoEntry: false }) === undefined) { - this.#emit('rename', folder); - this.#forget(folder); + } else if (folder === this.#rootPath) { + let stats; + try { + stats = statSync(folder, { throwIfNoEntry: false }); + } catch (error) { + this.emit('error', error); + return; + } + if (stats === undefined) { + this.#emit('rename', folder); + this.#forget(folder); + } } return; } @@ -284,7 +320,13 @@ class FSWatcher extends EventEmitter { return; } const { statSync } = lazyLoadFsSync(); - const stats = statSync(file, { throwIfNoEntry: false }); + let stats; + try { + stats = statSync(file, { throwIfNoEntry: false }); + } catch (error) { + this.emit('error', error); + return; + } if (stats === undefined) { this.#emit('rename', file); this.#forget(file); @@ -310,7 +352,7 @@ class FSWatcher extends EventEmitter { } else { this.emit('change', 'change', pathBasename(file)); } - })) { + }, true)) { this.#forget(file); } } diff --git a/test/parallel/test-fs-watch-recursive-dir-replaced-by-file.js b/test/parallel/test-fs-watch-recursive-dir-replaced-by-file.js new file mode 100755 index 000000000000..23ba12b2fdd4 --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-dir-replaced-by-file.js @@ -0,0 +1,35 @@ +'use strict'; + +const common = require('../common'); + +// Events naming entries of a watched directory that has meanwhile been +// replaced by a file must not make the recursive watcher throw from inside +// its own fs.watch() callbacks: those entries are simply gone. + +if (!common.isLinux) + common.skip('this test exercises the inotify-based recursive watcher'); + +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const root = tmpdir.resolve('root'); +const sub = path.join(root, 'sub'); +fs.mkdirSync(sub, { recursive: true }); + +const watcher = fs.watch(root, { recursive: true }); +watcher.on('error', common.mustNotCall()); +watcher.on('change', common.mustCallAtLeast((eventType, filename) => { + // Inotify reports events in order, so by the time the file created last is + // reported, the events naming entries of `sub` have been handled. + if (filename === 'after') { + watcher.close(); + } +})); + +fs.writeFileSync(path.join(sub, 'file'), ''); +fs.rmSync(sub, { recursive: true }); +fs.writeFileSync(sub, ''); +fs.writeFileSync(path.join(root, 'after'), ''); diff --git a/test/parallel/test-fs-watch-recursive-symlink-loop.js b/test/parallel/test-fs-watch-recursive-symlink-loop.js new file mode 100755 index 000000000000..74f56fdc2bad --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-symlink-loop.js @@ -0,0 +1,37 @@ +'use strict'; + +const common = require('../common'); + +// A symbolic link that cannot be watched (here, one of a pair pointing at +// each other) appearing in a watched tree must be reported through 'error' +// instead of being thrown from inside the recursive watcher's own fs.watch() +// callbacks, and the watcher must keep working. + +if (!common.isLinux) + common.skip('this test exercises the inotify-based recursive watcher'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const root = tmpdir.resolve('root'); +fs.mkdirSync(root); + +const watcher = fs.watch(root, { recursive: true }); +watcher.on('error', common.mustCallAtLeast((error) => { + assert.strictEqual(error.code, 'ELOOP'); +})); +watcher.on('change', common.mustCallAtLeast((eventType, filename) => { + // Inotify reports events in order, so by the time the file created last is + // reported, the events about the links have been handled. + if (filename === 'after') { + watcher.close(); + } +})); + +fs.symlinkSync(path.join(root, 'b'), path.join(root, 'a')); +fs.symlinkSync(path.join(root, 'a'), path.join(root, 'b')); +fs.writeFileSync(path.join(root, 'after'), ''); diff --git a/test/parallel/test-fs-watch-recursive-unreadable-dir.js b/test/parallel/test-fs-watch-recursive-unreadable-dir.js new file mode 100644 index 000000000000..6c2e9cd1c657 --- /dev/null +++ b/test/parallel/test-fs-watch-recursive-unreadable-dir.js @@ -0,0 +1,82 @@ +'use strict'; + +const common = require('../common'); + +// The recursive watcher must report directories it cannot look into through +// 'error' instead of throwing from inside its own fs.watch() callbacks, where +// nothing can catch the exception, and must keep working afterwards. + +if (!common.isLinux) + common.skip('this test exercises the inotify-based recursive watcher'); + +if (process.getuid() === 0) + common.skip('as this test should not be run as `root`'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); + +const root = tmpdir.resolve('root'); +const existing = path.join(root, 'existing'); +const created = path.join(root, 'created'); + +// Registered before tmpdir.refresh() so that it runs before tmpdir's own +// 'exit' cleanup, which cannot remove a directory it is unable to read. +process.on('exit', () => { + for (const dir of [existing, created]) { + try { + fs.chmodSync(dir, 0o755); + } catch { + // Never created. + } + } +}); + +tmpdir.refresh(); +fs.mkdirSync(existing, { recursive: true }); + +// Resolves once `filename` is reported. Inotify reports events in order, so +// by then the events caused before it have been handled. +function reported(watcher, filename) { + return new Promise((resolve) => { + watcher.on('change', function onChange(eventType, name) { + if (name === filename) { + watcher.off('change', onChange); + resolve(); + } + }); + }); +} + +(async () => { + { + // A watched directory that becomes unreadable. + const watcher = fs.watch(root, { recursive: true }); + watcher.on('error', common.mustCallAtLeast((error) => { + assert.strictEqual(error.code, 'EACCES'); + })); + const after = reported(watcher, 'after'); + fs.chmodSync(existing, 0o000); + fs.writeFileSync(path.join(root, 'after'), ''); + await after; + watcher.close(); + fs.chmodSync(existing, 0o755); + } + + { + // A directory that is already unreadable when its creation is handled is + // picked up once it can be read. + const watcher = fs.watch(root, { recursive: true }); + watcher.on('error', common.mustCallAtLeast((error) => { + assert.strictEqual(error.code, 'EACCES'); + fs.chmodSync(created, 0o755); + })); + const inside = reported(watcher, path.join('created', 'inside')); + fs.mkdirSync(created); + fs.writeFileSync(path.join(created, 'inside'), ''); + fs.chmodSync(created, 0o000); + await inside; + watcher.close(); + } +})().then(common.mustCall());