From 71af0fac1644821b9303815b680914b9eeca444d Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sun, 27 Dec 2020 20:30:51 -0800 Subject: [PATCH] Improve nest setup error handling (#44385) * Improve error handling user experience This is meant to make the nest integration quieter. Exceptions are handled with a single log error message. Co-authored-by: j-stienstra <65826735+j-stienstra@users.noreply.github.com> --- homeassistant/components/nest/__init__.py | 23 ++++- tests/components/nest/common.py | 4 +- tests/components/nest/test_config_flow_sdm.py | 2 +- tests/components/nest/test_init_sdm.py | 90 +++++++++++++++++++ 4 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/components/nest/test_init_sdm.py diff --git a/homeassistant/components/nest/__init__.py b/homeassistant/components/nest/__init__.py index ab235d7559a..e4be96cbf14 100644 --- a/homeassistant/components/nest/__init__.py +++ b/homeassistant/components/nest/__init__.py @@ -4,7 +4,11 @@ import asyncio import logging from google_nest_sdm.event import EventMessage -from google_nest_sdm.exceptions import AuthException, GoogleNestException +from google_nest_sdm.exceptions import ( + AuthException, + ConfigurationException, + GoogleNestException, +) from google_nest_sdm.google_nest_subscriber import GoogleNestSubscriber import voluptuous as vol @@ -43,6 +47,9 @@ _LOGGER = logging.getLogger(__name__) CONF_PROJECT_ID = "project_id" CONF_SUBSCRIBER_ID = "subscriber_id" DATA_NEST_CONFIG = "nest_config" +DATA_NEST_UNAVAILABLE = "nest_unavailable" + +NEST_SETUP_NOTIFICATION = "nest_setup" SENSOR_SCHEMA = vol.Schema( {vol.Optional(CONF_MONITORED_CONDITIONS): vol.All(cv.ensure_list)} @@ -173,18 +180,27 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): ) ) return False + except ConfigurationException as err: + _LOGGER.error("Configuration error: %s", err) + subscriber.stop_async() + return False except GoogleNestException as err: - _LOGGER.error("Subscriber error: %s", err) + if DATA_NEST_UNAVAILABLE not in hass.data[DOMAIN]: + _LOGGER.error("Subscriber error: %s", err) + hass.data[DOMAIN][DATA_NEST_UNAVAILABLE] = True subscriber.stop_async() raise ConfigEntryNotReady from err try: await subscriber.async_get_device_manager() except GoogleNestException as err: - _LOGGER.error("Device Manager error: %s", err) + if DATA_NEST_UNAVAILABLE not in hass.data[DOMAIN]: + _LOGGER.error("Device manager error: %s", err) + hass.data[DOMAIN][DATA_NEST_UNAVAILABLE] = True subscriber.stop_async() raise ConfigEntryNotReady from err + hass.data[DOMAIN].pop(DATA_NEST_UNAVAILABLE, None) hass.data[DOMAIN][DATA_SUBSCRIBER] = subscriber for component in PLATFORMS: @@ -213,5 +229,6 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry): ) if unload_ok: hass.data[DOMAIN].pop(DATA_SUBSCRIBER) + hass.data[DOMAIN].pop(DATA_NEST_UNAVAILABLE, None) return unload_ok diff --git a/tests/components/nest/common.py b/tests/components/nest/common.py index d7b78b98f8f..65a37563911 100644 --- a/tests/components/nest/common.py +++ b/tests/components/nest/common.py @@ -19,7 +19,7 @@ CONFIG = { "client_secret": "some-client-secret", # Required fields for using SDM API "project_id": "some-project-id", - "subscriber_id": "some-subscriber-id", + "subscriber_id": "projects/example/subscriptions/subscriber-id-9876", }, } @@ -65,7 +65,7 @@ class FakeSubscriber(GoogleNestSubscriber): """Capture the callback set by Home Assistant.""" self._callback = callback - async def start_async(self) -> DeviceManager: + async def start_async(self): """Return the fake device manager.""" return self._device_manager diff --git a/tests/components/nest/test_config_flow_sdm.py b/tests/components/nest/test_config_flow_sdm.py index e506f269d66..aad5621935e 100644 --- a/tests/components/nest/test_config_flow_sdm.py +++ b/tests/components/nest/test_config_flow_sdm.py @@ -14,7 +14,7 @@ from tests.async_mock import patch CLIENT_ID = "1234" CLIENT_SECRET = "5678" PROJECT_ID = "project-id-4321" -SUBSCRIBER_ID = "subscriber-id-9876" +SUBSCRIBER_ID = "projects/example/subscriptions/subscriber-id-9876" CONFIG = { DOMAIN: { diff --git a/tests/components/nest/test_init_sdm.py b/tests/components/nest/test_init_sdm.py new file mode 100644 index 00000000000..cb17f81d18a --- /dev/null +++ b/tests/components/nest/test_init_sdm.py @@ -0,0 +1,90 @@ +""" +Test for setup methods for the SDM API. + +The tests fake out the subscriber/devicemanager and simulate setup behavior +and failure modes. +""" + +import logging + +from google_nest_sdm.exceptions import GoogleNestException + +from homeassistant.components.nest import DOMAIN +from homeassistant.config_entries import ( + ENTRY_STATE_LOADED, + ENTRY_STATE_SETUP_ERROR, + ENTRY_STATE_SETUP_RETRY, +) +from homeassistant.setup import async_setup_component + +from .common import CONFIG, CONFIG_ENTRY_DATA, async_setup_sdm_platform + +from tests.async_mock import patch +from tests.common import MockConfigEntry + +PLATFORM = "sensor" + + +async def test_setup_success(hass, caplog): + """Test successful setup.""" + with caplog.at_level(logging.ERROR, logger="homeassistant.components.nest"): + await async_setup_sdm_platform(hass, PLATFORM) + assert not caplog.records + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ENTRY_STATE_LOADED + + +async def async_setup_sdm(hass, config=CONFIG): + """Prepare test setup.""" + MockConfigEntry(domain=DOMAIN, data=CONFIG_ENTRY_DATA).add_to_hass(hass) + with patch( + "homeassistant.helpers.config_entry_oauth2_flow.async_get_config_entry_implementation" + ): + await async_setup_component(hass, DOMAIN, config) + + +async def test_setup_configuration_failure(hass, caplog): + """Test configuration error.""" + config = CONFIG.copy() + config[DOMAIN]["subscriber_id"] = "invalid-subscriber-format" + + await async_setup_sdm(hass, config) + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ENTRY_STATE_SETUP_ERROR + + # This error comes from the python google-nest-sdm library, as a check added + # to prevent common misconfigurations (e.g. confusing topic and subscriber) + assert "Subscription misconfigured. Expected subscriber_id" in caplog.text + + +async def test_setup_susbcriber_failure(hass, caplog): + """Test configuration error.""" + with patch( + "homeassistant.components.nest.GoogleNestSubscriber.start_async", + side_effect=GoogleNestException(), + ), caplog.at_level(logging.ERROR, logger="homeassistant.components.nest"): + await async_setup_sdm(hass) + assert "Subscriber error:" in caplog.text + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ENTRY_STATE_SETUP_RETRY + + +async def test_setup_device_manager_failure(hass, caplog): + """Test configuration error.""" + with patch("homeassistant.components.nest.GoogleNestSubscriber.start_async"), patch( + "homeassistant.components.nest.GoogleNestSubscriber.async_get_device_manager", + side_effect=GoogleNestException(), + ), caplog.at_level(logging.ERROR, logger="homeassistant.components.nest"): + await async_setup_sdm(hass) + assert len(caplog.messages) == 1 + assert "Device manager error:" in caplog.text + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ENTRY_STATE_SETUP_RETRY