Translate service validation errors (#115024)

* Move service validation error message to translation cache

* Fix test

* Revert unrelated change

* Address review comments

* Improve error message

---------

Co-authored-by: J. Nick Koston <nick@koston.org>

homeassistant/components/homeassistant/strings.json

@@ -191,6 +191,18 @@
     },
     "service_not_found": {
       "message": "Service {domain}.{service} not found."
+    },
+    "service_does_not_supports_reponse": {
+      "message": "A service which does not return responses can't be called with {return_response}."
+    },
+    "service_lacks_response_request": {
+      "message": "The service call requires responses and must be called with {return_response}."
+    },
+    "service_reponse_invalid": {
+      "message": "Failed to process the returned service response data, expected a dictionary, but got {response_data_type}."
+    },
+    "service_should_be_blocking": {
+      "message": "A non blocking service call with argument {non_blocking_argument} can't be used together with argument {return_response}."
     }
   }
 }

homeassistant/core.py

@@ -86,6 +86,7 @@ from .exceptions import (
     InvalidStateError,
     MaxLengthExceeded,
     ServiceNotFound,
+    ServiceValidationError,
     Unauthorized,
 )
 from .helpers.deprecation import (
@@ -2571,16 +2572,27 @@ class ServiceRegistry:
 
         if return_response:
             if not blocking:
-                raise ValueError(
+                raise ServiceValidationError(
-                    "Invalid argument return_response=True when blocking=False"
+                    translation_domain=DOMAIN,
+                    translation_key="service_should_be_blocking",
+                    translation_placeholders={
+                        "return_response": "return_response=True",
+                        "non_blocking_argument": "blocking=False",
+                    },
                 )
             if handler.supports_response is SupportsResponse.NONE:
-                raise ValueError(
+                raise ServiceValidationError(
-                    "Invalid argument return_response=True when handler does not support responses"
+                    translation_domain=DOMAIN,
+                    translation_key="service_does_not_supports_reponse",
+                    translation_placeholders={
+                        "return_response": "return_response=True"
+                    },
                 )
         elif handler.supports_response is SupportsResponse.ONLY:
-            raise ValueError(
+            raise ServiceValidationError(
-                "Service call requires responses but caller did not ask for responses"
+                translation_domain=DOMAIN,
+                translation_key="service_lacks_response_request",
+                translation_placeholders={"return_response": "return_response=True"},
             )
 
         if target:
@@ -2628,7 +2640,11 @@ class ServiceRegistry:
             return None
         if not isinstance(response_data, dict):
             raise HomeAssistantError(
-                f"Service response data expected a dictionary, was {type(response_data)}"
+                translation_domain=DOMAIN,
+                translation_key="service_reponse_invalid",
+                translation_placeholders={
+                    "response_data_type": str(type(response_data))
+                },
             )
         return response_data
 

homeassistant/exceptions.py

@@ -255,7 +255,7 @@ class UnknownUser(Unauthorized):
     """When call is made with user ID that doesn't exist."""
 
 
-class ServiceNotFound(HomeAssistantError):
+class ServiceNotFound(ServiceValidationError):
     """Raised when a service is not found."""
 
     def __init__(self, domain: str, service: str) -> None:

tests/components/websocket_api/test_commands.py

@@ -204,7 +204,7 @@ async def test_return_response_error(hass: HomeAssistant, websocket_client) -> N
     assert msg["id"] == 8
     assert msg["type"] == const.TYPE_RESULT
     assert not msg["success"]
-    assert msg["error"]["code"] == "unknown_error"
+    assert msg["error"]["code"] == "service_validation_error"
 
 
 @pytest.mark.parametrize("command", ["call_service", "call_service_action"])

tests/test_core.py

@@ -55,6 +55,7 @@ from homeassistant.exceptions import (
     InvalidStateError,
     MaxLengthExceeded,
     ServiceNotFound,
+    ServiceValidationError,
 )
 from homeassistant.helpers.json import json_dumps
 from homeassistant.setup import async_setup_component
@@ -1791,8 +1792,9 @@ async def test_services_call_return_response_requires_blocking(
     hass: HomeAssistant,
 ) -> None:
     """Test that non-blocking service calls cannot ask for response data."""
+    await async_setup_component(hass, "homeassistant", {})
     async_mock_service(hass, "test_domain", "test_service")
-    with pytest.raises(ValueError, match="when blocking=False"):
+    with pytest.raises(ServiceValidationError, match="blocking=False") as exc:
         await hass.services.async_call(
             "test_domain",
             "test_service",
@@ -1800,6 +1802,10 @@ async def test_services_call_return_response_requires_blocking(
             blocking=False,
             return_response=True,
         )
+    assert (
+        str(exc.value)
+        == "A non blocking service call with argument blocking=False can't be used together with argument return_response=True"
+    )
 
 
 @pytest.mark.parametrize(
@@ -1816,6 +1822,7 @@ async def test_serviceregistry_return_response_invalid(
     hass: HomeAssistant, response_data: Any, expected_error: str
 ) -> None:
     """Test service call response data must be json serializable objects."""
+    await async_setup_component(hass, "homeassistant", {})
 
     def service_handler(call: ServiceCall) -> ServiceResponse:
         """Service handler coroutine."""
@@ -1842,8 +1849,8 @@ async def test_serviceregistry_return_response_invalid(
 @pytest.mark.parametrize(
     ("supports_response", "return_response", "expected_error"),
     [
-        (SupportsResponse.NONE, True, "not support responses"),
+        (SupportsResponse.NONE, True, "does not return responses"),
-        (SupportsResponse.ONLY, False, "caller did not ask for responses"),
+        (SupportsResponse.ONLY, False, "call requires responses"),
     ],
 )
 async def test_serviceregistry_return_response_arguments(
@@ -1853,6 +1860,7 @@ async def test_serviceregistry_return_response_arguments(
     expected_error: str,
 ) -> None:
     """Test service call response data invalid arguments."""
+    await async_setup_component(hass, "homeassistant", {})
 
     hass.services.async_register(
         "test_domain",
@@ -1861,7 +1869,7 @@ async def test_serviceregistry_return_response_arguments(
         supports_response=supports_response,
     )
 
-    with pytest.raises(ValueError, match=expected_error):
+    with pytest.raises(ServiceValidationError, match=expected_error):
         await hass.services.async_call(
             "test_domain",
             "test_service",