From 3f2bb22586d3e1aef765149b555ccc8e174ce7eb Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 8 Sep 2026 13:20:29 +0200 Subject: fix(hub): make the streamed download path reliable, and clean up after an abort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Firefox and Safari the service worker is the only unbounded way to write a download to disk: neither has the File System Access API, and OPFS is not a substitute — measured on Firefox 154, its quota is exactly 10% of the volume's size (389,233,459 bytes of a 3,892,334,592-byte volume, refused to the byte), which a film exceeds. So when this path declines, a large download has nowhere left to go, which makes its reliability a correctness property. Four ways it declined, all of them avoidable: - it was registered inside the first click on Download, so that click paid install, activate and claim while somebody watched a button do nothing; - `_swReady` cached a null for the life of the page. One slow first click left the tab unable to stream anything again, curable only by a reload nobody knew to do. Only a successful controller is remembered now; - control was waited for with a 3 s cap. It is 15 s, and a page that is active but not controlled asks the worker to claim again (`mbdl-claim`) instead of declaring the path unavailable; - a missed navigation gave up at once. It gets a second attempt with a fresh id and iframe, the failed one torn down completely first. Also closes a MessagePort leaked per download, and gives the reason a name (`lastStreamFailure`) so a refusal can say what happened. The timeouts became parameters: the defaults are the production values, no caller passes any, and the tests do not spend a minute waiting. `openTarget` gets an unrelated but adjacent fix, in the same file: it creates the destination with `getFileHandle({create: true})`, so an empty file exists before the first byte, and `abort()` leaves the target untouched — every cancelled download left a 0-byte file behind, and since `freeName` avoids collisions, three cancels left film.mkv, film (2).mkv and film (3).mkv, all empty. Its `abort()` now removes the entry. Safe here and only here, because `freeName` guarantees the name was not taken: the `showSaveFilePicker` path must not do the same, where the person may have picked an existing file whose contents `abort()` correctly preserves. Verified by hand in Chrome. test_streamed_download_reliability.py runs the real module under Node against a stubbed browser — it fails if the null is cached again. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST --- packages/meshbay-hub/tests/test_downloads.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'packages/meshbay-hub/tests/test_downloads.py') diff --git a/packages/meshbay-hub/tests/test_downloads.py b/packages/meshbay-hub/tests/test_downloads.py index 32d3e11..fc69fac 100644 --- a/packages/meshbay-hub/tests/test_downloads.py +++ b/packages/meshbay-hub/tests/test_downloads.py @@ -149,8 +149,12 @@ def test_a_length_is_only_promised_when_it_is_known(tmp_path): # and was lifted into file-utils.js's downloadDirectory (docs/photos.md # §3) so photos-app.js's own "zip this album" button calls the same # implementation rather than a second one. + # Anchored on the call, not on how its result is bound: the assignment + # became a bare `target = ...` inside a try when _openDownloadTarget gained + # the ability to refuse an oversized download (test_memory_ceiling.py). + # What this test is about -- the `0` -- did not move. app = (STATIC / "file-utils.js").read_text() - zip_call = app[app.index("const target = await _openDownloadTarget(suggested"):] + zip_call = app[app.index("_openDownloadTarget(suggested"):] zip_call = zip_call[:zip_call.index(");") + 2] assert zip_call.rstrip().endswith(", 0);"), ( "the zip download announces a Content-Length it will not match") @@ -201,9 +205,15 @@ def test_the_streamed_path_gives_up_rather_than_blocking_for_ever(): def test_an_uncontrolled_page_is_not_treated_as_ready(): """`registration.active` says a worker exists, not that it will see our fetch.""" + # Anchored on the streaming section rather than on one function: waiting + # for control moved into `_awaitControl`/`_claimController` when the budget + # became a parameter, and `serviceWorker()` no longer contains the words. + # The behaviour itself is executed in test_streamed_download_reliability.py; + # this stays as the cheap guard on the module's shape. src = DOWNLOADS.read_text() - fn = src[src.index("async function serviceWorker()"):] - fn = fn[:fn.index("\n}")] - assert "navigator.serviceWorker.controller" in fn - assert "controllerchange" in fn, ( + section = src[src.index("// ── Streaming to disk"):] + assert "navigator.serviceWorker.controller" in section + assert "controllerchange" in section, ( "control can arrive a tick after registration; waiting beats refusing") + assert "mbdl-claim" in section, ( + "an active-but-uncontrolled page must ask for a claim, not give up") -- cgit v1.2.3