-
-
Notifications
You must be signed in to change notification settings - Fork 700
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
Report-errors in emitter #738
base: master
Are you sure you want to change the base?
Conversation
Thanks for investigating this. IMHO having an error event can be useful for consumers to notice that the observer or emitter they are relying upon may need to be re-created. Unfortunately not all emitters are using the same |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank @tommorris!
I like the event idea, I think you can move forward (and sorry for the delay).
It is an important change, so if you can add tests, some documentation, and a line + your GitHub nickname in the changelog, that would be awesome!
try: | ||
self.queue_events(self.timeout) | ||
except Exception as ex: | ||
self.queue_event(FileObserverErrorEvent(self._watch.path, ex)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.queue_event(FileObserverErrorEvent(self._watch.path, ex)) | |
self.queue_event(FileObserverErrorEvent(self._watch.path, ex)) | |
# Prevent circular reference | |
ex = None | |
del ex |
self._exception = exception | ||
super(FileObserverErrorEvent, self).__init__(src_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self._exception = exception | |
super(FileObserverErrorEvent, self).__init__(src_path) | |
super().__init__(src_path) | |
self._exception = exception |
@tommorris my bad, sorry for the false alert :) |
@earonesty ⬆️ |
I am trying to use watchdog to monitor a NAS CIFS share from a windows VM as a windows service. It works well for a while but eventually I see that the observer thread dies with no information a la: #663 This seems to be getting at a way to figure out the cause of that thread death which seems like a good idea. I wonder, how would one use this when subclassing PatternMatchingEventHandler? Do we need to first add the EVENT_TYPE_ERROR to the FileSystemEventHandler dispatch so that we can trap the event (eg, "on_error") and dump its repr? |
Emitters can raise exceptions, for example if a handle becomes invalidated. Rather than handle it on a per-emitter basis, a catch-all can emit the underlying exception.
The caller can then choose to tear down an observer, or ignore the event as needed.
DRAFT: Does this look reasonable? Should I keep going (tests, use cases, docs).