handle MustScanSubDirs

[MarcCelani-at]

If the system is flooded with too many fsevents (for example, you are watching a directory that undergoes a huge number of changes all at once, and the user code to process these events is slow), a buffer of events can fill up and trigger a kFSEventStreamEventFlagMustScanSubDirs event.

See the following documentation:

The kFSEventStreamEventFlagUserDropped or kFSEventStreamEventFlagKernelDropped flags may be set in addition to the kFSEventStreamEventFlagMustScanSubDirs flag to indicate that a problem occurred in buffering the events (the particular flag set indicates where the problem occurred) and that the client must do a full scan of any directories (and their subdirectories, recursively) being monitored by this stream. If you asked to monitor multiple paths with this stream then you will be notified about all of them. Your code need only check for the kFSEventStreamEventFlagMustScanSubDirs flag; these flags (if present) only provide information to help you diagnose the problem.

When this happens, fsevents.info() returns an undefined type, which chokidar incorrectly interprets as an unlink. This PR explicitly checks for kFSEventStreamEventFlagMustScanSubDirs before calling fsevents.info and ignores events of this type.

Relevant: fsevents/fsevents#372

Fixes #1196

[paulmillr]

Thanks for the PR. Do you think it should be better handled instead of fsevents package?

[MarcCelani-at]

I'm assuming you meant "inside of the fsevents package"

How would you propose fsevents handle this? I see fsevents as a way to expose the MacOS FSEvents api to node programs. I would want it to be as true as possible to the actual underlying api. So it should definitely pass the exact flags it receives back to chokidar (in other words, I would not want fsevents to drop the callback).

I suppose that you could argue that getInfo(flags).type should return 'directory' here. But then, shouldn't MacOS set the kFSEventStreamEventFlagItemIsDir flag when it sends this event (it doesn't)? I'd argue that the fsevents module is just mirroring whatever MacOS does, and that's a pretty good thing for a module that is exposing something not otherwise available in nodejs.

I would say that fsevents should at least make more clear in the .d.ts file that getInfo().type can be undefined. I filed fsevents/fsevents#372 for that.

[MarcCelani-at]

@paulmillr , let me know how best to proceed here

[MarcCelani-at]

@paulmillr, any thoughts here?

[paulmillr]

fsevents maintainer opinion needs to be included, we'll wait for it

[MarcCelani-at]

I opened fsevents/fsevents#374 but it's been a few days with no answer. The fsevents project doesn't seem like it's under much development.

I guess I'd respectfully ask that this be included in the next version of chokidar if you don't hear back from the fsevents folks, so that we don't have to maintain a patch.

[paulmillr]

fsevents is a huge and important part of chokidar, so we'll wait for it.

[beorn]

One year and no answer from fsevents - make an assumption and move on?