Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error reporting mechanism for os.watch #71

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

utgheith
Copy link
Contributor

@utgheith utgheith commented Apr 5, 2021

os.watch.watch has 2 distinct phases: (1) recursively scan the watched paths, (2) fork a thread that listens for notifications and reports them back by calling on onEvent function.

Some (but not all) of the errors occurring caught during the second phase print a stack trace and terminate the watcher without informing the caller.

Source of errors:

  • overflow: the underlying watch mechanism usually has an in-kernel buffer that might overflow. Both Linux and OSX report those back as overflow events indicating loss of information.
  • Resource limits: too many open files, etc.
  • Yanking an external drive
  • Changing permissions
  • ...

My use case for os.watch restarts itself every 5 minutes to compensate for lack of reliable error reporting

The proposed API changes (should be backwards compatible with existing code)

  • Add a 3rd callback to os.watch.watch( onError: WatchError => ErrorResult)
  • Add a sealed trait for different kinds of errors (WatchError)
  • Add a sealed trait for error responses (ErrorResult)
  • Add a default error handler (defaultHandler) that behaves like the current implementation

Issues:

  • Ordering between events and errors is harder to establish then if the same callback handled both. Doing this would change the API in a non-backwards compatible way

@utgheith utgheith mentioned this pull request Apr 21, 2021
@lefou lefou marked this pull request as draft August 6, 2024 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant