Fix: Better URL error handling

Additional testing found that during the calendar validation some errors were getting overwritten by other errors and thus clear messaging to the end user was not happening. In particular, if a calendar platform returned a non-200 code this would present as an issue of invalid calendar data (technically accurate). It should, however have presented as an unknown error. Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
tykeal · Feb 3, 2022 · 88dfba6 · 88dfba6
1 parent 82d2947
commit 88dfba6
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 17 deletions.
diff --git a/custom_components/rental_control/config_flow.py b/custom_components/rental_control/config_flow.py
@@ -245,20 +245,24 @@ async def _start_config_flow(
             # We require that the URL be an SSL URL
             if not re.search("^https://", user_input["url"]):
                 errors["base"] = "invalid_url"
-
-            session = async_get_clientsession(
-                cls.hass, verify_ssl=user_input["verify_ssl"]
-            )
-            with async_timeout.timeout(REQUEST_TIMEOUT):
-                resp = await session.get(user_input["url"])
-            if resp.status != 200:
-                _LOGGER.error(
-                    "%s returned %s - %s", user_input["url"], resp.status, resp.reason
+            else:
+                session = async_get_clientsession(
+                    cls.hass, verify_ssl=user_input["verify_ssl"]
                 )
-                errors["base"] = "unknown"
-            # We require text/calendar in the content-type header
-            if "text/calendar" not in resp.content_type:
-                errors["base"] = "bad_ics"
+                with async_timeout.timeout(REQUEST_TIMEOUT):
+                    resp = await session.get(user_input["url"])
+                if resp.status != 200:
+                    _LOGGER.error(
+                        "%s returned %s - %s",
+                        user_input["url"],
+                        resp.status,
+                        resp.reason,
+                    )
+                    errors["base"] = "unknown"
+                else:
+                    # We require text/calendar in the content-type header
+                    if "text/calendar" not in resp.content_type:
+                        errors["base"] = "bad_ics"
         except vol.Invalid as err:
             _LOGGER.exception(err.msg)
             errors["base"] = "invalid_url"

diff --git a/custom_components/rental_control/strings.json b/custom_components/rental_control/strings.json
@@ -5,7 +5,7 @@
       "bad_time": "Check-in/out time is invalid. Use 24 hour time",
       "invalid_url": "Only https URLs are supported",
       "same_name": "Name already in use",
-      "unknown": "Unexpected error"
+      "unknown": "Unexpected error, check logs for more details"
     },
     "step": {
       "user": {
@@ -32,7 +32,7 @@
       "bad_time": "Check-in/out time is invalid. Use 24 hour time",
       "invalid_url": "Only https URLs are supported",
       "same_name": "Name already in use",
-      "unknown": "Unexpected error"
+      "unknown": "Unexpected error, check logs for more details"
     },
     "step": {
       "init": {

diff --git a/custom_components/rental_control/translations/en.json b/custom_components/rental_control/translations/en.json
@@ -5,7 +5,7 @@
       "bad_time": "Check-in/out time is invalid. Use 24 hour time",
       "invalid_url": "Only https URLs are supported",
       "same_name": "Name already in use",
-      "unknown": "Unexpected error"
+      "unknown": "Unexpected error, check logs for more details"
     },
     "step": {
       "user": {
@@ -32,7 +32,7 @@
       "bad_time": "Check-in/out time is invalid. Use 24 hour time",
       "invalid_url": "Only https URLs are supported",
       "same_name": "Name already in use",
-      "unknown": "Unexpected error"
+      "unknown": "Unexpected error, check logs for more details"
     },
     "step": {
       "init": {