watch: fix null fileName on windows systems#49891
watch: fix null fileName on windows systems#49891nodejs-github-bot merged 1 commit intonodejs:mainfrom
fileName on windows systems#49891Conversation
github-actions
bot
removed
the
request-ci
benjamingr
left a comment
benjamingr
left a comment
There was a problem hiding this comment.
Why is fileName null in this case? if it is do we want to fire #onChange ?
|
I'm not quite sure what the correct interpretation of null would be and what action to take. |
|
@nodejs/fs @bnoordhuis do you happen to know? (why/when null can appear as fileName) |
That can happen when the ReadDirectoryChanges IOCP request doesn't complete properly. The NT kernel has this half-way state where the request doesn't fail outright but neither does it produce a valid file change event. Libuv reports it as a UV_CHANGE event with a NULL path because presumably something changed even if we don't know what. I suppose libuv could pass the original path (the path it's been told to watch) although I'm not completely sure that's 100% harmless. |
| const watcher = watch(path, { recursive, signal: this.#signal }); | ||
| watcher.on('change', (eventType, fileName) => this | ||
| .#onChange(recursive ? resolve(path, fileName) : path)); | ||
| .#onChange(recursive ? resolve(path, fileName ?? '') : path)); |
There was a problem hiding this comment.
Can we add a comment to here referencing/mentioning @bnoordhuis's comments? It's important to not lose any knowledge we have.
lpinca
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in 9db4bf4 |
|
We shouldn't have merged this while having an open/pending pull request review comment. |
|
Was it a blocker? We can add the comment now without waiting further. This was open for 3 months. |
PR-URL: #49891 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #49891 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #49891 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
9 participants
This fixes a Node.js crash when running on Windows with
--watch.The issue is that
fs.watch()sometimes returnsnullfor thefilenameas described in the official Node.js documentation.The thrown error:
There is also a Stack Overflow question related to this issue.