mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-27 13:07:56 +00:00
efitenandClaude Opus 5
7f8f71e773
## Problem #1074 reports that after a proxy dropped the WebSocket, live updates only came back 8 to 10 minutes later. The client only reconnects from `onclose` (`public/app.js:791` on master). A half-open connection (a proxy or NAT dropping state without a FIN reaching the browser, a laptop that slept) can keep a WebSocket OPEN for minutes without `onclose`, and nothing retries in the meantime. The server does ping every 30s (`cmd/server/websocket.go:252` on master), but ping frames are answered by the browser below the page and JS cannot observe them. The only app-level frames are packet broadcasts (`websocket.go:337, 343, 366`), which stop on a quiet mesh. So the client had no signal to tell a quiet mesh from a dead socket. ## Change Server (`cmd/server/websocket.go`): - On the existing ping tick, `writePump` also writes the text frame `{"type":"heartbeat"}` (`:283`, bytes at `:80`). One 20-byte frame per client per 30s, from the goroutine that already writes the ping, no hub lock. - The interval moves to `Hub.pingInterval` (default 30s, `:118`) so a test can shorten it. Client (`public/app.js`): - Every frame refreshes `wsLastMessageAt` (`:841`). One timer (`checkWSLiveness`, `:806`) fires at last message + `WS_STALE_MS` (75s, one late or lost heartbeat of slack) and replaces the socket if it is still silent. It is armed at socket creation, so a stuck handshake is covered too. - `dropWS` (`:796`) detaches the old socket's handlers before `close()`, so a late close event cannot schedule a second connection. - `connectWS` (`:818`) cancels a pending reconnect and drops the previous socket, so the watchdog, resume checks, `onclose` and pull-to-reconnect cannot stack sockets. Before this, `pullReconnect` on a non-open socket left a third socket 3s later. The 3s `WS_RECONNECT_MS` delay after `onclose` is unchanged (`:837`). - `visibilitychange` (to visible) and `online` run the check immediately (`:861`), because a hidden or sleeping tab's timers can run late. - Heartbeat frames are matched by exact bytes (`:842`) and are not pulsed or dispatched to `onWS` listeners. Compatibility: tabs loaded before the deploy dispatch heartbeats to their listeners until reloaded. Every current listener filters on `msg.type`, so the visible effect is a logo pulse and a `/stats` cache refresh every 30s. Perf: one `Date.now()` and one string compare per WS message on the client; one extra 20-byte write per client per 30s on the server. ## Tests - `test-ws-stale-watchdog-1074.js`: real `app.js` in a vm with a fake clock, timers and WebSocket. 12 tests: silence past the threshold replaces the socket exactly once; a handshake that never opens is replaced; heartbeats and packet traffic keep the socket; heartbeats are not dispatched; resume and `online` after silence reconnect immediately, with recent traffic they do not, and hiding does not trigger a check; repeated resume events open one socket; after `onclose` only the reconnect timer is pending; pull-to-reconnect leaves one socket. 9 of 12 fail on master. 12 of 12 source mutations (threshold, reconnect path, detaching, timer cleanup, resume wiring, heartbeat filter) are caught. Registered in `test-all.sh` and the deploy.yml unit step. - `TestWritePumpSendsAppHeartbeat`: fails with a read timeout without the heartbeat, even with pings every 20ms. `TestHubDefaultPingInterval` pins the 30s interval that `WS_STALE_MS` assumes. - `go test ./...` in `cmd/server` passes; gofmt and go vet are clean. ## Browser validation On a staging instance (build `139e484e`, together with #1979's branch), in Chrome, no console errors: - A `{"type":"heartbeat"}` frame arrived on the open socket within the observation window. - Silent socket: after `ws.onmessage = null`, the page replaced the socket after 76.2s (threshold 75s plus a 250ms poll); the old socket ended in CLOSED, the new one OPEN. - Normal close: `ws.close()` led to a new OPEN socket after 4.1s, and exactly one new `WebSocket` was constructed. ## Not verified - The reporter's proxy setup was not reproduced; that their delay was a half-open socket is a hypothesis consistent with the symptom. Hence `Refs`, not `Fixes`. - Laptop sleep and the `visibilitychange` / `online` resume path were only covered by the unit test, not in a browser. - Behaviour under Chrome's intensive background-timer throttling and mobile tab freezing was not measured; a frozen but healthy tab may do one unnecessary reconnect on resume. - Go tests were run without `-race`. Refs #1074 ## Review follow-up (commit `72e5e906`) An independent review found no blocking bug: all data writes stay on the write goroutine, pong-based dead-client detection still works, and no ordering of onclose, watchdog, resume checks and pull ends with two live sockets or none. It reproduced the silent-socket case in headless Chromium through a blackholing TCP proxy (replacement 75.0 s after the last frame). Changed: 1. **Startup wiring tested.** The first resume test now boots through the page's real `DOMContentLoaded` listeners, so removing `setupWSResumeCheck()` from startup makes it fail. 2. **Wall clock stepping back.** If the clock steps back after the last message, the watchdog no longer re-arms for the size of the step (a 1 h step used to delay detection by about an hour). A negative silence reading is treated as stale, so the socket is replaced within `WS_STALE_MS` of the step (`public/app.js:810-814`). A step in either direction costs at most one extra reconnect on a healthy socket. `Date.now()` stays the clock so a tab resumed after sleep is still checked against real elapsed time. 3. **Pull-to-reconnect at once.** On an OPEN socket, pull-to-reconnect now replaces it through `connectWS()` instead of closing it and waiting for onclose, which took 63 s on a half-open connection in the review's measurement (`public/app.js:927-934`). This was slow on master too; it is safe now that `connectWS()` detaches the old socket. Tests: 12 to 16 in `test-ws-stale-watchdog-1074.js`; `test-pull-to-reconnect.js`, `test-pull-to-reconnect-1091.js` and `test-live.js` pass. Correction to the compatibility note: tabs opened before the deploy treat the heartbeat like any other message. Besides the logo pulse, `app.js` runs `updateNavStats` on every message and invalidates the cached `/stats` and `/nodes` responses 5 s later; `packets.js` also pushes every message into `pauseBuffer` unfiltered (~1310-1313), so an old tab with Packets paused sees its counter rise by 2 per minute. Cosmetic: heartbeats are filtered out on replay, and a reload ends it. Not verified: real hidden-tab or mobile freeze behaviour, Firefox and Safari, and the reporter's proxy setup. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>