Improve Monzo webhook retry logging (#179081)

homeassistant/components/monzo/webhook.py

@@ -70,6 +70,7 @@ class MonzoWebhookManager:
         self._active = True
         self._register_lock = asyncio.Lock()
         self._retry_cancel: CALLBACK_TYPE | None = None
+        self._retrying = False
         self._webhook_url: str | None = None
 
     async def async_setup(self) -> None:
@@ -120,7 +121,7 @@ class MonzoWebhookManager:
                             self.hass, self.entry.data[CONF_WEBHOOK_ID]
                         )
                     except cloud.CloudNotAvailable:
-                        self._schedule_retry()
+                        self._schedule_retry("Unable to create Monzo cloud webhook")
                         return
                     self.hass.config_entries.async_update_entry(
                         self.entry,
@@ -152,11 +153,11 @@ class MonzoWebhookManager:
                 return
             except (ClientError, InvalidMonzoAPIResponseError, TimeoutError) as err:
                 await self._async_rollback_remote_webhooks(registered_webhook_ids)
-                _LOGGER.warning("Unable to register Monzo webhooks: %s", err)
-                self._schedule_retry()
+                self._schedule_retry("Unable to register Monzo webhooks", err)
                 return
 
             self._cancel_retry()
+            self._log_retry_success()
             self._webhook_url = webhook_url
             if self.entry.data.get(CONF_WEBHOOK_URL) != webhook_url:
                 self.hass.config_entries.async_update_entry(
@@ -216,6 +217,8 @@ class MonzoWebhookManager:
     async def _async_remove_previous_remote_webhooks(self) -> None:
         """Remove remote webhooks when no callback URL is available."""
         if (previous_url := self.entry.data.get(CONF_WEBHOOK_URL)) is None:
+            self._cancel_retry()
+            self._retrying = False
             return
 
         try:
@@ -228,10 +231,11 @@ class MonzoWebhookManager:
             self.entry.async_start_reauth(self.hass)
             return
         except (ClientError, InvalidMonzoAPIResponseError, TimeoutError) as err:
-            _LOGGER.warning("Unable to remove obsolete Monzo webhooks: %s", err)
-            self._schedule_retry()
+            self._schedule_retry("Unable to remove obsolete Monzo webhooks", err)
             return
 
+        self._cancel_retry()
+        self._log_retry_success()
         self._webhook_url = None
         data = dict(self.entry.data)
         data.pop(CONF_WEBHOOK_URL, None)
@@ -329,14 +333,31 @@ class MonzoWebhookManager:
             name,
         )
 
-    def _schedule_retry(self) -> None:
+    def _schedule_retry(self, message: str, err: Exception | None = None) -> None:
         """Schedule another remote webhook registration attempt."""
         if self._retry_cancel is not None:
             return
+        if not self._retrying:
+            if err is None:
+                _LOGGER.info("%s; retrying in %s seconds", message, WEBHOOK_RETRY_DELAY)
+            else:
+                _LOGGER.info(
+                    "%s: %s; retrying in %s seconds",
+                    message,
+                    err,
+                    WEBHOOK_RETRY_DELAY,
+                )
+            self._retrying = True
         self._retry_cancel = async_call_later(
             self.hass, WEBHOOK_RETRY_DELAY, self._async_retry
         )
 
+    def _log_retry_success(self) -> None:
+        """Log when remote webhook management recovers."""
+        if self._retrying:
+            _LOGGER.info("Successfully updated Monzo webhooks after retrying")
+            self._retrying = False
+
     async def _async_retry(self, now: datetime) -> None:
         """Retry remote webhook registration."""
         self._retry_cancel = None

tests/components/monzo/test_webhook.py

@@ -2,6 +2,7 @@
 
 import asyncio
 from datetime import timedelta
+import logging
 from unittest.mock import AsyncMock, Mock, call, patch
 
 from aiohttp import ClientError
@@ -779,8 +780,10 @@ async def test_external_url_cleanup_failure_is_retried_once(
     monzo: AsyncMock,
     polling_config_entry: MockConfigEntry,
     freezer: FrozenDateTimeFactory,
+    caplog: pytest.LogCaptureFixture,
 ) -> None:
     """Test repeated URL updates share one remote cleanup retry."""
+    caplog.set_level(logging.INFO)
     await setup_integration(hass, polling_config_entry)
     monzo.user_account.list_account_webhooks.side_effect = [
         InvalidMonzoAPIResponseError(),
@@ -808,6 +811,8 @@ async def test_external_url_cleanup_failure_is_retried_once(
         call("old-current"),
         call("old-flex"),
     ]
+    assert caplog.text.count("Unable to remove obsolete Monzo webhooks") == 1
+    assert caplog.text.count("Successfully updated Monzo webhooks after retrying") == 1
 
 
 async def test_no_external_url_skips_remote_registration(
@@ -831,8 +836,10 @@ async def test_cloudhook_creation_failure_is_retried(
     monzo: AsyncMock,
     polling_config_entry: MockConfigEntry,
     freezer: FrozenDateTimeFactory,
+    caplog: pytest.LogCaptureFixture,
 ) -> None:
     """Test unavailable cloud connection schedules webhook registration retry."""
+    caplog.set_level(logging.INFO)
     with (
         patch.object(cloud, "async_active_subscription", return_value=True),
         patch.object(cloud, "async_is_connected", return_value=True),
@@ -853,6 +860,47 @@ async def test_cloudhook_creation_failure_is_retried(
         call("acc_curr", CLOUDHOOK_URL),
         call("acc_flex", CLOUDHOOK_URL),
     ]
+    assert "Unable to create Monzo cloud webhook; retrying in 60 seconds" in caplog.text
+    assert "Successfully updated Monzo webhooks after retrying" in caplog.text
+
+
+async def test_retry_is_cancelled_when_callback_url_becomes_unavailable(
+    hass: HomeAssistant,
+    monzo: AsyncMock,
+    polling_config_entry: MockConfigEntry,
+    freezer: FrozenDateTimeFactory,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Test a retry is cancelled when there is no callback or previous URL."""
+    caplog.set_level(logging.INFO)
+    with (
+        patch.object(cloud, "async_active_subscription", return_value=True),
+        patch.object(cloud, "async_is_connected", return_value=True),
+        patch.object(
+            cloud,
+            "async_get_or_create_cloudhook",
+            side_effect=cloud.CloudNotAvailable,
+        ),
+    ):
+        await setup_integration(hass, polling_config_entry)
+
+    with (
+        patch.object(cloud, "async_active_subscription", return_value=False),
+        patch(
+            "homeassistant.components.monzo.webhook.webhook.async_generate_url",
+            side_effect=NoURLAvailableError,
+        ) as generate_url,
+    ):
+        await hass.config.async_update(external_url=None)
+        await hass.async_block_till_done()
+
+        freezer.tick(timedelta(seconds=WEBHOOK_RETRY_DELAY))
+        async_fire_time_changed(hass)
+        await hass.async_block_till_done()
+
+    generate_url.assert_called_once()
+    monzo.user_account.list_account_webhooks.assert_not_awaited()
+    assert "Successfully updated Monzo webhooks after retrying" not in caplog.text
 
 
 async def test_registration_auth_failure_starts_reauthentication(
@@ -902,9 +950,12 @@ async def test_registration_failure_is_retried(
     monzo: AsyncMock,
     polling_config_entry: MockConfigEntry,
     freezer: FrozenDateTimeFactory,
+    caplog: pytest.LogCaptureFixture,
 ) -> None:
     """Test a transient invalid response schedules another registration attempt."""
+    caplog.set_level(logging.INFO)
     monzo.user_account.list_account_webhooks.side_effect = [
+        InvalidMonzoAPIResponseError(),
         InvalidMonzoAPIResponseError(),
         [],
         [],
@@ -917,4 +968,10 @@ async def test_registration_failure_is_retried(
     async_fire_time_changed(hass)
     await hass.async_block_till_done()
 
+    freezer.tick(timedelta(seconds=WEBHOOK_RETRY_DELAY))
+    async_fire_time_changed(hass)
+    await hass.async_block_till_done()
+
     assert monzo.user_account.register_webhook.await_count == 2
+    assert caplog.text.count("Unable to register Monzo webhooks") == 1
+    assert caplog.text.count("Successfully updated Monzo webhooks after retrying") == 1