From dd782f1bba2249bcd399b2bfc5a9306541e7ea52 Mon Sep 17 00:00:00 2001 From: TimL Date: Fri, 14 Aug 2026 22:00:24 +1000 Subject: [PATCH] Fix swallowed exceptions in smlight action handlers (#179078) --- homeassistant/components/smlight/light.py | 13 +++-- homeassistant/components/smlight/strings.json | 6 +++ homeassistant/components/smlight/update.py | 50 ++++++++++--------- tests/components/smlight/test_light.py | 15 +++--- tests/components/smlight/test_update.py | 32 ++++++------ 5 files changed, 68 insertions(+), 48 deletions(-) diff --git a/homeassistant/components/smlight/light.py b/homeassistant/components/smlight/light.py index 0db3e3ee027e..858ccbfd8500 100644 --- a/homeassistant/components/smlight/light.py +++ b/homeassistant/components/smlight/light.py @@ -17,8 +17,10 @@ from homeassistant.components.light import ( LightEntityFeature, ) from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback +from .const import DOMAIN from .coordinator import SmConfigEntry, SmDataUpdateCoordinator from .entity import SmEntity @@ -128,10 +130,13 @@ class SmLightEntity(SmEntity, LightEntity): effect_name: str = kwargs[ATTR_EFFECT] try: idx = self.entity_description.effect_list.index(effect_name) - # pylint: disable-next=home-assistant-action-swallowed-exception - except ValueError: - _LOGGER.warning("Unknown effect: %s", effect_name) - return + except ValueError as err: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="unknown_effect", + translation_placeholders={"effect": effect_name}, + ) from err + payload.ultLedMode = AmbiEffect(idx) elif not self.is_on: payload.ultLedMode = AmbiEffect.WSULT_SOLID diff --git a/homeassistant/components/smlight/strings.json b/homeassistant/components/smlight/strings.json index 624568ddd248..df8f2c73adce 100644 --- a/homeassistant/components/smlight/strings.json +++ b/homeassistant/components/smlight/strings.json @@ -193,8 +193,14 @@ "play_tone_failed": { "message": "Failed to play tone on {device_name}: {error}." }, + "reboot_timeout": { + "message": "Timeout waiting for {hostname} to reboot after update" + }, "send_ir_code_failed": { "message": "Failed to send IR code: {error}." + }, + "unknown_effect": { + "message": "Unknown effect: {effect}." } }, "issues": { diff --git a/homeassistant/components/smlight/update.py b/homeassistant/components/smlight/update.py index 37b738ab47e8..ebd3f0955f0a 100644 --- a/homeassistant/components/smlight/update.py +++ b/homeassistant/components/smlight/update.py @@ -15,12 +15,12 @@ from homeassistant.components.update import ( UpdateEntityDescription, UpdateEntityFeature, ) -from homeassistant.const import EntityCategory +from homeassistant.const import CONF_HOST, EntityCategory from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback -from .const import DOMAIN, LOGGER, ZWAVE_TYPES +from .const import DOMAIN, ZWAVE_TYPES from .coordinator import SmConfigEntry, SmFirmwareUpdateCoordinator, SmFwData from .entity import SmEntity @@ -251,26 +251,30 @@ class SmUpdateEntity(SmEntity, UpdateEntity): self._attr_update_percentage = None self.register_callbacks() - await self.coordinator.client.fw_update(self._firmware, self.idx) - - # block until update finished event received - await self._finished_event.wait() - - # allow time for SLZB-06 to reboot before updating coordinator data try: - async with asyncio.timeout(180): - while ( - self.coordinator.in_progress - and self.installed_version != self._firmware.ver - ): - await self.coordinator.async_refresh() - await asyncio.sleep(1) - # pylint: disable-next=home-assistant-action-swallowed-exception - except TimeoutError: - LOGGER.warning( - "Timeout waiting for %s to reboot after update", - self.coordinator.data.info.hostname, - ) + await self.coordinator.client.fw_update(self._firmware, self.idx) - self.coordinator.in_progress = False - self._finished_event.clear() + # block until update finished event received + await self._finished_event.wait() + + # allow time for SLZB-06 to reboot before updating coordinator data + try: + async with asyncio.timeout(180): + while ( + self.coordinator.in_progress + and self.installed_version != self._firmware.ver + ): + await self.coordinator.async_refresh() + await asyncio.sleep(1) + except TimeoutError as err: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="reboot_timeout", + translation_placeholders={ + "hostname": self.coordinator.config_entry.data[CONF_HOST], + }, + ) from err + finally: + self._update_done() + self.coordinator.in_progress = False + self._finished_event.clear() diff --git a/tests/components/smlight/test_light.py b/tests/components/smlight/test_light.py index b19c291304b0..ac82bb65ffb1 100644 --- a/tests/components/smlight/test_light.py +++ b/tests/components/smlight/test_light.py @@ -239,18 +239,19 @@ async def test_light_invalid_effect( mock_config_entry: MockConfigEntry, mock_ultima_client: MagicMock, ) -> None: - """Test handling of invalid effect name is ignored.""" + """Test handling of invalid effect name raises a translated error.""" await setup_integration(hass, mock_config_entry) entity_id = "light.mock_title_ambilight" mock_ultima_client.actions.ambilight.reset_mock() - await hass.services.async_call( - LIGHT_DOMAIN, - SERVICE_TURN_ON, - {ATTR_ENTITY_ID: entity_id, ATTR_EFFECT: "InvalidEffect"}, - blocking=True, - ) + with pytest.raises(HomeAssistantError, match="Unknown effect: InvalidEffect"): + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: entity_id, ATTR_EFFECT: "InvalidEffect"}, + blocking=True, + ) mock_ultima_client.actions.ambilight.assert_not_called() diff --git a/tests/components/smlight/test_update.py b/tests/components/smlight/test_update.py index 1945584df820..84096fc78d76 100644 --- a/tests/components/smlight/test_update.py +++ b/tests/components/smlight/test_update.py @@ -1,5 +1,6 @@ """Tests for the SMLIGHT update platform.""" +import asyncio from datetime import timedelta from unittest.mock import MagicMock, patch @@ -273,15 +274,12 @@ async def test_update_firmware_failed( assert state.attributes[ATTR_UPDATE_PERCENTAGE] is None -@patch("homeassistant.components.smlight.const.LOGGER.warning") async def test_update_reboot_timeout( - mock_warning: MagicMock, hass: HomeAssistant, - freezer: FrozenDateTimeFactory, mock_config_entry: MockConfigEntry, mock_smlight_client: MagicMock, ) -> None: - """Test firmware updates.""" + """Test reboot timeout raises a translated user-facing error.""" await setup_integration(hass, mock_config_entry) entity_id = "update.mock_title_core_firmware" state = hass.states.get(entity_id) @@ -299,26 +297,32 @@ async def test_update_reboot_timeout( return_value=None, ), ): - await hass.services.async_call( - UPDATE_DOMAIN, - SERVICE_INSTALL, - {ATTR_ENTITY_ID: entity_id}, - blocking=False, + install_task = hass.async_create_task( + hass.services.async_call( + UPDATE_DOMAIN, + SERVICE_INSTALL, + {ATTR_ENTITY_ID: entity_id}, + blocking=True, + ) ) + await asyncio.sleep(0) assert len(mock_smlight_client.fw_update.mock_calls) == 1 event_function = get_mock_event_function( mock_smlight_client, SmEvents.FW_UPD_done ) - event_function(MOCK_FIRMWARE_DONE) - freezer.tick(timedelta(seconds=5)) - async_fire_time_changed(hass) - await hass.async_block_till_done() + with pytest.raises( + HomeAssistantError, + match=r"Timeout waiting for .* to reboot after update", + ): + await install_task - mock_warning.assert_called_once() + state = hass.states.get(entity_id) + assert state.attributes[ATTR_IN_PROGRESS] is False + assert state.attributes[ATTR_UPDATE_PERCENTAGE] is None @pytest.mark.parametrize(