From a104ccc95c700c53a695844f8d0d60bf7927793d Mon Sep 17 00:00:00 2001 From: maziggy Date: Sun, 7 Jun 2026 15:40:22 +0200 Subject: [PATCH] fix(pwa): don't force-navigate first-install clients in SW activate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repro on every fresh demo subdomain: visitor lands on Printers OK, first sidebar click sticks on a spinner (Chromium) or trips "Corrupted Content Error" with sw.js stuck `activating` (Firefox). Manual reload recovers. Root cause is the `client.navigate(client.url)` call added to the activate handler in 18d534c9 (intended to force kiosks to pick up new bundles). Its guard — `client.url && typeof client.navigate === 'function'` — does not distinguish first install from upgrade. On any fresh origin (every demo session, every first-time visitor, every cleared profile) it still fired: Chromium raced it against the in-flight SPA mount; Firefox deadlocked the activate's waitUntil on `await client.navigate(...)` because the SW intercepts its own document fetch while still activating. Split the lifecycle correctly: - sw.js activate handler: just cache cleanup + clients.claim(). - sw-register.js: capture `hadController = !!serviceWorker.controller` at load, listen for `controllerchange`, reload only when hadController was true. Returning kiosk hits a new deploy -> had controller -> reloads as before; first-install visitor -> no controller -> no forced nav -> React mount completes. Bump CACHE_NAME v29->v30 and STATIC_CACHE v28->v29 so existing browsers fetching the new sw.js drop the old CacheStorage in the same pass. SpoolBuddy unregister branch and notificationclick's client.navigate are unrelated and unchanged. --- CHANGELOG.md | 2 ++ frontend/public/sw-register.js | 15 +++++++++++++ frontend/public/sw.js | 39 ++++++++++++---------------------- static/sw-register.js | 15 +++++++++++++ static/sw.js | 39 ++++++++++++---------------------- 5 files changed, 58 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c60aea36..f08a81488 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to Bambuddy will be documented in this file. ## [0.2.5b1] - Unreleased ### Fixed +- **Service-worker activate handler no longer hangs first-install browsers (demo site stuck spinner + Firefox Corrupted-Content)** — Reproduced live on the demo platform: a visitor lands on `{session}.demo.bambuddy.cool/`, the Printers page renders, but clicking any sidebar entry sticks the next page on a spinner; only a manual reload recovers. In Firefox the same race surfaces as a "Corrupted Content Error" with `sw.js` stuck in `activating` for the entire session. **Root cause:** the `client.navigate(client.url)` call added to the `activate` handler in `sw.js` (commit `18d534c9`, shipped 2026-06-04 alongside the Orca Cloud landing) was intended to force kiosks running an old SW to reload after a deploy, but its only guard was `client.url && typeof client.navigate === 'function'` — neither distinguishes a first install from an upgrade. On every fresh origin (every demo session is a new subdomain, but also any browser visiting Bambuddy for the first time, or after clearing site data) the activate handler still fired the forced navigation: Chromium raced it against React Router's in-flight SPA mount and wedged the page; Firefox's `event.waitUntil` deadlocked on `await client.navigate(...)` because the SW intercepts its own document fetch while still `activating`, the document load aborts, and the SW never reaches `activated`. The "first install on a never-controlled client" guard the commit's comment claimed simply didn't exist in code. **Fix: split the lifecycle correctly.** `sw.js` activate handler is reduced to cache cleanup + `clients.claim()` (matches the standard PWA lifecycle and lets activation complete in low single-digit ms regardless of in-flight document state). The deploy-pickup reload moves to `sw-register.js`: capture `hadController = !!navigator.serviceWorker.controller` at script load (true ⇔ a previous SW was controlling the document), listen for `controllerchange`, and only `location.reload()` when `hadController` was true. A returning kiosk hits a new deploy → had a controller → reloads as before. A first-install visitor (no prior SW, or hard-refresh, or first demo session) → no controller → no forced navigation → React mount completes cleanly. `CACHE_NAME` bumped `bambuddy-v29 → bambuddy-v30` and `STATIC_CACHE` `bambuddy-static-v28 → bambuddy-static-v29` so existing browsers fetching the new `sw.js` drop the old CacheStorage in the same pass — without the bump the SW file byte content might equal the cached one and the upgrade installs nothing. The SpoolBuddy-kiosk unregister branch at the top of `sw-register.js` is unchanged (still wipes registrations on `/spoolbuddy` paths). The `notificationclick` handler in `sw.js` (open-tab-on-push) still uses `client.navigate(url)` — different code path, unrelated, unchanged. + - **VP archive/queue names with `&` no longer render as `&amp;` + tooltip corrected for BambuStudio 2.7.x reality (#1658 follow-up, reported by @IndividualGhost1905)** — Two bugs surfaced on the same screenshot set: (A) Metadata-mode archive and queue names showed `PCB Vise &amp; Solder Station` where the 3MF's Title metadata is `PCB Vise & Solder Station`. **Root cause:** `ThreeMFParser._parse_3dmodel` (`backend/app/services/archive.py:495-538`) parsed the XML `…` payload via regex and stripped whitespace but never called `html.unescape()`. The raw `&` landed in the DB; React then auto-escaped the `&` again on render, producing `&amp;`. The sibling parser `ProjectPageParser` (line 754) already had a loop-until-stable unescape and a comment explaining why ("content is often triple-encoded" — observed BambuStudio behavior), the makerworld-fields path just didn't share it. **Fix:** module-level `import html` and the same loop-until-stable unescape pattern in `_parse_3dmodel`, applied uniformly to all `` values so `Title`, `Designer`, and any future fields all get peeled the same way. The loop terminates as soon as `html.unescape()` stops changing the string, so single-, double-, and triple-encoded payloads all converge to the correct value; plain ASCII passes through untouched. (B) Filename-mode showed the slugified project title (`PCB_Vise_&_Solder_Station`) instead of the user-typed Send-dialog text ("Main Parts"). **This is NOT a Bambuddy bug** — BambuStudio source confirms it. `PrintJob.cpp:314-325` (`src/slic3r/GUI/Jobs/PrintJob.cpp`) reads `BBL_DESIGNER_MODEL_TITLE_TAG` (defined as `"Title"` in `bbs_3mf.hpp`) from the 3MF, slugifies it (space → `_`, unusable chars `<>[]:/\|?*"` → `_`, collapse runs of `_`, truncate to 100 chars), and **unconditionally overwrites** the user-typed `m_project_name` with it before sending. `params.project_name` becomes both the FTP filename and the MQTT `subtask_name`. The user-typed string never leaves BambuStudio when a Title metadata exists — there is no MQTT field carrying it, so Bambuddy has no recovery path. The previous tooltip ("handy if you renamed the job in the 'send to printer' dialog") promised something BambuStudio strips, and the previous reply to the reporter dismissed this as "OrcaSlicer-style upload, working as designed" which was wrong on BambuStudio 2.7.1.57. **Fix:** tooltip rewritten in all 11 locales (de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW) to spell out the BambuStudio behavior — both modes often produce the same string because BS overwrites the Send-dialog name with the 3MF Title field when present. **Tests**: 3 new in `test_archive_service.py::TestThreeMFMetadataHTMLUnescape` — `Title` with `&` unescapes to `&` (the reporter's exact case), `Title` with triple-encoded `&amp;amp;` peels all three layers (the BambuStudio worst-case ProjectPageParser already documents), plain `Title=Benchy` passes through unchanged (regression guard against accidentally munging non-encoded payloads). Full 104-test archive suite green; ruff clean; i18n parity holds (5065 leaves × 11 locales); frontend build clean. - **FTP passive-port pool now sliced per-VP (10 ports each) so bridge-mode Docker drops from ~3.5 GB to ~210 MB host RAM (#1646, reported by @TheFou — followed up with corrections we acted on)** — Reporter on a Linux Docker VM (`network_mode: host` not viable because other containers already bind the same ports) measured 2002 `docker-proxy` host processes spawned from the previously-exposed `50000-51000:50000-51000` range — one process per port per address family, ~3.5 MB RSS each, ~3.5 GB total that doesn't show up in `docker stats` because it's host-level not container-level. **Root cause: shared port pool, treated as symptom not cause.** `VirtualPrinterFTPServer` exposed `PASSIVE_PORT_MIN/MAX` as **class constants** (`backend/app/services/virtual_printer/ftp_server.py:573-574`), so every VP's FTP session passed the same `(50000, 51000)` range into `_bind_passive_port` and competed on the same 0.0.0.0 binds. The widening from 100 → 1001 ports in an earlier round had been collision-avoidance headroom for multi-VP-on-shared-bind, but the cost was paid by every install — including the reporter's single-VP install that only ever needed ~10 ports of headroom. **Fix: per-VP non-overlapping slices, allocated by VP id.** New module-level `compute_passive_port_slice(vp_id) → (port_min, port_max)` returns a 10-port window: VP id 1 → 50000-50009, VP id 2 → 50010-50019, …, VP id 100 → 50990-50999. Class constants are gone; `VirtualPrinterFTPServer.__init__` now takes `passive_port_min` / `passive_port_max` instance args. `manager.py` computes the slice at server-construction time from `self.id` and passes it in. Result for the reporter (single VP): 10 exposed ports → 20 docker-proxy processes → ~70 MB instead of ~3.5 GB. Three VPs → 30 ports → ~210 MB. **Wrap-around behaviour pinned**: VP ids beyond `PASSIVE_MAX_SLOTS = 100` wrap modulo 100 (an install that's churned through many VPs over time still produces a valid in-range slice). A same-slot collision (vp_id 101 lands on the same slice as vp_id 1) falls back to the per-session 10-attempt random retry that pre-#1646 code already had — same recovery, no regression. **Compose default narrowed**: `docker-compose.yml` now exposes `50000-50029:50000-50029` by default (covers 3 VPs out of the box) instead of the 1001-port range. The comment explains how to widen for more VPs (`50000-500N9` for `N = vp_count - 1`) and that proxy-mode VPs still need `50000-50100:50000-50100` because proxy mode forwards the real printer's full range — that codepath uses a separate `TCPProxy.FTP_DATA_PORT_MIN/MAX` and isn't sliced (the real printer owns that range, not Bambuddy). **Doc corrections in the same drop**: the previous warning over-stated `userland-proxy: false` as "confirmed by the reporter" — TheFou had flagged it as theoretical, not tested; the new comment doesn't push it as a recommendation at all (it's a global daemon flag, too blunt for a per-container problem). The new comment also explicitly names Linux multi-service hosts (NAS, dedicated Docker VMs, Unraid, Synology DSM) as a primary bridge-mode audience instead of leaving the warning under a "macOS/Windows" framing that TheFou pointed out missed his use case. Acknowledges that host-mode default is a deliberate trade-off for SSDP discovery, not a security-blind default. **Tests**: 10 new in `test_vp_ftp_port_slicing.py` — `compute_passive_port_slice` pins: vp_id=1 starts at base, consecutive vp_ids get adjacent non-overlapping slices, no two distinct vp_ids within MAX_SLOTS share a port (exhaustive across all 100 slots), wraps modulo MAX_SLOTS, top slot stays within the documented pool, non-positive vp_ids clamp to slot 0 (defensive — never produce a negative port that would crash `asyncio.start_server`). Two `VirtualPrinterFTPServer` instance tests pin: two instances constructed with different slices stay independent (regression guard against re-introducing class-level state), default-arg construction yields a valid one-slice window. Existing proxy-mode test at `test_virtual_printer.py:2269` (101 ports for `_ftp_data_proxies`) stays green — that path is unchanged. Full 130-test VP suite green. diff --git a/frontend/public/sw-register.js b/frontend/public/sw-register.js index 3b174266e..220886e86 100644 --- a/frontend/public/sw-register.js +++ b/frontend/public/sw-register.js @@ -9,6 +9,21 @@ if ('serviceWorker' in navigator) { } }); } else { + // Capture controller state at script-load. Used to decide whether a + // subsequent `controllerchange` is a deploy-pickup (had a prior SW → + // reload so the new bundle takes over) or a first install (no prior SW → + // skip the reload; the in-flight React mount would otherwise race the + // forced navigation, leaving the page wedged on a spinner. The previous + // approach — `client.navigate(client.url)` from the SW's activate + // handler — exhibited that race in Chromium and a waitUntil hang in + // Firefox, both surfaced on every fresh demo subdomain). + const hadController = !!navigator.serviceWorker.controller; + let reloading = false; + navigator.serviceWorker.addEventListener('controllerchange', () => { + if (!hadController || reloading) return; + reloading = true; + location.reload(); + }); window.addEventListener('load', () => { navigator.serviceWorker.register('/sw.js') .then((registration) => { diff --git a/frontend/public/sw.js b/frontend/public/sw.js index f7bb88ea4..42035c068 100644 --- a/frontend/public/sw.js +++ b/frontend/public/sw.js @@ -1,6 +1,6 @@ // Bambuddy Service Worker -const CACHE_NAME = 'bambuddy-v29'; -const STATIC_CACHE = 'bambuddy-static-v28'; +const CACHE_NAME = 'bambuddy-v30'; +const STATIC_CACHE = 'bambuddy-static-v29'; // Static assets to cache on install const STATIC_ASSETS = [ @@ -31,12 +31,17 @@ self.addEventListener('install', (event) => { self.skipWaiting(); }); -// Activate event - clean up old caches, then force-reload any controlled -// windows so they pick up the new bundle. Important for the SpoolBuddy kiosk -// (Pi + Chromium-in-kiosk-mode, no devtools, no manual reload control): -// without this hop, restarting Chromium installs the new SW but the existing -// document keeps running the previously-cached bundle until a navigation -// happens — which on a locked kiosk never occurs. +// Activate event - clean up old caches and claim existing clients. +// +// The forced reload that picks up a new bundle on already-open clients (the +// kiosk deploy-pickup scenario) lives in sw-register.js via a +// `controllerchange` listener, gated on whether the page already had a SW +// controller at load time. That gate distinguishes first-install (where a +// reload would race the in-flight React mount — observed on every fresh +// *.demo.bambuddy.cool subdomain, and in Firefox the activate's waitUntil +// hung on `client.navigate` until the document load was aborted with a +// Corrupted-Content error) from upgrade-on-existing-client (where the reload +// is wanted). self.addEventListener('activate', (event) => { console.log('[SW] Activating service worker...'); event.waitUntil( @@ -50,25 +55,7 @@ self.addEventListener('activate', (event) => { return caches.delete(name); }), ); - // Take control immediately. await self.clients.claim(); - // Force a fresh navigation in any window that this SW now controls. - // ``client.navigate(client.url)`` re-requests the page through the - // network-first fetch handler, picking up the new index.html + the - // new content-hashed JS bundle. Guarded so the very first install on - // a never-controlled client doesn't trigger an unwanted reload. - const clients = await self.clients.matchAll({ type: 'window' }); - for (const client of clients) { - try { - if (client.url && typeof client.navigate === 'function') { - await client.navigate(client.url); - } - } catch (e) { - // Some browsers reject navigate on cross-origin or detached - // clients — swallow so one bad client doesn't break the rest. - console.warn('[SW] Forced reload skipped for client:', client.url, e); - } - } })(), ); }); diff --git a/static/sw-register.js b/static/sw-register.js index 3b174266e..220886e86 100644 --- a/static/sw-register.js +++ b/static/sw-register.js @@ -9,6 +9,21 @@ if ('serviceWorker' in navigator) { } }); } else { + // Capture controller state at script-load. Used to decide whether a + // subsequent `controllerchange` is a deploy-pickup (had a prior SW → + // reload so the new bundle takes over) or a first install (no prior SW → + // skip the reload; the in-flight React mount would otherwise race the + // forced navigation, leaving the page wedged on a spinner. The previous + // approach — `client.navigate(client.url)` from the SW's activate + // handler — exhibited that race in Chromium and a waitUntil hang in + // Firefox, both surfaced on every fresh demo subdomain). + const hadController = !!navigator.serviceWorker.controller; + let reloading = false; + navigator.serviceWorker.addEventListener('controllerchange', () => { + if (!hadController || reloading) return; + reloading = true; + location.reload(); + }); window.addEventListener('load', () => { navigator.serviceWorker.register('/sw.js') .then((registration) => { diff --git a/static/sw.js b/static/sw.js index f7bb88ea4..42035c068 100644 --- a/static/sw.js +++ b/static/sw.js @@ -1,6 +1,6 @@ // Bambuddy Service Worker -const CACHE_NAME = 'bambuddy-v29'; -const STATIC_CACHE = 'bambuddy-static-v28'; +const CACHE_NAME = 'bambuddy-v30'; +const STATIC_CACHE = 'bambuddy-static-v29'; // Static assets to cache on install const STATIC_ASSETS = [ @@ -31,12 +31,17 @@ self.addEventListener('install', (event) => { self.skipWaiting(); }); -// Activate event - clean up old caches, then force-reload any controlled -// windows so they pick up the new bundle. Important for the SpoolBuddy kiosk -// (Pi + Chromium-in-kiosk-mode, no devtools, no manual reload control): -// without this hop, restarting Chromium installs the new SW but the existing -// document keeps running the previously-cached bundle until a navigation -// happens — which on a locked kiosk never occurs. +// Activate event - clean up old caches and claim existing clients. +// +// The forced reload that picks up a new bundle on already-open clients (the +// kiosk deploy-pickup scenario) lives in sw-register.js via a +// `controllerchange` listener, gated on whether the page already had a SW +// controller at load time. That gate distinguishes first-install (where a +// reload would race the in-flight React mount — observed on every fresh +// *.demo.bambuddy.cool subdomain, and in Firefox the activate's waitUntil +// hung on `client.navigate` until the document load was aborted with a +// Corrupted-Content error) from upgrade-on-existing-client (where the reload +// is wanted). self.addEventListener('activate', (event) => { console.log('[SW] Activating service worker...'); event.waitUntil( @@ -50,25 +55,7 @@ self.addEventListener('activate', (event) => { return caches.delete(name); }), ); - // Take control immediately. await self.clients.claim(); - // Force a fresh navigation in any window that this SW now controls. - // ``client.navigate(client.url)`` re-requests the page through the - // network-first fetch handler, picking up the new index.html + the - // new content-hashed JS bundle. Guarded so the very first install on - // a never-controlled client doesn't trigger an unwanted reload. - const clients = await self.clients.matchAll({ type: 'window' }); - for (const client of clients) { - try { - if (client.url && typeof client.navigate === 'function') { - await client.navigate(client.url); - } - } catch (e) { - // Some browsers reject navigate on cross-origin or detached - // clients — swallow so one bad client doesn't break the rest. - console.warn('[SW] Forced reload skipped for client:', client.url, e); - } - } })(), ); });