Skip to content

fix(pwa): the service worker swallowed every PDOK-via-openconnector call - #780

Merged
rubenvdlinde merged 2 commits into
developmentfrom
test-debt/procest-pdok-e2e
Aug 10, 2026
Merged

fix(pwa): the service worker swallowed every PDOK-via-openconnector call#780
rubenvdlinde merged 2 commits into
developmentfrom
test-debt/procest-pdok-e2e

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What was red

E2E Tests (Playwright) on development — run 31388005994, 83 expected / 38 skipped / 4 unexpected. Three of the four are spec-coverage/pdok-via-openconnector.spec.ts, all with page.evaluate: TypeError: Failed to fetch. (The fourth is the #719 bootstrap stall — analysed in a comment on #719, untouched here, and deliberately not "fixed" with a threshold change.)

Root cause — proven, not argued

The throwing frame core-main.js?v=…:1:80554 is the await t(e,n) inside Nextcloud core's patched window.fetch (verified against stable32's shipped dist/core-main.js), so the network request itself failed: the trace records net::ERR_FAILED and contains no Route.fulfill call at all — the handler never ran. A positive control proves Route.fulfill is traced when it runs.

A diagnostic spec pushed to a scratch branch and run on a real CI runner (job 93482170902) settled it:

probe result
no page.route() registered at all TypeError: Failed to fetch
original glob / catch-all ** / regex / predicate / registered-before-navigation handler hits 0, all throw (catch-all ** saw 0 requests)
same request, page loaded outside the worker's scope status 404, handler hits 1
real server, no browser 404 text/html

Not Playwright. The app's own service worker claimed the request.

public/service-worker.js matched map tiles with /(brtachtergrondkaart|wmts|pdok|service\.pdok\.nl)/i.test(url.host + url.pathname) — a substring test that also runs over the same-origin path. Since migrate-pdok-to-openconnector, this app's address lookups live at /apps/openconnector/api/pdok/*, so all four of them matched on the literal pdok.

And a second defect underneath it: a Service Worker inherits the CSP of its own script response. DashboardController::serviceWorker() sent Nextcloud's default default-src 'none' with no connect-src, so every fetch() the worker made was blocked. Measured inside the worker on NC 32: fetch(request), fetch(request.url) and fetch(url, {mode:'same-origin'}) all threw. Both strategies always degraded to Response.error() — the worker could only break a request, never serve one, and the offline sync cache could never be populated.

Net effect for users: pdokService.handleNetworkError() rethrows on an error with no HTTP status, so the address field rejected instead of surfacing the 503/404 warning openspec/specs/pdok-consumer/spec.md requires.

Why CI-only: the worker's scope keeps the /index.php prefix unless front_controller_active is set. On CI (php -S) the spec's URL is inside the scope and the worker controls the page; on the docker images developers use, the same URL is outside it and the worker is invisible.

Fixes

Product

  • public/service-worker.js — exact third-party tile-host allow-list; a request back to this Nextcloud is never a tile.
  • lib/Controller/DashboardController.php — the worker script now ships connect-src 'self' https://service.pdok.nl.

Test

  • the specs navigate into the worker's own scope and wait for navigator.serviceWorker.controller, so the CI/dev coin flip is gone. (Deliberately not serviceWorkers: 'block' — that would hide the defect.)
  • assert the ITEM: Array.isArray(result) was true for the empty array an unintercepted answer produces, and toBe(404) passed on Nextcloud's own 404 (openconnector is genuinely absent on CI). Fulfilled responses now carry a marker header a real server cannot produce.
  • addressesRegisterAvailable() returned status() !== 404, so a 401 read as "installed"; now only 2xx counts.
  • that test also resolved NEXTCLOUD_URL || 'http://localhost:8080' — the shared dev container — and seeded/deleted OpenRegister objects there. Now uses the single BASE_URL resolver.
  • @e2e anchors repointed from the archived change dir to canonical openspec/specs/pdok-consumer/spec.md.

Both-direction proof

Run on a disposable Nextcloud 32 + Postgres serving this branch, one mutation at a time:

mutation result
none (fixed) 5 passed, 1 skipped
old tile predicate restored the 3 pdok specs red, CSP specs green
CSP block removed the 2 service-worker specs red, pdok specs green
test 3's route glob made non-matching (real 404 arrives) red: Expected "suggest-404", Received null — the assertion the old spec had would have passed

A fourth planned test (scope discipline asserted from the response alone) went green under the planted defect and was removed rather than kept as a check that cannot fail.

Refs #719

`E2E Tests (Playwright)` was red on `development` with three
`page.evaluate: TypeError: Failed to fetch` failures in
`spec-coverage/pdok-via-openconnector.spec.ts`, thrown from inside
Nextcloud core's patched `window.fetch` (`core-main.js:1:80554` is the
`await t(e,n)` in that wrapper — checked against stable32's shipped
`dist/core-main.js`). It is not a test problem and not a Playwright
problem. Measured on a CI runner with NO `page.route()` registered at
all, the same fetch still throws; and with the page loaded OUTSIDE the
service worker's scope, it returns 404 and Playwright's route handler
fires normally.

TWO PRODUCT DEFECTS, both in the mobiel-inspectie-offline PWA layer:

1. `public/service-worker.js` decided "is this a map tile?" with
   `/(brtachtergrondkaart|wmts|pdok|service\.pdok\.nl)/i.test(url.host +
   url.pathname)` — a substring test that also runs over the SAME-ORIGIN
   path. Since migrate-pdok-to-openconnector this app's own address
   lookups live at `/apps/openconnector/api/pdok/{suggest,lookup,free,
   reverse}`, so every one of them matched on the literal `pdok` and was
   answered cache-first out of the map-tile cache. The worker is
   registered at app-root scope, so this hit every procest page.
   Replaced with an exact third-party host allow-list: a request back to
   this Nextcloud is never a tile.

2. A Service Worker inherits the CSP of its OWN script response.
   `DashboardController::serviceWorker()` sent Nextcloud's default
   `default-src 'none'` with no `connect-src`, under which EVERY
   `fetch()` the worker makes is blocked. Measured on Nextcloud 32:
   `fetch(request)`, `fetch(request.url)` and
   `fetch(url, {mode:'same-origin'})` all threw inside the worker. Both
   strategies (`cacheFirst` / `networkFirst`) therefore always fell
   through to `Response.error()` — the worker could only ever break a
   request, never serve one, and the offline sync cache could never be
   populated in the first place. Now sends
   `connect-src 'self' https://service.pdok.nl`.

Together these made `src/services/pdokService.js` REJECT rather than
degrade: `handleNetworkError()` rethrows on an error with no HTTP status,
so the address field broke outright instead of surfacing the 503/404
warning `openspec/specs/pdok-consumer/spec.md` requires.

Why it was CI-only: the worker's scope is `generateUrl('/apps/procest/')`,
which keeps the `/index.php` prefix unless `front_controller_active` is
set. On CI (`php -S`) the spec's `/index.php/apps/procest/dashboard` is
INSIDE that scope and the worker controls the document; on the docker
images every developer uses, the same URL is OUTSIDE it and the worker is
invisible. The specs now navigate into the worker's own scope and wait
for `navigator.serviceWorker.controller`, so the coin flip is gone.

Test hardening in the same files (each mutation-verified):
- assert the ITEM, not the container. `expect(Array.isArray(result))` was
  true for the empty array an unintercepted server answer produces, and
  `expect(status).toBe(404)` passed on Nextcloud's own 404 — openconnector
  is genuinely absent on CI. Both fulfilled responses now carry a marker
  header that a real server answer cannot produce, and the payload
  assertion names `Lauriergracht 116`.
- `addressesRegisterAvailable()` returned `status() !== 404`, so 401 from
  an unauthenticated context read as "the register is installed" and the
  test went on to seed against it. Now only a 2xx counts.
- that same test resolved `process.env.NEXTCLOUD_URL ||
  'http://localhost:8080'` — the SHARED dev container — so it seeded and
  deleted OpenRegister objects in somebody else's environment whenever the
  suite was pointed elsewhere. Now uses the single `BASE_URL` resolver.

New `spec-coverage/service-worker-scope.spec.ts` guards defect 2 directly.
A third test that would have guarded defect 1 from the response alone was
written, found unable to fail under the planted defect, and left out
rather than kept as a check that always passes.

Refs #719
The coverage ratchet caught the 4 new statements in
DashboardController::serviceWorker() as untested — correctly: the CSP on
that response is the difference between a Service Worker that can fetch
and one whose every request is blocked, and nothing else in the suite
looks at it.

Proven able to fail: with the CSP block removed,
testServiceWorkerScriptGrantsConnectSrc reports
"Failed asserting that 'default-src 'none';base-uri 'none';manifest-src
'self';frame-ancestors 'none'' contains \"connect-src\"."
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/procest @ 9e07e69

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue3-compile
test-l10n
composer ✅ 100/100
npm ✅ 552/552
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-10 15:08 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/procest @ b88eb79

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue3-compile
test-l10n
composer ✅ 100/100
npm ✅ 552/552
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-10 15:41 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 461357c into development Aug 10, 2026
30 of 32 checks passed
@rubenvdlinde
rubenvdlinde deleted the test-debt/procest-pdok-e2e branch August 10, 2026 16:14
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