Repository navigation
fs: report errors from recursive watch events - #66792
Open
codeNinja62 wants to merge 1 commit into
Open
codeNinja62 wants to merge 1 commit into
codeNinja62 wants to merge 1 commit into
Conversation
On Linux, the recursive fs.watch() fallback resolves inotify events with stat() and lstat() calls, and arms fs.watch() on directories and symbolic links that appear later. Errors from those calls other than ENOENT were thrown from inside the FSEvent callback, where nothing can catch them, so the process exited: EACCES when a watched directory became unreadable, or when a new directory or the target of a new symbolic link was unreadable by the time it was handled; ELOOP for a symbolic link loop; and ENOTDIR when a watched directory was replaced by a file while events naming its entries were still queued. Report such errors through 'error', as #scanFolder() already does for readdir(), and take ENOTDIR from lstat() to mean that the entry is gone, as statSync() and #scanFolder() do. A directory or link that cannot be watched is dropped, so a later event about it picks it up again. Failures during the initial scan still propagate out of fs.watch(). Fixes: nodejs#66791 Signed-off-by: Sameer Ahmed <166151464+codeNinja62@users.noreply.github.com>
Contributor
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #66791
On Linux, recursive
fs.watch()is the JavaScript watcher inlib/internal/fs/recursive_watch.js. Since #65486 it handles each inotify event with astat()/lstat()of the entry it names, and callsfs.watch()on directories and symlinks that show up later. When one of those calls fails with anything other thanENOENT, the error is thrown inside theFSEventcallback, where no'error'listener can see it, and the process exits:chmod 000)lstatSync()in#onFolderEvent()EACCES, exit 1'error', keeps runningfs.watch()in#watch()EACCES, exit 1'error', keeps runningfs.watch()in#watch()EACCES/ELOOP, exit 1'error', keeps runninglstatSync()in#onFolderEvent()ENOTDIR, exit 1What this changes:
statSync()/lstatSync()calls in#onFolderEvent()and#onEntryEvent()now report failures through'error', the same way#scanFolder()already handlesreaddirSync()failures (fs: ignore deleted dirs in recursive watch scan #63686).ENOTDIRfromlstatSync()is treated as the entry being gone, which is whatstatSync()withthrowIfNoEntry: falseand#scanFolder()already do.#watch()now gets theinitialflag its callers already have. After the initial scan, a failure to watch a directory or symlink is reported through'error'and the entry is dropped, so a later event about it (for example thechmodthat makes it readable again) picks it up. During the initial scan, errors still propagate out offs.watch(), as fs: fix recursive watch error handling #65635 and fs: handle recursive watch setup races #65699 intended.Why
'error'instead of silently forgetting the entry: the watcher can no longer see inside that directory, the native watchers report this kind of failure through'error'too, and long-running consumers such as file-watching daemons need to know. Without an'error'listener the emitter still throws, like any otherFSWatchererror.I didn't add a
#closedcheck before the newemit('error')calls: both handlers return early once the watcher is closed, and so does#watch(). The only code that can run in between is a user-suppliedignorefunction, which can already lead to'change'being emitted afterclose().Tests (each one crashes the process without this change and passes with it):
test-fs-watch-recursive-unreadable-dir.js: a directory made unreadable while watched, and a directory that is already unreadable when its creation is handled and is picked up once it becomes readable. Skipped as root.test-fs-watch-recursive-dir-replaced-by-file.js: theENOTDIRcase; no'error'is expected.test-fs-watch-recursive-symlink-loop.js: a symlink loop created while watching;'error'withELOOPis expected. This covers the symlink branch of#addEntry().Checked locally on a build of
main(Ubuntu on WSL2): the new tests fail before the change and pass after it, 10 runs in a row;tools/test.py 'parallel/test-fs-watch*' 'sequential/test-fs-watch*'passes (49 tests);make lint-jsis clean.v24.22.0 has the same code, since #65486 landed there too, so this should go to v24.x as well.
I used an AI coding assistant to help investigate this and draft the change. I reproduced the crashes myself, reviewed the diff, and ran the tests and lint locally.