From 33ce795695ebf6875a6ad9880a5314fb795363dd Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Thu, 23 Jan 2025 18:26:28 +0100 Subject: [PATCH] Improve error handling for incomfort gateway (#136317) --- .../components/incomfort/__init__.py | 13 ++-- .../components/incomfort/config_flow.py | 17 +---- .../components/incomfort/coordinator.py | 11 +-- .../components/incomfort/test_config_flow.py | 32 ++++---- tests/components/incomfort/test_init.py | 76 +++++++++++-------- 5 files changed, 74 insertions(+), 75 deletions(-) diff --git a/homeassistant/components/incomfort/__init__.py b/homeassistant/components/incomfort/__init__.py index 5a57f9f4198..909a4731e84 100644 --- a/homeassistant/components/incomfort/__init__.py +++ b/homeassistant/components/incomfort/__init__.py @@ -3,7 +3,7 @@ from __future__ import annotations from aiohttp import ClientResponseError -from incomfortclient import IncomfortError, InvalidHeaterList +from incomfortclient import InvalidGateway, InvalidHeaterList from homeassistant.config_entries import ConfigEntry from homeassistant.const import Platform @@ -35,12 +35,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: InComfortConfigEntry) -> await heater.update() except InvalidHeaterList as exc: raise NoHeaters from exc - except IncomfortError as exc: - if isinstance(exc.message, ClientResponseError): - if exc.message.status == 401: - raise ConfigEntryAuthFailed("Incorrect credentials") from exc - if exc.message.status == 404: - raise NotFound from exc + except InvalidGateway as exc: + raise ConfigEntryAuthFailed("Incorrect credentials") from exc + except ClientResponseError as exc: + if exc.status == 404: + raise NotFound from exc raise InConfortUnknownError from exc except TimeoutError as exc: raise InConfortTimeout from exc diff --git a/homeassistant/components/incomfort/config_flow.py b/homeassistant/components/incomfort/config_flow.py index 47db9b701bf..779b0e97777 100644 --- a/homeassistant/components/incomfort/config_flow.py +++ b/homeassistant/components/incomfort/config_flow.py @@ -5,8 +5,7 @@ from __future__ import annotations from collections.abc import Mapping from typing import Any -from aiohttp import ClientResponseError -from incomfortclient import IncomfortError, InvalidHeaterList +from incomfortclient import InvalidGateway, InvalidHeaterList import voluptuous as vol from homeassistant.config_entries import ( @@ -77,11 +76,6 @@ OPTIONS_SCHEMA = vol.Schema( } ) -ERROR_STATUS_MAPPING: dict[int, tuple[str, str]] = { - 401: (CONF_PASSWORD, "auth_error"), - 404: ("base", "not_found"), -} - async def async_try_connect_gateway( hass: HomeAssistant, config: dict[str, Any] @@ -89,15 +83,10 @@ async def async_try_connect_gateway( """Try to connect to the Lan2RF gateway.""" try: await async_connect_gateway(hass, config) + except InvalidGateway: + return {"base": "auth_error"} except InvalidHeaterList: return {"base": "no_heaters"} - except IncomfortError as exc: - if isinstance(exc.message, ClientResponseError): - scope, error = ERROR_STATUS_MAPPING.get( - exc.message.status, ("base", "unknown") - ) - return {scope: error} - return {"base": "unknown"} except TimeoutError: return {"base": "timeout_error"} except Exception: # noqa: BLE001 diff --git a/homeassistant/components/incomfort/coordinator.py b/homeassistant/components/incomfort/coordinator.py index d1370f613ad..3436d40298a 100644 --- a/homeassistant/components/incomfort/coordinator.py +++ b/homeassistant/components/incomfort/coordinator.py @@ -9,7 +9,7 @@ from aiohttp import ClientResponseError from incomfortclient import ( Gateway as InComfortGateway, Heater as InComfortHeater, - IncomfortError, + InvalidHeaterList, ) from homeassistant.const import CONF_HOST @@ -70,9 +70,10 @@ class InComfortDataCoordinator(DataUpdateCoordinator[InComfortData]): await heater.update() except TimeoutError as exc: raise UpdateFailed("Timeout error") from exc - except IncomfortError as exc: - if isinstance(exc.message, ClientResponseError): - if exc.message.status == 401: - raise ConfigEntryError("Incorrect credentials") from exc + except ClientResponseError as exc: + if exc.status == 401: + raise ConfigEntryError("Incorrect credentials") from exc + raise UpdateFailed(exc.message) from exc + except InvalidHeaterList as exc: raise UpdateFailed(exc.message) from exc return self.incomfort_data diff --git a/tests/components/incomfort/test_config_flow.py b/tests/components/incomfort/test_config_flow.py index e102595657f..e3579182b3d 100644 --- a/tests/components/incomfort/test_config_flow.py +++ b/tests/components/incomfort/test_config_flow.py @@ -4,7 +4,7 @@ from typing import Any from unittest.mock import AsyncMock, MagicMock, patch from aiohttp import ClientResponseError -from incomfortclient import IncomfortError, InvalidHeaterList +from incomfortclient import InvalidGateway, InvalidHeaterList import pytest from homeassistant.components.incomfort.const import DOMAIN @@ -81,24 +81,22 @@ async def test_entry_already_configured( ("exc", "error", "base"), [ ( - IncomfortError(ClientResponseError(None, None, status=401)), + InvalidGateway, "auth_error", - CONF_PASSWORD, - ), - ( - IncomfortError(ClientResponseError(None, None, status=404)), - "not_found", "base", ), ( - IncomfortError(ClientResponseError(None, None, status=500)), + InvalidHeaterList, + "no_heaters", + "base", + ), + ( + ClientResponseError(None, None, status=500), "unknown", "base", ), - (IncomfortError, "unknown", "base"), - (ValueError, "unknown", "base"), (TimeoutError, "timeout_error", "base"), - (InvalidHeaterList, "no_heaters", "base"), + (ValueError, "unknown", "base"), ], ) async def test_form_validation( @@ -243,7 +241,7 @@ async def test_dhcp_flow_wih_auth( with patch.object( mock_incomfort(), "heaters", - side_effect=IncomfortError(ClientResponseError(None, None, status=401)), + side_effect=InvalidGateway, ): result = await hass.config_entries.flow.async_configure( result["flow_id"], {CONF_HOST: "192.168.1.12"} @@ -251,7 +249,7 @@ async def test_dhcp_flow_wih_auth( assert result["type"] is FlowResultType.FORM assert result["step_id"] == "dhcp_auth" - assert result["errors"] == {CONF_PASSWORD: "auth_error"} + assert result["errors"] == {"base": "auth_error"} # Submit the form with added credentials result = await hass.config_entries.flow.async_configure( @@ -300,14 +298,14 @@ async def test_reauth_flow_failure( with patch.object( mock_incomfort(), "heaters", - side_effect=IncomfortError(ClientResponseError(None, None, status=401)), + side_effect=InvalidGateway, ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={CONF_PASSWORD: "incorrect-password"}, ) assert result["type"] is FlowResultType.FORM - assert result["errors"] == {CONF_PASSWORD: "auth_error"} + assert result["errors"] == {"base": "auth_error"} result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -352,14 +350,14 @@ async def test_reconfigure_flow_failure( with patch.object( mock_incomfort(), "heaters", - side_effect=IncomfortError(ClientResponseError(None, None, status=401)), + side_effect=InvalidGateway, ): result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input=MOCK_CONFIG | {CONF_PASSWORD: "wrong-password"}, ) assert result["type"] is FlowResultType.FORM - assert result["errors"] == {CONF_PASSWORD: "auth_error"} + assert result["errors"] == {"base": "auth_error"} result = await hass.config_entries.flow.async_configure( result["flow_id"], diff --git a/tests/components/incomfort/test_init.py b/tests/components/incomfort/test_init.py index f603c3ce27b..a9b3a8e4e3a 100644 --- a/tests/components/incomfort/test_init.py +++ b/tests/components/incomfort/test_init.py @@ -5,10 +5,9 @@ from unittest.mock import AsyncMock, MagicMock, patch from aiohttp import ClientResponseError, RequestInfo from freezegun.api import FrozenDateTimeFactory -from incomfortclient import IncomfortError +from incomfortclient import InvalidGateway, InvalidHeaterList import pytest -from homeassistant.components.incomfort import InvalidHeaterList from homeassistant.components.incomfort.coordinator import UPDATE_INTERVAL from homeassistant.config_entries import ConfigEntry, ConfigEntryState from homeassistant.const import STATE_UNAVAILABLE @@ -66,20 +65,27 @@ async def test_coordinator_updates( @pytest.mark.parametrize( "exc", [ - IncomfortError(ClientResponseError(None, None, status=401)), - IncomfortError( - ClientResponseError( - RequestInfo( - url="http://example.com", - method="GET", - headers=[], - real_url="http://example.com", - ), - None, - status=500, - ) + ClientResponseError( + RequestInfo( + url="http://example.com", + method="GET", + headers=[], + real_url="http://example.com", + ), + None, + status=401, + ), + InvalidHeaterList, + ClientResponseError( + RequestInfo( + url="http://example.com", + method="GET", + headers=[], + real_url="http://example.com", + ), + None, + status=500, ), - IncomfortError(ValueError("some_error")), TimeoutError, ], ) @@ -113,30 +119,36 @@ async def test_coordinator_update_fails( ("exc", "config_entry_state"), [ ( - IncomfortError(ClientResponseError(None, None, status=401)), - ConfigEntryState.SETUP_ERROR, - ), - ( - IncomfortError(ClientResponseError(None, None, status=404)), + InvalidGateway, ConfigEntryState.SETUP_ERROR, ), (InvalidHeaterList, ConfigEntryState.SETUP_RETRY), ( - IncomfortError( - ClientResponseError( - RequestInfo( - url="http://example.com", - method="GET", - headers=[], - real_url="http://example.com", - ), - None, - status=500, - ) + ClientResponseError( + RequestInfo( + url="http://example.com", + method="GET", + headers=[], + real_url="http://example.com", + ), + None, + status=404, + ), + ConfigEntryState.SETUP_ERROR, + ), + ( + ClientResponseError( + RequestInfo( + url="http://example.com", + method="GET", + headers=[], + real_url="http://example.com", + ), + None, + status=500, ), ConfigEntryState.SETUP_RETRY, ), - (IncomfortError(ValueError("some_error")), ConfigEntryState.SETUP_RETRY), (TimeoutError, ConfigEntryState.SETUP_RETRY), ], )