-
Notifications
You must be signed in to change notification settings - Fork 33
Reconnect the live update stream after a transient failure #418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rhammen
wants to merge
16
commits into
custom-components:master
Choose a base branch
from
rhammen:fix/issue-417-stream-reconnect
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 11 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
f31cd08
Add design spec for stream reconnect fix (#417)
rhammen bd9ea14
Add implementation plan for stream reconnect fix (#417)
rhammen a24ca88
Ignore .superpowers/ SDD scratch workspace
rhammen cd33db5
Let stream_main() propagate transient failures instead of swallowing …
rhammen 80f5e5a
Add reconnect-with-backoff supervisor for the live update stream
rhammen 2123293
Address final-review findings: backoff test coverage, jitter-at-cap f…
rhammen 7bc348f
Tighten _stream_supervisor() docstring wording
rhammen ef93474
Trim _stream_supervisor() docstring to the correctness-critical parts
rhammen baa2db8
Trim backoff-reset test's comments to the load-bearing facts
rhammen 1085a7d
Trim stream_main test docstrings to match file convention
rhammen 4405b1e
docs: remove planning docs, archived on docs/ai-planning-archive
rhammen 85c015f
Drop the .superpowers/ gitignore entry
rhammen 49c3522
Report stream reconnects and reset the outage state on connect
rhammen d98b7f8
Keep the stream traceback only for unexpected failures
rhammen ce63801
Widen the expected stream errors and cover the reset paths
rhammen 7c2b4c7
Require 5 minutes of uptime before a stream counts as recovered
rhammen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ __pycache__ | |
| coverage.xml | ||
| .ruff_cache | ||
| htmlcov/ | ||
| .superpowers/ | ||
|
|
||
| # Home Assistant configuration | ||
| config/* | ||
|
|
||
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| """Tests for custom_components.zaptec.manager.""" | ||
|
|
||
| import asyncio | ||
| import logging | ||
| from types import SimpleNamespace | ||
| from unittest.mock import AsyncMock | ||
|
|
||
| import pytest | ||
|
|
||
| from custom_components.zaptec import manager as manager_module | ||
| from custom_components.zaptec.manager import ( | ||
| STREAM_RECONNECT_INIT_DELAY, | ||
| STREAM_RECONNECT_MAX_DELAY, | ||
| _stream_supervisor, | ||
| ) | ||
|
|
||
|
|
||
| def _fake_install() -> SimpleNamespace: | ||
| """Return a stand-in for Installation carrying only what _stream_supervisor uses.""" | ||
| return SimpleNamespace(qual_id="Installation[nst-1]", stream_main=AsyncMock()) | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_stream_supervisor_stops_when_stream_main_returns_normally() -> None: | ||
| """stream_main() returning None (e.g. 403/Forbidden) is a permanent stop.""" | ||
| install = _fake_install() | ||
| install.stream_main.return_value = None | ||
|
|
||
| await _stream_supervisor(install, cb=AsyncMock(), ssl_context=None) | ||
|
|
||
| install.stream_main.assert_awaited_once() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_stream_supervisor_retries_on_exception(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| """A raised exception is retried, not left dead, using the initial backoff delay.""" | ||
| install = _fake_install() | ||
| install.stream_main.side_effect = [ConnectionError("boom"), None] | ||
| sleep_mock = AsyncMock() | ||
| monkeypatch.setattr(asyncio, "sleep", sleep_mock) | ||
| # Neutralize jitter so the first delay is deterministically the init delay. | ||
| monkeypatch.setattr(manager_module.random, "normalvariate", lambda mu, sigma: mu) # noqa: ARG005 | ||
|
|
||
| await _stream_supervisor(install, cb=AsyncMock(), ssl_context=None) | ||
|
|
||
| assert install.stream_main.await_count == 2 # noqa: PLR2004 | ||
| sleep_mock.assert_awaited_once() | ||
| (delay,), _ = sleep_mock.await_args | ||
| assert delay == pytest.approx(STREAM_RECONNECT_INIT_DELAY) | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_stream_supervisor_backoff_never_exceeds_max_delay( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """Across many consecutive failures, every sleep delay stays within the cap.""" | ||
| install = _fake_install() | ||
| install.stream_main.side_effect = [ConnectionError("boom")] * 10 + [None] | ||
| sleep_mock = AsyncMock() | ||
| monkeypatch.setattr(asyncio, "sleep", sleep_mock) | ||
|
|
||
| await _stream_supervisor(install, cb=AsyncMock(), ssl_context=None) | ||
|
|
||
| assert install.stream_main.await_count == 11 # noqa: PLR2004 | ||
| assert sleep_mock.await_count == 10 # noqa: PLR2004 | ||
| for (delay,), _ in sleep_mock.await_args_list: | ||
| assert delay <= STREAM_RECONNECT_MAX_DELAY | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_stream_supervisor_propagates_cancelled_error_without_retrying( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """Task cancellation (integration unload/reload) is not treated as a retryable failure.""" | ||
| install = _fake_install() | ||
| install.stream_main.side_effect = asyncio.CancelledError() | ||
| monkeypatch.setattr(asyncio, "sleep", AsyncMock()) | ||
|
|
||
| with pytest.raises(asyncio.CancelledError): | ||
| await _stream_supervisor(install, cb=AsyncMock(), ssl_context=None) | ||
|
|
||
| install.stream_main.assert_awaited_once() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_stream_supervisor_logs_warning_once_then_debug( | ||
| monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture | ||
| ) -> None: | ||
| """Only the first failure of an outage logs at warning; the rest log at debug.""" | ||
| install = _fake_install() | ||
| install.stream_main.side_effect = [ConnectionError("1"), ConnectionError("2"), None] | ||
| monkeypatch.setattr(asyncio, "sleep", AsyncMock()) | ||
|
|
||
| with caplog.at_level(logging.DEBUG, logger="custom_components.zaptec.manager"): | ||
| await _stream_supervisor(install, cb=AsyncMock(), ssl_context=None) | ||
|
|
||
| warnings = [r for r in caplog.records if r.levelno == logging.WARNING] | ||
| debugs = [r for r in caplog.records if r.levelno == logging.DEBUG] | ||
| assert len(warnings) == 1 | ||
| assert len(debugs) == 1 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_stream_supervisor_resets_backoff_after_long_lived_connection( | ||
| monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture | ||
| ) -> None: | ||
| """A connection that outlived the max backoff delay counts as a fresh outage. | ||
|
|
||
| Verified via logging: a failure treated as a new outage warns again (see | ||
| test_stream_supervisor_logs_warning_once_then_debug for the same-outage | ||
| case, which stays at DEBUG). | ||
| """ | ||
| install = _fake_install() | ||
| install.stream_main.side_effect = [ConnectionError("1"), ConnectionError("2"), None] | ||
| monkeypatch.setattr(asyncio, "sleep", AsyncMock()) | ||
| monkeypatch.setattr(manager_module, "STREAM_RECONNECT_MAX_DELAY", 100.0) | ||
| # 5 monotonic() calls: connected_at + failure-check per failed attempt (x2), | ||
| # then connected_at for the successful 3rd. Attempt 2's gap (0.0 -> 200.0) | ||
| # exceeds MAX_DELAY (100.0), so it counts as a new outage. | ||
| # Fallback (not bare next(clock)): Windows' ProactorEventLoop calls | ||
| # monotonic() more times during teardown, after the coroutine has returned. | ||
| clock = iter([0.0, 0.0, 0.0, 200.0, 500.0]) | ||
| monkeypatch.setattr(manager_module.time, "monotonic", lambda: next(clock, 500.0)) | ||
|
|
||
| with caplog.at_level(logging.DEBUG, logger="custom_components.zaptec.manager"): | ||
| await _stream_supervisor(install, cb=AsyncMock(), ssl_context=None) | ||
|
|
||
| warnings = [r for r in caplog.records if r.levelno == logging.WARNING] | ||
| assert len(warnings) == 2 # noqa: PLR2004 # both failures counted as separate outages |
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.