From 3e498d289b99c6780a013f9a4cd8dc2ff91fb7db Mon Sep 17 00:00:00 2001 From: Maikel Punie Date: Tue, 23 Dec 2025 13:39:16 +0100 Subject: [PATCH] Velbus make sure the services throw exceptions (#159583) --- .../components/velbus/quality_scale.yaml | 2 +- homeassistant/components/velbus/services.py | 52 +++++++++++++---- homeassistant/components/velbus/strings.json | 15 +++++ tests/components/velbus/test_services.py | 58 ++++++++++++++++++- 4 files changed, 113 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/velbus/quality_scale.yaml b/homeassistant/components/velbus/quality_scale.yaml index 829f48e6f52..83b8bce6c0c 100644 --- a/homeassistant/components/velbus/quality_scale.yaml +++ b/homeassistant/components/velbus/quality_scale.yaml @@ -23,7 +23,7 @@ rules: unique-config-entry: done # Silver - action-exceptions: todo + action-exceptions: done config-entry-unloading: done docs-configuration-parameters: todo docs-installation-parameters: todo diff --git a/homeassistant/components/velbus/services.py b/homeassistant/components/velbus/services.py index de1fb1b2fc8..773d3d742d0 100644 --- a/homeassistant/components/velbus/services.py +++ b/homeassistant/components/velbus/services.py @@ -2,7 +2,6 @@ from __future__ import annotations -from contextlib import suppress import os import shutil from typing import TYPE_CHECKING @@ -12,7 +11,7 @@ import voluptuous as vol from homeassistant.config_entries import ConfigEntryState from homeassistant.const import CONF_ADDRESS from homeassistant.core import HomeAssistant, ServiceCall, callback -from homeassistant.exceptions import ServiceValidationError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers import config_validation as cv, selector from homeassistant.helpers.storage import STORAGE_DIR @@ -36,8 +35,7 @@ def async_setup_services(hass: HomeAssistant) -> None: async def get_config_entry(call: ServiceCall) -> VelbusConfigEntry: """Get the config entry for this service call.""" - if CONF_CONFIG_ENTRY in call.data: - entry_id = call.data[CONF_CONFIG_ENTRY] + entry_id: str = call.data[CONF_CONFIG_ENTRY] if not (entry := hass.config_entries.async_get_entry(entry_id)): raise ServiceValidationError( translation_domain=DOMAIN, @@ -55,26 +53,52 @@ def async_setup_services(hass: HomeAssistant) -> None: async def scan(call: ServiceCall) -> None: """Handle a scan service call.""" entry = await get_config_entry(call) - await entry.runtime_data.controller.scan() + try: + await entry.runtime_data.controller.scan() + except OSError as exc: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="scan_failed", + translation_placeholders={"error": str(exc)}, + ) from exc async def syn_clock(call: ServiceCall) -> None: """Handle a sync clock service call.""" entry = await get_config_entry(call) - await entry.runtime_data.controller.sync_clock() + try: + await entry.runtime_data.controller.sync_clock() + except OSError as exc: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="sync_clock_failed", + translation_placeholders={"error": str(exc)}, + ) from exc async def set_memo_text(call: ServiceCall) -> None: """Handle Memo Text service call.""" entry = await get_config_entry(call) memo_text = call.data[CONF_MEMO_TEXT] - module = entry.runtime_data.controller.get_module(call.data[CONF_ADDRESS]) + address = call.data[CONF_ADDRESS] + module = entry.runtime_data.controller.get_module(address) if not module: - raise ServiceValidationError("Module not found") - await module.set_memo_text(memo_text) + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="module_not_found", + translation_placeholders={"address": str(address)}, + ) + try: + await module.set_memo_text(memo_text) + except OSError as exc: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="set_memo_text_failed", + translation_placeholders={"error": str(exc)}, + ) from exc async def clear_cache(call: ServiceCall) -> None: """Handle a clear cache service call.""" entry = await get_config_entry(call) - with suppress(FileNotFoundError): + try: if call.data.get(CONF_ADDRESS): await hass.async_add_executor_job( os.unlink, @@ -88,6 +112,14 @@ def async_setup_services(hass: HomeAssistant) -> None: shutil.rmtree, hass.config.path(STORAGE_DIR, f"velbuscache-{entry.entry_id}/"), ) + except FileNotFoundError: + pass # It's okay if the file doesn't exist + except OSError as exc: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="clear_cache_failed", + translation_placeholders={"error": str(exc)}, + ) from exc # call a scan to repopulate await scan(call) diff --git a/homeassistant/components/velbus/strings.json b/homeassistant/components/velbus/strings.json index cdbe7328955..ff51ad066e5 100644 --- a/homeassistant/components/velbus/strings.json +++ b/homeassistant/components/velbus/strings.json @@ -57,14 +57,29 @@ } }, "exceptions": { + "clear_cache_failed": { + "message": "Could not cleat the Velbus cache: {error}" + }, "integration_not_found": { "message": "Integration \"{target}\" not found in registry." }, "invalid_hvac_mode": { "message": "Climate mode {hvac_mode} is not supported." }, + "module_not_found": { + "message": "Module with address {address} not found." + }, "not_loaded": { "message": "{target} is not loaded." + }, + "scan_failed": { + "message": "Scan service: {error}." + }, + "set_memo_text_failed": { + "message": "Failed to set the memo text on the Velbus module: {error}." + }, + "sync_clock_failed": { + "message": "Failed to sync the Velbus clock: {error}." } }, "issues": { diff --git a/tests/components/velbus/test_services.py b/tests/components/velbus/test_services.py index bd59e4946ba..f7f480b9260 100644 --- a/tests/components/velbus/test_services.py +++ b/tests/components/velbus/test_services.py @@ -1,6 +1,6 @@ """Velbus services tests.""" -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, patch import pytest import voluptuous as vol @@ -16,7 +16,7 @@ from homeassistant.components.velbus.const import ( ) from homeassistant.const import CONF_ADDRESS from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ServiceValidationError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from . import init_integration @@ -64,6 +64,26 @@ async def test_global_services_with_config_entry( blocking=True, ) + # Test scan with OSError + config_entry.runtime_data.controller.scan.side_effect = OSError("Boom") + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + DOMAIN, + SERVICE_SCAN, + {CONF_CONFIG_ENTRY: config_entry.entry_id}, + blocking=True, + ) + + # Test sync_clock with OSError + config_entry.runtime_data.controller.sync_clock.side_effect = OSError("Boom") + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + DOMAIN, + SERVICE_SYNC, + {CONF_CONFIG_ENTRY: config_entry.entry_id}, + blocking=True, + ) + async def test_set_memo_text( hass: HomeAssistant, @@ -87,9 +107,26 @@ async def test_set_memo_text( 1 ).set_memo_text.assert_called_once_with("Test") + # Test with OSError + controller.return_value.get_module.return_value.set_memo_text.side_effect = OSError( + "Boom" + ) + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + DOMAIN, + SERVICE_SET_MEMO_TEXT, + { + CONF_CONFIG_ENTRY: config_entry.entry_id, + CONF_MEMO_TEXT: "Test", + CONF_ADDRESS: 2, + }, + blocking=True, + ) + controller.return_value.get_module.return_value.set_memo_text.side_effect = None + # Test with unfound module controller.return_value.get_module.return_value = None - with pytest.raises(ServiceValidationError, match="Module not found"): + with pytest.raises(ServiceValidationError, match="Module with address 2 not found"): await hass.services.async_call( DOMAIN, SERVICE_SET_MEMO_TEXT, @@ -124,3 +161,18 @@ async def test_clear_cache( blocking=True, ) assert config_entry.runtime_data.controller.scan.call_count == 2 + + # Test with OSError + with ( + patch("os.unlink", side_effect=OSError("Boom")), + pytest.raises(HomeAssistantError), + ): + await hass.services.async_call( + DOMAIN, + SERVICE_CLEAR_CACHE, + { + CONF_CONFIG_ENTRY: config_entry.entry_id, + CONF_ADDRESS: 2, + }, + blocking=True, + )