notify-rs / notify

🔭 Cross-platform filesystem notification library for Rust.
https://docs.rs/notify
2.68k stars 215 forks source link

PollWatcher: Ignore IO "file not found" exceptions when accessing entry metadata?🥺 #581

Open earshinov opened 6 months ago

earshinov commented 6 months ago

For local development, we are running our Django app inside a Linux Docker container, launched from Windows host with help of WSL2, with sources typically mounted from the Windows file system. We are using watchfiles + django_watchfiles instead of Django's built-in reloader. Watchfiles uses notify under the hood (source). Thanks to this code in watchfiles, it understands that it is launched from a Windows host and falls back to PollWatcher (👌).

We are facing an issue that notify hits an exception (also bringing down the Django app and the Docker container) when we launch mypy in the same running container:

_rust_notify.WatchfilesRustInternalError: error in underlying watcher: IO error for operation on /opt/project/.mypy_cache/3.10/iniconfig/__init__.meta.json.894e30fd1ecd2163: No such file or directory (os error 2) about ["/opt/project/.mypy_cache/3.10/iniconfig/__init__.meta.json.894e30fd1ecd2163"]

I am mediocre at best in Rust, but I assume that this exception is coming from here: https://github.com/notify-rs/notify/blob/2511ebc004c3680bebc0a81f7eb8a7928ecdc76d/notify/src/poll.rs#L301

Could we perhaps have "Not found" errors ignored?

System details

Software

watchfiles 0.21 (latest release) notify 5.1.0 (source)

I see the same code in latest notify here in the repo, so I guess we would have the same problem with watchfiles rebuilt with latest notify.

OS

Host: Windows 11 x64 Version 23H2 (0S Build 22631.3296)

> wsl cat /proc/version
Linux version 5.10.102.1-microsoft-standard-WSL2 (oe-user@oe-host) (x86_64-msft-linux-gcc (GCC) 9.3.0, GNU ld (GNU Binutils) 2.34.0.20200220) #1 SMP Wed Mar 2 00:30:59 UTC 2022

Docker container: python:3.11-bullseye-slim

What you expected

Errors when accessing metadata of a presumably changed file being silently ignored

What happened

A chain reaction notify ⇒ watchfiles ⇒ Django app ⇒ Docker container 💀

earshinov commented 6 months ago

All references at a glance:

earshinov commented 6 months ago

I think, it would be more appropriate to ignore errors in watchfiles rather than here, so I am going to close this until I am convinced otherwise (if ever).

tomako commented 4 months ago

Hey @earshinov, I ran into the same issue. However I don't think that this should be handled somewhere else. The WatchfilesRustInternalError is raised because of an unexpectedly missing file. Files are just like that. They are being created and removed, so this kind of error should be handled here. I'm even not sure that it should be ignored. A notification about a deleted file seems also correct. My environment is also WSL (Ubuntu 22.04) and python watchfiles (0.21.0). Seemingly the error is related to temporary files being generated in the watched folder. In my case it is relatively easy to reproduce if I switch git branch and the referred file is .git/index.lock (not always though). I would consider to re-open this ticket.

earshinov commented 4 months ago

@tomako , I agree, let's reopen the ticket and let the developers decide on further actions.

samuelcolvin commented 1 month ago

Background, I think as of #507 the current behaviour is intended — applications using notify are expected to process the error, instead of it being completely ignored, that's what's implemented in https://github.com/samuelcolvin/watchfiles/pull/301. However #507 had some problematic behaviour — returning generic errors instead of IO errors in some cases which I think I've fixed in #634.

Please can someone who has experienced his error install watchfiles from https://github.com/samuelcolvin/watchfiles/pull/301 and check if it is indeed fixed.

earshinov commented 6 days ago

@samuelcolvin, thank you so much for looking into it! The fix seems to be working nicely for us. Waiting impatiently for the next release 😄