Skip to content

Reconnect the live update stream after a transient failure - #418

Open
rhammen wants to merge 16 commits into
custom-components:masterfrom
rhammen:fix/issue-417-stream-reconnect
Open

Reconnect the live update stream after a transient failure#418
rhammen wants to merge 16 commits into
custom-components:masterfrom
rhammen:fix/issue-417-stream-reconnect

Conversation

@rhammen

@rhammen rhammen commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #417.

The live update stream (Service Bus/AMQP connection) currently dies
permanently after any transient failure (e.g. a brief internet outage) —
it swallows the exception, logs it, and just stops, requiring a full
integration reload to recover.

  • stream_main() no longer catches its own exceptions; only the
    existing 403/Forbidden case still returns cleanly ("permanent stop").
  • New _stream_supervisor() in manager.py wraps stream_main() in a
    reconnect loop with exponential backoff + jitter, retrying any
    transient failure indefinitely while leaving CancelledError (task
    cancellation on unload/reload) unaffected.
  • New STREAM_RECONNECT_* constants in const.py.

Personally reviewed the code changes and trimmed AI-generated
docstrings/inline comments for length.

rhammen and others added 11 commits July 28, 2026 21:49
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…them

Only the existing 403/Forbidden case still returns cleanly; any other
failure now raises so a caller can distinguish "retry me" from
"permanent stop". Prep for issue custom-components#417's reconnect supervisor.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wraps stream_main() in an exponential-backoff retry loop so a transient
connection failure (e.g. a brief home-internet outage) reconnects
automatically instead of permanently killing live updates until the
user reloads the integration. Fixes custom-components#417.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ix, docstring

- Add test coverage for the actual asyncio.sleep delay values (first-call
  value and max-delay cap enforcement across a sustained outage)
- Clamp exponential growth to STREAM_RECONNECT_MAX_DELAY before applying
  jitter, so jitter isn't nullified once backoff saturates
- Document _stream_supervisor()'s actual warn-once-per-outage semantics

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per this repo's demonstrated review bar (PR custom-components#415, commit 2359337):
maintainers cut explanatory comments even when accurate, not just
stale/duplicative ones. The warn-once-per-outage paragraph explained
a behavior detail already covered by the inline comment at the reset
condition -- kept the return/raise contract and the CancelledError
note since those are safety-critical to not breaking the function.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same bar as the manager.py docstring trim: keep what's necessary to
not accidentally break the test (the clock-sequence meaning, why the
next(clock, 500.0) fallback exists), cut the restatement/elaboration.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every other test in this file uses a single-line docstring; these two
stood out at 3 lines each. Kept the issue-custom-components#417 reference as a compact
parenthetical, matching the existing precedent in test_init.py:25.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rhammen
rhammen marked this pull request as ready for review July 28, 2026 22:09
Comment thread .gitignore Outdated
Comment thread custom_components/zaptec/manager.py
Comment thread custom_components/zaptec/zaptec/api.py
@sveinse sveinse added this to the v0.8.8 milestone Aug 31, 2026
@sveinse

sveinse commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

@rhammen Is this a fix to #423?

rhammen and others added 5 commits September 1, 2026 20:08
Local-only tooling directory; excluded via .git/info/exclude instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stream_main() gained an optional on_connect callback, fired once the
service bus receiver is live. The supervisor uses it to log "connected
after N reconnect attempt(s)" and to time the connection, so a stream
that stayed up past STREAM_RECONNECT_STABLE_TIME resets the backoff and
the warn-once state instead of relying on the max-delay heuristic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The supervisor names the exception in its "disconnected" warning and
attaches a traceback only when it isn't one of STREAM_TRANSIENT_ERRORS
(service bus, network, timeout, request retry). Repeat failures still
log the traceback at debug level.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
STREAM_TRANSIENT_ERRORS missed the failure issue custom-components#417 is actually about:
a non-403 RequestError out of live_stream_connection_details() was
classified unexpected. It now lists ZaptecApiError instead of three of
its subclasses, plus MessageAlreadySettled, which derives from
ValueError rather than ServiceBusError.

on_connect now fires from inside the receiver context, and tests cover
the backoff reset, the quiet first connect, and an attempt that fails
before connecting at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Measured on a live installation, stream observations arrive every ~3
minutes while charging (median gap 164s) and in a two-message burst
every ~68 minutes when idle. At 60s a connection could be declared
healthy having carried nothing, so a stream flapping just above that
reset the backoff and re-warned on every cycle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@rhammen

rhammen commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

No — #423 looks like an account-side problem rather than a code bug, so this PR won't fix it.

What it does change for that failure: RequestRetryError now propagates as a transient error, so the stream retries with backoff instead of dying on the first failure. If a messagingConnectionDetails failure is temporary, that self-heals. #423's isn't — I checked the portal today and the reporter's messaging subscription appears not to be provisioned (their "subscription enabled" toggle is greyed out where mine is on), which no amount of retrying will fix. Details in #423.

One consequence worth your view before this merges. For a permanently failing stream, the supervisor now warns once and then logs only at debug, retrying every 5 minutes forever. That's right for a transient outage but hides a terminal one — a #423-style user would see a single warning and then silence. Should it re-warn periodically (say hourly) while an outage persists?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Live update stream doesn't reconnect after a transient connection failure (e.g. AMQP/network drop)

2 participants