From d4adb140b9e7250b0f02d9651e4b9db87b74df81 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Wed, 9 Sep 2026 11:21:50 +0200 Subject: fix(spa): stop reloading a healthy page at boot Reported from Chrome: connecting to a group triggered a page refresh within seconds, taking the WebRTC session down with it. The boot check added with the bypass repair asked its question by *performing a download* -- a four-byte stream through a hidden iframe. Chrome rations the downloads a page may start without a user gesture to about three, measured: on a first visit three consecutive attempts went served, served, refused. So the check competed with the person's own downloads for that budget, and its answer depended on how much of the budget was left. On a healthy page it concluded the worker could not serve, and reloaded. The same mistake the repair was written to fix, from the other side: paying a capability to obtain a diagnostic. The replacement costs nothing and asks nothing. Measured on Chrome, at document start, before anything registers: first visit controller false, registration false ordinary reload controller true, registration true hard reload controller false, registration true Being uncontrolled while an active registration already exists names a hard-reloaded document exactly, so that is now the whole of the evidence. A first visit is uncontrolled too and is not a bypass -- the worker is installing and will claim the page in a moment -- which is precisely the case that was reloading. Four cases, each checked against the unfixed source, including that priming performs no download at all. Hub suite 847 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST --- .../src/meshbay_hub/static/downloads.js | 111 ++++++++------------- .../tests/test_streamed_download_reliability.py | 104 ++++++++++--------- 2 files changed, 99 insertions(+), 116 deletions(-) diff --git a/packages/meshbay-hub/src/meshbay_hub/static/downloads.js b/packages/meshbay-hub/src/meshbay_hub/static/downloads.js index 6ee7cf0..ce1901c 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/downloads.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/downloads.js @@ -380,8 +380,8 @@ async function _claimController(budgetMs) { */ export function primeServiceWorker() { if (!STREAMS_VIA_SW) return; - _priming = serviceWorker() - .then((worker) => worker && _repairIfBypassed(worker)) + _priming = _repairIfBypassed() + .then(() => serviceWorker()) .catch(() => {}); } @@ -391,85 +391,58 @@ export function primeServiceWorker() { // path that is about to be repaired. let _priming = null; -// How long to wait for the worker to answer the one-byte self-test below. -// Milliseconds when it works; a page that cannot stream at all is worth four -// seconds to find out about, once, at boot. -const SELF_TEST_BUDGET_MS = 4000; -// Set for the life of this tab, so the repair below can happen at most once and +// Set for the life of this tab, so the repair below happens at most once and // can never become a reload loop. const REPAIRED_KEY = 'meshbay.sw-repaired'; /** - * Can this document actually have a download served, or only talk to the worker? + * Was this document loaded with the service worker bypassed? * - * Being controlled is not the same thing, and the gap between them is a real - * failure people hit. A document fetched by a **hard** reload — Ctrl+F5, - * Ctrl+Shift+R — is loaded with the service worker bypassed. It can still be - * claimed afterwards, so `navigator.serviceWorker.controller` comes back and - * every check in `_claimController` passes; but the navigations that document - * starts keep missing the worker, and the hidden iframe a streamed download - * needs *is* a navigation. Every download then fails with "the worker did not - * answer", for the life of that page — on Firefox and Safari, the only path - * there is for a file too large to hold in memory. + * A document fetched by a **hard** reload — Ctrl+F5, Ctrl+Shift+R — is loaded + * with the worker bypassed. It can still be claimed afterwards, so + * `navigator.serviceWorker.controller` comes back and every control check + * passes; but the navigations it starts keep missing the worker, and the hidden + * iframe a streamed download needs *is* a navigation. On Firefox and Safari + * that is the only way to write a file too large to hold in memory, so every + * download fails for the life of that page. * - * Reported after an operator was told, by this author, to hard-reload after - * each deployment: four downloads out of four worked on a freshly started - * browser, and the first attempt after a Ctrl+F5 failed, every time. + * Measured on Chrome, at document start, before anything registers: * - * This asks the question directly rather than inferring it: a four-byte stream - * and a hidden iframe, exactly as a real download would. - */ -async function _canServeDownloads(worker) { - const id = `selftest-${Math.random().toString(36).slice(2, 10)}`; - let readable, writable; - try { - ({ readable, writable } = new TransformStream()); - } catch { - return true; // No transferable streams: a different failure, not this one. - } - const chan = new MessageChannel(); - const serving = new Promise((resolve) => { - chan.port1.onmessage = (e) => { - if (e.data && e.data.type === 'mbdl-serving') resolve(true); - }; - }); - try { - worker.postMessage({ type: 'mbdl', id, filename: 'meshbay-selftest.bin', - size: 4, readable, port: chan.port2 }, - [readable, chan.port2]); - } catch { - return true; // Same: not the bypass this is looking for. - } - const frame = document.createElement('iframe'); - frame.hidden = true; - frame.src = `${PREFIX_PATH}${id}`; - document.body.appendChild(frame); - const served = await Promise.race([ - serving, - new Promise((r) => setTimeout(() => r(false), SELF_TEST_BUDGET_MS)), - ]); - frame.remove(); - try { chan.port1.close(); } catch { /* already gone */ } - // Never completed, so the browser has nothing to save and no file appears. - try { await writable.abort('self-test'); } catch { /* already gone */ } - return served; -} - -/** - * An ordinary reload puts the document back under the worker, so do that once. + * first visit controller false, registration false + * ordinary reload controller true, registration true + * hard reload controller false, registration true + * + * So being uncontrolled while an active registration already exists names the + * case exactly, and costs nothing to ask. * - * Only at boot, where nothing is in flight and the reload costs a flicker. - * Guarded by a session flag rather than a variable: the point is to survive the - * reload it triggers, and to stop rather than loop if reloading does not help. + * The first version of this asked by *performing a download* — a four-byte + * stream through a hidden iframe — which was both unreliable and expensive: + * Chrome rations downloads a page starts without a user gesture to about three, + * so the test competed with the person's real downloads for that budget and its + * answer depended on how many had been spent. It reloaded a healthy page on + * every first visit, taking the group's WebRTC session down with it. */ -async function _repairIfBypassed(worker) { +const _controlledAtLoad = STREAMS_VIA_SW + && Boolean(navigator.serviceWorker.controller); +// Started here, at module load, because `register()` would make the answer +// true whatever it was. +const _registeredAtLoad = STREAMS_VIA_SW + ? navigator.serviceWorker.getRegistration('/') + .then((reg) => Boolean(reg && reg.active)).catch(() => false) + : Promise.resolve(false); + +/** An ordinary reload puts the document back under the worker, so do that once. */ +async function _repairIfBypassed() { + if (_controlledAtLoad) return; + // Uncontrolled with nothing registered is a first visit, not a bypass: the + // worker is being installed right now and the page is fine after it claims. + if (!await _registeredAtLoad) return; let repaired = false; try { repaired = sessionStorage.getItem(REPAIRED_KEY) === '1'; } catch { /* blocked */ } if (repaired) return; - if (await _canServeDownloads(worker)) return; - console.warn('[MeshBay] this page cannot be served by the download worker — ' - + 'reloading once to put it back under the worker\u2019s control ' - + '(a hard reload leaves a page in this state)'); + console.warn('[MeshBay] this page was loaded with the download worker ' + + 'bypassed (a hard reload does that) — reloading once to put it ' + + 'back under the worker\u2019s control'); try { sessionStorage.setItem(REPAIRED_KEY, '1'); } catch { /* blocked */ } location.reload(); } diff --git a/packages/meshbay-hub/tests/test_streamed_download_reliability.py b/packages/meshbay-hub/tests/test_streamed_download_reliability.py index da745d0..bfd4a89 100644 --- a/packages/meshbay-hub/tests/test_streamed_download_reliability.py +++ b/packages/meshbay-hub/tests/test_streamed_download_reliability.py @@ -81,6 +81,10 @@ Object.defineProperty(globalThis, 'navigator', { value: { serviceWorker: { get controller() { return controller; }, + // What the document started with, which is the whole of the repair's + // evidence now. `getRegistration` is asked before anything registers. + getRegistration: async () => (PLAN.registeredAtLoad + ? {active: makeController()} : undefined), register: async () => { log.registers += 1; if (PLAN.registerThrows) throw new Error('registration blocked'); @@ -118,6 +122,8 @@ Object.defineProperty(globalThis, 'navigator', { globalThis.window = globalThis; globalThis.isSecureContext = true; +// Set before the module is imported, because it reads it at evaluation. +controller = %(controlled)s ? makeController() : null; // The self-test's repair reloads once and remembers it for the tab; both have // to exist here or priming the worker throws instead of repairing. const session = new Map(); @@ -160,7 +166,8 @@ const out = {}; def _run(tmp_path, body, *, control_after_ms=0, active=True, serve="always", register_throws=False, control_budget_ms=800, ready_settles=True, register_hangs=False, - active_after_unregister=False, control_on_claim=False): + active_after_unregister=False, control_on_claim=False, + controlled_at_load=False, registered_at_load=False): module = tmp_path / "downloads.mjs" module.write_text(DOWNLOADS.read_text()) (tmp_path / "package.json").write_text('{"type":"module"}') @@ -173,11 +180,13 @@ def _run(tmp_path, body, *, control_after_ms=0, active=True, "registerHangs": register_hangs, "activeAfterUnregister": active_after_unregister, "controlOnClaim": control_on_claim, + "registeredAtLoad": registered_at_load, } script = tmp_path / "case.mjs" script.write_text( (PRELUDE % {"plan": json.dumps(plan), "module": module.as_posix(), - "control": control_budget_ms}) + "control": control_budget_ms, + "controlled": json.dumps(controlled_at_load)}) + body + "\nout.log = log;\nconsole.log(JSON.stringify(out));\n") proc = subprocess.run(["node", str(script)], capture_output=True, text=True, @@ -398,68 +407,70 @@ if (target) await target.writable.close(); "discarding it did not get the page a worker it could stream to") -# ── A page the worker cannot serve ────────────────────────────────────────── +# ── A page loaded with the worker bypassed ────────────────────────────────── -def test_a_page_the_worker_cannot_serve_reloads_itself_once(tmp_path): - """Being controlled is not being servable, and the gap is a real failure. - A document fetched by a hard reload — Ctrl+F5, Ctrl+Shift+R — is loaded with - the service worker bypassed. It can be claimed afterwards, so `controller` - comes back and every check in `_claimController` passes; but the navigations - it starts keep missing the worker, and the hidden iframe a streamed download - needs is a navigation. Every download then fails with "the worker did not - answer" for the life of that page — on Firefox and Safari, the only path - there is for a file too large to hold in memory. +def test_a_hard_reloaded_page_reloads_itself_once(tmp_path): + """Uncontrolled at load while an active registration already exists is a + document fetched by a hard reload — Ctrl+F5, Ctrl+Shift+R — and nothing + else. Measured on Chrome at document start: a first visit has neither, an + ordinary reload has both, a hard reload has the registration and no + controller. - Reported after an operator was told to hard-reload after each deployment: - four downloads out of four worked on a freshly started browser, and the - first attempt after a Ctrl+F5 failed, every time. An ordinary reload puts - the document back under the worker, so priming does exactly that, once. + Such a page can still be claimed, so every control check passes; but the + navigations it starts keep missing the worker, and the hidden iframe a + streamed download needs is one. On Firefox and Safari that is the only way + to write a file too large to hold in memory. An ordinary reload undoes it. """ out = _run(tmp_path, """ M.primeServiceWorker(); - await new Promise((r) => setTimeout(r, 6000)); + await new Promise((r) => setTimeout(r, 200)); out.reloads = log.reloads; - """, serve="never") - assert out["reloads"] == 1, ( - "a page that cannot be served by the worker was left that way") + """, controlled_at_load=False, registered_at_load=True) + assert out["reloads"] == 1 -def test_a_page_that_works_is_not_reloaded(tmp_path): - """The self-test costs milliseconds when it passes, and must cost nothing - else. Reloading a healthy page at boot would be a flicker on every visit.""" +def test_a_first_visit_is_not_a_bypass(tmp_path): + """Also uncontrolled at load, and perfectly healthy: the worker is being + installed right now and will claim the page in a moment. Reloading here + would be a flicker on everybody's first visit — and it was, taking the + group's WebRTC session down with it when it landed mid-connection.""" out = _run(tmp_path, """ M.primeServiceWorker(); - await new Promise((r) => setTimeout(r, 3000)); + await new Promise((r) => setTimeout(r, 200)); out.reloads = log.reloads; - """) + """, controlled_at_load=False, registered_at_load=False) + assert out["reloads"] == 0 + + +def test_a_controlled_page_does_not_reload(tmp_path): + """The ordinary case, which must cost nothing at all: no reload, and no + download spent asking. Chrome rations the downloads a page may start + without a user gesture to about three, and the first version of this check + asked its question by performing one — competing with the person's own + downloads for that budget.""" + out = _run(tmp_path, """ + M.primeServiceWorker(); + await new Promise((r) => setTimeout(r, 200)); + out.reloads = log.reloads; + out.navigations = log.navigations; + """, controlled_at_load=True, registered_at_load=True) assert out["reloads"] == 0 + assert out["navigations"] == 0, ( + "priming performed a download; that budget belongs to the person") def test_the_repair_happens_at_most_once(tmp_path): """The flag is in sessionStorage rather than a variable because the point is - to survive the reload it triggers. If reloading does not help, the page - stays broken and says so — it does not reload again, and again.""" + to survive the reload it triggers, and because a page that is still bypassed + afterwards must stop rather than reload again, and again.""" out = _run(tmp_path, """ sessionStorage.setItem('meshbay.sw-repaired', '1'); M.primeServiceWorker(); - await new Promise((r) => setTimeout(r, 6000)); + await new Promise((r) => setTimeout(r, 200)); out.reloads = log.reloads; - """, serve="never") - assert out["reloads"] == 0, "a page that had already been repaired reloaded again" - - -def test_the_self_test_leaves_no_file_behind(tmp_path): - """It opens a real download target to ask a real question, so it must also - tear it down: a completed one would drop `meshbay-selftest.bin` into the - download folder on every page load.""" - src = DOWNLOADS.read_text() - fn = src[src.index("async function _canServeDownloads"):] - fn = fn[:fn.index("\n}\n")] - assert "writable.abort" in fn, ( - "the self-test's stream is never aborted, so the browser keeps what it " - "was given") - assert "frame.remove" in fn + """, controlled_at_load=False, registered_at_load=True) + assert out["reloads"] == 0 # ── The claim is asked for, not waited for ────────────────────────────────── @@ -487,10 +498,9 @@ def test_an_uncontrolled_page_asks_at_once_rather_than_after_the_budget(tmp_path "after the wait, not before it") -def test_a_download_waits_for_the_self_test(tmp_path): - """A click that lands while the check is still running must not race it. - On a page that turns out to be unservable it would otherwise spend the full - two attempts failing on a path that is about to be repaired.""" +def test_a_download_waits_for_priming(tmp_path): + """A click that lands while priming is still running must not race it: on a + page about to reload, the attempt would fail for nothing.""" out = _run(tmp_path, """ M.primeServiceWorker(); const t0 = Date.now(); -- cgit v1.2.3