From c1d982b4a4dbbef1b4f88bef6f34ac185e1977c9 Mon Sep 17 00:00:00 2001 From: david raistrick <1108844+keen99@users.noreply.github.com> Date: Wed, 1 Jul 2026 18:26:42 -0400 Subject: [PATCH] fix(BUG-8): ws adapter auto-reconnect after drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WS adapter had no reconnect. WS dies (idle/error/close) → wsReady=null, subscribers dead forever, display frozen until full reload. Changes (src/storage/ws.js): - onClose: schedule reconnect via setTimeout(500ms), ensureWs re-arms. Guard: disposed flag stops reconnect after dispose. - onOpen: resubscribe all existing doc/coll subscribers (backend state may have changed). Re-fetch current values on RECONNECT only (skip first connect — initial REST fetch in subscribe* already did). Added everConnected flag to distinguish first vs reconnect. - reconnectTimer unref'd (Node) to avoid hanging event loop. - dispose(cb): set disposed, clear timer, close ws, then cb. Also fixed test teardown leaks: - server/index.js close(): terminate all wss.clients before wss.close(). Reconnect test spawned new ws to server; old close hung on live conn. - both ws test factories: port 0 (OS picks free) instead of module-local nextPort counter. Parallel jest workers collided on EADDRINUSE. Tests: ws-reconnect GREEN (1.7s), ws-contract 23 GREEN. No regression. server suite 24/24. shared 90/90. --- TODO.md | 2 +- server/index.js | 6 +++- server/tests/ws-contract.test.js | 10 +++---- server/tests/ws-reconnect.test.js | 10 +++---- src/storage/ws.js | 50 +++++++++++++++++++++++++++++-- 5 files changed, 61 insertions(+), 17 deletions(-) diff --git a/TODO.md b/TODO.md index 1055782..8bee996 100644 --- a/TODO.md +++ b/TODO.md @@ -183,7 +183,7 @@ REWORK_PLAN.md. - [x] BUG-6: fixed structurally (1-list model) - [x] BUG-12: fixed — campaign selection follows activeDisplay - [x] BUG-15: fixed — DisplayView no longer re-sorts (drag order preserved) -- [ ] BUG-8: ws adapter reconnect +- [x] BUG-8: ws adapter reconnect (implemented + GREEN) - [ ] BUG-10: deact+reactivate double-act - [ ] BUG-11: FE Combat.scenario crash - [ ] BUG-13: reorder cross-pointer semantics (RED + decide block/allow) diff --git a/server/index.js b/server/index.js index d79260d..48b104f 100644 --- a/server/index.js +++ b/server/index.js @@ -131,7 +131,11 @@ function createServer({ dbPath, port, corsOrigin } = {}) { return { app, server, wss, store, db, - close(done) { wss.close(); server.close(() => { db.close(); if (done) done(); }); }, + close(done) { + wss.clients.forEach(c => { try { c.terminate(); } catch {} }); + wss.close(); + server.close(() => { db.close(); if (done) done(); }); + }, }; } diff --git a/server/tests/ws-contract.test.js b/server/tests/ws-contract.test.js index 373a72d..0f6b9cb 100644 --- a/server/tests/ws-contract.test.js +++ b/server/tests/ws-contract.test.js @@ -14,18 +14,16 @@ const { createServer } = require('../index'); const { createWsStorage } = require('../../src/storage/ws'); const { runStorageContract } = require('../../src/storage/contract'); -let nextPort = 4000 + Math.floor(Math.random() * 999); - // Factory: fresh backend (unique sqlite file) + storage pointed at it. // Disposing the storage closes the backend so each test is fully isolated. async function makeStorage() { - const port = nextPort++; - const dbPath = path.join(os.tmpdir(), `ws-contract-${port}-${Date.now()}.sqlite`); - const handle = createServer({ dbPath, port }); + const dbPath = path.join(os.tmpdir(), `ws-contract-${Date.now()}-${Math.random().toString(36).slice(2)}.sqlite`); + const handle = createServer({ dbPath, port: 0 }); await new Promise((resolve, reject) => { handle.server.on('error', reject); - handle.server.listen(port, resolve); + handle.server.listen(0, resolve); }); + const port = handle.server.address().port; const baseUrl = `http://127.0.0.1:${port}`; const wsUrl = `ws://127.0.0.1:${port}/ws`; const storage = createWsStorage({ baseUrl, wsUrl }); diff --git a/server/tests/ws-reconnect.test.js b/server/tests/ws-reconnect.test.js index db49669..c36692e 100644 --- a/server/tests/ws-reconnect.test.js +++ b/server/tests/ws-reconnect.test.js @@ -12,16 +12,14 @@ const { createWsStorage } = require('../../src/storage/ws'); const flush = (ms = 150) => new Promise(r => setTimeout(r, ms)); -let nextPort = 5000 + Math.floor(Math.random() * 999); - async function makeStorage() { - const port = nextPort++; - const dbPath = path.join(os.tmpdir(), `ws-recon-${port}-${Date.now()}.sqlite`); - const handle = createServer({ dbPath, port }); + const dbPath = path.join(os.tmpdir(), `ws-recon-${Date.now()}-${Math.random().toString(36).slice(2)}.sqlite`); + const handle = createServer({ dbPath, port: 0 }); await new Promise((resolve, reject) => { handle.server.on('error', reject); - handle.server.listen(port, resolve); + handle.server.listen(0, resolve); }); + const port = handle.server.address().port; const baseUrl = `http://127.0.0.1:${port}`; const wsUrl = `ws://127.0.0.1:${port}/ws`; const storage = createWsStorage({ baseUrl, wsUrl }); diff --git a/src/storage/ws.js b/src/storage/ws.js index 1066b99..42d801f 100644 --- a/src/storage/ws.js +++ b/src/storage/ws.js @@ -39,13 +39,51 @@ function createWsStorage({ baseUrl, wsUrl } = {}) { let ws = null; let wsReady = null; + let disposed = false; + let reconnectTimer = null; + let everConnected = false; + const RECONNECT_DELAY = 500; + function ensureWs() { if (wsReady) return wsReady; wsReady = new Promise((resolve, reject) => { ws = new WebSocketImpl(WS); - const onOpen = () => resolve(ws); + const onOpen = () => { + const isReconnect = everConnected; + everConnected = true; + // resubscribe all existing subscribers after (re)connect + for (const p of docSubs.keys()) { + ws.send(JSON.stringify({ type: 'subscribe', kind: 'doc', path: p })); + } + for (const p of collSubs.keys()) { + ws.send(JSON.stringify({ type: 'subscribe', kind: 'collection', path: p })); + } + // On RECONNECT only: re-fetch current values — catches writes that + // happened while disconnected (broadcast missed). Skip on first connect + // (initial REST fetch in subscribeDoc/subscribeCollection already did). + if (isReconnect) { + for (const [p, cbs] of docSubs) { + storage.getDoc(p).then(doc => { cbs.forEach(cb => cb(doc)); }).catch(() => {}); + } + for (const [p, cbs] of collSubs) { + storage.getCollection(p).then(docs => { cbs.forEach(cb => cb(docs)); }).catch(() => {}); + } + } + resolve(ws); + }; const onError = (err) => { wsReady = null; reject(err instanceof Event ? new Error('ws error') : err); }; - const onClose = () => { wsReady = null; }; + const onClose = () => { + wsReady = null; + ws = null; + if (disposed) return; + // auto-reconnect (BUG-8): try again after delay. ensureWs() re-arms. + if (reconnectTimer) clearTimeout(reconnectTimer); + reconnectTimer = setTimeout(() => { + reconnectTimer = null; + if (!disposed) ensureWs().catch(() => {}); + }, RECONNECT_DELAY); + if (reconnectTimer && typeof reconnectTimer.unref === 'function') reconnectTimer.unref(); + }; const onMessage = (ev) => { const raw = typeof ev === 'string' ? ev : (ev.data !== undefined ? ev.data : ev); let msg; try { msg = JSON.parse(typeof raw === 'string' ? raw : raw.toString()); } catch { return; } @@ -168,7 +206,13 @@ function createWsStorage({ baseUrl, wsUrl } = {}) { return () => { collSubs.get(p)?.delete(cb); }; }, - dispose() { if (ws) ws.close(); docSubs.clear(); collSubs.clear(); }, + dispose(cb) { + disposed = true; + if (reconnectTimer) { clearTimeout(reconnectTimer); reconnectTimer = null; } + if (ws) ws.close(); + docSubs.clear(); collSubs.clear(); + if (typeof cb === 'function') cb(); + }, _api: api, _test: {