Fix MQTT reconnection to retry with backoff instead of returning on first failure#101
Merged
Conversation
…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.
There was a problem hiding this comment.
Pull request overview
This PR fixes the Home Assistant integration’s MQTT reconnection behavior by making NWP500MqttManager.force_reconnect() continuously retry with exponential backoff instead of returning after the first failed attempt, preventing the integration from getting stuck disconnected after transient auth/network errors.
Changes:
- Converted
force_reconnect()from a single-attempt reconnect to an internal retry loop with exponential backoff (2s → 5s → 15s → 30s → 60s cap). - Improved cancellation handling so the reconnect task cleanly propagates
CancelledError. - Continues retrying when
setup()fails (including exceptions during setup), resetting counters after a successful reconnection.
Comment on lines
+502
to
+507
| 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] |
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.
Auto-format code according to ruff rules.
Comment on lines
+544
to
547
| except Exception as err: | ||
| _LOGGER.debug( | ||
| "Setup attempt failed: %s", err, exc_info=True | ||
| ) |
Comment on lines
+295
to
+308
| # 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 | ||
|
|
Comment on lines
+357
to
+360
| task = asyncio.create_task(manager.force_reconnect([mock_device])) | ||
| await asyncio.sleep(0.1) # Let task start | ||
| task.cancel() | ||
|
|
- 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()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When
force_reconnect()encounters a transient auth service failure (e.g., 'Cannot connect to host nlus.naviensmartcontrol.com:443'), it logs 'Reconnection failed (attempt 1). Next backoff: 5s' but never actually reschedules the retry.The task completes after a single failed attempt. The backoff message is misleading—there is no 'next' attempt scheduled.
This causes the integration to become permanently stuck disconnected if the first reconnection attempt fails, requiring manual HA restart to recover.
Root Cause
force_reconnect()attempts reconnection once with exponential backoff, but doesn't reschedule retries. If the attempt fails, the function returns False and the task completes. The reconnection is only triggered again if the coordinator detects more timeouts, which may never happen or take minutes.Solution
Changed
force_reconnect()to loop internally and continuously retry with exponential backoff (2s, 5s, 15s, 30s, 60s cap) until:This ensures automatic recovery when auth service or network issues are transient.
Changes
mqtt_manager.force_reconnect()to loop with exponential backoffTesting
Fixes issue #100.