From cdabcce83af12c853837e36eb03260dc92ef35d5 Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Thu, 14 Apr 2022 22:43:14 +0200 Subject: [PATCH] Add ability to continue scripts/automations on error (#70004) --- homeassistant/const.py | 1 + homeassistant/helpers/config_validation.py | 6 +- homeassistant/helpers/script.py | 45 ++++- tests/helpers/test_script.py | 181 ++++++++++++++++++++- 4 files changed, 225 insertions(+), 8 deletions(-) diff --git a/homeassistant/const.py b/homeassistant/const.py index 6897a9c9b9f..7661e21b2d1 100644 --- a/homeassistant/const.py +++ b/homeassistant/const.py @@ -116,6 +116,7 @@ CONF_COMMAND_STATE: Final = "command_state" CONF_COMMAND_STOP: Final = "command_stop" CONF_CONDITION: Final = "condition" CONF_CONDITIONS: Final = "conditions" +CONF_CONTINUE_ON_ERROR: Final = "continue_on_error" CONF_CONTINUE_ON_TIMEOUT: Final = "continue_on_timeout" CONF_COUNT: Final = "count" CONF_COVERS: Final = "covers" diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 3ad178483fd..af4bf9bf599 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -36,6 +36,7 @@ from homeassistant.const import ( CONF_CHOOSE, CONF_CONDITION, CONF_CONDITIONS, + CONF_CONTINUE_ON_ERROR, CONF_CONTINUE_ON_TIMEOUT, CONF_COUNT, CONF_DEFAULT, @@ -1056,7 +1057,10 @@ def script_action(value: Any) -> dict: SCRIPT_SCHEMA = vol.All(ensure_list, [script_action]) -SCRIPT_ACTION_BASE_SCHEMA = {vol.Optional(CONF_ALIAS): string} +SCRIPT_ACTION_BASE_SCHEMA = { + vol.Optional(CONF_ALIAS): string, + vol.Optional(CONF_CONTINUE_ON_ERROR): boolean, +} EVENT_SCHEMA = vol.Schema( { diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index f723b351b9c..c26a5c1b852 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -28,6 +28,7 @@ from homeassistant.const import ( CONF_CHOOSE, CONF_CONDITION, CONF_CONDITIONS, + CONF_CONTINUE_ON_ERROR, CONF_CONTINUE_ON_TIMEOUT, CONF_COUNT, CONF_DEFAULT, @@ -404,6 +405,8 @@ class _ScriptRun: self._finish() async def _async_step(self, log_exceptions): + continue_on_error = self._action.get(CONF_CONTINUE_ON_ERROR, False) + with trace_path(str(self._step)): async with trace_action(self._hass, self, self._stop, self._variables): if self._stop.is_set(): @@ -411,12 +414,10 @@ class _ScriptRun: try: handler = f"_async_{cv.determine_script_action(self._action)}_step" await getattr(self, handler)() - except Exception as ex: - if not isinstance(ex, (_AbortScript, _StopScript)) and ( - self._log_exceptions or log_exceptions - ): - self._log_exception(ex) - raise + except Exception as ex: # pylint: disable=broad-except + self._handle_exception( + ex, continue_on_error, self._log_exceptions or log_exceptions + ) def _finish(self) -> None: self._script._runs.remove(self) # pylint: disable=protected-access @@ -430,6 +431,38 @@ class _ScriptRun: self._stop.set() await self._stopped.wait() + def _handle_exception( + self, exception: Exception, continue_on_error: bool, log_exceptions: bool + ) -> None: + if not isinstance(exception, (_AbortScript, _StopScript)) and log_exceptions: + self._log_exception(exception) + + if not continue_on_error: + raise exception + + # An explicit request to stop the script has been raised. + if isinstance(exception, _StopScript): + raise exception + + # These are incorrect scripts, and not runtime errors that need to + # be handled and thus cannot be stopped by `continue_on_error`. + if isinstance( + exception, + ( + vol.Invalid, + exceptions.TemplateError, + exceptions.ServiceNotFound, + exceptions.InvalidEntityFormatError, + exceptions.NoEntitySpecifiedError, + exceptions.ConditionError, + ), + ): + raise exception + + # Only Home Assistant errors can be ignored. + if not isinstance(exception, exceptions.HomeAssistantError): + raise exception + def _log_exception(self, exception): action_type = cv.determine_script_action(self._action) diff --git a/tests/helpers/test_script.py b/tests/helpers/test_script.py index 1fbf2b2625e..b04ad4a4ddf 100644 --- a/tests/helpers/test_script.py +++ b/tests/helpers/test_script.py @@ -28,9 +28,10 @@ from homeassistant.core import ( Context, CoreState, HomeAssistant, + ServiceCall, callback, ) -from homeassistant.exceptions import ConditionError, ServiceNotFound +from homeassistant.exceptions import ConditionError, HomeAssistantError, ServiceNotFound from homeassistant.helpers import ( config_validation as cv, entity_registry as er, @@ -4257,3 +4258,181 @@ async def test_error_action(hass, caplog): }, expected_script_execution="aborted", ) + + +async def test_continue_on_error(hass: HomeAssistant) -> None: + """Test if automation continue when a step fails.""" + events = async_capture_events(hass, "test_event") + + @callback + def broken_service(service: ServiceCall) -> None: + """Break this service with an error.""" + raise HomeAssistantError("It is not working!") + + hass.services.async_register("broken", "service", broken_service) + + sequence = cv.SCRIPT_SCHEMA( + [ + {"event": "test_event"}, + { + "continue_on_error": True, + "service": "broken.service", + }, + {"event": "test_event"}, + { + "continue_on_error": False, + "service": "broken.service", + }, + {"event": "test_event"}, + ] + ) + script_obj = script.Script(hass, sequence, "Test Name", "test_domain") + + with pytest.raises(exceptions.HomeAssistantError, match="It is not working!"): + await script_obj.async_run(context=Context()) + + assert len(events) == 2 + + assert_action_trace( + { + "0": [{"result": {"event": "test_event", "event_data": {}}}], + "1": [ + { + "result": { + "limit": 10, + "params": { + "domain": "broken", + "service": "service", + "service_data": {}, + "target": {}, + }, + "running_script": False, + }, + } + ], + "2": [{"result": {"event": "test_event", "event_data": {}}}], + "3": [ + { + "error_type": HomeAssistantError, + "result": { + "limit": 10, + "params": { + "domain": "broken", + "service": "service", + "service_data": {}, + "target": {}, + }, + "running_script": False, + }, + } + ], + }, + expected_script_execution="error", + ) + + +async def test_continue_on_error_with_stop(hass: HomeAssistant) -> None: + """Test continue on error doesn't work with explicit an stop.""" + sequence = cv.SCRIPT_SCHEMA( + [ + { + "continue_on_error": True, + "stop": "Stop it!", + }, + ] + ) + script_obj = script.Script(hass, sequence, "Test Name", "test_domain") + + await script_obj.async_run(context=Context()) + + assert_action_trace( + { + "0": [{"result": {"stop": "Stop it!"}}], + }, + expected_script_execution="finished", + ) + + +async def test_continue_on_error_automation_issue(hass: HomeAssistant) -> None: + """Test continue on error doesn't block action automation errors.""" + sequence = cv.SCRIPT_SCHEMA( + [ + { + "continue_on_error": True, + "service": "service.not_found", + }, + ] + ) + script_obj = script.Script(hass, sequence, "Test Name", "test_domain") + + with pytest.raises(exceptions.ServiceNotFound): + await script_obj.async_run(context=Context()) + + assert_action_trace( + { + "0": [ + { + "error_type": ServiceNotFound, + "result": { + "limit": 10, + "params": { + "domain": "service", + "service": "not_found", + "service_data": {}, + "target": {}, + }, + "running_script": False, + }, + } + ], + }, + expected_script_execution="error", + ) + + +async def test_continue_on_error_unknown_error(hass: HomeAssistant) -> None: + """Test continue on error doesn't block unknown errors from e.g., libraries.""" + + class MyLibraryError(Exception): + """My custom library error.""" + + @callback + def some_service(service: ServiceCall) -> None: + """Break this service with an error.""" + raise MyLibraryError("It is not working!") + + hass.services.async_register("some", "service", some_service) + + sequence = cv.SCRIPT_SCHEMA( + [ + { + "continue_on_error": True, + "service": "some.service", + }, + ] + ) + script_obj = script.Script(hass, sequence, "Test Name", "test_domain") + + with pytest.raises(MyLibraryError): + await script_obj.async_run(context=Context()) + + assert_action_trace( + { + "0": [ + { + "error_type": MyLibraryError, + "result": { + "limit": 10, + "params": { + "domain": "some", + "service": "service", + "service_data": {}, + "target": {}, + }, + "running_script": False, + }, + } + ], + }, + expected_script_execution="error", + )