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
Handle Alert exception on notification failure #93632
Conversation
Hey there @home-assistant/core, @frenck, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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.
Thanks @karwosts.
Can you please rebase your PR and fix the merge conflicts. Thanks :)
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
35230fb
to
81df0f4
Compare
Change has been rebased/merged. |
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.
Thanks @karwosts 👍
Proposed change
Currently when Alert integration attemps to send a notification, it will throw an unhandled exception if any of its notifiers do not exist. This can happen in two situations:
When this happens the alert gets into a bad state, where its internal
self._firing
flag is set, but it throws an exception before it can callasync_write_ha_state
. Because of a notification failure, even though the watched condition may be true, the alert does not update its state toon
. It will forever remain inidle
state if any notifications cannot be delivered.This change adds a simple try/catch around each attempt to send notification. This ensures that even if one notification fails, the Alert state is always accurate, and all other notifiers that could receive the notification continue to do so.
If the failure was due to trying to send a notification on boot before a notifier was setup, that notifier will then continue receiving subsequent notifications at the original repeat interval.
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: