From e6edc3b3c272d1552c593a7d173216babb82f004 Mon Sep 17 00:00:00 2001 From: Martin <32802427+mstu01@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:58:59 +0200 Subject: [PATCH] Fix swallowed exceptions in action handlers for Slack (#177049) --- homeassistant/components/slack/notify.py | 10 +- homeassistant/components/slack/strings.json | 5 + tests/components/slack/test_notify.py | 206 +++++++++++--------- 3 files changed, 129 insertions(+), 92 deletions(-) diff --git a/homeassistant/components/slack/notify.py b/homeassistant/components/slack/notify.py index efea07cb39ed..f44e0b3a0c86 100644 --- a/homeassistant/components/slack/notify.py +++ b/homeassistant/components/slack/notify.py @@ -20,6 +20,7 @@ from homeassistant.components.notify import ( ) from homeassistant.const import ATTR_ICON, CONF_PATH from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers import aiohttp_client, config_validation as cv, template from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType @@ -34,6 +35,7 @@ from .const import ( ATTR_USERNAME, CONF_DEFAULT_CHANNEL, DATA_CLIENT, + DOMAIN, SLACK_DATA, ) from .utils import upload_file_to_slack @@ -278,10 +280,12 @@ class SlackNotificationService(BaseNotificationService): try: DATA_SCHEMA(data) - # pylint: disable-next=home-assistant-action-swallowed-exception except vol.Invalid as err: - _LOGGER.error("Invalid message data: %s", err) - data = {} + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="invalid_message_data", + translation_placeholders={"error": str(err)}, + ) from err title = kwargs.get(ATTR_TITLE) targets = _async_sanitize_channel_names( diff --git a/homeassistant/components/slack/strings.json b/homeassistant/components/slack/strings.json index cefa5a412630..32cdfdfcd772 100644 --- a/homeassistant/components/slack/strings.json +++ b/homeassistant/components/slack/strings.json @@ -32,5 +32,10 @@ "name": "Do not disturb until" } } + }, + "exceptions": { + "invalid_message_data": { + "message": "Invalid message data: {error}" + } } } diff --git a/tests/components/slack/test_notify.py b/tests/components/slack/test_notify.py index 29c7e27d107e..f31619381aa8 100644 --- a/tests/components/slack/test_notify.py +++ b/tests/components/slack/test_notify.py @@ -1,110 +1,138 @@ """Test slack notifications.""" -from unittest.mock import AsyncMock, Mock +from unittest.mock import AsyncMock + +import pytest from homeassistant.components import notify from homeassistant.components.slack import DOMAIN -from homeassistant.components.slack.notify import ( - ATTR_THREAD_TS, - CONF_DEFAULT_CHANNEL, - SlackNotificationService, +from homeassistant.components.slack.notify import ATTR_THREAD_TS +from homeassistant.const import ATTR_ICON +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError + +from . import CONF_DATA, TEAM_ID, mock_connection + +from tests.common import MockConfigEntry +from tests.test_util.aiohttp import AiohttpClientMocker + +SERVICE_NAME = "test_team" + + +async def _async_setup_notify_service( + hass: HomeAssistant, + aioclient_mock: AiohttpClientMocker, + entry_data: dict[str, str], +) -> AsyncMock: + """Set up the slack integration and mock the message client.""" + entry = MockConfigEntry(domain=DOMAIN, data=entry_data, unique_id=TEAM_ID) + entry.add_to_hass(hass) + mock_connection(aioclient_mock) + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + assert hass.services.has_service(notify.DOMAIN, SERVICE_NAME) + mock_fn = AsyncMock() + entry.runtime_data.client.chat_postMessage = mock_fn + return mock_fn + + +@pytest.mark.parametrize( + ("entry_icon", "service_data", "expected_key", "expected_icon"), + [ + pytest.param( + ":robot_face:", + {notify.ATTR_MESSAGE: "test"}, + "icon_emoji", + ":robot_face:", + id="default_emoji", + ), + pytest.param( + "default_icon", + {notify.ATTR_MESSAGE: "test", notify.ATTR_DATA: {ATTR_ICON: ":new:"}}, + "icon_emoji", + ":new:", + id="emoji_overrides_default", + ), + pytest.param( + "https://example.com/hass.png", + {notify.ATTR_MESSAGE: "test"}, + "icon_url", + "https://example.com/hass.png", + id="default_icon_url", + ), + pytest.param( + "default_icon", + { + notify.ATTR_MESSAGE: "test", + notify.ATTR_DATA: {ATTR_ICON: "https://example.com/hass.png"}, + }, + "icon_url", + "https://example.com/hass.png", + id="icon_url_overrides_default", + ), + ], ) -from homeassistant.const import ATTR_ICON, CONF_API_KEY, CONF_NAME, CONF_PLATFORM - -from . import CONF_DATA - -SERVICE_NAME = f"notify_{DOMAIN}" - -DEFAULT_CONFIG = { - notify.DOMAIN: [ - { - CONF_PLATFORM: DOMAIN, - CONF_NAME: SERVICE_NAME, - CONF_API_KEY: "12345", - CONF_DEFAULT_CHANNEL: "channel", - } - ] -} - - -async def test_message_includes_default_emoji() -> None: - """Tests that default icon is used when no message icon is given.""" - mock_client = Mock() - mock_client.chat_postMessage = AsyncMock() - expected_icon = ":robot_face:" - service = SlackNotificationService( - None, mock_client, CONF_DATA | {ATTR_ICON: expected_icon} +async def test_message_icon( + hass: HomeAssistant, + aioclient_mock: AiohttpClientMocker, + entry_icon: str, + service_data: dict[str, str], + expected_key: str, + expected_icon: str, +) -> None: + """Test that the message icon comes from the entry data or the service data.""" + mock_fn = await _async_setup_notify_service( + hass, aioclient_mock, CONF_DATA | {ATTR_ICON: entry_icon} ) - await service.async_send_message("test") - - mock_fn = mock_client.chat_postMessage - mock_fn.assert_called_once() - _, kwargs = mock_fn.call_args - assert kwargs["icon_emoji"] == expected_icon - - -async def test_message_emoji_overrides_default() -> None: - """Tests that overriding the default icon emoji when sending a message works.""" - mock_client = Mock() - mock_client.chat_postMessage = AsyncMock() - service = SlackNotificationService( - None, mock_client, CONF_DATA | {ATTR_ICON: "default_icon"} + await hass.services.async_call( + notify.DOMAIN, SERVICE_NAME, service_data, blocking=True ) - expected_icon = ":new:" - await service.async_send_message("test", data={"icon": expected_icon}) - - mock_fn = mock_client.chat_postMessage mock_fn.assert_called_once() _, kwargs = mock_fn.call_args - assert kwargs["icon_emoji"] == expected_icon + assert kwargs[expected_key] == expected_icon -async def test_message_includes_default_icon_url() -> None: - """Tests that overriding the default icon url when sending a message works.""" - mock_client = Mock() - mock_client.chat_postMessage = AsyncMock() - expected_icon = "https://example.com/hass.png" - service = SlackNotificationService( - None, mock_client, CONF_DATA | {ATTR_ICON: expected_icon} - ) - - await service.async_send_message("test") - - mock_fn = mock_client.chat_postMessage - mock_fn.assert_called_once() - _, kwargs = mock_fn.call_args - assert kwargs["icon_url"] == expected_icon - - -async def test_message_icon_url_overrides_default() -> None: - """Tests that overriding the default icon url when sending a message works.""" - mock_client = Mock() - mock_client.chat_postMessage = AsyncMock() - service = SlackNotificationService( - None, mock_client, CONF_DATA | {ATTR_ICON: "default_icon"} - ) - - expected_icon = "https://example.com/hass.png" - await service.async_send_message("test", data={ATTR_ICON: expected_icon}) - - mock_fn = mock_client.chat_postMessage - mock_fn.assert_called_once() - _, kwargs = mock_fn.call_args - assert kwargs["icon_url"] == expected_icon - - -async def test_message_as_reply() -> None: +async def test_message_as_reply( + hass: HomeAssistant, aioclient_mock: AiohttpClientMocker +) -> None: """Tests that a message pointer will be passed to Slack if specified.""" - mock_client = Mock() - mock_client.chat_postMessage = AsyncMock() - service = SlackNotificationService(None, mock_client, CONF_DATA) + mock_fn = await _async_setup_notify_service(hass, aioclient_mock, CONF_DATA) expected_ts = "1624146685.064129" - await service.async_send_message("test", data={ATTR_THREAD_TS: expected_ts}) + await hass.services.async_call( + notify.DOMAIN, + SERVICE_NAME, + { + notify.ATTR_MESSAGE: "test", + notify.ATTR_DATA: {ATTR_THREAD_TS: expected_ts}, + }, + blocking=True, + ) - mock_fn = mock_client.chat_postMessage mock_fn.assert_called_once() _, kwargs = mock_fn.call_args assert kwargs["thread_ts"] == expected_ts + + +async def test_invalid_message_data( + hass: HomeAssistant, aioclient_mock: AiohttpClientMocker +) -> None: + """Tests that invalid message data raises an error and sends no message.""" + mock_fn = await _async_setup_notify_service(hass, aioclient_mock, CONF_DATA) + + with pytest.raises(ServiceValidationError) as exc_info: + await hass.services.async_call( + notify.DOMAIN, + SERVICE_NAME, + { + notify.ATTR_MESSAGE: "test", + notify.ATTR_DATA: {"not_a_valid_key": "value"}, + }, + blocking=True, + ) + + assert exc_info.value.translation_domain == DOMAIN + assert exc_info.value.translation_key == "invalid_message_data" + mock_fn.assert_not_called()