blastoise/todo/websocket-robustness/overview.md

5.8 KiB
Raw Permalink Blame History

WebSocket Client Robustness: Parse Guard, Reconnect Backoff, Dead-Channel Fallback

Priority: High · Effort: LowMedium · Risk: Low

Problem

The WebSocket client in public/channelSync.js is brittle in three ways that can leave the UI silently desynced or hammering a dead server/channel:

  1. JSON.parse(e.data) has no try/catch (channelSync.js:262). A single malformed or partial frame throws, the onmessage handler aborts, and the client stops processing all further state updates while the socket appears "open."
  2. Reconnect has no backoff, no jitter, no max-retries (channelSync.js:353-369). A down server produces a fixed 23s reconnect storm with no cap.
  3. No fallback to the default channel. If the current channel is deleted server-side while the client is disconnected, every reconnect 404s and the client loops forever against a dead channel ID.

Separately, the message dispatcher falls through to M.handleUpdate(data) for any unknown data.type (channelSync.js:346), so a new server message type silently corrupts state. Worth hardening in the same pass.

Affected Locations

  • public/channelSync.js:262 — unguarded JSON.parse
  • public/channelSync.js:346handleUpdate fallback for unknown types
  • public/channelSync.js:353-369 — reconnect logic
  • public/channelSync.js:245-261 — socket setup (reference for structure)

Implementation Plan

Step 1 — Wrap JSON.parse in try/catch

M.ws.onmessage = (e) => {
  let data;
  try {
    data = JSON.parse(e.data);
  } catch (err) {
    console.warn("[channelSync] Dropping malformed WS frame:", err);
    return;
  }
  // ... existing dispatch ...
};

Keep behavior: a bad frame is dropped, the socket stays open, subsequent good frames still process.

Step 2 — Strict dispatch with explicit unknown-type handling

Replace the implicit fall-through to M.handleUpdate(data). Make the dispatch explicit:

switch (data.type) {
  case "channel_list": /* ... */ break;
  case "switched":     /* ... */ break;
  case "track":        /* fallthrough */
  case "state":        M.handleUpdate(data); break;   // explicit state-shape types only
  // ... all other known types ...
  default:
    console.warn("[channelSync] Unknown WS message type:", data.type, data);
}

(If the server currently sends type values other than the ones handled, gather the full set first via grep for "type": / type: " in the server broadcast paths and list them in this switch.)

Step 3 — Reconnect with exponential backoff + jitter + max attempts

Track reconnect state on M (e.g. M.wsReconnect = { attempts: 0, timer: null }). On close:

M.ws.onclose = () => {
  M.ws = null;
  // clear any user-facing "connected" state
  if (!M.wantSync) return;

  const recon = M.wsReconnect;
  recon.attempts++;
  const base = 1000;                 // 1s
  const cap = 30000;                 // 30s max
  const exp = Math.min(cap, base * 2 ** recon.attempts);
  const jitter = Math.random() * 500; // 0500ms
  const delay = exp + jitter;

  recon.timer = setTimeout(() => {
    connectChannel(M.currentChannelId);
  }, delay);
};

Reset recon.attempts = 0 on a successful open. Cancel recon.timer on any explicit/manual connectChannel call to avoid double-connects.

On max attempts: rather than giving up entirely (a music app should keep trying), cap the delay at 30s but keep retrying. Optionally surface a "reconnecting…" indicator after the first few failures so the user knows the stream is stale.

Step 4 — Dead-channel fallback

In connectChannel, if the upgrade/open fails or the server responds with a channel-not-found error message (the server already sends { type: "error", message: "Channel not found" } per websocket.ts:50), fall back to the default channel:

// inside onmessage error handler:
if (data.type === "error" && /not found/i.test(data.message)) {
  console.warn(`[channelSync] Channel ${M.currentChannelId} not found, falling back to default`);
  // fetch default channel id from /api/channels (isDefault === true)
  const def = await fetchDefaultChannelId();
  if (def && def !== M.currentChannelId) {
    M.currentChannelId = def;
    M.saveChannelId(def);
    connectChannel(def);
    M.showToast("This channel no longer exists — switched to the default channel.");
    return;
  }
}

Add a small helper fetchDefaultChannelId() that GETs /api/channels and returns the id where isDefault === true. Cache it on M.defaultChannelId after first successful list load so we don't re-fetch on every failure.

Validation

  • Malformed frame: from server console or a test, ws.send("not json") to a client — confirm the client logs a warning and continues processing subsequent valid frames (state still updates).
  • Server kill / restart: stop the server, confirm reconnect attempts grow with backoff (log each attempt + delay) rather than firing every 2s. Restart, confirm the client reconnects and attempts resets.
  • Channel deletion while disconnected: note a non-default channel ID, stop server, delete the channel row from blastoise.db, restart server, ensure the client reconnects → gets "not found" → falls back to the default channel and shows the toast.
  • Dispatch: inject an unknown { type: "future_thing" } message server-side (or via a debug WS send) and confirm it logs a warning and does not call handleUpdate.

Risk / Rollback

  • All changes are additive hardening; normal-path behavior is preserved.
  • Backoff introduces up to a 30s worst-case reconnect delay — acceptable and better than a tight loop. If a deployment wants faster recovery, tune base/cap.
  • Default-channel fallback adds one HTTP fetch on the error path only; cache the result to avoid repeats.
  • Rollback = revert the file; no schema or wire-protocol change.