diff --git a/homeassistant/core.py b/homeassistant/core.py index 1993e657368..47b52d9ff76 100644 --- a/homeassistant/core.py +++ b/homeassistant/core.py @@ -1753,6 +1753,17 @@ class ServiceRegistry: """ return service.lower() in self._services.get(domain.lower(), []) + def supports_response(self, domain: str, service: str) -> SupportsResponse: + """Return whether or not the service supports response data. + + This exists so that callers can return more helpful error messages given + the context. Will return NONE if the service does not exist as there is + other error handling when calling the service if it does not exist. + """ + if not (handler := self._services[domain][service]): + return SupportsResponse.NONE + return handler.supports_response + def register( self, domain: str, diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index ee4346ff388..93d04d5f6df 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -69,6 +69,7 @@ from homeassistant.core import ( Event, HassJob, HomeAssistant, + SupportsResponse, callback, ) from homeassistant.util import slugify @@ -661,12 +662,30 @@ class _ScriptRun: self._hass, self._action, self._variables ) + # Validate response data paraters. This check ignores services that do + # not exist which will raise an appropriate error in the service call below. + response_variable = self._action.get(CONF_RESPONSE_VARIABLE) + return_response = response_variable is not None + if self._hass.services.has_service(params[CONF_DOMAIN], params[CONF_SERVICE]): + supports_response = self._hass.services.supports_response( + params[CONF_DOMAIN], params[CONF_SERVICE] + ) + if supports_response == SupportsResponse.ONLY and not return_response: + raise vol.Invalid( + f"Script requires '{CONF_RESPONSE_VARIABLE}' for response data " + f"for service call {params[CONF_DOMAIN]}.{params[CONF_SERVICE]}" + ) + if supports_response == SupportsResponse.NONE and return_response: + raise vol.Invalid( + f"Script does not support '{CONF_RESPONSE_VARIABLE}' for service " + f"'{CONF_RESPONSE_VARIABLE}' which does not support response data." + ) + running_script = ( params[CONF_DOMAIN] == "automation" and params[CONF_SERVICE] == "trigger" or params[CONF_DOMAIN] in ("python_script", "script") ) - response_variable = self._action.get(CONF_RESPONSE_VARIABLE) trace_set_result(params=params, running_script=running_script) response_data = await self._async_run_long_action( self._hass.async_create_task( @@ -674,7 +693,7 @@ class _ScriptRun: **params, blocking=True, context=self._context, - return_response=(response_variable is not None), + return_response=return_response, ) ), ) diff --git a/tests/helpers/test_script.py b/tests/helpers/test_script.py index de13557024a..7f66ec25977 100644 --- a/tests/helpers/test_script.py +++ b/tests/helpers/test_script.py @@ -28,6 +28,7 @@ from homeassistant.core import ( HomeAssistant, ServiceCall, ServiceResponse, + SupportsResponse, callback, ) from homeassistant.exceptions import ConditionError, HomeAssistantError, ServiceNotFound @@ -333,7 +334,7 @@ async def test_calling_service_template(hass: HomeAssistant) -> None: async def test_calling_service_response_data( hass: HomeAssistant, caplog: pytest.LogCaptureFixture ) -> None: - """Test the calling of a service with return values.""" + """Test the calling of a service with response data.""" context = Context() def mock_service(call: ServiceCall) -> ServiceResponse: @@ -342,7 +343,9 @@ async def test_calling_service_response_data( return {"data": "value-12345"} return None - hass.services.async_register("test", "script", mock_service, supports_response=True) + hass.services.async_register( + "test", "script", mock_service, supports_response=SupportsResponse.OPTIONAL + ) sequence = cv.SCRIPT_SCHEMA( [ { @@ -404,6 +407,50 @@ async def test_calling_service_response_data( ) +@pytest.mark.parametrize( + ("supports_response", "params", "expected_error"), + [ + ( + SupportsResponse.NONE, + {"response_variable": "foo"}, + "does not support 'response_variable'", + ), + (SupportsResponse.ONLY, {}, "requires 'response_variable'"), + ], +) +async def test_service_response_data_errors( + hass: HomeAssistant, + supports_response: SupportsResponse, + params: dict[str, str], + expected_error: str, +) -> None: + """Test the calling of a service with response data error cases.""" + context = Context() + + def mock_service(call: ServiceCall) -> ServiceResponse: + """Mock service call.""" + raise ValueError("Never invoked") + + hass.services.async_register( + "test", "script", mock_service, supports_response=supports_response + ) + + sequence = cv.SCRIPT_SCHEMA( + [ + { + "alias": "service step1", + "service": "test.script", + **params, + }, + ] + ) + script_obj = script.Script(hass, sequence, "Test Name", "test_domain") + + with pytest.raises(vol.Invalid, match=expected_error): + await script_obj.async_run(context=context) + await hass.async_block_till_done() + + async def test_data_template_with_templated_key(hass: HomeAssistant) -> None: """Test the calling of a service with a data_template with a templated key.""" context = Context()