From 98a87147053be17156075d056dbf6586b6b53368 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 22 Feb 2024 15:38:46 -1000 Subject: [PATCH] Improve error reporting in tplink config flow (#111166) * Improve error reporting in tplink config flow * coverage, fixes --- .../components/tplink/config_flow.py | 39 +++++++++++++------ homeassistant/components/tplink/strings.json | 3 +- tests/components/tplink/test_config_flow.py | 15 ++++--- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/homeassistant/components/tplink/config_flow.py b/homeassistant/components/tplink/config_flow.py index 67f0285f21d..643748f175e 100644 --- a/homeassistant/components/tplink/config_flow.py +++ b/homeassistant/components/tplink/config_flow.py @@ -148,6 +148,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self._discovered_device = device return await self.async_step_discovery_confirm() + placeholders = self._async_make_placeholders_from_discovery() + if user_input: username = user_input[CONF_USERNAME] password = user_input[CONF_PASSWORD] @@ -156,17 +158,18 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): device = await self._async_try_connect( self._discovered_device, credentials ) - except AuthenticationException: + except AuthenticationException as ex: errors[CONF_PASSWORD] = "invalid_auth" - except SmartDeviceException: + placeholders["error"] = str(ex) + except SmartDeviceException as ex: errors["base"] = "cannot_connect" + placeholders["error"] = str(ex) else: self._discovered_device = device await set_credentials(self.hass, username, password) self.hass.async_create_task(self._async_reload_requires_auth_entries()) return self._async_create_entry_from_device(self._discovered_device) - placeholders = self._async_make_placeholders_from_discovery() self.context["title_placeholders"] = placeholders return self.async_show_form( step_id="discovery_auth_confirm", @@ -204,7 +207,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self, user_input: dict[str, Any] | None = None ) -> FlowResult: """Handle the initial step.""" - errors = {} + errors: dict[str, str] = {} + placeholders: dict[str, str] = {} + if user_input is not None: if not (host := user_input[CONF_HOST]): return await self.async_step_pick_device() @@ -217,8 +222,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): ) except AuthenticationException: return await self.async_step_user_auth_confirm() - except SmartDeviceException: + except SmartDeviceException as ex: errors["base"] = "cannot_connect" + placeholders["error"] = str(ex) else: return self._async_create_entry_from_device(device) @@ -226,14 +232,17 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): step_id="user", data_schema=vol.Schema({vol.Optional(CONF_HOST, default=""): str}), errors=errors, + description_placeholders=placeholders, ) async def async_step_user_auth_confirm( self, user_input: dict[str, Any] | None = None ) -> FlowResult: """Dialog that informs the user that auth is required.""" - errors = {} + errors: dict[str, str] = {} host = self.context[CONF_HOST] + placeholders: dict[str, str] = {CONF_HOST: host} + assert self._discovered_device is not None if user_input: username = user_input[CONF_USERNAME] @@ -243,10 +252,12 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): device = await self._async_try_connect( self._discovered_device, credentials ) - except AuthenticationException: + except AuthenticationException as ex: errors[CONF_PASSWORD] = "invalid_auth" - except SmartDeviceException: + placeholders["error"] = str(ex) + except SmartDeviceException as ex: errors["base"] = "cannot_connect" + placeholders["error"] = str(ex) else: await set_credentials(self.hass, username, password) self.hass.async_create_task(self._async_reload_requires_auth_entries()) @@ -256,7 +267,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): step_id="user_auth_confirm", data_schema=STEP_AUTH_DATA_SCHEMA, errors=errors, - description_placeholders={CONF_HOST: host}, + description_placeholders=placeholders, ) async def async_step_pick_device( @@ -402,6 +413,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): ) -> FlowResult: """Dialog that informs the user that reauth is required.""" errors: dict[str, str] = {} + placeholders: dict[str, str] = {} reauth_entry = self.reauth_entry assert reauth_entry is not None entry_data = reauth_entry.data @@ -417,10 +429,12 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): credentials=credentials, raise_on_progress=True, ) - except AuthenticationException: + except AuthenticationException as ex: errors[CONF_PASSWORD] = "invalid_auth" - except SmartDeviceException: + placeholders["error"] = str(ex) + except SmartDeviceException as ex: errors["base"] = "cannot_connect" + placeholders["error"] = str(ex) else: await set_credentials(self.hass, username, password) self.hass.async_create_task(self._async_reload_requires_auth_entries()) @@ -430,7 +444,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): alias = entry_data.get(CONF_ALIAS) or "unknown" model = entry_data.get(CONF_MODEL) or "unknown" - placeholders = {"name": alias, "model": model, "host": host} + placeholders.update({"name": alias, "model": model, "host": host}) + self.context["title_placeholders"] = placeholders return self.async_show_form( step_id="reauth_confirm", diff --git a/homeassistant/components/tplink/strings.json b/homeassistant/components/tplink/strings.json index 4aa4a3856bd..c2e16063e4a 100644 --- a/homeassistant/components/tplink/strings.json +++ b/homeassistant/components/tplink/strings.json @@ -49,7 +49,8 @@ } }, "error": { - "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]" + "cannot_connect": "Connection error: {error}", + "invalid_auth": "Invalid authentication: {error}" }, "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", diff --git a/tests/components/tplink/test_config_flow.py b/tests/components/tplink/test_config_flow.py index b1937598e38..6163afac564 100644 --- a/tests/components/tplink/test_config_flow.py +++ b/tests/components/tplink/test_config_flow.py @@ -151,8 +151,8 @@ async def test_discovery_auth( @pytest.mark.parametrize( ("error_type", "errors_msg", "error_placement"), [ - (AuthenticationException, "invalid_auth", CONF_PASSWORD), - (SmartDeviceException, "cannot_connect", "base"), + (AuthenticationException("auth_error_details"), "invalid_auth", CONF_PASSWORD), + (SmartDeviceException("smart_device_error_details"), "cannot_connect", "base"), ], ids=["invalid-auth", "unknown-error"], ) @@ -195,6 +195,7 @@ async def test_discovery_auth_errors( assert result2["type"] is FlowResultType.FORM assert result2["errors"] == {error_placement: errors_msg} + assert result2["description_placeholders"]["error"] == str(error_type) await hass.async_block_till_done() @@ -524,8 +525,8 @@ async def test_manual_auth( @pytest.mark.parametrize( ("error_type", "errors_msg", "error_placement"), [ - (AuthenticationException, "invalid_auth", CONF_PASSWORD), - (SmartDeviceException, "cannot_connect", "base"), + (AuthenticationException("auth_error_details"), "invalid_auth", CONF_PASSWORD), + (SmartDeviceException("smart_device_error_details"), "cannot_connect", "base"), ], ids=["invalid-auth", "unknown-error"], ) @@ -570,6 +571,7 @@ async def test_manual_auth_errors( assert result3["type"] is FlowResultType.FORM assert result3["step_id"] == "user_auth_confirm" assert result3["errors"] == {error_placement: errors_msg} + assert result3["description_placeholders"]["error"] == str(error_type) mock_connect["connect"].side_effect = default_connect_side_effect result4 = await hass.config_entries.flow.async_configure( @@ -962,8 +964,8 @@ async def test_reauth_no_update_if_config_and_ip_the_same( @pytest.mark.parametrize( ("error_type", "errors_msg", "error_placement"), [ - (AuthenticationException, "invalid_auth", CONF_PASSWORD), - (SmartDeviceException, "cannot_connect", "base"), + (AuthenticationException("auth_error_details"), "invalid_auth", CONF_PASSWORD), + (SmartDeviceException("smart_device_error_details"), "cannot_connect", "base"), ], ids=["invalid-auth", "unknown-error"], ) @@ -1002,6 +1004,7 @@ async def test_reauth_errors( mock_discovery["mock_device"].update.assert_called_once_with() assert result2["type"] is FlowResultType.FORM assert result2["errors"] == {error_placement: errors_msg} + assert result2["description_placeholders"]["error"] == str(error_type) mock_discovery["discover_single"].reset_mock() mock_discovery["mock_device"].update.reset_mock(side_effect=True)