Abort Hue config flow if bridge can not be reached (#88893)

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
Co-authored-by: Franck Nijhof <git@frenck.dev>

homeassistant/components/hue/config_flow.py

@@ -78,7 +78,13 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
             bridge = await discover_bridge(
                 host, websession=aiohttp_client.async_get_clientsession(self.hass)
             )
-        except aiohttp.ClientError:
+        except aiohttp.ClientError as err:
+            LOGGER.warning(
+                "Error while attempting to retrieve discovery information, "
+                "is there a bridge alive on IP %s ?",
+                host,
+                exc_info=err,
+            )
             return None
         if bridge_id is not None:
             bridge_id = normalize_bridge_id(bridge_id)
@@ -147,7 +153,9 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
             )
 
         self._async_abort_entries_match({"host": user_input["host"]})
-        self.bridge = await self._get_bridge(user_input[CONF_HOST])
+        if (bridge := await self._get_bridge(user_input[CONF_HOST])) is None:
+            return self.async_abort(reason="cannot_connect")
+        self.bridge = bridge
         return await self.async_step_link()
 
     async def async_step_link(
@@ -224,9 +232,12 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         )
 
         # we need to query the other capabilities too
-        self.bridge = await self._get_bridge(
+        bridge = await self._get_bridge(
             discovery_info.host, discovery_info.properties["bridgeid"]
         )
+        if bridge is None:
+            return self.async_abort(reason="cannot_connect")
+        self.bridge = bridge
         return await self.async_step_link()
 
     async def async_step_homekit(
@@ -238,7 +249,10 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         as the unique identifier. Therefore, this method uses discovery without
         a unique ID.
         """
-        self.bridge = await self._get_bridge(discovery_info.host)
+        bridge = await self._get_bridge(discovery_info.host)
+        if bridge is None:
+            return self.async_abort(reason="cannot_connect")
+        self.bridge = bridge
         await self._async_handle_discovery_without_unique_id()
         return await self.async_step_link()
 
@@ -254,7 +268,10 @@ class HueFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         # Check if host exists, abort if so.
         self._async_abort_entries_match({"host": import_info["host"]})
 
-        self.bridge = await self._get_bridge(import_info["host"])
+        bridge = await self._get_bridge(import_info["host"])
+        if bridge is None:
+            return self.async_abort(reason="cannot_connect")
+        self.bridge = bridge
         return await self.async_step_link()
 
 

tests/components/hue/test_config_flow.py

@@ -15,7 +15,7 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers import device_registry as dr
 
 from tests.common import MockConfigEntry
-from tests.test_util.aiohttp import AiohttpClientMocker
+from tests.test_util.aiohttp import AiohttpClientMocker, ClientError
 
 
 @pytest.fixture(name="hue_setup", autouse=True)
@@ -645,3 +645,76 @@ async def test_bridge_zeroconf_ipv6(hass: HomeAssistant) -> None:
 
     assert result["type"] == "abort"
     assert result["reason"] == "invalid_host"
+
+
+async def test_bridge_connection_failed(
+    hass: HomeAssistant,
+    aioclient_mock: AiohttpClientMocker,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Test that connection errors to the bridge are handled."""
+    create_mock_api_discovery(aioclient_mock, [])
+
+    with patch(
+        "homeassistant.components.hue.config_flow.discover_bridge",
+        side_effect=ClientError,
+    ):
+        result = await hass.config_entries.flow.async_init(
+            const.DOMAIN, context={"source": config_entries.SOURCE_USER}
+        )
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"], user_input={"host": "blah"}
+        )
+
+        # a warning message should have been logged that the bridge could not be reached
+        assert "Error while attempting to retrieve discovery information" in caplog.text
+
+        assert result["type"] == "abort"
+        assert result["reason"] == "cannot_connect"
+
+        # test again with zeroconf discovered wrong bridge IP
+        result = await hass.config_entries.flow.async_init(
+            const.DOMAIN,
+            context={"source": config_entries.SOURCE_ZEROCONF},
+            data=zeroconf.ZeroconfServiceInfo(
+                host="blah",
+                addresses=["1.2.3.4"],
+                port=443,
+                hostname="Philips-hue.local",
+                type="_hue._tcp.local.",
+                name="Philips Hue - ABCABC._hue._tcp.local.",
+                properties={
+                    "_raw": {"bridgeid": b"ecb5fafffeabcabc", "modelid": b"BSB002"},
+                    "bridgeid": "ecb5fafffeabcabc",
+                    "modelid": "BSB002",
+                },
+            ),
+        )
+        assert result["type"] == "abort"
+        assert result["reason"] == "cannot_connect"
+
+        # test again with homekit discovered wrong bridge IP
+        result = await hass.config_entries.flow.async_init(
+            const.DOMAIN,
+            context={"source": config_entries.SOURCE_HOMEKIT},
+            data=zeroconf.ZeroconfServiceInfo(
+                host="0.0.0.0",
+                addresses=["0.0.0.0"],
+                hostname="mock_hostname",
+                name="mock_name",
+                port=None,
+                properties={zeroconf.ATTR_PROPERTIES_ID: "aa:bb:cc:dd:ee:ff"},
+                type="mock_type",
+            ),
+        )
+        assert result["type"] == "abort"
+        assert result["reason"] == "cannot_connect"
+
+        # repeat test with import flow
+        result = await hass.config_entries.flow.async_init(
+            const.DOMAIN,
+            context={"source": config_entries.SOURCE_IMPORT},
+            data={"host": "blah"},
+        )
+        assert result["type"] == "abort"
+        assert result["reason"] == "cannot_connect"