Error code in Windows ReadDirectoryChanges event handler is not propagated to listener

[mchesser]

Windows provides an error code to the completion routine that is called when a new file event is received. In the code (i.e. here: https://github.com/passcod/notify/blob/0709c940dd7294ef07b176504a89949d462bbe98/src/windows.rs#L300), the only error that is checked for is ERROR_OPERATION_ABORTED however it's possible for other error codes to be generated.

Notably, when a watched folder is deleted the error code appears to be 5 (access denied), however the file is not properly deleted until the watch handle is closed. This results in various unwanted behavior depending on how the folder is deleted. Deleting the file directly generally results in being marked as deleted (but not actually deleted) and any attempt to use the directory returning an access denied status code. Deleting the file through explorer, sometimes results in the watcher being flooded with a continuous stream of notifications.

This happens fairly frequently in tools like rust-analyzer that like to dynamically watch subfolders to avoid listening to all notification events in busy root directories.

One solution might be to stop the watcher like in the ERROR_OPERATION_ABORTED case (possibly notifying any clients about the error), allowing the handle to be closed (e.g. maybe like this: https://github.com/mchesser/notify/commit/e9236511c991fcb89f4fd3b768bd1dd935cfea76#diff-0d46108ce4a3c312f023e721b094d40cR300). However I'm not certain whether there are other errors that could occur where it is reasonable to continue listening for events.

Another solution might be just to propagate back to the user and force them to deal with stopping the watcher.

[passcod]

I think for the specific case of the watch root being deleted, the watcher should close, and for any other errors, they should be ignored for now, as more would be a breaking change (I think) and propagated forward in the next major (with the hope that users who get errors they think Notify should handle itself will comment and eventually we'll get good handling for all relevant cases).

[mchesser]

Sounds reasonable, though one thing that is not clear, is whether request.buffer is valid if there is an error. It in the access denied case, it seems valid to me (since the filename looks fine), but the Action field is 0 which is not documented.

[passcod]

I've tried a bunch of things to implement this but need more debugging... currently don't have a Windows machine dev-ready, and CI round trips are fairly long, so would appreciate help if available... anyone interested can look at the try-abort-on-deleted-root branch.