diff --git a/CHANGELOG.md b/CHANGELOG.md index fbfd117..466ec9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ ## [Unreleased] +### Fixed +- **MQTT reconnection retry loop**: Fixed a critical bug where + `force_reconnect()` would fail to retry after a transient auth service or + network error. Previously, when reconnection failed during the initial + attempt, the method would return instead of rescheduling retries with + exponential backoff. This caused the integration to become permanently stuck + disconnected, requiring manual Home Assistant restart to recover. The fix + converts `force_reconnect()` to loop internally with exponential backoff + (2s, 5s, 15s, 30s, 60s cap), continuously retrying until successful + reconnection or task cancellation. This ensures automatic recovery from + transient auth/network failures. Fixes + [issue #100](https://github.com/eman/ha_nwp500/issues/100). + ## [0.16.1] - 2026-07-14 ### Fixed diff --git a/custom_components/nwp500/mqtt_manager.py b/custom_components/nwp500/mqtt_manager.py index 157a80c..977c202 100644 --- a/custom_components/nwp500/mqtt_manager.py +++ b/custom_components/nwp500/mqtt_manager.py @@ -492,67 +492,77 @@ async def force_reconnect(self, devices: list[Device]) -> bool: Uses increasing delays between attempts: 2s, 5s, 15s, 30s, 60s (cap). Backoff resets on successful reconnection. + Retries indefinitely with exponential backoff until successful. """ if self.reconnection_in_progress: return False self.reconnection_in_progress = True try: - # Calculate backoff delay based on attempt count - delay_index = min( - self._reconnect_attempts, len(_RECONNECT_BACKOFF_DELAYS) - 1 - ) - backoff_delay = _RECONNECT_BACKOFF_DELAYS[delay_index] - - _LOGGER.warning( - "Forcing MQTT reconnection (attempt %d, backoff %.0fs)...", - self._reconnect_attempts + 1, - backoff_delay, - ) - - # Full teardown - await self.disconnect() - - # Wait with exponential backoff before reconnecting - await asyncio.sleep(backoff_delay) - - # Re-initialize and connect (connect() will refresh auth tokens) - if await self.setup(): - # Only update timestamp on successful reconnection - # This prevents rate-limiting from blocking retries on failed attempts - self._last_reconnect_time = time.time() + while True: + # Calculate backoff delay based on attempt count + delay_index = min( + self._reconnect_attempts, len(_RECONNECT_BACKOFF_DELAYS) - 1 + ) + backoff_delay = _RECONNECT_BACKOFF_DELAYS[delay_index] - _LOGGER.info( - "Reconnection successful after %d attempt(s)", + _LOGGER.warning( + "Forcing MQTT reconnection (attempt %d, backoff %.0fs)...", self._reconnect_attempts + 1, + backoff_delay, ) - self.consecutive_timeouts = 0 - self._reconnect_attempts = 0 # Reset backoff on success - - # Re-subscribe to all devices and restart periodic tasks - for device in devices: - await self.subscribe_device(device) - await self.start_periodic_requests(device) - return True - # Failed - increment attempt counter for next backoff - self._reconnect_attempts += 1 - _LOGGER.warning( - "Reconnection failed (attempt %d). Next backoff: %.0fs", - self._reconnect_attempts, - _RECONNECT_BACKOFF_DELAYS[ - min( - self._reconnect_attempts, - len(_RECONNECT_BACKOFF_DELAYS) - 1, + # Full teardown + await self.disconnect() + + # Wait with exponential backoff before reconnecting + try: + await asyncio.sleep(backoff_delay) + except asyncio.CancelledError: + _LOGGER.debug("MQTT reconnection task was cancelled") + raise + + try: + # Re-initialize and connect (connect() will refresh auth tokens) + if await self.setup(): + # Only update timestamp on successful reconnection + # This prevents rate-limiting from blocking retries on failed attempts + self._last_reconnect_time = time.time() + + _LOGGER.info( + "Reconnection successful after %d attempt(s)", + self._reconnect_attempts + 1, + ) + self.consecutive_timeouts = 0 + self._reconnect_attempts = 0 # Reset backoff on success + + # Re-subscribe to all devices and restart periodic tasks + for device in devices: + await self.subscribe_device(device) + await self.start_periodic_requests(device) + return True + except asyncio.CancelledError: + raise + except Exception as err: + _LOGGER.debug( + "Setup attempt failed: %s", err, exc_info=True ) - ], - ) - return False - except Exception as err: - self._reconnect_attempts += 1 - _LOGGER.error("Error during forced reconnect: %s", err) - return False + # Failed - increment attempt counter for next backoff + self._reconnect_attempts += 1 + next_backoff_index = min( + self._reconnect_attempts, len(_RECONNECT_BACKOFF_DELAYS) - 1 + ) + next_backoff = _RECONNECT_BACKOFF_DELAYS[next_backoff_index] + _LOGGER.warning( + "Reconnection failed (attempt %d). Retrying in %.0fs...", + self._reconnect_attempts, + next_backoff, + ) + # Loop continues - will retry after backoff + + except asyncio.CancelledError: + raise finally: self.reconnection_in_progress = False diff --git a/tests/unit/test_mqtt_manager.py b/tests/unit/test_mqtt_manager.py index 9f3ca80..8e20ff3 100644 --- a/tests/unit/test_mqtt_manager.py +++ b/tests/unit/test_mqtt_manager.py @@ -2,6 +2,7 @@ from __future__ import annotations +import asyncio from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -284,9 +285,96 @@ async def test_force_reconnect(manager, mock_mqtt_client, mock_device): @pytest.mark.asyncio -async def test_callbacks(manager, mock_mqtt_client): - """Test that callbacks are registered correctly.""" +async def test_force_reconnect_retries_on_setup_failure( + manager, mock_mqtt_client, mock_device +): + """Test that force_reconnect retries when setup fails temporarily.""" + await manager.setup() + await manager.subscribe_device(mock_device) + + # Simulate setup failures on first attempt, then delegate to real setup + setup_call_count = 0 + + async def setup_with_one_failure(): + nonlocal setup_call_count + setup_call_count += 1 + if setup_call_count == 1: + raise ConnectionError("Transient auth service failure") + # Call the real setup to actually reconnect + return await original_setup() + + # Wrap setup to fail once then succeed with real reconnection + original_setup = manager.setup + manager.setup = setup_with_one_failure + + # Create a task with generous timeout to account for backoff delays + try: + result = await asyncio.wait_for( + manager.force_reconnect([mock_device]), timeout=15 + ) + assert result is True, "force_reconnect should succeed after retry" + assert setup_call_count == 2, ( + "Setup should be called twice (1 failure + 1 success)" + ) + # Verify MQTT client was actually reconnected + assert manager.mqtt_client is not None, ( + "MQTT client should be connected" + ) + finally: + manager.setup = original_setup + + +@pytest.mark.asyncio +async def test_force_reconnect_resets_backoff_on_success( + manager, mock_mqtt_client, mock_device +): + """Test that reconnection attempt counter resets on successful reconnect.""" await manager.setup() + await manager.subscribe_device(mock_device) + + # Manually increment attempt counter to simulate prior failures + manager._reconnect_attempts = 3 + + result = await manager.force_reconnect([mock_device]) + + assert result is True + # Counter should be reset to 0 after successful reconnection + assert manager._reconnect_attempts == 0 + + +@pytest.mark.asyncio +async def test_force_reconnect_handles_cancellation( + manager, mock_mqtt_client, mock_device +): + """Test that force_reconnect properly handles task cancellation from setup.""" + await manager.setup() + await manager.subscribe_device(mock_device) + + # Mock setup to hang indefinitely so we can cancel it during setup + async def setup_hang(): + await asyncio.sleep(10) + return True + + original_setup = manager.setup + manager.setup = setup_hang + + try: + task = asyncio.create_task(manager.force_reconnect([mock_device])) + # Use patch to set backoff delays to 0 so task reaches setup() immediately + with patch( + "custom_components.nwp500.mqtt_manager._RECONNECT_BACKOFF_DELAYS", + [0, 0, 0, 0, 0], + ): + await asyncio.sleep(0.1) # Let task reach setup() + task.cancel() + + try: + await task + pytest.fail("Task should have been cancelled") + except asyncio.CancelledError: + pass # Expected - CancelledError should propagate + finally: + manager.setup = original_setup # Verify callbacks are registered with the client assert mock_mqtt_client.on.call_count >= 3