Simplify error handling in otbr async_setup_entry (#98395)

* Simplify error handling in otbr async_setup_entry

* Create issue if the OTBR does not support border agent ID

* Update test

* Improve test coverage

* Catch the right exception

homeassistant/components/otbr/__init__.py

@@ -2,7 +2,6 @@
 from __future__ import annotations
 
 import asyncio
-import contextlib
 
 import aiohttp
 import python_otbr_api
@@ -11,7 +10,7 @@ from homeassistant.components.thread import async_add_dataset
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady, HomeAssistantError
-from homeassistant.helpers import config_validation as cv
+from homeassistant.helpers import config_validation as cv, issue_registry as ir
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.typing import ConfigType
 
@@ -37,6 +36,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
     otbrdata = OTBRData(entry.data["url"], api, entry.entry_id)
     try:
+        border_agent_id = await otbrdata.get_border_agent_id()
         dataset_tlvs = await otbrdata.get_active_dataset_tlvs()
     except (
         HomeAssistantError,
@@ -44,20 +44,24 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         asyncio.TimeoutError,
     ) as err:
         raise ConfigEntryNotReady("Unable to connect") from err
+    if border_agent_id is None:
+        ir.async_create_issue(
+            hass,
+            DOMAIN,
+            f"get_get_border_agent_id_unsupported_{otbrdata.entry_id}",
+            is_fixable=False,
+            is_persistent=False,
+            severity=ir.IssueSeverity.WARNING,
+            translation_key="get_get_border_agent_id_unsupported",
+        )
+        return False
     if dataset_tlvs:
-        border_agent_id: str | None = None
-        with contextlib.suppress(
-            HomeAssistantError, aiohttp.ClientError, asyncio.TimeoutError
-        ):
-            border_agent_bytes = await otbrdata.get_border_agent_id()
-            if border_agent_bytes:
-                border_agent_id = border_agent_bytes.hex()
         await update_issues(hass, otbrdata, dataset_tlvs)
         await async_add_dataset(
             hass,
             DOMAIN,
             dataset_tlvs.hex(),
-            preferred_border_agent_id=border_agent_id,
+            preferred_border_agent_id=border_agent_id.hex(),
         )
 
     entry.async_on_unload(entry.add_update_listener(async_reload_entry))

homeassistant/components/otbr/strings.json

@@ -16,6 +16,10 @@
     }
   },
   "issues": {
+    "get_get_border_agent_id_unsupported": {
+      "title": "The OTBR does not support border agent ID",
+      "description": "Your OTBR does not support border agent ID.\n\nTo fix this issue, update the OTBR to the latest version and restart Home Assistant.\nTo update the OTBR, update the Open Thread Border Router or Silicon Labs Multiprotocol add-on if you use the OTBR from the add-on, otherwise update your self managed OTBR."
+    },
     "insecure_thread_network": {
       "title": "Insecure Thread network settings detected",
       "description": "Your Thread network is using a default network key or pass phrase.\n\nThis is a security risk, please create a new Thread network."

homeassistant/components/otbr/util.py

@@ -83,9 +83,12 @@ class OTBRData:
             await self.delete_active_dataset()
 
     @_handle_otbr_error
-    async def get_border_agent_id(self) -> bytes:
+    async def get_border_agent_id(self) -> bytes | None:
-        """Get the border agent ID."""
+        """Get the border agent ID or None if not supported by the router."""
-        return await self.api.get_border_agent_id()
+        try:
+            return await self.api.get_border_agent_id()
+        except python_otbr_api.GetBorderAgentIdNotSupportedError:
+            return None
 
     @_handle_otbr_error
     async def set_enabled(self, enabled: bool) -> None:

tests/components/otbr/test_init.py

@@ -89,12 +89,16 @@ async def test_import_share_radio_channel_collision(
     config_entry.add_to_hass(hass)
     with patch(
         "python_otbr_api.OTBR.get_active_dataset_tlvs", return_value=DATASET_CH16
+    ), patch(
+        "python_otbr_api.OTBR.get_border_agent_id", return_value=TEST_BORDER_AGENT_ID
     ), patch(
         "homeassistant.components.thread.dataset_store.DatasetStore.async_add"
     ) as mock_add:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
 
-    mock_add.assert_called_once_with(otbr.DOMAIN, DATASET_CH16.hex(), None)
+    mock_add.assert_called_once_with(
+        otbr.DOMAIN, DATASET_CH16.hex(), TEST_BORDER_AGENT_ID.hex()
+    )
     assert issue_registry.async_get_issue(
         domain=otbr.DOMAIN,
         issue_id=f"otbr_zha_channel_collision_{config_entry.entry_id}",
@@ -122,12 +126,16 @@ async def test_import_share_radio_no_channel_collision(
     config_entry.add_to_hass(hass)
     with patch(
         "python_otbr_api.OTBR.get_active_dataset_tlvs", return_value=dataset
+    ), patch(
+        "python_otbr_api.OTBR.get_border_agent_id", return_value=TEST_BORDER_AGENT_ID
     ), patch(
         "homeassistant.components.thread.dataset_store.DatasetStore.async_add"
     ) as mock_add:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
 
-    mock_add.assert_called_once_with(otbr.DOMAIN, dataset.hex(), None)
+    mock_add.assert_called_once_with(
+        otbr.DOMAIN, dataset.hex(), TEST_BORDER_AGENT_ID.hex()
+    )
     assert not issue_registry.async_get_issue(
         domain=otbr.DOMAIN,
         issue_id=f"otbr_zha_channel_collision_{config_entry.entry_id}",
@@ -153,12 +161,16 @@ async def test_import_insecure_dataset(hass: HomeAssistant, dataset: bytes) -> N
     config_entry.add_to_hass(hass)
     with patch(
         "python_otbr_api.OTBR.get_active_dataset_tlvs", return_value=dataset
+    ), patch(
+        "python_otbr_api.OTBR.get_border_agent_id", return_value=TEST_BORDER_AGENT_ID
     ), patch(
         "homeassistant.components.thread.dataset_store.DatasetStore.async_add"
     ) as mock_add:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
 
-    mock_add.assert_called_once_with(otbr.DOMAIN, dataset.hex(), None)
+    mock_add.assert_called_once_with(
+        otbr.DOMAIN, dataset.hex(), TEST_BORDER_AGENT_ID.hex()
+    )
     assert issue_registry.async_get_issue(
         domain=otbr.DOMAIN, issue_id=f"insecure_thread_network_{config_entry.entry_id}"
     )
@@ -186,6 +198,25 @@ async def test_config_entry_not_ready(hass: HomeAssistant, error) -> None:
         assert not await hass.config_entries.async_setup(config_entry.entry_id)
 
 
+async def test_border_agent_id_not_supported(hass: HomeAssistant) -> None:
+    """Test border router does not support border agent ID."""
+
+    config_entry = MockConfigEntry(
+        data=CONFIG_ENTRY_DATA_MULTIPAN,
+        domain=otbr.DOMAIN,
+        options={},
+        title="My OTBR",
+    )
+    config_entry.add_to_hass(hass)
+    with patch(
+        "python_otbr_api.OTBR.get_active_dataset_tlvs", return_value=DATASET_CH16
+    ), patch(
+        "python_otbr_api.OTBR.get_border_agent_id",
+        side_effect=python_otbr_api.GetBorderAgentIdNotSupportedError,
+    ):
+        assert not await hass.config_entries.async_setup(config_entry.entry_id)
+
+
 async def test_config_entry_update(hass: HomeAssistant) -> None:
     """Test update config entry settings."""
     config_entry = MockConfigEntry(
@@ -197,6 +228,7 @@ async def test_config_entry_update(hass: HomeAssistant) -> None:
     config_entry.add_to_hass(hass)
     mock_api = MagicMock()
     mock_api.get_active_dataset_tlvs = AsyncMock(return_value=None)
+    mock_api.get_border_agent_id = AsyncMock(return_value=TEST_BORDER_AGENT_ID)
     with patch("python_otbr_api.OTBR", return_value=mock_api) as mock_otrb_api:
         assert await hass.config_entries.async_setup(config_entry.entry_id)