From 1b2e66930249df0893eba710b8f38fc5042bef2a Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Mon, 26 Feb 2024 11:04:55 +0100 Subject: [PATCH] Improve handling mqtt command template exceptions (#110499) * Improve handling mqtt command template exceptions * Fix test * Cleanup stale exception handler * Throw on topic template exception --- homeassistant/components/mqtt/__init__.py | 57 +++++---------- homeassistant/components/mqtt/models.py | 49 +++++++++++-- homeassistant/components/mqtt/strings.json | 6 ++ tests/components/mqtt/test_init.py | 81 +++++++++++++++------- 4 files changed, 127 insertions(+), 66 deletions(-) diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index 4eadc0e98b3..1412ad63e68 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -320,49 +320,30 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: qos: int = call.data[ATTR_QOS] retain: bool = call.data[ATTR_RETAIN] if msg_topic_template is not None: + rendered_topic: Any = MqttCommandTemplate( + template.Template(msg_topic_template), + hass=hass, + ).async_render() try: - rendered_topic: Any = template.Template( - msg_topic_template, hass - ).async_render(parse_result=False) msg_topic = valid_publish_topic(rendered_topic) - except TEMPLATE_ERRORS as exc: - _LOGGER.error( - ( - "Unable to publish: rendering topic template of %s " - "failed because %s" - ), - msg_topic_template, - exc, - ) - return except vol.Invalid as err: - _LOGGER.error( - ( - "Unable to publish: topic template '%s' produced an " - "invalid topic '%s' after rendering (%s)" - ), - msg_topic_template, - rendered_topic, - err, - ) - return + err_str = str(err) + raise ServiceValidationError( + f"Unable to publish: topic template '{msg_topic_template}' produced an " + f"invalid topic '{rendered_topic}' after rendering ({err_str})", + translation_domain=DOMAIN, + translation_key="invalid_publish_topic", + translation_placeholders={ + "error": err_str, + "topic": str(rendered_topic), + "topic_template": str(msg_topic_template), + }, + ) from err if payload_template is not None: - try: - payload = MqttCommandTemplate( - template.Template(payload_template), hass=hass - ).async_render() - except TEMPLATE_ERRORS as exc: - _LOGGER.error( - ( - "Unable to publish to %s: rendering payload template of " - "%s failed because %s" - ), - msg_topic, - payload_template, - exc, - ) - return + payload = MqttCommandTemplate( + template.Template(payload_template), hass=hass + ).async_render() if TYPE_CHECKING: assert msg_topic is not None diff --git a/homeassistant/components/mqtt/models.py b/homeassistant/components/mqtt/models.py index 8b1f21c2775..1295bfb8ff3 100644 --- a/homeassistant/components/mqtt/models.py +++ b/homeassistant/components/mqtt/models.py @@ -15,6 +15,7 @@ import voluptuous as vol from homeassistant.const import ATTR_ENTITY_ID, ATTR_NAME from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback +from homeassistant.exceptions import ServiceValidationError, TemplateError from homeassistant.helpers import template from homeassistant.helpers.entity import Entity from homeassistant.helpers.service_info.mqtt import ReceivePayloadType @@ -29,7 +30,7 @@ if TYPE_CHECKING: from .discovery import MQTTDiscoveryPayload from .tag import MQTTTagScanner -from .const import TEMPLATE_ERRORS +from .const import DOMAIN, TEMPLATE_ERRORS class PayloadSentinel(StrEnum): @@ -111,6 +112,38 @@ class MqttOriginInfo(TypedDict, total=False): support_url: str +class MqttCommandTemplateException(ServiceValidationError): + """Handle MqttCommandTemplate exceptions.""" + + def __init__( + self, + *args: object, + base_exception: Exception, + command_template: str, + value: PublishPayloadType, + entity_id: str | None = None, + ) -> None: + """Initialize exception.""" + super().__init__(base_exception, *args) + value_log = str(value) + self.translation_domain = DOMAIN + self.translation_key = "command_template_error" + self.translation_placeholders = { + "error": str(base_exception), + "entity_id": str(entity_id), + "command_template": command_template, + } + entity_id_log = "" if entity_id is None else f" for entity '{entity_id}'" + self._message = ( + f"{type(base_exception).__name__}: {base_exception} rendering template{entity_id_log}" + f", template: '{command_template}' and payload: {value_log}" + ) + + def __str__(self) -> str: + """Return exception message string.""" + return self._message + + class MqttCommandTemplate: """Class for rendering MQTT payload with command templates.""" @@ -177,9 +210,17 @@ class MqttCommandTemplate: values, self._command_template, ) - return _convert_outgoing_payload( - self._command_template.async_render(values, parse_result=False) - ) + try: + return _convert_outgoing_payload( + self._command_template.async_render(values, parse_result=False) + ) + except TemplateError as exc: + raise MqttCommandTemplateException( + base_exception=exc, + command_template=self._command_template.template, + value=value, + entity_id=self._entity.entity_id if self._entity is not None else None, + ) from exc class MqttValueTemplate: diff --git a/homeassistant/components/mqtt/strings.json b/homeassistant/components/mqtt/strings.json index 2c3a87a515b..4c37de8204c 100644 --- a/homeassistant/components/mqtt/strings.json +++ b/homeassistant/components/mqtt/strings.json @@ -246,9 +246,15 @@ } }, "exceptions": { + "command_template_error": { + "message": "Parsing template `{command_template}` for entity `{entity_id}` failed with error: {error}." + }, "invalid_platform_config": { "message": "Reloading YAML config for manually configured MQTT `{domain}` item failed. See logs for more details." }, + "invalid_publish_topic": { + "message": "Unable to publish: topic template `{topic_template}` produced an invalid topic `{topic}` after rendering ({error})" + }, "mqtt_not_setup_cannot_subscribe": { "message": "Cannot subscribe to topic '{topic}', make sure MQTT is set up correctly." }, diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index 95ac94751ad..9fe394bd797 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -16,7 +16,11 @@ from homeassistant.components import mqtt from homeassistant.components.mqtt import debug_info from homeassistant.components.mqtt.client import EnsureJobAfterCooldown from homeassistant.components.mqtt.mixins import MQTT_ENTITY_DEVICE_INFO_SCHEMA -from homeassistant.components.mqtt.models import MessageCallbackType, ReceiveMessage +from homeassistant.components.mqtt.models import ( + MessageCallbackType, + MqttCommandTemplateException, + ReceiveMessage, +) from homeassistant.config_entries import ConfigEntryDisabler, ConfigEntryState from homeassistant.const import ( ATTR_ASSUMED_STATE, @@ -30,7 +34,7 @@ from homeassistant.const import ( ) import homeassistant.core as ha from homeassistant.core import CoreState, HomeAssistant, callback -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers import device_registry as dr, entity_registry as er, template from homeassistant.helpers.entity import Entity from homeassistant.helpers.entity_platform import async_get_platforms @@ -369,6 +373,15 @@ async def test_command_template_variables( assert state and state.state == "milk" +async def test_command_template_fails(hass: HomeAssistant) -> None: + """Test the exception handling of an MQTT command template.""" + tpl = template.Template("{{ value * 2 }}") + cmd_tpl = mqtt.MqttCommandTemplate(tpl, hass=hass) + with pytest.raises(MqttCommandTemplateException) as exc: + cmd_tpl.async_render(None) + assert "unsupported operand type(s) for *: 'NoneType' and 'int'" in str(exc.value) + + async def test_value_template_value(hass: HomeAssistant) -> None: """Test the rendering of MQTT value template.""" @@ -497,14 +510,20 @@ async def test_service_call_with_invalid_topic_template_does_not_publish( ) -> None: """Test the service call with a problematic topic template.""" mqtt_mock = await mqtt_mock_entry() - await hass.services.async_call( - mqtt.DOMAIN, - mqtt.SERVICE_PUBLISH, - { - mqtt.ATTR_TOPIC_TEMPLATE: "test/{{ 1 | no_such_filter }}", - mqtt.ATTR_PAYLOAD: "payload", - }, - blocking=True, + with pytest.raises(MqttCommandTemplateException) as exc: + await hass.services.async_call( + mqtt.DOMAIN, + mqtt.SERVICE_PUBLISH, + { + mqtt.ATTR_TOPIC_TEMPLATE: "test/{{ 1 | no_such_filter }}", + mqtt.ATTR_PAYLOAD: "payload", + }, + blocking=True, + ) + assert str(exc.value) == ( + "TemplateError: TemplateAssertionError: No filter named 'no_such_filter'. " + "rendering template, template: " + "'test/{{ 1 | no_such_filter }}' and payload: None" ) assert not mqtt_mock.async_publish.called @@ -538,14 +557,20 @@ async def test_service_call_with_template_topic_renders_invalid_topic( If a wildcard topic is rendered, then fail. """ mqtt_mock = await mqtt_mock_entry() - await hass.services.async_call( - mqtt.DOMAIN, - mqtt.SERVICE_PUBLISH, - { - mqtt.ATTR_TOPIC_TEMPLATE: "test/{{ '+' if True else 'topic' }}/topic", - mqtt.ATTR_PAYLOAD: "payload", - }, - blocking=True, + with pytest.raises(ServiceValidationError) as exc: + await hass.services.async_call( + mqtt.DOMAIN, + mqtt.SERVICE_PUBLISH, + { + mqtt.ATTR_TOPIC_TEMPLATE: "test/{{ '+' if True else 'topic' }}/topic", + mqtt.ATTR_PAYLOAD: "payload", + }, + blocking=True, + ) + assert str(exc.value) == ( + "Unable to publish: topic template 'test/{{ '+' if True else 'topic' }}/topic' " + "produced an invalid topic 'test/+/topic' after rendering " + "(Wildcards cannot be used in topic names)" ) assert not mqtt_mock.async_publish.called @@ -611,13 +636,21 @@ async def test_service_call_with_bad_template( ) -> None: """Test the service call with a bad template does not publish.""" mqtt_mock = await mqtt_mock_entry() - await hass.services.async_call( - mqtt.DOMAIN, - mqtt.SERVICE_PUBLISH, - {mqtt.ATTR_TOPIC: "test/topic", mqtt.ATTR_PAYLOAD_TEMPLATE: "{{ 1 | bad }}"}, - blocking=True, - ) + with pytest.raises(MqttCommandTemplateException) as exc: + await hass.services.async_call( + mqtt.DOMAIN, + mqtt.SERVICE_PUBLISH, + { + mqtt.ATTR_TOPIC: "test/topic", + mqtt.ATTR_PAYLOAD_TEMPLATE: "{{ 1 | bad }}", + }, + blocking=True, + ) assert not mqtt_mock.async_publish.called + assert str(exc.value) == ( + "TemplateError: TemplateAssertionError: No filter named 'bad'. " + "rendering template, template: '{{ 1 | bad }}' and payload: None" + ) async def test_service_call_with_payload_doesnt_render_template(