diff options
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/static/downloads.js | 84 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_streamed_download_reliability.py | 115 |
2 files changed, 186 insertions, 13 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/downloads.js b/packages/meshbay-hub/src/meshbay_hub/static/downloads.js index cfb0051..7e4802a 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/downloads.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/downloads.js @@ -261,19 +261,89 @@ function _awaitControl(budgetMs) { }); } +/** + * The promise's value, or `TIMED_OUT` once `ms` is spent. + * + * A rejection is still a rejection — the caller reports those — and the timer + * is cleared either way, so nothing is left running behind a fast answer. + */ +const TIMED_OUT = Symbol('timed out'); + +function _within(promise, ms) { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => resolve(TIMED_OUT), ms); + promise.then((v) => { clearTimeout(timer); resolve(v); }, + (err) => { clearTimeout(timer); reject(err); }); + }); +} + async function _claimController(budgetMs) { - const reg = await navigator.serviceWorker.register(SW_PATH, { scope: '/' }); - // `ready` resolves on an *active* registration; being active is not being in - // control. An uncontrolled page's requests never reach the fetch handler, so - // the worker would take our stream and never be asked for it — the download - // then freezes after exactly one chunk, which is how this was found. - await navigator.serviceWorker.ready; - const controller = await _awaitControl(budgetMs); + // Every wait in here is inside one budget. Neither of the first two used to + // have any deadline at all, and `_swPromise` is shared, so a single one of + // them left every download on the page waiting on the same promise for ever + // — four rows stuck at "preparing", with nothing in the node's journal + // because no transfer had been asked for yet. + const deadline = Date.now() + budgetMs; + const left = () => Math.max(0, deadline - Date.now()); + + let reg = await _within( + navigator.serviceWorker.register(SW_PATH, { scope: '/' }), left()); + if (reg === TIMED_OUT) { + _lastFailure = `the worker did not register within ${budgetMs / 1000}s`; + return null; + } + // `register()` resolves as soon as the registration object exists, with + // nothing but an *installing* worker; `ready` is what waits for an active + // one. A worker that never finishes installing leaves `ready` pending + // indefinitely — measured on Firefox 154: an install handler that rejects + // leaves `ready` unsettled past ten seconds while `register()` returns in + // seven milliseconds. + let ready = await _within(navigator.serviceWorker.ready, left()); + if (ready === TIMED_OUT && !reg.active) { + // A registration stuck with nothing but an installing worker does not heal + // on its own: every later visit finds the same registration and waits on + // the same `ready`. Left alone it is permanent, and it costs Firefox the + // only unbounded way it has to write a download to disk — so the stuck + // registration is thrown away and asked for once more, with its own budget, + // rather than reported and lived with. + console.warn('[MeshBay] the download worker never became active; ' + + 'discarding the registration and asking again'); + try { + await _within(reg.unregister(), budgetMs); + } catch (err) { + console.warn('[MeshBay] could not discard it:', err.message); + } + const again = await _within( + navigator.serviceWorker.register(SW_PATH, { scope: '/' }), budgetMs); + if (again === TIMED_OUT) { + _lastFailure = `the worker did not register within ${budgetMs / 1000}s`; + return null; + } + reg = again; + ready = await _within(navigator.serviceWorker.ready, budgetMs); + if (ready === TIMED_OUT && !reg.active) { + _lastFailure = `the worker did not become active within ${budgetMs / 1000}s` + + ', even after its registration was discarded'; + return null; + } + } + // Past here `ready` may still be waiting on a *newer* worker that cannot + // install while an older one is perfectly able to serve. An active worker is + // all this path needs, so a stuck `ready` is not on its own a reason to give + // up a capability Firefox has nothing else to offer for. + // + // Being active is not being in control, either. An uncontrolled page's + // requests never reach the fetch handler, so the worker would take our stream + // and never be asked for it — the download then freezes after exactly one + // chunk, which is how that was found. + const controller = await _awaitControl(left()); if (controller) return controller; // Active but not controlling after the whole budget. `sw.js` calls // `clients.claim()` on activate, so this is rare; when it happens the page // was loaded before any worker existed and the claim was missed. Ask the // active worker to claim again rather than declare the path unavailable. + // This wait is deliberately outside the budget above: giving up here would + // cost Firefox the only unbounded way it has to write a download to disk. if (reg.active) { try { reg.active.postMessage({ type: 'mbdl-claim' }); } catch { /* gone */ } return await _awaitControl(2000); diff --git a/packages/meshbay-hub/tests/test_streamed_download_reliability.py b/packages/meshbay-hub/tests/test_streamed_download_reliability.py index 45138b4..9e6f77d 100644 --- a/packages/meshbay-hub/tests/test_streamed_download_reliability.py +++ b/packages/meshbay-hub/tests/test_streamed_download_reliability.py @@ -49,7 +49,8 @@ globalThis.localStorage = { removeItem: k => store.delete(k), }; const PLAN = %(plan)s; -const log = { registers: 0, claims: 0, navigations: 0, served: 0 }; +const log = { registers: 0, claims: 0, navigations: 0, served: 0, + unregisters: 0 }; // The worker as the page sees it: something with postMessage. It answers a // navigation by posting mbdl-serving back on the port it was handed, which is @@ -75,15 +76,32 @@ Object.defineProperty(globalThis, 'navigator', { register: async () => { log.registers += 1; if (PLAN.registerThrows) throw new Error('registration blocked'); - if (PLAN.controlAfterMs !== null) { + // A registration that never answers at all. Distinct from one that + // rejects: nothing is reported, nothing fails, the caller just waits. + if (PLAN.registerHangs) await new Promise(() => {}); + // A worker that only becomes installable once the stuck registration + // has been thrown away -- the browser this was reported from. + const healed = PLAN.activeAfterUnregister && log.unregisters > 0; + if (PLAN.controlAfterMs !== null || healed) { setTimeout(() => { controller = makeController(); for (const fn of listeners) fn(); - }, PLAN.controlAfterMs); + }, healed ? 0 : PLAN.controlAfterMs); } - return {active: PLAN.active ? makeController() : null}; + return { + active: (PLAN.active || healed) ? makeController() : null, + unregister: async () => { log.unregisters += 1; return true; }, + }; + }, + // `register()` resolves as soon as the registration object exists, with + // nothing but an installing worker; `ready` is what waits for an active + // one. Measured on Firefox 154: an install handler that rejects leaves + // `ready` unsettled past ten seconds while `register()` returns in 7 ms. + get ready() { + const healed = PLAN.activeAfterUnregister && log.unregisters > 0; + return (PLAN.readySettles || healed) + ? Promise.resolve({}) : new Promise(() => {}); }, - ready: Promise.resolve({}), addEventListener: (type, fn) => { if (type === 'controllerchange') listeners.add(fn); }, removeEventListener: (type, fn) => { listeners.delete(fn); }, }, @@ -122,7 +140,9 @@ const out = {}; def _run(tmp_path, body, *, control_after_ms=0, active=True, - serve="always", register_throws=False, control_budget_ms=800): + serve="always", register_throws=False, control_budget_ms=800, + ready_settles=True, register_hangs=False, + active_after_unregister=False): module = tmp_path / "downloads.mjs" module.write_text(DOWNLOADS.read_text()) (tmp_path / "package.json").write_text('{"type":"module"}') @@ -131,6 +151,9 @@ def _run(tmp_path, body, *, control_after_ms=0, active=True, "active": active, "serveOnNavigation": serve, "registerThrows": register_throws, + "readySettles": ready_settles, + "registerHangs": register_hangs, + "activeAfterUnregister": active_after_unregister, } script = tmp_path / "case.mjs" script.write_text( @@ -274,3 +297,83 @@ def test_the_worker_answers_a_re_claim(tmp_path): if sw.js implements the other half.""" sw = (STATIC / "sw.js").read_text() assert "mbdl-claim" in sw and "clients.claim()" in sw + + +# ── Nothing on this path may wait for ever ────────────────────────────────── + +def test_a_worker_that_never_installs_does_not_hang_every_download(tmp_path): + """The one that reached a person: four downloads stuck at "preparing", for + ever, with nothing in the node's journal because no transfer had been asked + for yet. + + `register()` resolves as soon as the registration object exists — with + nothing but an *installing* worker — and `ready` waits for an active one. + Measured on Firefox 154: an install handler that rejects leaves `ready` + unsettled past ten seconds while `register()` returns in seven + milliseconds. Neither had a deadline, and `_swPromise` is shared, so every + download on the page waited on the same promise that would never settle. + """ + out = _run(tmp_path, """ +const t0 = Date.now(); +out.worker = await M.openStreamedDownload('film.mkv', 1, FAST); +out.ms = Date.now() - t0; +out.why = M.lastStreamFailure(); +""", ready_settles=False, active=False, control_after_ms=None, + control_budget_ms=300) + assert out["worker"] is None + assert out["ms"] < 8000, ( + f"gave up after {out['ms']}ms — a budget that is not enforced is not a " + "budget, and the row above it says 'preparing' the whole time") + assert "active" in out["why"], out["why"] + + +def test_a_stuck_ready_does_not_throw_away_a_working_worker(tmp_path): + """`ready` can be waiting on a *newer* worker that cannot install while an + older one is perfectly able to serve. Giving up then would cost Firefox the + only unbounded way it has to write a download to disk — a deadline must + bound the waiting, never remove the capability.""" + out = _run(tmp_path, """ +const target = await M.openStreamedDownload('film.mkv', 1, FAST); +out.target = target !== null; +// Closing stops the keep-alive; left open, its interval keeps this process +// alive well past the test's own timeout. +if (target) await target.writable.close(); +""", ready_settles=False, active=True, control_budget_ms=300) + assert out["target"] is True + + +def test_a_registration_that_never_answers_gives_up_too(tmp_path): + """The other unbounded await. It rejects loudly in the case above; this is + the case where it says nothing at all.""" + out = _run(tmp_path, """ +const t0 = Date.now(); +out.worker = await M.openStreamedDownload('film.mkv', 1, FAST); +out.ms = Date.now() - t0; +out.why = M.lastStreamFailure(); +""", register_hangs=True, active=False, control_after_ms=None, + control_budget_ms=300) + assert out["worker"] is None + assert out["ms"] < 8000, f"gave up after {out['ms']}ms" + assert "register" in out["why"], out["why"] + + +def test_a_registration_stuck_installing_is_discarded_and_asked_for_again(tmp_path): + """A deadline turns an invisible hang into a named failure, which is better + but is not a fix: a registration stuck with nothing but an installing worker + does not heal on its own. Every later visit finds the same registration and + waits on the same `ready`, so the browser stays unable to stream a download + until somebody opens developer tools — and on Firefox there is nothing else + that can write a film to disk. + + So the stuck registration is thrown away and asked for once more. + """ + out = _run(tmp_path, """ +const target = await M.openStreamedDownload('film.mkv', 1, FAST); +out.target = target !== null; +if (target) await target.writable.close(); +""", ready_settles=False, active=False, control_after_ms=None, + active_after_unregister=True, control_budget_ms=300) + assert out["log"]["unregisters"] == 1, ( + "the stuck registration was left in place") + assert out["target"] is True, ( + "discarding it did not get the page a worker it could stream to") |