mirror of
https://github.com/home-assistant/core.git
synced 2026-04-02 08:26:41 +01:00
Add retry logic and resilience for Withings webhook subscription (#162189)
Co-authored-by: delize <4028612+delize@users.noreply.github.com> Co-authored-by: abmantis <amfcalt@gmail.com>
This commit is contained in:
@@ -16,7 +16,13 @@ from aiohttp import ClientError
|
||||
from aiohttp.hdrs import METH_POST
|
||||
from aiohttp.web import Request, Response
|
||||
from aiowithings import NotificationCategory, WithingsClient
|
||||
from aiowithings.exceptions import WithingsError
|
||||
from aiowithings.exceptions import (
|
||||
WithingsAuthenticationFailedError,
|
||||
WithingsError,
|
||||
WithingsInvalidParamsError,
|
||||
WithingsTooManyRequestsError,
|
||||
WithingsUnauthorizedError,
|
||||
)
|
||||
from aiowithings.util import to_enum
|
||||
from yarl import URL
|
||||
|
||||
@@ -36,7 +42,7 @@ from homeassistant.const import (
|
||||
EVENT_HOMEASSISTANT_STOP,
|
||||
Platform,
|
||||
)
|
||||
from homeassistant.core import HomeAssistant
|
||||
from homeassistant.core import CALLBACK_TYPE, HomeAssistant
|
||||
from homeassistant.exceptions import ConfigEntryNotReady
|
||||
from homeassistant.helpers.aiohttp_client import async_get_clientsession
|
||||
from homeassistant.helpers.config_entry_oauth2_flow import (
|
||||
@@ -62,6 +68,8 @@ PLATFORMS = [Platform.BINARY_SENSOR, Platform.CALENDAR, Platform.SENSOR]
|
||||
|
||||
SUBSCRIBE_DELAY = timedelta(seconds=5)
|
||||
UNSUBSCRIBE_DELAY = timedelta(seconds=1)
|
||||
WEBHOOK_REGISTER_DELAY = 1
|
||||
MAX_WEBHOOK_RETRY_INTERVAL = 1800
|
||||
CONF_CLOUDHOOK_URL = "cloudhook_url"
|
||||
type WithingsConfigEntry = ConfigEntry[WithingsData]
|
||||
|
||||
@@ -162,14 +170,18 @@ async def async_setup_entry(hass: HomeAssistant, entry: WithingsConfigEntry) ->
|
||||
if cloud.async_active_subscription(hass):
|
||||
if cloud.async_is_connected(hass):
|
||||
entry.async_on_unload(
|
||||
async_call_later(hass, 1, webhook_manager.register_webhook)
|
||||
async_call_later(
|
||||
hass, WEBHOOK_REGISTER_DELAY, webhook_manager.register_webhook
|
||||
)
|
||||
)
|
||||
entry.async_on_unload(
|
||||
cloud.async_listen_connection_change(hass, manage_cloudhook)
|
||||
)
|
||||
else:
|
||||
entry.async_on_unload(
|
||||
async_call_later(hass, 1, webhook_manager.register_webhook)
|
||||
async_call_later(
|
||||
hass, WEBHOOK_REGISTER_DELAY, webhook_manager.register_webhook
|
||||
)
|
||||
)
|
||||
|
||||
await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
|
||||
@@ -213,20 +225,36 @@ async def async_subscribe_webhooks(client: WithingsClient, webhook_url: str) ->
|
||||
class WithingsWebhookManager:
|
||||
"""Manager that manages the Withings webhooks."""
|
||||
|
||||
_webhooks_registered = False
|
||||
_webhook_url_invalid = False
|
||||
_register_lock = asyncio.Lock()
|
||||
|
||||
def __init__(self, hass: HomeAssistant, entry: WithingsConfigEntry) -> None:
|
||||
"""Initialize webhook manager."""
|
||||
self.hass = hass
|
||||
self.entry = entry
|
||||
self._subscribe_attempt: int = 0
|
||||
self._webhook_url: str | None = None
|
||||
self._webhooks_registered: bool = False
|
||||
self._webhook_url_invalid: bool = False
|
||||
self._ha_webhook_registered: bool = False
|
||||
self._cancel_retry: CALLBACK_TYPE | None = None
|
||||
self._register_lock = asyncio.Lock()
|
||||
|
||||
@property
|
||||
def withings_data(self) -> WithingsData:
|
||||
"""Return Withings data."""
|
||||
return self.entry.runtime_data
|
||||
|
||||
def _cancel_pending_retry(self) -> None:
|
||||
"""Cancel any pending retry."""
|
||||
if self._cancel_retry is not None:
|
||||
self._cancel_retry()
|
||||
self._cancel_retry = None
|
||||
|
||||
def _schedule_retry(self, delay: int) -> None:
|
||||
"""Schedule a webhook registration retry."""
|
||||
self._cancel_pending_retry()
|
||||
self._cancel_retry = async_call_later(
|
||||
self.hass, delay, self._async_subscribe_webhook
|
||||
)
|
||||
|
||||
async def unregister_webhook(
|
||||
self,
|
||||
_: Any,
|
||||
@@ -236,10 +264,14 @@ class WithingsWebhookManager:
|
||||
LOGGER.debug(
|
||||
"Unregister Withings webhook (%s)", self.entry.data[CONF_WEBHOOK_ID]
|
||||
)
|
||||
self._cancel_pending_retry()
|
||||
webhook_unregister(self.hass, self.entry.data[CONF_WEBHOOK_ID])
|
||||
self._ha_webhook_registered = False
|
||||
self._webhook_url = None
|
||||
for coordinator in self.withings_data.coordinators:
|
||||
coordinator.webhook_subscription_listener(False)
|
||||
self._webhooks_registered = False
|
||||
self._subscribe_attempt = 0
|
||||
try:
|
||||
await async_unsubscribe_webhooks(self.withings_data.client)
|
||||
except WithingsError as ex:
|
||||
@@ -253,6 +285,7 @@ class WithingsWebhookManager:
|
||||
async with self._register_lock:
|
||||
if self._webhooks_registered:
|
||||
return
|
||||
self._cancel_pending_retry()
|
||||
if cloud.async_active_subscription(self.hass):
|
||||
webhook_url = await _async_cloudhook_generate_url(self.hass, self.entry)
|
||||
else:
|
||||
@@ -277,30 +310,94 @@ class WithingsWebhookManager:
|
||||
self._webhook_url_invalid = True
|
||||
return
|
||||
|
||||
webhook_name = "Withings"
|
||||
if self.entry.title != DEFAULT_TITLE:
|
||||
webhook_name = f"{DEFAULT_TITLE} {self.entry.title}"
|
||||
if not self._ha_webhook_registered:
|
||||
webhook_name = "Withings"
|
||||
if self.entry.title != DEFAULT_TITLE:
|
||||
webhook_name = f"{DEFAULT_TITLE} {self.entry.title}"
|
||||
|
||||
webhook_register(
|
||||
self.hass,
|
||||
DOMAIN,
|
||||
webhook_name,
|
||||
self.entry.data[CONF_WEBHOOK_ID],
|
||||
get_webhook_handler(self.withings_data),
|
||||
allowed_methods=[METH_POST],
|
||||
)
|
||||
LOGGER.debug("Registered Withings webhook at hass: %s", webhook_url)
|
||||
|
||||
await async_subscribe_webhooks(self.withings_data.client, webhook_url)
|
||||
for coordinator in self.withings_data.coordinators:
|
||||
coordinator.webhook_subscription_listener(True)
|
||||
LOGGER.debug("Registered Withings webhook at Withings: %s", webhook_url)
|
||||
self.entry.async_on_unload(
|
||||
self.hass.bus.async_listen_once(
|
||||
EVENT_HOMEASSISTANT_STOP, self.unregister_webhook
|
||||
webhook_register(
|
||||
self.hass,
|
||||
DOMAIN,
|
||||
webhook_name,
|
||||
self.entry.data[CONF_WEBHOOK_ID],
|
||||
get_webhook_handler(self.withings_data),
|
||||
allowed_methods=[METH_POST],
|
||||
)
|
||||
)
|
||||
self._webhooks_registered = True
|
||||
self._ha_webhook_registered = True
|
||||
LOGGER.debug("Registered Withings webhook at hass: %s", webhook_url)
|
||||
|
||||
self._webhook_url = webhook_url
|
||||
self._subscribe_attempt = 0
|
||||
|
||||
await self._async_subscribe_webhook()
|
||||
|
||||
async def _async_subscribe_webhook(self, _: Any = None) -> None:
|
||||
"""Attempt to subscribe to Withings webhooks."""
|
||||
|
||||
async with self._register_lock:
|
||||
if self._webhooks_registered or self._webhook_url is None:
|
||||
return
|
||||
|
||||
try:
|
||||
await async_subscribe_webhooks(
|
||||
self.withings_data.client, self._webhook_url
|
||||
)
|
||||
except (
|
||||
WithingsUnauthorizedError,
|
||||
WithingsAuthenticationFailedError,
|
||||
) as err:
|
||||
LOGGER.error(
|
||||
"Authentication failed while subscribing to webhooks: %s",
|
||||
err,
|
||||
)
|
||||
self.entry.async_start_reauth(self.hass)
|
||||
return
|
||||
except WithingsInvalidParamsError as err:
|
||||
LOGGER.error(
|
||||
"Webhook URL rejected by Withings: %s",
|
||||
err,
|
||||
)
|
||||
return
|
||||
except WithingsTooManyRequestsError as err:
|
||||
delay = min(
|
||||
300 * (self._subscribe_attempt + 1), MAX_WEBHOOK_RETRY_INTERVAL
|
||||
)
|
||||
LOGGER.warning(
|
||||
"Rate limited by Withings API (attempt %d): %s. "
|
||||
"Retrying in %d seconds",
|
||||
self._subscribe_attempt + 1,
|
||||
err,
|
||||
delay,
|
||||
)
|
||||
except WithingsError as err:
|
||||
base_delay = 30
|
||||
delay = min(
|
||||
base_delay * (2**self._subscribe_attempt),
|
||||
MAX_WEBHOOK_RETRY_INTERVAL,
|
||||
)
|
||||
LOGGER.warning(
|
||||
"Failed to subscribe to Withings webhooks "
|
||||
"(attempt %d): %s. Retrying in %d seconds",
|
||||
self._subscribe_attempt + 1,
|
||||
err,
|
||||
delay,
|
||||
)
|
||||
else:
|
||||
for coordinator in self.withings_data.coordinators:
|
||||
coordinator.webhook_subscription_listener(True)
|
||||
LOGGER.debug(
|
||||
"Registered Withings webhook at Withings: %s", self._webhook_url
|
||||
)
|
||||
self.entry.async_on_unload(
|
||||
self.hass.bus.async_listen_once(
|
||||
EVENT_HOMEASSISTANT_STOP, self.unregister_webhook
|
||||
)
|
||||
)
|
||||
self._webhooks_registered = True
|
||||
return
|
||||
|
||||
self._subscribe_attempt += 1
|
||||
self._schedule_retry(delay)
|
||||
|
||||
|
||||
async def async_unsubscribe_webhooks(client: WithingsClient) -> None:
|
||||
|
||||
@@ -67,7 +67,7 @@ async def setup_integration(
|
||||
async def prepare_webhook_setup(
|
||||
hass: HomeAssistant, freezer: FrozenDateTimeFactory
|
||||
) -> None:
|
||||
"""Prepare webhooks are registered by waiting a second."""
|
||||
"""Prepare webhooks by advancing past the registration delay."""
|
||||
freezer.tick(timedelta(seconds=1))
|
||||
async_fire_time_changed(hass)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
@@ -196,9 +196,7 @@ def mock_withings():
|
||||
|
||||
@pytest.fixture(name="disable_webhook_delay", autouse=True)
|
||||
def disable_webhook_delay():
|
||||
"""Disable webhook delay."""
|
||||
|
||||
mock = AsyncMock()
|
||||
"""Disable webhook delays for faster tests."""
|
||||
with (
|
||||
patch(
|
||||
"homeassistant.components.withings.SUBSCRIBE_DELAY",
|
||||
@@ -208,5 +206,9 @@ def disable_webhook_delay():
|
||||
"homeassistant.components.withings.UNSUBSCRIBE_DELAY",
|
||||
timedelta(seconds=0),
|
||||
),
|
||||
patch(
|
||||
"homeassistant.components.withings.WEBHOOK_REGISTER_DELAY",
|
||||
0,
|
||||
),
|
||||
):
|
||||
yield mock
|
||||
yield
|
||||
|
||||
@@ -11,6 +11,9 @@ from aiowithings import (
|
||||
NotificationCategory,
|
||||
WithingsAuthenticationFailedError,
|
||||
WithingsConnectionError,
|
||||
WithingsError,
|
||||
WithingsInvalidParamsError,
|
||||
WithingsTooManyRequestsError,
|
||||
WithingsUnauthorizedError,
|
||||
)
|
||||
from freezegun.api import FrozenDateTimeFactory
|
||||
@@ -725,3 +728,122 @@ async def test_oauth_implementation_not_available(
|
||||
await hass.async_block_till_done()
|
||||
|
||||
assert webhook_config_entry.state is ConfigEntryState.SETUP_RETRY
|
||||
|
||||
|
||||
async def test_webhook_subscription_retry_on_failure(
|
||||
hass: HomeAssistant,
|
||||
withings: AsyncMock,
|
||||
webhook_config_entry: MockConfigEntry,
|
||||
freezer: FrozenDateTimeFactory,
|
||||
) -> None:
|
||||
"""Test webhook subscription is retried on failure."""
|
||||
call_count = 0
|
||||
|
||||
async def subscribe_side_effect(*args, **kwargs):
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count <= 2:
|
||||
raise WithingsError("The callback URL is either absent or incorrect")
|
||||
|
||||
withings.subscribe_notification.side_effect = subscribe_side_effect
|
||||
|
||||
await setup_integration(hass, webhook_config_entry)
|
||||
await prepare_webhook_setup(hass, freezer)
|
||||
|
||||
# Trigger first retry (30s delay for attempt 0)
|
||||
freezer.tick(timedelta(seconds=30))
|
||||
async_fire_time_changed(hass)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
# Trigger second retry (60s delay for attempt 1)
|
||||
freezer.tick(timedelta(seconds=60))
|
||||
async_fire_time_changed(hass)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
assert withings.subscribe_notification.call_count >= 6
|
||||
|
||||
|
||||
async def test_webhook_subscription_continues_retrying(
|
||||
hass: HomeAssistant,
|
||||
withings: AsyncMock,
|
||||
webhook_config_entry: MockConfigEntry,
|
||||
freezer: FrozenDateTimeFactory,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Test webhook subscription continues retrying with exponential backoff."""
|
||||
withings.subscribe_notification.side_effect = WithingsError(
|
||||
"The callback URL is either absent or incorrect"
|
||||
)
|
||||
|
||||
await setup_integration(hass, webhook_config_entry)
|
||||
await prepare_webhook_setup(hass, freezer)
|
||||
|
||||
# Tick through retry delays with exponential backoff: 30, 60, 120, 240, 480
|
||||
for delay in (30, 60, 120, 240, 480):
|
||||
freezer.tick(timedelta(seconds=delay))
|
||||
async_fire_time_changed(hass)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
assert "Failed to subscribe to Withings webhooks" in caplog.text
|
||||
|
||||
|
||||
async def test_webhook_subscription_rate_limited(
|
||||
hass: HomeAssistant,
|
||||
withings: AsyncMock,
|
||||
webhook_config_entry: MockConfigEntry,
|
||||
freezer: FrozenDateTimeFactory,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Test webhook subscription handles rate limiting."""
|
||||
withings.subscribe_notification.side_effect = WithingsTooManyRequestsError(
|
||||
"Too many requests"
|
||||
)
|
||||
|
||||
await setup_integration(hass, webhook_config_entry)
|
||||
await prepare_webhook_setup(hass, freezer)
|
||||
|
||||
assert "Rate limited by Withings API" in caplog.text
|
||||
|
||||
|
||||
async def test_webhook_subscription_auth_failure(
|
||||
hass: HomeAssistant,
|
||||
withings: AsyncMock,
|
||||
webhook_config_entry: MockConfigEntry,
|
||||
freezer: FrozenDateTimeFactory,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Test webhook subscription stops on auth failure and triggers reauth."""
|
||||
withings.subscribe_notification.side_effect = WithingsAuthenticationFailedError(
|
||||
"Authentication failed"
|
||||
)
|
||||
|
||||
await setup_integration(hass, webhook_config_entry)
|
||||
await prepare_webhook_setup(hass, freezer)
|
||||
|
||||
assert "Authentication failed while subscribing to webhooks" in caplog.text
|
||||
assert withings.subscribe_notification.call_count == 1
|
||||
|
||||
flows = hass.config_entries.flow.async_progress()
|
||||
assert any(
|
||||
flow["handler"] == DOMAIN and flow["context"]["source"] == "reauth"
|
||||
for flow in flows
|
||||
)
|
||||
|
||||
|
||||
async def test_webhook_subscription_invalid_params(
|
||||
hass: HomeAssistant,
|
||||
withings: AsyncMock,
|
||||
webhook_config_entry: MockConfigEntry,
|
||||
freezer: FrozenDateTimeFactory,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Test invalid params error stops retrying."""
|
||||
withings.subscribe_notification.side_effect = WithingsInvalidParamsError(
|
||||
"Invalid callback URL"
|
||||
)
|
||||
|
||||
await setup_integration(hass, webhook_config_entry)
|
||||
await prepare_webhook_setup(hass, freezer)
|
||||
|
||||
assert "Webhook URL rejected by Withings" in caplog.text
|
||||
assert withings.subscribe_notification.call_count == 1
|
||||
|
||||
Reference in New Issue
Block a user