Velbus make sure the services throw exceptions (#159583)

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

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)
 

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": {

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,
+        )