From 0a21db20d50639ef0eebd37f50871b6cbf711808 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Tue, 14 Jul 2026 10:21:59 -0700 Subject: [PATCH 1/5] fix: Make MQTT reconnection retry loop instead of returning on first failure The force_reconnect() method now continuously retries with exponential backoff (2s, 5s, 15s, 30s, 60s cap) until successful reconnection, rather than returning after a single failed attempt. This prevents the integration from becoming permanently stuck when reconnection is triggered during transient auth service failures (e.g., network timeouts). Previously, when force_reconnect() failed due to auth service unavailability, it would log 'Next backoff: 5s' but never actually reschedule the retry. The reconnection task would complete, blocking any future reconnection attempts until the coordinator detected more timeouts, which could take minutes or never occur if request patterns changed. Now the task continuously retries with exponential backoff until: - Successful reconnection (returns True and resets attempts) - Task cancellation (propagates CancelledError) - Coordinator shutdown This ensures automatic recovery when auth service or network issues are transient. Fixes issue #100. --- custom_components/nwp500/mqtt_manager.py | 106 ++++++++++++----------- 1 file changed, 56 insertions(+), 50 deletions(-) diff --git a/custom_components/nwp500/mqtt_manager.py b/custom_components/nwp500/mqtt_manager.py index 157a80c..577d394 100644 --- a/custom_components/nwp500/mqtt_manager.py +++ b/custom_components/nwp500/mqtt_manager.py @@ -492,67 +492,73 @@ 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, - ) - ], - ) - return False + # 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 Exception as err: + _LOGGER.debug("Setup attempt failed: %s", err) + + # 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 Exception as err: - self._reconnect_attempts += 1 - _LOGGER.error("Error during forced reconnect: %s", err) - return False + except asyncio.CancelledError: + raise finally: self.reconnection_in_progress = False From 788a663479d66e79d5f2fa210adcd3c2cb929015 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Tue, 14 Jul 2026 10:34:31 -0700 Subject: [PATCH 2/5] test: Add comprehensive unit tests for MQTT reconnection retry logic Add three new unit tests to cover the retry behavior of force_reconnect(): 1. test_force_reconnect_retries_on_setup_failure: Verifies that force_reconnect() retries when setup() fails temporarily and eventually succeeds on retry. 2. test_force_reconnect_resets_backoff_on_success: Confirms that the reconnection attempt counter resets to 0 after a successful reconnection. 3. test_force_reconnect_handles_cancellation: Validates that force_reconnect() properly handles task cancellation by propagating CancelledError. Also improves debug logging to include exception context (exc_info=True) so tracebacks are captured when setup() fails, aiding diagnosis of reconnection issues. These tests prevent regressions in the reconnection retry logic and validate the fix for issue #100. --- custom_components/nwp500/mqtt_manager.py | 2 +- tests/unit/test_mqtt_manager.py | 83 +++++++++++++++++++++++- 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/custom_components/nwp500/mqtt_manager.py b/custom_components/nwp500/mqtt_manager.py index 577d394..fa59d72 100644 --- a/custom_components/nwp500/mqtt_manager.py +++ b/custom_components/nwp500/mqtt_manager.py @@ -542,7 +542,7 @@ async def force_reconnect(self, devices: list[Device]) -> bool: await self.start_periodic_requests(device) return True except Exception as err: - _LOGGER.debug("Setup attempt failed: %s", err) + _LOGGER.debug("Setup attempt failed: %s", err, exc_info=True) # Failed - increment attempt counter for next backoff self._reconnect_attempts += 1 diff --git a/tests/unit/test_mqtt_manager.py b/tests/unit/test_mqtt_manager.py index 9f3ca80..08ee98c 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,87 @@ 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 success + 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") + return True + + # Mock setup to fail once then succeed + 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)" + 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.""" + await manager.setup() + await manager.subscribe_device(mock_device) + + # Mock setup to hang indefinitely so we can cancel it + 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])) + await asyncio.sleep(0.1) # Let task start + task.cancel() + + try: + await task + pytest.fail("Task should have been cancelled") + except asyncio.CancelledError: + pass # Expected + finally: + manager.setup = original_setup + + + # Verify callbacks are registered with the client assert mock_mqtt_client.on.call_count >= 3 From 55e29dc1364452dac915b0e06a9ba4e28f1c5d1e Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Tue, 14 Jul 2026 10:37:34 -0700 Subject: [PATCH 3/5] style: Apply ruff formatting Auto-format code according to ruff rules. --- custom_components/nwp500/mqtt_manager.py | 4 +++- tests/unit/test_mqtt_manager.py | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/custom_components/nwp500/mqtt_manager.py b/custom_components/nwp500/mqtt_manager.py index fa59d72..f67a1af 100644 --- a/custom_components/nwp500/mqtt_manager.py +++ b/custom_components/nwp500/mqtt_manager.py @@ -542,7 +542,9 @@ async def force_reconnect(self, devices: list[Device]) -> bool: await self.start_periodic_requests(device) return True except Exception as err: - _LOGGER.debug("Setup attempt failed: %s", err, exc_info=True) + _LOGGER.debug( + "Setup attempt failed: %s", err, exc_info=True + ) # Failed - increment attempt counter for next backoff self._reconnect_attempts += 1 diff --git a/tests/unit/test_mqtt_manager.py b/tests/unit/test_mqtt_manager.py index 08ee98c..dcfa4cd 100644 --- a/tests/unit/test_mqtt_manager.py +++ b/tests/unit/test_mqtt_manager.py @@ -312,7 +312,9 @@ async def setup_with_one_failure(): 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)" + assert setup_call_count == 2, ( + "Setup should be called twice (1 failure + 1 success)" + ) finally: manager.setup = original_setup @@ -364,9 +366,6 @@ async def setup_hang(): finally: manager.setup = original_setup - - - # Verify callbacks are registered with the client assert mock_mqtt_client.on.call_count >= 3 From 38b99bc29fb9612d0b3028a3caf707b2168d74ad Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Tue, 14 Jul 2026 10:40:20 -0700 Subject: [PATCH 4/5] docs: Add changelog entry for MQTT reconnection retry loop fix --- CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 From b1f649c5aa1fbad9d6a52d977dcc7fa7339a9e4c Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Tue, 14 Jul 2026 14:02:43 -0700 Subject: [PATCH 5/5] fix: Improve CancelledError handling and strengthen unit tests - Re-raise asyncio.CancelledError explicitly before generic exception handler in force_reconnect() to prevent swallowing cancellation during setup() (coordinator shutdown case) - Strengthen test_force_reconnect_retries_on_setup_failure to wrap real setup() instead of stub, ensuring test validates actual reconnection and doesn't weaken the contract - Improve test_force_reconnect_handles_cancellation to patch backoff delays to 0, ensuring cancellation occurs during setup() call rather than backoff sleep, validating CancelledError propagation from within setup() --- custom_components/nwp500/mqtt_manager.py | 2 ++ tests/unit/test_mqtt_manager.py | 36 +++++++++++++++--------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/custom_components/nwp500/mqtt_manager.py b/custom_components/nwp500/mqtt_manager.py index f67a1af..977c202 100644 --- a/custom_components/nwp500/mqtt_manager.py +++ b/custom_components/nwp500/mqtt_manager.py @@ -541,6 +541,8 @@ async def force_reconnect(self, devices: list[Device]) -> bool: 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 diff --git a/tests/unit/test_mqtt_manager.py b/tests/unit/test_mqtt_manager.py index dcfa4cd..8e20ff3 100644 --- a/tests/unit/test_mqtt_manager.py +++ b/tests/unit/test_mqtt_manager.py @@ -292,7 +292,7 @@ async def test_force_reconnect_retries_on_setup_failure( await manager.setup() await manager.subscribe_device(mock_device) - # Simulate setup failures on first attempt, then success + # Simulate setup failures on first attempt, then delegate to real setup setup_call_count = 0 async def setup_with_one_failure(): @@ -300,9 +300,10 @@ async def setup_with_one_failure(): setup_call_count += 1 if setup_call_count == 1: raise ConnectionError("Transient auth service failure") - return True + # Call the real setup to actually reconnect + return await original_setup() - # Mock setup to fail once then succeed + # Wrap setup to fail once then succeed with real reconnection original_setup = manager.setup manager.setup = setup_with_one_failure @@ -315,6 +316,10 @@ async def setup_with_one_failure(): 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 @@ -341,11 +346,11 @@ async def test_force_reconnect_resets_backoff_on_success( async def test_force_reconnect_handles_cancellation( manager, mock_mqtt_client, mock_device ): - """Test that force_reconnect properly handles task cancellation.""" + """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 + # Mock setup to hang indefinitely so we can cancel it during setup async def setup_hang(): await asyncio.sleep(10) return True @@ -355,14 +360,19 @@ async def setup_hang(): try: task = asyncio.create_task(manager.force_reconnect([mock_device])) - await asyncio.sleep(0.1) # Let task start - task.cancel() - - try: - await task - pytest.fail("Task should have been cancelled") - except asyncio.CancelledError: - pass # Expected + # 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