Skip to content

Harden caching, engine updates and the generator (0.2.0 → next) - #31

Open
mike-doctorina wants to merge 6 commits into
masterfrom
fix/audit-30
Open

mike-doctorina wants to merge 6 commits into
masterfrom
fix/audit-30

Conversation

@mike-doctorina

Copy link
Copy Markdown
Collaborator

One PR for everything since 0.2.0. It replaces the stack #27 → #28 → #29 and adds the fixes for the audit in #30. Every change is described in detail in CHANGELOG.md under Unreleased.

Commits

  1. Harden caching, bootstrap deadlines and generator config (was Harden caching, bootstrap deadlines and generator config #27). Covers:
    • --no-web-resources-cdn is honoured;
    • the keep-set keeps the Chromium canvaskit.js;
    • keys with spaces or non-ASCII characters are decoded;
    • an HTML fallback is never cached as a script or wasm;
    • bounded deadlines for foreign-worker recovery and applyUpdate();
    • keep-maps, no-cleanup and comments work from YAML.
  2. Switch to the new worker first when a deploy changes the engine (was Switch to the new worker first when a deploy changes the engine #28). The first load after a Flutter upgrade no longer runs the old Dart code on the new engine.
  3. Keep a failing activate step from erasing caches it does not own (was Keep a failing activate step from erasing caches it does not own #29). Swap, cleanup and navigation preload fail separately. Cache names are matched on a - boundary. Pending installs keep their temp caches.
  4. Fix keys under an encoded scope, version encoding and review findings. Covers:
    • a <base href> with escapes;
    • sw.js?v= encoding;
    • no reload when the page is already on the new worker;
    • stricter flutter_bootstrap.js detection.
  5. Let git work on the container's checkout in the publish check. A CI fix.
  6. Fix the audit findings from Audit: reliability, caching, CLI, CI and UX findings #30.
    • Service worker:
      • Bodies are size-checked against the manifest while they stream into the cache. A transfer that is cut short is never cached.
      • A host that re-encodes a file is tolerated, either through its Content-Length or through a second fetch.
      • Range requests pass through to the network, and only a plain 200 is cached lazily.
      • 5xx and 429 responses are retried.
      • Temp caches left by abandoned installs are removed, and quota errors are logged.
    • Bootstrap:
      • When the app fails on a CDN engine, the page reloads once with the local engine.
      • Engine-init and mount failures are now surfaced.
      • The reset button deletes only this app's caches.
      • Loading widget accessibility and CSP fixes.
    • Generator:
      • Clear errors and exit codes, and warnings for invalid config.
      • MD5 is streamed.
    • Docs: no-cache hosting examples.
    • CI: the publish workflow now runs the tests.

#30 is addressed, not closed. These items are deliberately left for later because the change is riskier than the problem:

  • cache reuse across versions;
  • the double download on the first visit;
  • changes to navigation retry and stall handling;
  • an inline splash;
  • narrowing the public API.

Behaviour changes

  • A CLI usage error exits with 64 (EX_USAGE) instead of 1.
  • The README now recommends Cache-Control: no-cache for every file. Flutter's file names carry no hash.

Tests

  • Vitest: 488 tests; tsc is clean.
  • Dart: 175 unit tests and 14 integration tests; dart analyze is clean.
  • Playwright: 19 tests against the regenerated example.

For each fix in #30, I confirmed that its test fails without the fix.

Service worker:
- Decode percent-escapes in resource keys. The manifest is keyed by the
  file name on disk, so `assets/my photo.png` requested as
  `my%20photo.png` never hit the cache.
- Refuse to cache an HTML page served for a non-HTML file: the app shell
  an SPA rewrite returns with status 200 for a file the deploy lacks.
  Precache fails the entry; the lazy cache skips it.

Bootstrap:
- Bound the foreign-controller cleanup, which runs before the pipeline's
  deadline and waits on the same job queue a stuck install wedges.
- Bound Bootstrap.applyUpdate(): a waiting worker that turns redundant
  never fires controllerchange.
- Register the flutter-first-frame listener before the app loads.

Generator:
- Read keep-maps, no-cleanup and comments from sw.yaml and the
  environment, as documented, instead of from the CLI only.
- Keep flutter_bootstrap.js, with a warning, while index.html still
  loads it; deleting it left a blank page.
- Strip flutter.js's dangling sourceMappingURL from bootstrap.js and
  separate the two scripts with a semicolon.
On the first load after a deploy the previous worker still controls the
page and serves its cached main.dart.*, while index.html and bootstrap.js
are the new build's and the bootstrap loads the engine for the new
revision. On the same engine that mix runs; after a Flutter upgrade it
fails inside the engine.

The worker now reports its build's engineRevision on getVersion. The
bootstrap asks the controller for it:

- Same engine: boot with the controller at once, without the 4s
  activation wait it spent for nothing; the new worker takes over on
  the next load.
- Different or unknown engine (earlier workers don't report it): wait
  for the new worker to install, hand control to it before any app file
  is requested, and reload once if the handover is lost. A failed
  install leaves the page on its controller, as before.
Activating one version deleted the temp cache of a version installing
beside it, so that version activated with an empty content cache. And
any failure during activate, navigation preload included, deleted every
cache starting with the prefix: this version's fresh content cache, a
pending install's temp cache, another app's caches with a longer prefix.

- Cleanup keeps the temp caches of the registration's installing and
  waiting workers, read from their ?v=.
- Swap, cleanup and navigation preload fail separately. A failed swap
  drops only this version's caches and starts over from the network.
- Cache names match on a `-` boundary.

Docs: offline is not supported. The shell loads bootstrap.js, which is
never cached so that shell, bootstrap and worker come from one deploy;
the README no longer promises otherwise. Apps sharing an origin need
distinct prefixes.
Review of everything since 0.2.0, with tests for each finding:

- Decode the scope path like the keys it is compared with, so a base
  href such as /my%20app/ re-bases keys again instead of missing every
  manifest entry and disarming the __/ pass-through.
- Encode the version in sw.js?v=, so a --version like 1.0.0+3 reads
  back unchanged and its pending temp cache survives activate.
- Skip the engine-update reload when the page is already on the new
  worker (another tab handed over, or controllerchange came late).
- Put the ; between flutter.js and the bootstrap on its own line.
- Ignore commented-out, data-src and look-alike tags when deciding to
  keep flutter_bootstrap.js.
- Run the publish-set check last, on the whole tree, and fail when the
  dry run fails.
- Docs: version default, categories, env variables, engineRevision,
  --comments, cache prefix boundary.

Tests: Vitest 308 -> 370, Dart 101 -> 135 (without integration).
Service worker:
- Count pre-cached and lazily cached bodies against the manifest size
  while they stream, so a transfer cut short is never cached. A host
  that re-encodes a file and declares its Content-Length is checked
  against that length; without the header, precache fetches once more
  and keeps a length that repeats or a longer body.
- Pass Range requests to the network and cache only a plain 200.
- Retry 5xx and 429 (Retry-After capped at 5 s) for precache and the
  refill, which now bypasses the HTTP cache.
- Remove temp caches of abandoned installs; log quota errors with the
  storage estimate.

Bootstrap:
- Reload once with the local CanvasKit when the app fails on a CDN
  engine, but not for a missing Flutter loader; remember a failed CDN
  probe for the session.
- Surface initializeEngine() failures and a widget that fails to mount.
- Reset deletes only this app's caches (cachePrefix in the config).
- Loading widget: no innerHTML, CSP nonce, ARIA progressbar and live
  status, transitionend and pointer-events fixes, reduced motion.

Generator:
- Clear errors (exit 64 for usage) and warnings for unknown YAML keys,
  bad theme/color/progress values, unknown renderers, symlinks and
  ignored *.part.js files; a clear message for an already processed
  build.
- Stream MD5 and hash manifest entries only; create output directories;
  prune without following symlinks.

Docs and CI: no-cache hosting examples (nginx types block removed),
wasm MIME and isolation notes, service-worker docs for the new checks;
the publish workflow now runs the TS and Dart tests.
pdcruze added a commit to NeoriseTech/service-worker-generator that referenced this pull request Oct 30, 2026
A load that finds a newer worker waiting posts skipWaiting from
activateWaitingAtBootstrap. Usually the handover lands in milliseconds.
But activation stops the old worker, and a request the page makes at
that moment (the browser's favicon fetch, a logo, the web manifest) is
still dispatched to the old worker, which restarts it; Chromium then
keeps the new worker waiting for as long as the page is open, and
re-sending skipWaiting does not help. The bootstrap waited the full
SW_REGISTRATION_TIMEOUT_MS (4s) for a controllerchange that could not
come, and then booted with the old controller, or on an engine change
reloaded once.

The handover is now bounded by SW_HANDOFF_TIMEOUT_MS (500ms). A lost
handover costs that instead of 4s; where the page ends up is unchanged.
A first visit with a waiting worker and no controller still waits for
activation after the bound.

This is what is left of DoctorinaAI#25 on top of DoctorinaAI#31: the other half of DoctorinaAI#25, a
controlled page not waiting for a worker that cannot activate, is
already part of DoctorinaAI#31.

Tests: the handover tests that used SW_REGISTRATION_TIMEOUT_MS as the
handover's timeout now use SW_HANDOFF_TIMEOUT_MS and check both sides of
the bound: a lost handover on a controlled page resolves after it and not
before; an uncontrolled page still waits for activation after it; on an
engine change the reload follows it, including when the install lands
first and when the controller changes just after it. Five fail without
the fix. Vitest (488), Dart unit (175) and integration tests, and the
Playwright suite pass; dart analyze and tsc are clean; the embedded
bootstrap template is regenerated with npm run build:all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant