Implement basic support for failure notifications.

[libklein]

This is a draft PR to implement notifications on watch failure. The system always sends a notification in the current state of the PR, but I am happy to address that (See #1678 for suggestions). I am creating the PR at this point to create a discussion which kinds of handlers should/should not be supported. Please comment if you have an opinion on this!

I'd further like to refactor the error handling system in update_worker.py. The idea is to delegate exception handling to one or multiple handler classes, registered on startup. I believe that this would clean up the quite convoluted code. I'd further like to have something similar for the notification handling (See the TODO's in _update_watch). I'd be happy for feedback on this idea.

See #1678.

[dgtlmoon]

Heya! thanks for the PR!

This is a draft PR to implement notifications on watch failure. The system always sends a notification in the current state of the PR, but I am happy to address that (See #1678 for suggestions). I am creating the PR at this point to create a discussion which kinds of handlers should/should not be supported. Please comment if you have an opinion on this!

So far the only big thing that it absolutely requires is the "threshold", many sites will "naturally" fail sometimes, its never a great idea to always panic every time the site doesnt load (unless that's what you want :) )

See here

changedetection.io/changedetectionio/forms.py

Line 537 in f9f69bf

filter_failure_notification_threshold_attempts = IntegerField('Number of times the filter can be missing before sending a notification',

I see you are handling this by inserting a call on every Exception

                    except content_fetcher.EmptyReply as e:
                        # Some kind of custom to-str handler in the exception handler that does this?
                        err_text = "EmptyReply - try increasing 'Wait seconds before extracting text', Status Code {}".format(
                            e.status_code)
                        self._update_watch(uuid=uuid, update_obj={'last_error': err_text,
                                                                  'last_check_status': e.status_code},

I have been struggling with how todo this for a long time in a cleaner way, My initial thoughts was that Python can tell in a single call if there was an exception thrown, without having to place this call into every exception catch block..

try:
   something()
except CustomException as e:
   some_custom_message()
except Exception:
   some_custom_message()
alwaysIfAnyException:
   log()

but at https://docs.python.org/3/tutorial/errors.html#defining-clean-up-actions there is really only finally: which is not what we want.. there is no ExceptAlwaysIfAnyExceptionWasThrown in python

(or some system call to get any last exception thrown?)

The other idea is to store the exception, then at the end of the try/catch block, we can ask "is the exception that was set one that has a baseclass of our custom list? if so, check the configs if the exception class name is one we 'care about'"

https://docs.python.org/3/tutorial/errors.html#enriching-exceptions-with-notes

btw: #1941 I want to merge this in next week or so, which will cause a few conflicts, this is also a really good cleanup

[libklein]

Hi @dgtlmoon, thanks for the encouraging reply!

I see you are handling this by inserting a call on every Exception

                    except content_fetcher.EmptyReply as e:
                        # Some kind of custom to-str handler in the exception handler that does this?
                        err_text = "EmptyReply - try increasing 'Wait seconds before extracting text', Status Code {}".format(
                            e.status_code)
                        self._update_watch(uuid=uuid, update_obj={'last_error': err_text,
                                                                  'last_check_status': e.status_code},

I have been struggling with how todo this for a long time in a cleaner way

I think I might have an idea on doing this, this has been bothering me as well. The code is really convoluted and there is a lot of repetition. I was planning on including a refactor of the logic with this PR. Let me go ahead and implement my basic idea and we can then perhaps iterate on that. I think it would be worthwhile to rewrite a lot of the testing boilerplate as well, but that is work for another PR!

Another thing: I've so far assumed that we were aiming to remain compatible with python 3.7 (see setup.py). Is this the case? If not, what version are we supporting?

[dgtlmoon]

hey - yeah sometimes the convoluted way is the easiest/best way, atleast looking at that long try/except block its easy to understand what each exception does... so maybe adding the exception like you did is perfectly fine... see what you can find out

3.7+ is good, 3.10 is what we are testing for

[dgtlmoon]

One extra thing, would be nice to maybe configure the 'profile' for what we want to know about

for some users, knowing when its 500 is also handy for example

so maybe it could be a configurable profile

[libklein]

I've been working on refactoring the exception chain but believe that it would be better to address that in a separate PR. How do you feel about that?

Can you elaborate on the "profile" configuration option?

[libklein]

Hey @dgtlmoon, so, how should we proceed with this?

[libklein]

Hello @dgtlmoon, do you have any plans to consider this PR for merging? I'd be happy to update the PR if you deem this feature important enough.

[rasmusson]

@dgtlmoon I so need this one. Can I do anything to help?