[Bug][E2E flake] /geography/:countryCode click-through — Leaflet "Cannot read properties of undefined (reading 'appendChild')" pageerror on CI only #363

New Issue

2026-07-04T20:08:48Z

spikerj commented

2026-07-04 20:08:48 +00:00

Epic #307 (E2E walk findings). Failed e2e-anonymous on master run 793 (sha 98d3778, the art-studio viewer merge — not caused by that PR: the identical commit passes locally, including --repeat-each=6).

Symptom: anonymous-click-throughs.spec.ts:41 (geography grid → country detail) fails the console-error assertion with [pageerror] Cannot read properties of undefined (reading 'appendChild').

Suspected root cause: Leaflet/markercluster post-destroy race in country-map.component.ts (used by geography-explorer). The component's own setTimeout(100/150ms) callbacks are destroyed-guarded, but Leaflet-internal timers/animation callbacks (tile load, zoom anim, cluster spiderfy) are not — on fast grid→detail navigation the component re-creates while a queued Leaflet callback fires against a removed map whose panes are gone → container.appendChild on undefined. Slower CI hardware widens the window; M5-local never hits it (6/6 pass).

Repro: CI-timing only so far; locally green on the same sha. Error context artifact: test-results/anonymous-click-throughs-a-*/error-context.md on run 793.

Fix directions: guard/teardown — cancel Leaflet animations before map.remove() (map.stop(), zoomAnimation:false on CI-sized viewports, or defer remove() until whenReady); markercluster removeLayers before map removal; alternatively harden the spec to retry-on-known-flake only if the component fix proves elusive (component fix preferred — a real pageerror reaches students' consoles too).

Impact beyond CI: the same failed run skipped build_and_publish_frontend, so a red e2e-anonymous blocks frontend deploys — flakes here directly stall the deploy train.

Epic #307 (E2E walk findings). Failed `e2e-anonymous` on master run [793](https://git.spikersoft.com/spikerj/spikersoft-angular/actions/runs/793/jobs/1) (sha `98d3778`, the art-studio viewer merge — **not caused by that PR**: the identical commit passes locally, including `--repeat-each=6`). **Symptom:** `anonymous-click-throughs.spec.ts:41` (geography grid → country detail) fails the console-error assertion with `[pageerror] Cannot read properties of undefined (reading 'appendChild')`. **Suspected root cause:** Leaflet/markercluster post-destroy race in `country-map.component.ts` (used by `geography-explorer`). The component's own `setTimeout(100/150ms)` callbacks are `destroyed`-guarded, but **Leaflet-internal** timers/animation callbacks (tile load, zoom anim, cluster spiderfy) are not — on fast grid→detail navigation the component re-creates while a queued Leaflet callback fires against a removed map whose panes are gone → `container.appendChild` on undefined. Slower CI hardware widens the window; M5-local never hits it (6/6 pass). **Repro:** CI-timing only so far; locally green on the same sha. Error context artifact: `test-results/anonymous-click-throughs-a-*/error-context.md` on run 793. **Fix directions:** guard/teardown — cancel Leaflet animations before `map.remove()` (`map.stop()`, `zoomAnimation:false` on CI-sized viewports, or defer `remove()` until `whenReady`); markercluster `removeLayers` before map removal; alternatively harden the spec to retry-on-known-flake only if the component fix proves elusive (component fix preferred — a real pageerror reaches students' consoles too). **Impact beyond CI:** the same failed run skipped `build_and_publish_frontend`, so a red e2e-anonymous blocks frontend deploys — flakes here directly stall the deploy train.

spikerj added the bug label 2026-07-04 20:08:48 +00:00

spikerj commented

2026-07-04 20:44:38 +00:00

Resolved in spikersoft-angular PR #116 (merged to master). Leaflet teardown hardened in country-map.component.ts — animations stopped, listeners detached, cluster markers cleared before map.remove(), guarded so internal races can't escape as pageerrors. 1840/1840 unit tests; geography click-through green locally. The merge also re-triggers the master pipeline, which redelivers the blocked frontend deploy. Closing.

Resolved in spikersoft-angular PR #116 (merged to `master`). Leaflet teardown hardened in `country-map.component.ts` — animations stopped, listeners detached, cluster markers cleared before `map.remove()`, guarded so internal races can't escape as pageerrors. 1840/1840 unit tests; geography click-through green locally. The merge also re-triggers the master pipeline, which redelivers the blocked frontend deploy. Closing.

spikerj closed this issue

2026-07-04 20:44:38 +00:00

spikerj reopened this issue

2026-07-07 17:55:42 +00:00

spikerj commented

2026-07-07 17:55:57 +00:00

Reopened — the flake recurred on master ee927b8 (e2e-anonymous passed as task 8852, then failed on a later run of the same commit — the classic intermittent signature).

Root cause pinned more precisely than the original hypothesis. It's not (only) Leaflet-internal timers — it's three of our own async callbacks in country-map.component.ts that touch the map after remove(), and unlike the setTimeout callbacks (which are destroyed-guarded) these had no guard:

loadGeoData() HTTP next → renderGeoLayer() adds an L.geoJSON layer / applyCountryFitBounds() invalidates size
reloadCountryGeoData() HTTP next
the ResizeObserver callback → invalidateSize()

On a fast grid→detail nav the map is remove()d before the geo-JSON GET resolves; adding a layer to / invalidating the removed map appends to panes that no longer exist → the uncaught appendChild pageerror. The original fix wrapped only the synchronous ngOnDestroy, so these async throws escaped it.

Fix in spikersoft-angular PR #149 (open, for review): if (this.destroyed) return; at the top of all three, matching the existing guard pattern. Added a unit test asserting a GeoJSON response that arrives after fixture.destroy() neither throws nor applies to the torn-down map. nx lint + nx test spikersoft (1870) green. Full confirmation is the next e2e-anonymous run after merge.

**Reopened — the flake recurred on master `ee927b8`** (`e2e-anonymous` passed as task 8852, then failed on a later run of the *same commit* — the classic intermittent signature). **Root cause pinned more precisely than the original hypothesis.** It's not (only) Leaflet-*internal* timers — it's **three of our own async callbacks in `country-map.component.ts` that touch the map after `remove()`**, and unlike the `setTimeout` callbacks (which *are* `destroyed`-guarded) these had no guard: - `loadGeoData()` HTTP `next` → `renderGeoLayer()` adds an `L.geoJSON` layer / `applyCountryFitBounds()` invalidates size - `reloadCountryGeoData()` HTTP `next` - the `ResizeObserver` callback → `invalidateSize()` On a fast grid→detail nav the map is `remove()`d before the geo-JSON GET resolves; adding a layer to / invalidating the removed map appends to panes that no longer exist → the uncaught `appendChild` pageerror. The original fix wrapped only the synchronous `ngOnDestroy`, so these async throws escaped it. **Fix in spikersoft-angular PR #149** (open, for review): `if (this.destroyed) return;` at the top of all three, matching the existing guard pattern. Added a unit test asserting a GeoJSON response that arrives after `fixture.destroy()` neither throws nor applies to the torn-down map. `nx lint` + `nx test spikersoft` (1870) green. Full confirmation is the next `e2e-anonymous` run after merge.

spikerj commented

2026-07-07 18:19:53 +00:00

Resolved in spikersoft-angular PR #149 (merged to master, 74dff52). Extended the this.destroyed guard to the three async callbacks in country-map.component.ts that touched the Leaflet map after remove() — loadGeoData()/reloadCountryGeoData() HTTP next handlers and the ResizeObserver callback — closing the exact escape paths that let the "appendChild of undefined" pageerror surface on fast grid→detail navigation (the original fix guarded only the synchronous ngOnDestroy). Added a unit test asserting a GeoJSON response arriving after fixture.destroy() neither throws nor applies to the torn-down map. Closing; the next e2e-anonymous run on master is the live confirmation.

Resolved in spikersoft-angular PR #149 (merged to `master`, `74dff52`). Extended the `this.destroyed` guard to the three async callbacks in `country-map.component.ts` that touched the Leaflet map after `remove()` — `loadGeoData()`/`reloadCountryGeoData()` HTTP `next` handlers and the `ResizeObserver` callback — closing the exact escape paths that let the "appendChild of undefined" pageerror surface on fast grid→detail navigation (the original fix guarded only the synchronous `ngOnDestroy`). Added a unit test asserting a GeoJSON response arriving after `fixture.destroy()` neither throws nor applies to the torn-down map. Closing; the next `e2e-anonymous` run on master is the live confirmation.

spikerj closed this issue

2026-07-07 18:19:57 +00:00

spikerj reopened this issue

2026-07-07 20:28:22 +00:00

spikerj commented

2026-07-07 20:28:37 +00:00

Reopened again — recurred on master 74dff52 (e2e-anonymous task 8884), which already contains both prior rounds of hardening:

6f20b71 — ngOnDestroy stop()/off()/clearLayers()/remove() in try-catch
6dbca58 (PR #149) — this.destroyed guards on the async geo-JSON/resize callbacks

Neither reaches the remaining escape: Leaflet's requestAnimationFrame-driven _animateZoom / tile _fadeIn, which append to map panes from an rAF callback that can fire after teardown. map.stop() only races to cancel an already-queued frame.

This is exactly the zoomAnimation:false direction called out in the original report. PR #150 (open) disables zoomAnimation + fadeAnimation + markerZoomAnimation at map construction so no frame is ever queued — no visual regression since the country map already fits bounds with { animate: false }. Unit test added; nx lint + nx test spikersoft (1871) green.

Leaving open until #150 merges and the next e2e-anonymous on master confirms the walk is clean.

(Caveat: CI action logs aren't reachable from the agent env, so 8884's failing spec isn't 100% confirmed as geography vs. another intermittent item — but the signature + both prior guards already being in the failing tree point squarely at the animation path.)

**Reopened again — recurred on master `74dff52`** (`e2e-anonymous` task 8884), which already contains **both** prior rounds of hardening: - `6f20b71` — `ngOnDestroy` `stop()`/`off()`/`clearLayers()`/`remove()` in try-catch - `6dbca58` (PR #149) — `this.destroyed` guards on the async geo-JSON/resize callbacks Neither reaches the remaining escape: Leaflet's **`requestAnimationFrame`-driven** `_animateZoom` / tile `_fadeIn`, which append to map panes from an rAF callback that can fire *after* teardown. `map.stop()` only *races* to cancel an already-queued frame. **This is exactly the `zoomAnimation:false` direction called out in the original report.** PR #150 (open) disables `zoomAnimation` + `fadeAnimation` + `markerZoomAnimation` at map construction so no frame is ever queued — no visual regression since the country map already fits bounds with `{ animate: false }`. Unit test added; `nx lint` + `nx test spikersoft` (1871) green. Leaving open until #150 merges and the next `e2e-anonymous` on master confirms the walk is clean. _(Caveat: CI action logs aren't reachable from the agent env, so 8884's failing spec isn't 100% confirmed as geography vs. another intermittent item — but the signature + both prior guards already being in the failing tree point squarely at the animation path.)_

spikerj commented

2026-07-07 20:34:12 +00:00

Resolved in spikersoft-angular PR #150 (merged to master, b1024b1). Disabled Leaflet's zoomAnimation/fadeAnimation/markerZoomAnimation at map construction in country-map.component.ts, so the requestAnimationFrame-driven _animateZoom / tile _fadeIn callbacks that appended to map panes after teardown are never queued — the remaining escape path after the earlier ngOnDestroy teardown (6f20b71) and async-callback guards (PR #149, 6dbca58). This is the zoomAnimation:false direction the original report called out. No visual regression (the map already fits bounds with { animate: false }); unit test asserts the flags.

Closing. The next e2e-anonymous on master is the live confirmation the /geography/:countryCode walk stays green.

Resolved in spikersoft-angular PR #150 (merged to `master`, `b1024b1`). Disabled Leaflet's `zoomAnimation`/`fadeAnimation`/`markerZoomAnimation` at map construction in `country-map.component.ts`, so the `requestAnimationFrame`-driven `_animateZoom` / tile `_fadeIn` callbacks that appended to map panes after teardown are never queued — the remaining escape path after the earlier `ngOnDestroy` teardown (`6f20b71`) and async-callback guards (PR #149, `6dbca58`). This is the `zoomAnimation:false` direction the original report called out. No visual regression (the map already fits bounds with `{ animate: false }`); unit test asserts the flags. Closing. The next `e2e-anonymous` on master is the live confirmation the `/geography/:countryCode` walk stays green.

spikerj closed this issue

2026-07-07 20:34:16 +00:00

spikerj referenced this issue

2026-07-07 21:48:36 +00:00

[Art Studio] ArtPipe Generate-drawer composer parity — remaining deltas vs Eric's original (#446) #454

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#363