From 554aefed423d672c431ff21c78b50c596573cce5 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Sat, 16 Mar 2024 22:56:48 +0100 Subject: [PATCH] Generate HomeAssistantError message from English translations (#113305) * Fetch exception message from translation cache * Improve tests * Return translation key without path, cleanup * Fetch translations when string variant is requested * Move import * revert changes ConfigValidationError * mypy * Remove _str__ method instead * Type _message for mqtt template exception classes * Revert changes made to test_config.py * Undo changes TemplateError * Follow up comments and test coverage --- homeassistant/components/climate/__init__.py | 2 - homeassistant/components/mqtt/models.py | 4 ++ homeassistant/exceptions.py | 38 +++++++++++-- homeassistant/helpers/translation.py | 32 ++++++++++- tests/components/climate/test_init.py | 16 +++--- tests/test_exceptions.py | 57 ++++++++++++++++++++ 6 files changed, 136 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/climate/__init__.py b/homeassistant/components/climate/__init__.py index 7265b5192e8..2382c535413 100644 --- a/homeassistant/components/climate/__init__.py +++ b/homeassistant/components/climate/__init__.py @@ -647,8 +647,6 @@ class ClimateEntity(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): elif mode_type == "fan": translation_key = "not_valid_fan_mode" raise ServiceValidationError( - f"The {mode_type}_mode {mode} is not a valid {mode_type}_mode:" - f" {modes_str}", translation_domain=DOMAIN, translation_key=translation_key, translation_placeholders={ diff --git a/homeassistant/components/mqtt/models.py b/homeassistant/components/mqtt/models.py index 9c961a3b543..6e6ae784eec 100644 --- a/homeassistant/components/mqtt/models.py +++ b/homeassistant/components/mqtt/models.py @@ -116,6 +116,8 @@ class MqttOriginInfo(TypedDict, total=False): class MqttCommandTemplateException(ServiceValidationError): """Handle MqttCommandTemplate exceptions.""" + _message: str + def __init__( self, *args: object, @@ -227,6 +229,8 @@ class MqttCommandTemplate: class MqttValueTemplateException(TemplateError): """Handle MqttValueTemplate exceptions.""" + _message: str + def __init__( self, *args: object, diff --git a/homeassistant/exceptions.py b/homeassistant/exceptions.py index a5999510f9f..409fa3450bd 100644 --- a/homeassistant/exceptions.py +++ b/homeassistant/exceptions.py @@ -13,6 +13,9 @@ if TYPE_CHECKING: class HomeAssistantError(Exception): """General Home Assistant exception occurred.""" + _message: str | None = None + generate_message: bool = False + def __init__( self, *args: object, @@ -21,11 +24,42 @@ class HomeAssistantError(Exception): translation_placeholders: dict[str, str] | None = None, ) -> None: """Initialize exception.""" + if not args and translation_key and translation_domain: + self.generate_message = True + args = (translation_key,) + super().__init__(*args) self.translation_domain = translation_domain self.translation_key = translation_key self.translation_placeholders = translation_placeholders + def __str__(self) -> str: + """Return exception message. + + If no message was passed to `__init__`, the exception message is generated from + the translation_key. The message will be in English, regardless of the configured + language. + """ + + if self._message: + return self._message + + if not self.generate_message: + self._message = super().__str__() + return self._message + + if TYPE_CHECKING: + assert self.translation_key is not None + assert self.translation_domain is not None + + # pylint: disable-next=import-outside-toplevel + from .helpers.translation import async_get_exception_message + + self._message = async_get_exception_message( + self.translation_domain, self.translation_key, self.translation_placeholders + ) + return self._message + class ConfigValidationError(HomeAssistantError, ExceptionGroup[Exception]): """A validation exception occurred when validating the configuration.""" @@ -47,10 +81,6 @@ class ConfigValidationError(HomeAssistantError, ExceptionGroup[Exception]): ) self._message = message - def __str__(self) -> str: - """Return exception message string.""" - return self._message - class ServiceValidationError(HomeAssistantError): """A validation exception occurred when calling a service.""" diff --git a/homeassistant/helpers/translation.py b/homeassistant/helpers/translation.py index 641d200afe3..841226ac584 100644 --- a/homeassistant/helpers/translation.py +++ b/homeassistant/helpers/translation.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio from collections.abc import Iterable, Mapping +from contextlib import suppress import logging import string from typing import Any @@ -13,7 +14,7 @@ from homeassistant.const import ( STATE_UNAVAILABLE, STATE_UNKNOWN, ) -from homeassistant.core import Event, HomeAssistant, callback +from homeassistant.core import Event, HomeAssistant, async_get_hass, callback from homeassistant.loader import ( Integration, async_get_config_flows, @@ -528,6 +529,35 @@ def async_translations_loaded(hass: HomeAssistant, components: set[str]) -> bool ) +@callback +def async_get_exception_message( + translation_domain: str, + translation_key: str, + translation_placeholders: dict[str, str] | None = None, +) -> str: + """Return a translated exception message. + + Defaults to English, requires translations to already be cached. + """ + language = "en" + hass = async_get_hass() + localize_key = ( + f"component.{translation_domain}.exceptions.{translation_key}.message" + ) + translations = async_get_cached_translations(hass, language, "exceptions") + if localize_key in translations: + if message := translations[localize_key]: + message = message.rstrip(".") + if not translation_placeholders: + return message + with suppress(KeyError): + message = message.format(**translation_placeholders) + return message + + # We return the translation key when was not found in the cache + return translation_key + + @callback def async_translate_state( hass: HomeAssistant, diff --git a/tests/components/climate/test_init.py b/tests/components/climate/test_init.py index 3950dea59ed..65b7287f549 100644 --- a/tests/components/climate/test_init.py +++ b/tests/components/climate/test_init.py @@ -300,7 +300,7 @@ async def test_preset_mode_validation( with pytest.raises( ServiceValidationError, - match="The preset_mode invalid is not a valid preset_mode: home, away", + match="Preset mode invalid is not valid. Valid preset modes are: home, away", ) as exc: await hass.services.async_call( DOMAIN, @@ -313,13 +313,13 @@ async def test_preset_mode_validation( ) assert ( str(exc.value) - == "The preset_mode invalid is not a valid preset_mode: home, away" + == "Preset mode invalid is not valid. Valid preset modes are: home, away" ) assert exc.value.translation_key == "not_valid_preset_mode" with pytest.raises( ServiceValidationError, - match="The swing_mode invalid is not a valid swing_mode: auto, off", + match="Swing mode invalid is not valid. Valid swing modes are: auto, off", ) as exc: await hass.services.async_call( DOMAIN, @@ -331,13 +331,14 @@ async def test_preset_mode_validation( blocking=True, ) assert ( - str(exc.value) == "The swing_mode invalid is not a valid swing_mode: auto, off" + str(exc.value) + == "Swing mode invalid is not valid. Valid swing modes are: auto, off" ) assert exc.value.translation_key == "not_valid_swing_mode" with pytest.raises( ServiceValidationError, - match="The fan_mode invalid is not a valid fan_mode: auto, off", + match="Fan mode invalid is not valid. Valid fan modes are: auto, off", ) as exc: await hass.services.async_call( DOMAIN, @@ -348,7 +349,10 @@ async def test_preset_mode_validation( }, blocking=True, ) - assert str(exc.value) == "The fan_mode invalid is not a valid fan_mode: auto, off" + assert ( + str(exc.value) + == "Fan mode invalid is not valid. Valid fan modes are: auto, off" + ) assert exc.value.translation_key == "not_valid_fan_mode" diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 3f18b3527b7..f5b60627ce2 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -2,12 +2,17 @@ from __future__ import annotations +from typing import Any +from unittest.mock import patch + import pytest +from homeassistant.core import HomeAssistant from homeassistant.exceptions import ( ConditionErrorContainer, ConditionErrorIndex, ConditionErrorMessage, + HomeAssistantError, TemplateError, ) @@ -62,3 +67,55 @@ def test_template_message(arg: str | Exception, expected: str) -> None: """Ensure we can create TemplateError.""" template_error = TemplateError(arg) assert str(template_error) == expected + + +@pytest.mark.parametrize( + ("exception_args", "exception_kwargs", "args_base_class", "message"), + [ + ((), {}, (), ""), + (("bla",), {}, ("bla",), "bla"), + ((None,), {}, (None,), "None"), + ((type_error_bla := TypeError("bla"),), {}, (type_error_bla,), "bla"), + ( + (), + {"translation_domain": "test", "translation_key": "test"}, + ("test",), + "test", + ), + ( + (), + {"translation_domain": "test", "translation_key": "bla"}, + ("bla",), + "{bla} from cache", + ), + ( + (), + { + "translation_domain": "test", + "translation_key": "bla", + "translation_placeholders": {"bla": "Bla"}, + }, + ("bla",), + "Bla from cache", + ), + ], +) +async def test_home_assistant_error( + hass: HomeAssistant, + exception_args: tuple[Any,], + exception_kwargs: dict[str, Any], + args_base_class: tuple[Any], + message: str, +) -> None: + """Test edge cases with HomeAssistantError.""" + + with patch( + "homeassistant.helpers.translation.async_get_cached_translations", + return_value={"component.test.exceptions.bla.message": "{bla} from cache"}, + ): + with pytest.raises(HomeAssistantError) as exc: + raise HomeAssistantError(*exception_args, **exception_kwargs) + assert exc.value.args == args_base_class + assert str(exc.value) == message + # Get string of exception again from the cache + assert str(exc.value) == message