From 54cf7010cdb6f08243377e57cb15744bb65d4b35 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Mon, 6 Nov 2023 15:45:04 +0100 Subject: [PATCH] Add ServiceValidationError and translation support (#102592) * Add ServiceValidationError * Add translation support * Extend translation support to HomeAssistantError * Add translation support for ServiceNotFound exc * Frontend translation & translation_key from caller * Improve fallback message * Set websocket_api as default translation_domain * Add MQTT ServiceValidationError exception * Follow up comments * Revert removing gueard on translation_key * Revert test changes to fix CI test * Follow up comments * Fix CI test * Follow up * Improve language * Follow up comment --- .../components/homeassistant/strings.json | 5 ++ homeassistant/components/mqtt/__init__.py | 16 ++++- homeassistant/components/mqtt/strings.json | 5 ++ .../components/websocket_api/commands.py | 46 +++++++++++++- .../components/websocket_api/connection.py | 23 ++++++- .../components/websocket_api/const.py | 1 + .../components/websocket_api/messages.py | 21 ++++++- .../components/websocket_api/strings.json | 7 +++ homeassistant/exceptions.py | 27 +++++++- script/hassfest/translations.py | 4 ++ tests/components/trace/test_websocket_api.py | 6 +- .../components/websocket_api/test_commands.py | 63 ++++++++++++++++++- 12 files changed, 206 insertions(+), 18 deletions(-) create mode 100644 homeassistant/components/websocket_api/strings.json diff --git a/homeassistant/components/homeassistant/strings.json b/homeassistant/components/homeassistant/strings.json index 26871522819..f14d9f8148c 100644 --- a/homeassistant/components/homeassistant/strings.json +++ b/homeassistant/components/homeassistant/strings.json @@ -136,5 +136,10 @@ "name": "Reload all", "description": "Reload all YAML configuration that can be reloaded without restarting Home Assistant." } + }, + "exceptions": { + "service_not_found": { + "message": "Service {domain}.{service} not found." + } } } diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index be283271dee..effff9fdf12 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -24,7 +24,12 @@ from homeassistant.const import ( SERVICE_RELOAD, ) from homeassistant.core import HassJob, HomeAssistant, ServiceCall, callback -from homeassistant.exceptions import HomeAssistantError, TemplateError, Unauthorized +from homeassistant.exceptions import ( + HomeAssistantError, + ServiceValidationError, + TemplateError, + Unauthorized, +) from homeassistant.helpers import config_validation as cv, event as ev, template from homeassistant.helpers.device_registry import DeviceEntry from homeassistant.helpers.dispatcher import async_dispatcher_connect @@ -246,7 +251,14 @@ async def async_check_config_schema( message, _ = conf_util._format_config_error( ex, domain, config, integration.documentation ) - raise HomeAssistantError(message) from ex + raise ServiceValidationError( + message, + translation_domain=DOMAIN, + translation_key="invalid_platform_config", + translation_placeholders={ + "domain": domain, + }, + ) from ex async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: diff --git a/homeassistant/components/mqtt/strings.json b/homeassistant/components/mqtt/strings.json index 6197e580b1d..db0ed741ac0 100644 --- a/homeassistant/components/mqtt/strings.json +++ b/homeassistant/components/mqtt/strings.json @@ -207,5 +207,10 @@ "name": "[%key:common::action::reload%]", "description": "Reloads MQTT entities from the YAML-configuration." } + }, + "exceptions": { + "invalid_platform_config": { + "message": "Reloading YAML config for manually configured MQTT `{domain}` failed. See logs for more details." + } } } diff --git a/homeassistant/components/websocket_api/commands.py b/homeassistant/components/websocket_api/commands.py index 369eca38925..471bbc4745a 100644 --- a/homeassistant/components/websocket_api/commands.py +++ b/homeassistant/components/websocket_api/commands.py @@ -22,6 +22,7 @@ from homeassistant.core import Context, Event, HomeAssistant, State, callback from homeassistant.exceptions import ( HomeAssistantError, ServiceNotFound, + ServiceValidationError, TemplateError, Unauthorized, ) @@ -238,14 +239,53 @@ async def handle_call_service( connection.send_result(msg["id"], {"context": context}) except ServiceNotFound as err: if err.domain == msg["domain"] and err.service == msg["service"]: - connection.send_error(msg["id"], const.ERR_NOT_FOUND, "Service not found.") + connection.send_error( + msg["id"], + const.ERR_NOT_FOUND, + f"Service {err.domain}.{err.service} not found.", + translation_domain=err.translation_domain, + translation_key=err.translation_key, + translation_placeholders=err.translation_placeholders, + ) else: - connection.send_error(msg["id"], const.ERR_HOME_ASSISTANT_ERROR, str(err)) + # The called service called another service which does not exist + connection.send_error( + msg["id"], + const.ERR_HOME_ASSISTANT_ERROR, + f"Service {err.domain}.{err.service} called service " + f"{msg['domain']}.{msg['service']} which was not found.", + translation_domain=const.DOMAIN, + translation_key="child_service_not_found", + translation_placeholders={ + "domain": err.domain, + "service": err.service, + "child_domain": msg["domain"], + "child_service": msg["service"], + }, + ) except vol.Invalid as err: connection.send_error(msg["id"], const.ERR_INVALID_FORMAT, str(err)) + except ServiceValidationError as err: + connection.logger.error(err) + connection.logger.debug("", exc_info=err) + connection.send_error( + msg["id"], + const.ERR_SERVICE_VALIDATION_ERROR, + f"Validation error: {err}", + translation_domain=err.translation_domain, + translation_key=err.translation_key, + translation_placeholders=err.translation_placeholders, + ) except HomeAssistantError as err: connection.logger.exception(err) - connection.send_error(msg["id"], const.ERR_HOME_ASSISTANT_ERROR, str(err)) + connection.send_error( + msg["id"], + const.ERR_HOME_ASSISTANT_ERROR, + str(err), + translation_domain=err.translation_domain, + translation_key=err.translation_key, + translation_placeholders=err.translation_placeholders, + ) except Exception as err: # pylint: disable=broad-except connection.logger.exception(err) connection.send_error(msg["id"], const.ERR_UNKNOWN_ERROR, str(err)) diff --git a/homeassistant/components/websocket_api/connection.py b/homeassistant/components/websocket_api/connection.py index 1dbda62ab95..4581b3be773 100644 --- a/homeassistant/components/websocket_api/connection.py +++ b/homeassistant/components/websocket_api/connection.py @@ -134,9 +134,26 @@ class ActiveConnection: self.send_message(messages.event_message(msg_id, event)) @callback - def send_error(self, msg_id: int, code: str, message: str) -> None: - """Send a error message.""" - self.send_message(messages.error_message(msg_id, code, message)) + def send_error( + self, + msg_id: int, + code: str, + message: str, + translation_key: str | None = None, + translation_domain: str | None = None, + translation_placeholders: dict[str, Any] | None = None, + ) -> None: + """Send an error message.""" + self.send_message( + messages.error_message( + msg_id, + code, + message, + translation_key=translation_key, + translation_domain=translation_domain, + translation_placeholders=translation_placeholders, + ) + ) @callback def async_handle_binary(self, handler_id: int, payload: bytes) -> None: diff --git a/homeassistant/components/websocket_api/const.py b/homeassistant/components/websocket_api/const.py index 4b9a0943d9a..9a44f80a5c8 100644 --- a/homeassistant/components/websocket_api/const.py +++ b/homeassistant/components/websocket_api/const.py @@ -32,6 +32,7 @@ ERR_NOT_ALLOWED: Final = "not_allowed" ERR_NOT_FOUND: Final = "not_found" ERR_NOT_SUPPORTED: Final = "not_supported" ERR_HOME_ASSISTANT_ERROR: Final = "home_assistant_error" +ERR_SERVICE_VALIDATION_ERROR: Final = "service_validation_error" ERR_UNKNOWN_COMMAND: Final = "unknown_command" ERR_UNKNOWN_ERROR: Final = "unknown_error" ERR_UNAUTHORIZED: Final = "unauthorized" diff --git a/homeassistant/components/websocket_api/messages.py b/homeassistant/components/websocket_api/messages.py index 12e649219bc..34ca6886b5e 100644 --- a/homeassistant/components/websocket_api/messages.py +++ b/homeassistant/components/websocket_api/messages.py @@ -65,12 +65,29 @@ def construct_result_message(iden: int, payload: str) -> str: return f'{{"id":{iden},"type":"result","success":true,"result":{payload}}}' -def error_message(iden: int | None, code: str, message: str) -> dict[str, Any]: +def error_message( + iden: int | None, + code: str, + message: str, + translation_key: str | None = None, + translation_domain: str | None = None, + translation_placeholders: dict[str, Any] | None = None, +) -> dict[str, Any]: """Return an error result message.""" + error_payload: dict[str, Any] = { + "code": code, + "message": message, + } + # In case `translation_key` is `None` we do not set it, nor the + # `translation`_placeholders` and `translation_domain`. + if translation_key is not None: + error_payload["translation_key"] = translation_key + error_payload["translation_placeholders"] = translation_placeholders + error_payload["translation_domain"] = translation_domain return { "id": iden, **BASE_ERROR_MESSAGE, - "error": {"code": code, "message": message}, + "error": error_payload, } diff --git a/homeassistant/components/websocket_api/strings.json b/homeassistant/components/websocket_api/strings.json new file mode 100644 index 00000000000..10b95637b6b --- /dev/null +++ b/homeassistant/components/websocket_api/strings.json @@ -0,0 +1,7 @@ +{ + "exceptions": { + "child_service_not_found": { + "message": "Service {domain}.{service} called service {child_domain}.{child_service} which was not found." + } + } +} diff --git a/homeassistant/exceptions.py b/homeassistant/exceptions.py index 2946c8c3743..262b0e338ff 100644 --- a/homeassistant/exceptions.py +++ b/homeassistant/exceptions.py @@ -12,6 +12,23 @@ if TYPE_CHECKING: class HomeAssistantError(Exception): """General Home Assistant exception occurred.""" + def __init__( + self, + *args: object, + translation_domain: str | None = None, + translation_key: str | None = None, + translation_placeholders: dict[str, str] | None = None, + ) -> None: + """Initialize exception.""" + super().__init__(*args) + self.translation_domain = translation_domain + self.translation_key = translation_key + self.translation_placeholders = translation_placeholders + + +class ServiceValidationError(HomeAssistantError): + """A validation exception occurred when calling a service.""" + class InvalidEntityFormatError(HomeAssistantError): """When an invalid formatted entity is encountered.""" @@ -165,13 +182,19 @@ class ServiceNotFound(HomeAssistantError): def __init__(self, domain: str, service: str) -> None: """Initialize error.""" - super().__init__(self, f"Service {domain}.{service} not found") + super().__init__( + self, + f"Service {domain}.{service} not found.", + translation_domain="homeassistant", + translation_key="service_not_found", + translation_placeholders={"domain": domain, "service": service}, + ) self.domain = domain self.service = service def __str__(self) -> str: """Return string representation.""" - return f"Unable to find service {self.domain}.{self.service}" + return f"Service {self.domain}.{self.service} not found." class MaxLengthExceeded(HomeAssistantError): diff --git a/script/hassfest/translations.py b/script/hassfest/translations.py index 4483aacd804..950eeb827ba 100644 --- a/script/hassfest/translations.py +++ b/script/hassfest/translations.py @@ -328,6 +328,10 @@ def gen_strings_schema(config: Config, integration: Integration) -> vol.Schema: ), slug_validator=cv.slug, ), + vol.Optional("exceptions"): cv.schema_with_slug_keys( + {vol.Optional("message"): translation_value_validator}, + slug_validator=cv.slug, + ), vol.Optional("services"): cv.schema_with_slug_keys( { vol.Required("name"): translation_value_validator, diff --git a/tests/components/trace/test_websocket_api.py b/tests/components/trace/test_websocket_api.py index 1041208fa61..1197719328b 100644 --- a/tests/components/trace/test_websocket_api.py +++ b/tests/components/trace/test_websocket_api.py @@ -205,7 +205,7 @@ async def test_get_trace( _assert_raw_config(domain, sun_config, trace) assert trace["blueprint_inputs"] is None assert trace["context"] - assert trace["error"] == "Unable to find service test.automation" + assert trace["error"] == "Service test.automation not found." assert trace["state"] == "stopped" assert trace["script_execution"] == "error" assert trace["item_id"] == "sun" @@ -893,7 +893,7 @@ async def test_list_traces( assert len(_find_traces(response["result"], domain, "sun")) == 1 trace = _find_traces(response["result"], domain, "sun")[0] assert trace["last_step"] == last_step[0].format(prefix=prefix) - assert trace["error"] == "Unable to find service test.automation" + assert trace["error"] == "Service test.automation not found." assert trace["state"] == "stopped" assert trace["script_execution"] == script_execution[0] assert trace["timestamp"] @@ -1632,7 +1632,7 @@ async def test_trace_blueprint_automation( assert trace["config"]["id"] == "sun" assert trace["blueprint_inputs"] == sun_config assert trace["context"] - assert trace["error"] == "Unable to find service test.automation" + assert trace["error"] == "Service test.automation not found." assert trace["state"] == "stopped" assert trace["script_execution"] == "error" assert trace["item_id"] == "sun" diff --git a/tests/components/websocket_api/test_commands.py b/tests/components/websocket_api/test_commands.py index f200c44acca..34424545666 100644 --- a/tests/components/websocket_api/test_commands.py +++ b/tests/components/websocket_api/test_commands.py @@ -19,7 +19,7 @@ from homeassistant.components.websocket_api.auth import ( from homeassistant.components.websocket_api.const import FEATURE_COALESCE_MESSAGES, URL from homeassistant.const import SIGNAL_BOOTSTRAP_INTEGRATIONS from homeassistant.core import Context, HomeAssistant, State, callback -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers import device_registry as dr from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.loader import async_get_integration @@ -343,6 +343,13 @@ async def test_call_service_not_found( assert msg["type"] == const.TYPE_RESULT assert not msg["success"] assert msg["error"]["code"] == const.ERR_NOT_FOUND + assert msg["error"]["message"] == "Service domain_test.test_service not found." + assert msg["error"]["translation_placeholders"] == { + "domain": "domain_test", + "service": "test_service", + } + assert msg["error"]["translation_key"] == "service_not_found" + assert msg["error"]["translation_domain"] == "homeassistant" async def test_call_service_child_not_found( @@ -370,6 +377,18 @@ async def test_call_service_child_not_found( assert msg["type"] == const.TYPE_RESULT assert not msg["success"] assert msg["error"]["code"] == const.ERR_HOME_ASSISTANT_ERROR + assert ( + msg["error"]["message"] == "Service non.existing called service " + "domain_test.test_service which was not found." + ) + assert msg["error"]["translation_placeholders"] == { + "domain": "non", + "service": "existing", + "child_domain": "domain_test", + "child_service": "test_service", + } + assert msg["error"]["translation_key"] == "child_service_not_found" + assert msg["error"]["translation_domain"] == "websocket_api" async def test_call_service_schema_validation_error( @@ -450,10 +469,26 @@ async def test_call_service_error( @callback def ha_error_call(_): - raise HomeAssistantError("error_message") + raise HomeAssistantError( + "error_message", + translation_domain="test", + translation_key="custom_error", + translation_placeholders={"option": "bla"}, + ) hass.services.async_register("domain_test", "ha_error", ha_error_call) + @callback + def service_error_call(_): + raise ServiceValidationError( + "error_message", + translation_domain="test", + translation_key="custom_error", + translation_placeholders={"option": "bla"}, + ) + + hass.services.async_register("domain_test", "service_error", service_error_call) + async def unknown_error_call(_): raise ValueError("value_error") @@ -474,18 +509,40 @@ async def test_call_service_error( assert msg["success"] is False assert msg["error"]["code"] == "home_assistant_error" assert msg["error"]["message"] == "error_message" + assert msg["error"]["translation_placeholders"] == {"option": "bla"} + assert msg["error"]["translation_key"] == "custom_error" + assert msg["error"]["translation_domain"] == "test" await websocket_client.send_json( { "id": 6, "type": "call_service", "domain": "domain_test", + "service": "service_error", + } + ) + + msg = await websocket_client.receive_json() + assert msg["id"] == 6 + assert msg["type"] == const.TYPE_RESULT + assert msg["success"] is False + assert msg["error"]["code"] == "service_validation_error" + assert msg["error"]["message"] == "Validation error: error_message" + assert msg["error"]["translation_placeholders"] == {"option": "bla"} + assert msg["error"]["translation_key"] == "custom_error" + assert msg["error"]["translation_domain"] == "test" + + await websocket_client.send_json( + { + "id": 7, + "type": "call_service", + "domain": "domain_test", "service": "unknown_error", } ) msg = await websocket_client.receive_json() - assert msg["id"] == 6 + assert msg["id"] == 7 assert msg["type"] == const.TYPE_RESULT assert msg["success"] is False assert msg["error"]["code"] == "unknown_error"