From a7741be9bbbd2f3fc52cb01fb11dd65cf2c041fd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 20 Jan 2021 15:13:21 -0600 Subject: [PATCH] Wait for all triggers when one fails to attach (#45361) --- .../components/automation/__init__.py | 4 +-- homeassistant/helpers/script.py | 20 +++++++++----- homeassistant/helpers/trigger.py | 15 ++++++++--- tests/helpers/test_script.py | 26 +++++++++++++++++++ 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/automation/__init__.py b/homeassistant/components/automation/__init__.py index e693d2ed814..f1b6df48bde 100644 --- a/homeassistant/components/automation/__init__.py +++ b/homeassistant/components/automation/__init__.py @@ -462,8 +462,8 @@ class AutomationEntity(ToggleEntity, RestoreEntity): ) -> Optional[Callable[[], None]]: """Set up the triggers.""" - def log_cb(level, msg): - self._logger.log(level, "%s %s", msg, self._name) + def log_cb(level, msg, **kwargs): + self._logger.log(level, "%s %s", msg, self._name, **kwargs) return await async_initialize_triggers( cast(HomeAssistant, self.hass), diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index a2328901d36..ff87312dbc2 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -227,8 +227,12 @@ class _ScriptRun: # pylint: disable=protected-access return await self._script._async_get_condition(config) - def _log(self, msg: str, *args: Any, level: int = logging.INFO) -> None: - self._script._log(msg, *args, level=level) # pylint: disable=protected-access + def _log( + self, msg: str, *args: Any, level: int = logging.INFO, **kwargs: Any + ) -> None: + self._script._log( # pylint: disable=protected-access + msg, *args, level=level, **kwargs + ) async def async_run(self) -> None: """Run script.""" @@ -623,8 +627,8 @@ class _ScriptRun: } done.set() - def log_cb(level, msg): - self._log(msg, level=level) + def log_cb(level, msg, **kwargs): + self._log(msg, level=level, **kwargs) to_context = None remove_triggers = await async_initialize_triggers( @@ -1128,11 +1132,13 @@ class Script: self._choose_data[step] = choose_data return choose_data - def _log(self, msg: str, *args: Any, level: int = logging.INFO) -> None: + def _log( + self, msg: str, *args: Any, level: int = logging.INFO, **kwargs: Any + ) -> None: msg = f"%s: {msg}" args = (self.name, *args) if level == _LOG_EXCEPTION: - self._logger.exception(msg, *args) + self._logger.exception(msg, *args, **kwargs) else: - self._logger.log(level, msg, *args) + self._logger.log(level, msg, *args, **kwargs) diff --git a/homeassistant/helpers/trigger.py b/homeassistant/helpers/trigger.py index f9dd91dc2f1..2c7275a9cc3 100644 --- a/homeassistant/helpers/trigger.py +++ b/homeassistant/helpers/trigger.py @@ -75,12 +75,19 @@ async def async_initialize_triggers( platform = await _async_get_trigger_platform(hass, conf) triggers.append(platform.async_attach_trigger(hass, conf, action, info)) - removes = await asyncio.gather(*triggers) + attach_results = await asyncio.gather(*triggers, return_exceptions=True) + removes = [] - if None in removes: - log_cb(logging.ERROR, "Error setting up trigger") + for result in attach_results: + if isinstance(result, Exception): + log_cb(logging.ERROR, "Error setting up trigger", exc_info=result) + elif result is None: + log_cb( + logging.ERROR, "Unknown error while setting up trigger (empty result)" + ) + else: + removes.append(result) - removes = list(filter(None, removes)) if not removes: return None diff --git a/tests/helpers/test_script.py b/tests/helpers/test_script.py index 18e510b7582..923384099ab 100644 --- a/tests/helpers/test_script.py +++ b/tests/helpers/test_script.py @@ -955,7 +955,33 @@ async def test_wait_for_trigger_bad(hass, caplog): hass.async_create_task(script_obj.async_run()) await hass.async_block_till_done() + assert "Unknown error while setting up trigger" in caplog.text + + +async def test_wait_for_trigger_generated_exception(hass, caplog): + """Test bad wait_for_trigger.""" + script_obj = script.Script( + hass, + cv.SCRIPT_SCHEMA( + {"wait_for_trigger": {"platform": "state", "entity_id": "sensor.abc"}} + ), + "Test Name", + "test_domain", + ) + + async def async_attach_trigger_mock(*args, **kwargs): + raise ValueError("something bad") + + with mock.patch( + "homeassistant.components.homeassistant.triggers.state.async_attach_trigger", + wraps=async_attach_trigger_mock, + ): + hass.async_create_task(script_obj.async_run()) + await hass.async_block_till_done() + assert "Error setting up trigger" in caplog.text + assert "ValueError" in caplog.text + assert "something bad" in caplog.text async def test_condition_basic(hass):