Detect silently dropped WebSocket connections and retry until subscriptions are restored by markfrancisonly · Pull Request #7164 · home-assistant/android

markfrancisonly · 2026-07-11T17:22:06Z

Summary

When the WebSocket dropped and the single reconnection attempt failed — typically because Home Assistant was restarting — active subscriptions were never restored even after the server came back. For devices relying on local push, notifications stayed dead until the app was manually restarted, with notify failing as not connected to local push notifications. Connections that die silently (a container restart where the address simply disappears, an access point roam, a NAT timeout) were never detected at all: OkHttp keeps buffering writes into the half-open socket, no close callback ever fires, and the periodic ping result was discarded.

The reconnection is now retried while there are active subscriptions to restore, stopping on authentication failure since retrying would not help. This replaces the TODO referencing this issue. Attempts start 10 seconds apart and back off up to a 2 minute cap; the backoff resets when a subscription is actually restored, so persistent failures — an unreachable server or a subscription the server keeps rejecting — keep retrying indefinitely without hammering, and are never silently abandoned while their flow is still collected.
A subscription's old registration is only dropped once the new one is acknowledged. A resubscription the server rejects is retried on the live connection with the capped backoff. One that receives no acknowledgement leaves the server state ambiguous — it may have been accepted with the answer lost — so the connection is cancelled and restoration resumes on a clean connection, where no possibly-accepted attempt can linger as a duplicate registration. Closing a subscription also unsubscribes any attempt still in flight for it.
The application-level ping now cancels the connection when no pong arrives on a connection that is believed to be established, surfacing silently dropped connections to the same close handling and restore path. A connection (re)established while waiting for the pong is never cancelled.
The WebsocketManager worker keeps supervising while the settings require it and restarts collection jobs that stopped (now covered by a test), covering the case where the app starts while the server is restarting and the very first subscribe fails.

Complementary to — but fully independent of — home-assistant/core#176833, the server-side half of the local push story; see the comment below for how the two fit together.

Fixes #5259
Fixes #5939
Likely also resolves #6926

Checklist

New or updated tests have been added to cover the changes following the testing guidelines.
The code follows the project's code style and best_practices.
The changes have been thoroughly tested, and edge cases have been considered.
Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Any other notes

Added regression tests for the previously uncovered cases: the server staying unavailable through several retries and then recovering (the original Flow receives events under the new subscription ID), an unacknowledged resubscription being restored on the following reconnection, a resubscription rejected on a still-live connection being retried until it succeeds, an unanswered ping cancelling the connection and its subscription being restored, the backoff schedule while the server stays unreachable, rejection retries backing off without ever dropping the subscription, and the worker restarting a collection job that stopped. Verified on-device: with the server stopped, the app retries continuously and notifications resume within seconds of the server coming back; a silently dropped connection is detected within one ping interval.

When the connection dropped and the single reconnection attempt failed, for instance because Home Assistant was restarting, the active subscriptions were never restored even after the server came back, leaving local push notifications dead until the app was restarted. - Keep retrying the reconnection while there are active subscriptions, stopping on authentication failure since retrying would not help - Only drop a subscription's old registration once the new one is acknowledged, so a failed resubscription is retried on the next reconnection instead of being silently lost - Keep the WebsocketManager worker supervising while the settings require it and restart collection jobs that stopped, so a collection that could not subscribe, like when the app starts while the server is restarting, is started again Fixes home-assistant#5259 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR improves the WebSocket subscription restoration logic so local push notification subscriptions are retried and restored after transient server unavailability (e.g., during Home Assistant restarts), instead of staying disconnected until the app restarts.

Changes:

Add a retry loop that keeps reconnecting/resubscribing every 10 seconds while there are active subscriptions, and stops retrying on authentication failure.
Make resubscription attempts retryable when rejected or unacknowledged, preserving subscription flows across reconnects.
Update the WebsocketManager worker to continue supervising/restarting collection jobs that stop unexpectedly, plus add regression tests and a changelog entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketCoreImpl.kt	Adds reconnect/resubscribe retry loop and resubscription restore behavior
common/src/test/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketCoreImplTest.kt	Adds regression tests for retrying reconnect/resubscribe scenarios
app/src/main/kotlin/io/homeassistant/companion/android/websocket/WebsocketManager.kt	Keeps the worker supervising and restarting stopped server jobs
app/src/main/res/xml/changelog_master.xml	Documents the user-visible local push reconnect behavior change

Addresses the review comments: the restore loop is now cancelled and replaced under the connection lock so concurrent callbacks of the same closing cannot start two loops, a closing already handled by another callback is skipped, and closing a subscription removes every entry tracked for its message so an in flight resubscription cannot leave an orphan behind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

markfrancisonly · 2026-07-11T18:32:02Z

this pr resolves issues that were preventing notification commands from being received reliability. good stuff, enjoy

…ropped connections The restore loop added for local push only runs when OkHttp reports the connection closed. When the server goes away without the TCP connection being reset — for example a Home Assistant container restart where the IP simply disappears (macvlan/ipvlan), an AP roam or a NAT timeout — no close frame or reset ever reaches the device. OkHttp keeps buffering writes into the half-open socket without failing, onFailure/onClosed never fire, and the subscription stays dead until the app is restarted, even though WebsocketManager keeps pinging every 30 seconds: the ping result was discarded and a missing pong had no effect. Move the ping into WebSocketCore: when a pong is not received on a connection that is believed to be established, cancel the WebSocket. The cancel surfaces as onFailure, runs handleClosingSocket and hands over to the restore loop, which reconnects and resubscribes once the server is reachable again. A connection (re)established while waiting for the pong is never cancelled thanks to an identity check on the connection holder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Retrying at a fixed 10 second cadence is fine for a quick server restart but wasteful on a long outage, where a battery powered device would attempt a connection every 10 seconds indefinitely. Double the delay between attempts after each failed reconnection, up to a cap of 2 minutes, and restart from 10 seconds as soon as the server is reachable again so rejected resubscriptions on a live connection keep retrying quickly. A short outage is still picked up fast (attempts at 10s, 20s and 40s cover a typical restart) while a long one settles at one attempt every 2 minutes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

TimoPtr

Shutdown needs to clear the reconnection and close too if I'm not wrong.

I like the idea of the ping you had it's a nice safe guard.

home-assistant · 2026-07-13T12:47:58Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

GimliTheGreat · 2026-07-17T12:11:11Z

I hope you guys work it out, I would love to stop rebooting my Amazon Echos every time I restart HA 😀

…scribe-retry # Conflicts: # app/src/main/res/xml/changelog_master.xml

markfrancisonly · 2026-07-19T17:27:40Z

Relationship to home-assistant/core#176833 (mobile_app: keep the push channel registered, degrade routing on repeated confirm timeouts)

The two PRs are complementary halves of the same user-visible failure — not connected to local push notifications — and are fully independent: neither depends on the other and they can land in any order.

Failure	Who can detect it	Fixed by
Socket alive, but the server tore down the push channel after one late notification confirmation	Only the server — the app-level ping added here gets its pong (the socket is fine), so no client change can see this	core#176833: the channel stays registered; repeated timeouts degrade routing to cloud with a periodic local probe instead of tearing down
Socket dead (server restart, NAT timeout, network roam) with the old server process gone	Only the client — no server code can inform a client holding a socket to an endpoint that no longer exists	This PR: the unanswered ping cancels the connection and the restore loop retries with backoff until subscriptions are re-established

They also compose cleanly: when the restore loop here re-subscribes, the server replaces its push channel object, which resets the degraded/breaker state from core#176833 — a reconnect always starts with a clean local push path.

Tracks each re-subscription attempt as a temporary ActiveMessage.Reconnecting entry keyed by the id the server will use, while the original subscription stays under its old id until the attempt is acknowledged and promoted. This replaces the duplicate Subscription entries (and the awaitClose cleanup loop) previously created during re-subscription, so findSubscription stays unambiguous and orphans cannot linger. Events arriving for the new id before the acknowledgement is processed are routed to the original subscription's flow. Also hardens the restore loop per review: - close() cancels the running restore loop so a shutdown cannot leave it re-establishing the connection it just closed - a subscription the server rejects is retried indefinitely with capped backoff instead of being silently abandoned; dropping it would leave its collector waiting on a flow that can never emit again - an unanswered resubscription cancels the socket: the server may have accepted it with the answer lost, so restoration resumes on a clean connection instead of risking a duplicate registration; only the connection the attempt was sent on is cancelled, never a replacement established while waiting - closing a subscription also unsubscribes the pending id of any attempt still in flight for it - the worker's restart of stopped collectors is now covered by a test, with the work dispatcher injectable for virtual time - shouldAttemptReconnect names the reconnect condition and reconnectJob is annotated with GuardedBy(connectedMutex) - drops the changelog entry Client half of the local push fix: home-assistant/core#176833 keeps the server-side push channel registered across confirm timeouts, while this change makes the client detect dead sockets and retry until its subscriptions are restored. A successful re-subscribe replaces the server's push channel, which also resets its degraded state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

markfrancisonly · 2026-07-19T18:47:10Z

CI note: the Emulator.wtf APK build hit D8: java.lang.OutOfMemoryError: Java heap space while merging external-dependency dex archives. Compilation passed and this branch does not change any dependencies, so this looks like a runner flake — closing/reopening to retrigger.

markfrancisonly · 2026-07-19T18:54:53Z

Thanks for the review! Everything is addressed in 3a4e4b3 (today's push — the review predates it):

shutdown: close() now cancels the restore loop under connectedMutex before closing the socket
the changelog entry is dropped
shouldAttemptReconnect stays as a named variable, and reconnectJob carries @GuardedBy("connectedMutex")
rejected subscriptions retry with capped exponential backoff (10 s → 2 min) instead of a fixed 10 s loop
and the bigger one: your explicit-type idea is implemented — re-subscription attempts are a dedicated ActiveMessage.Reconnecting tracked under the pending id, the duplicate Subscription entries and the awaitClose sweep are gone (details in the inline replies)

Glad the ping safeguard reads well!

Duplicate terminal callbacks of the same closing raced the reconnect decision: the first callback transitioned the connection state before the cleanup coroutines ran, so the second captured wasActive = false — and whichever coroutine won the holder decided with its own stale snapshot. Losing that race left the subscriptions disconnected, exactly what this change is meant to eliminate. The snapshot, state transition, close reason consumption and holder removal now all happen in the same connectedMutex critical section: nothing is captured at callback time anymore, only the coroutine that wins the holder decides on reconnection, and a duplicate serializes behind it, finds the state already Closed and no holder, and exits. A callback arriving after close() still transitions the state without starting a restore loop, since close() already removed the holder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

markfrancisonly · 2026-07-19T20:07:30Z

The duplicate-close race is fixed in 3bb983f: the state snapshot, transition, close-reason consumption and holder removal now happen in one connectedMutex critical section, so nothing is captured at callback time and only the coroutine winning the holder decides on reconnection. Includes a regression test delivering both terminal callbacks before cleanup runs, asserting exactly one restore loop starts. Ready for another look @TimoPtr.

markfrancisonly · 2026-07-19T22:41:08Z

Delivering on the Home Assistant promise to be local-first, this PR and the corresponding core home-assistant/core#176833 solve a critical android local notification reliability issue.

Without a solid notification communication channel, no android wallpanel/kiosk notification command works reliably. My other PRs #7162 and #7163 are dependent on this PR.

I'm out of time to iterate further — maintainers, please feel free to push any refactoring or polish directly to the branch. I'll be production testing tonight on my SM-X110s and will report back. Hope you enjoy these changes!

TimoPtr · 2026-07-21T07:24:47Z

Hi @markfrancisonly I invite you to read our recently published AI policy https://github.com/home-assistant/android/blob/main/AI_POLICY.md from this point I expect to speak to a human only, you can use LLMs to shape your answer but let's keep it between humans.

TimoPtr

Thanks for pushing this through. It does cover edge cases and should improve the stability of the app. I have few requests still to harden it even more. Then I would like @jpelgrom to also look at it since it's a quite important/fragile part of the app.

TimoPtr · 2026-07-21T07:56:13Z

+    @VisibleForTesting
+    internal var dispatcher: CoroutineDispatcher = Dispatchers.IO


Move this above the EntryPoint definition.

TimoPtr · 2026-07-21T08:21:59Z

+                Timber.w("${toRestore.size} subscriptions not restored, retrying in $retryDelay")
+            } else {
+                if (getConnectionState() == WebSocketState.ClosedAuth) {
+                    Timber.e("Authentication failed, not retrying to resubscribe to active subscriptions")


When this happens the subscriptions stay in activeMessages but are never resubscribed. Even if auth is fixed later (a new token for instance), the next successful connection comes up with zero registrations on the server side and nothing restores them until the next socket closing.

The more I look at this, the more I think the resubscription trigger belongs at the end of connect() rather than in the closing path: when a fresh connection is established and activeMessages still contains Subscription entries from a previous connection, start the restore job for them. Semantically it reads as "connect, then re-register anything left over from the previous connection". A fresh socket has no registrations by definition, so any tracked subscription at that point needs restoring. This matters especially because any sendMessage can theoretically reconnect the WebSocket (the worker's ping does this every 30s) and leave the subscriptions tracked but not subscribed.

Note: it would need to launch the job (guarded by reconnectJob like today) rather than resubscribe inline: inline it would deadlock on connectedMutex and make unrelated sendMessage callers wait on the restoration. The closing-path loop would still be needed to actively retry while the server is unreachable; the connect() trigger would be the safety net for every other way a connection comes back.

This is an idea to explore.

The more I look at this, the more I think the resubscription trigger belongs at the end of connect() rather than in the closing path: when a fresh connection is established and activeMessages still contains Subscription entries from a previous connection, start the restore job for them. Semantically it reads as "connect, then re-register anything left over from the previous connection".

This makes logical sense. However, one of the reasons for putting it in the closing was to ensure a delay before retrying. Most connection failures are temporary due to a network change - immediately retrying will fail but a couple of seconds later there is a network again so it will work. The delay before attempting to setup a new connection should stay.

I agree, I want to move only the logic of opening again the pending subscription out of this and directly in the connect path. I'm going to propose something within the PR

TimoPtr · 2026-07-21T08:40:16Z

+    }
+
+    @Test
+    fun `Given a resubscription rejected on a live connection Then it is retried until restored`() = runTest {


Reformulate to add When

TimoPtr · 2026-07-21T08:40:38Z

+    }
+
+    @Test
+    fun `Given a subscription the server keeps rejecting Then it is retried with backoff and never abandoned`() = runTest {


Reformulate to add When

TimoPtr · 2026-07-21T09:20:28Z

+                val wasActive = previousState == WebSocketState.Active
+                pendingCloseReason = null
+
+                // Transition to closed state unless already closed


Suggested change

// Transition to closed state unless already closed

TimoPtr · 2026-07-21T09:21:05Z

+                // Snapshot and transition under the mutex: a duplicate terminal callback of the
+                // same closing serializes behind the first one, sees the Closed state and no
+                // holder, and can neither suppress nor repeat the reconnect decision


I'm not sure this comments brings a lot of value here. @jpelgrom wdyt?

No, we already have comments in this function every couple of lines and mutexes avoiding two calls racing each other is basics we don't need in a comment.

TimoPtr · 2026-07-21T09:29:47Z

+        class Resubscription(request: WebSocketRequest, val original: ActiveMessage.Subscription) :
+            WithAnswer(request) {
+
+            override fun toActiveMessage(): ActiveMessage {
+                return ActiveMessage.Reconnecting(responseDeferred, original)
+            }
+        }


Suggested change

class Resubscription(request: WebSocketRequest, val original: ActiveMessage.Subscription) :

WithAnswer(request) {

override fun toActiveMessage(): ActiveMessage {

return ActiveMessage.Reconnecting(responseDeferred, original)

}

}

class Resubscription(val original: ActiveMessage.Subscription) :

WithAnswer(original.request) {

override fun toActiveMessage(): ActiveMessage {

return ActiveMessage.Reconnecting(responseDeferred, original)

}

}

Let's avoid mistake by only taking the original as parameter.

TimoPtr · 2026-07-21T09:31:07Z

+            val response = sendMessage(
+                Command.WithAnswer.Resubscription(request = original.request, original = original),
+            )


Suggested change

val response = sendMessage(

Command.WithAnswer.Resubscription(request = original.request, original = original),

)

val response = sendMessage(Command.WithAnswer.Resubscription(original))

TimoPtr · 2026-07-21T09:43:17Z

-            // TODO https://github.com/home-assistant/android/issues/5259 handle re-connection gracefully or terminates the flows
-            Timber.w("Unable to reconnect, cannot resubscribe to active subscriptions")
+
+                response.success != true -> {


With this it means that an always failing request for instance if a template fail it will retry with the backoff all the time until it is addressed.

TimoPtr · 2026-07-28T08:46:08Z

@markfrancisonly I did push to your PR to address my comments and made the logic changes. It would be nice if you have some time to look at my changes (even if it's merged I'll address the comments).

jpelgrom

Thanks for picking this up @TimoPtr. I'm not seeing any issues while testing and your comments make sense, OK to merge this to see if it improves as far as I'm concerned.

Code comments are a bit much in my opinion, especially in the tests which are a distinctly different style, but that's partially your style and the AI gen origins I'm afraid. I've added some suggestions for the worst ones.

Co-authored-by: Joris Pelgröm <jpelgrom@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 11, 2026 17:22

home-assistant Bot added the cla-signed label Jul 11, 2026

Copilot started reviewing on behalf of markfrancisonly July 11, 2026 17:22 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

Comment thread ...in/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketCoreImpl.kt Outdated

Comment thread ...in/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/WebSocketCoreImpl.kt Outdated

markfrancisonly mentioned this pull request Jul 11, 2026

Local Notifications sometimes failing (device I'd, webhook) #6926

Closed

markfrancisonly and others added 3 commits July 12, 2026 07:52

Merge branch 'main' into fix/websocket-resubscribe-retry

2e636e4

markfrancisonly changed the title ~~Retry reconnecting the WebSocket while subscriptions are active~~ Detect silently dropped WebSocket connections and retry until subscriptions are restored Jul 12, 2026

markfrancisonly mentioned this pull request Jul 13, 2026

mobile_app: one late local push confirm tears down the push channel without notifying the subscriber, leaving the device permanently 'not connected to local push' home-assistant/core#176371

Open

TimoPtr requested changes Jul 13, 2026

View reviewed changes

home-assistant Bot marked this pull request as draft July 13, 2026 12:47

markfrancisonly mentioned this pull request Jul 19, 2026

Fix mobile_app local push failing permanently after a missed confirmation home-assistant/core#176833

Open

21 tasks

Merge remote-tracking branch 'upstream/main' into fix/websocket-resub…

d60e7a5

…scribe-retry # Conflicts: # app/src/main/res/xml/changelog_master.xml

markfrancisonly closed this Jul 19, 2026

markfrancisonly reopened this Jul 19, 2026

Merge branch 'main' into fix/websocket-resubscribe-retry

9960362

markfrancisonly requested a review from TimoPtr July 19, 2026 19:00

markfrancisonly marked this pull request as ready for review July 19, 2026 22:43

TimoPtr reviewed Jul 21, 2026

View reviewed changes

TimoPtr added 3 commits July 28, 2026 10:23

Move resubscription within connect

28c5d56

Address remaining comments

ea186d7

Merge remote-tracking branch 'origin/main' into pull/7164

08e61d0

TimoPtr requested review from TimoPtr and jpelgrom July 28, 2026 08:46

TimoPtr approved these changes Jul 28, 2026

View reviewed changes

jpelgrom approved these changes Jul 28, 2026

View reviewed changes

TimoPtr and others added 2 commits July 29, 2026 11:05

Apply suggestions from code review

2e979bc

Co-authored-by: Joris Pelgröm <jpelgrom@users.noreply.github.com>

Properly nest test for WebSocketCoreImplTest

53f6b47

TimoPtr enabled auto-merge (squash) July 29, 2026 09:21

TimoPtr merged commit 6d227b3 into home-assistant:main Jul 29, 2026
20 checks passed

		@VisibleForTesting
		internal var dispatcher: CoroutineDispatcher = Dispatchers.IO

Uh oh!

Uh oh!

Conversation

markfrancisonly commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Any other notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

markfrancisonly commented Jul 11, 2026

Uh oh!

TimoPtr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

home-assistant Bot commented Jul 13, 2026

Uh oh!

GimliTheGreat commented Jul 17, 2026

Uh oh!

markfrancisonly commented Jul 19, 2026

Uh oh!

markfrancisonly commented Jul 19, 2026

Uh oh!

markfrancisonly commented Jul 19, 2026

Uh oh!

markfrancisonly commented Jul 19, 2026

Uh oh!

markfrancisonly commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TimoPtr commented Jul 21, 2026

Uh oh!

TimoPtr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimoPtr commented Jul 28, 2026

Uh oh!

jpelgrom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

markfrancisonly commented Jul 11, 2026 •

edited

Loading

markfrancisonly commented Jul 19, 2026 •

edited

Loading