Skip to content

fix(#160): a failed reconnect no longer latches HTTP mode forever - #161

Merged
adnaan merged 2 commits into
mainfrom
fix-160-reconnect-latch
Nov 4, 2026
Merged

adnaan merged 2 commits into
mainfrom
fix-160-reconnect-latch

Conversation

@adnaan

@adnaan adnaan commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #160.

Problem

A page that was live over WebSocket loses its socket, and the reconnect runs while the network is still down: a phone waking from sleep, a Wi-Fi or tailnet blip. WebSocketManager.connect() falls into its catch ("WebSocket open failed, falling back to HTTP"), the client sets useHTTP = true, and the transport is nulled. Every self-healing path skips that state:

  • heartbeatTick returns early on useHTTP and on a missing transport.
  • The visibility handler and the scheduleVisibilityReconnect timer require !useHTTP && readyState !== undefined.

HTTP mode gets no server push, so the page freezes until a reload.

Fix

  • Report why it fell back. WebSocketManager.connect() now sets reason on its result: "unavailable" when the probe says the server has no WebSocket, "failed" when the open errored, closed before opening, or timed out.
  • Mark the socket lost. A reconnect that comes back "failed" marks the socket lost. Actions keep going over HTTP as before, but every reconnect trigger now retries the socket instead of skipping it:
    • heartbeat tick: the first retry comes on the next tick, then the wait doubles from one interval up to a 5-minute cap, and it never stops. A tab left open after its server ended costs one attempt per 5 minutes, not one per tick.
    • return to visible: retries regardless of how long the page was hidden.
    • pageshow (bfcache): retries.
    • window online: new; retries immediately.
  • Unchanged: a server that reports no WebSocket, or a socket that fails on the first connect, still falls back to HTTP for good.

Tests

  • Coverage: tests/heartbeat.test.ts has a new "Lost-socket recovery (A failed reconnect latches the client into HTTP mode forever — live updates stop until a full reload #160)" block with 10 tests. Every test counts constructed WebSockets, i.e. an observable retry, so a gate that still blocks the lost state fails the test instead of passing on a flag check. tests/websocket.test.ts asserts the reason values.
  • Mutation checks: against the old client, 9 of the 10 new tests fail. Reverting only the scheduleVisibilityReconnect gate fails the 4 event-trigger tests.
  • Full suite: npm test passes, 43 suites and 897 tests.
  • Real browser: a new prereview chromedp e2e takes the page offline until the console shows the HTTP fallback, changes server state, then brings the network back with no reload. It fails on v0.25.1 (stale after 20s) and passes with this branch, catching up 600ms after the network returns via online. It lands in prereview after this is released.

Test-harness note

The existing jest fetch stubs build new Response(...), but jsdom has no global Response. So the HEAD probe threw, its catch assumed WebSocket was available, and no test ever read X-LiveTemplate-WebSocket. The new block's stub returns a plain object with headers.get. I left the existing stubs alone; they're still passing by accident.

Follow-up (not in this PR)

The transport has its own backoff (scheduleReconnect: 16s cap, 10 attempts, used only with autoReconnect), and this PR adds a second one. The deeper fix is to keep the transport alive across reconnects instead of rebuilding it. That's a design change for another issue.

🤖 Generated with Claude Code

https://claude.ai/code/session_01H17jLWktjd8RmZim6vf8RJ

A live page whose socket dropped would, if the reconnect ran while the
network was still down (phone wake, Wi-Fi blip), fall back to HTTP and
never try the WebSocket again: heartbeatTick and the visibility paths
all skip HTTP mode / a missing transport, and HTTP gets no server push,
so the page froze until a reload.

WebSocketManager.connect() now says why it fell back
(reason: "unavailable" from the probe, "failed" from the open
error/timeout). A RE-connect that fails with "failed" marks the socket
lost: actions keep going over HTTP, but the heartbeat tick (next tick,
then doubling, capped at 5 min, never stopping), a return to visible,
pageshow and the window `online` event all retry it. A server with no
WebSocket, or a first-connect failure, still falls back for good.

Tests count constructed sockets (an observable retry) per trigger;
mutation-checked against the old client (9/10 fail) and per gate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H17jLWktjd8RmZim6vf8RJ
@claude

claude Bot commented Nov 4, 2026

Copy link
Copy Markdown

Review

Overall this looks solid. The lost-socket state is explicit (wsRetryFailures) and doesn't overload useHTTP. reason: "failed" | "unavailable" cleanly separates a permanent fallback from a transient one. disconnect() clears the state and the online listener. The tests count constructed sockets rather than flags, which is the right thing to assert. I found no blocking bugs or security issues.

Minor points, none blocking:

  1. Backoff comment doesn't match the code. The comment on markWSLost says the wait "doubles from one heartbeat interval". The code gives 0 for failure 1, then HB * 2^(n-1), so the first timed wait is 2×HB (n=2). The tests assume 2×HB, so the code is fine and only the comment is off.
  2. Event-triggered retries advance the backoff. An online, pageshow or visibility retry that fails increments wsRetryFailures, so a flapping network can push the next timed retry out faster than elapsed time justifies. The cap is 5 min, so the cost is bounded. Resetting wsRetryAt only for timer-driven failures would be more precise, but this is probably fine.
  3. heartbeatMs = 0 (opt-out). Retries are then event-only, because no tick runs. The online handler comment notes this. Consider mentioning it in the CHANGELOG or docs for apps that opt out.
  4. Jitter. Many clients dropped by the same server restart will retry on the same HB * 2^n schedule. A little jitter would avoid a thundering herd on the server. It's optional, since ticks are already offset by each client's connect time.

LGTM.

Review nits: the first timed wait is 2× the heartbeat (failure 2), not
1×; apps with data-lvt-heartbeat-ms="0" recover on events only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H17jLWktjd8RmZim6vf8RJ
@claude

claude Bot commented Nov 4, 2026

Copy link
Copy Markdown

Review

Overall this looks good. The fix is well-scoped: reason: "failed" | "unavailable" cleanly separates a transient failure from a permanent HTTP fallback, and wsLost is cleared in disconnect() and on every successful or unavailable reconnect. The tests count constructed sockets, so they check real retries rather than flags. I didn't run the suite locally.

No blocking issues. A few things to consider:

  1. Heartbeat timer must outlive the failed reconnect. The retry clock depends on the heartbeat interval still running while wsLost is true. The tests cover this. Please confirm that no onDisconnected/close path calls stopHeartbeat() before the lost state is entered. If one does, the timed retries silently stop and only the event triggers remain.
  2. Event triggers skip the backoff. online, visibilitychange and pageshow retry immediately, with no rate limit beyond the 500ms settle delay. A flapping connection that fires online repeatedly could hit the server with a HEAD probe plus a WS attempt each time. This is probably acceptable. A minimum interval between event-driven attempts would be cheap insurance.
  3. Catch path accounting. In the catch of performVisibilityReconnect, markWSLost() only runs if wsLost was already true. A throw during the first reconnect, before markWSLost is reached, leaves the state unchanged and so is not retried by the lost-socket logic. That is probably fine because the live-socket paths still apply, but it deserves a comment.
  4. Backoff test is slow and loose. It simulates 45 minutes in 10s steps, and the cap + HB tolerance is wide. It works, but a direct unit test of markWSLost wait values would be faster and stricter.

The CHANGELOG entry is clear.

@adnaan
adnaan merged commit 2e14080 into main Nov 4, 2026
7 checks passed
@adnaan
adnaan deleted the fix-160-reconnect-latch branch November 4, 2026 07:25
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.

A failed reconnect latches the client into HTTP mode forever — live updates stop until a full reload

1 participant