Repository navigation
fs.watchFile inconsistent stat return #15364
Description
- Version: tested on v8.4.0 and v6.11.2
- Platform: GNU/Linux (Centos 7)
Hi!
I encountered the following problem when using fs.watchFile. I start watching a file, let's say file1, the file is renamed into file2 and then back into file1.
On the first rename, my listener is called with currentStat filled with 0s (correct, as the file does not exist) and some values for the previousStat.
On the second rename, when the file gets the name it had initially, the previousStat is the same one as the previousStat of the first rename (I think it should be the one filled with 0s, as the file did not exist previously) and has the same modified time as the currentStat. This way, I cannot determine that the file has reappeared.
I am comparing that the modified time has changed in order to avoid false alarms triggered by file access as specified in the Node.js documentation.
Activity
bnoordhuis commentedon Sep 12, 2017
The node.js documentation doesn't mention it but that's the correct and expected behavior for
uv_fs_poll_start(), the libuv function thatfs.watchFile()uses under the hood.If the mtime is the same, the file was moved rather than created anew. If you get a callback with an all-fields-zero stats object, the next callback is a signal that the file has reappeared.
I'll add documentation labels.
dragosnicolae commentedon Sep 12, 2017
Thank you for the quick response.
niveditn commentedon Sep 14, 2017
Hey @bnoordhuis, I can pick this up if that's okay.
Through what I understand from the discussion above and my own examination of the behavior mentioned, I need to add a Note at the bottom of this section, is that correct?
Please let me know if there is anything I need to keep in mind. Meanwhile, I'll get started on this.
Thanks!
acarstoiu commentedon Sep 14, 2017
The expected behaviour of
fs.watchFile()is to hide state processing, not to increase the burden on the programmer who otherwise has to keep track of previously reported zero-ed states.When a watched file reappears either because it was deleted and recreated or simply renamed twice (back and forth), the previous stat object should definitely be zero-ed.
Just try putting yourself in the shoes of someone supervising the file. The only certainty stated by documentation is
All other properties are uncertain, so this is the only real check that the programmer can make. But the way
fs.watchFile()returns now the previous stat object in the said cases makes the check miss the change. 🔕acarstoiu commentedon Sep 14, 2017
➡️ Also mind that
fs.watchFile()is watching paths as opposed to inodes, watched byfs.watch()(which throws if the given path does not resolve to an existing file/directory).bnoordhuis commentedon Sep 14, 2017
@niveditn Yes, that's right.
@acarstoiu
fs.watch( )doesn't necessarily watch inodes, it depends on the platform.acarstoiu commentedon Sep 18, 2017
Doesn't matter, the discussion was about
fs.watchFile().bnoordhuis commentedon Sep 18, 2017
Yes... you were the one who brought up
fs.watch(), remember?acarstoiu commentedon Sep 18, 2017
I'll say it more clearly: read and comment my arguments against your dismissal of this ticket as a documentation bug (when in fact it's an implementation issue).
Do not steer the discussion sideways, please.
bnoordhuis commentedon Sep 18, 2017
I've outlined why it's working as expected per the current implementation, which goes back to node.js v0.6 and behaved mostly identical even before that.
Your comment starts off with "The expected behaviour of fs.watchFile() is..." when in fact that's merely your expectation.
acarstoiu commentedon Sep 18, 2017
By "expected" I meant what is presumable, sane and efficient. If you deem the legacy behaviour as being "expected" in all cases, then you're effectively denying improvements.
(Off-topic: it's like the two meanings of "normal" - what is frequently done vs. what the rules/norms indicate that should be done)
Also, this doesn't seem at all an awfully breaking change. In fact, nothing bad happens with the code implemented as advised by you (i.e. retain state also in client code), it will just be more suboptimal than it is right now.
bnoordhuis commentedon Sep 18, 2017
Undocumented doesn't mean unused.
fs.watchFile()is used a lot and has behaved this way a long time. Any change will almost certainly break someone's code, and for what exactly?If you still think your suggestion is a good idea, open a pull request and argue your case.
doc: added note to fs.watchFile on previousStat
doc: corrected position of note on fs.watchFile
doc: updated content of note on fs.watchFile
doc: added note to fs.watchFile on previousStat
doc: added note to fs.watchFile on previousStat
doc: added note to fs.watchFile on previousStat
doc: added note to fs.watchFile on previousStat
doc: added note to fs.watchFile on previousStat