From 0732f8f95c52ad8e7de72a355fbec8af969b3bc4 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Sun, 13 Sep 2026 14:47:26 -0700 Subject: [PATCH] fix(traffic): recover road loading after city navigation --- CHANGELOG.md | 7 + docs/CURRENT-STATE.md | 8 + scripts/format-scope.json | 4 +- scripts/qa-traffic-navigation.mjs | 125 ++++++++++++++ src/data/trafficTiming.test.mjs | 2 +- src/layers/traffic/controls.js | 2 +- src/layers/traffic/flow.js | 3 +- src/layers/traffic/ingestion.js | 37 ++++- src/layers/traffic/lifecycle.js | 16 +- src/layers/traffic/navigation.test.mjs | 218 +++++++++++++++++++++++++ src/layers/traffic/state.js | 6 + src/layers/traffic/viewport.js | 8 + 12 files changed, 425 insertions(+), 11 deletions(-) create mode 100644 scripts/qa-traffic-navigation.mjs create mode 100644 src/layers/traffic/navigation.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 29271a1f5..e5ee6b519 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,13 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md ## [Unreleased] +### Fixed + +- Traffic now retries a failed destination after city navigation without a layer + toggle. Camera departure cancels pending work, arrival checks the final view, + and superseded requests cannot keep a newer view loading. + + - Extract vessel feed, store, rendering, selection, trail and card components with explicit source and scene services. - Bound contact retention for incomplete vessel observations, preserve source freshness and refresh history references in place. - Cancel pending vessel history during selection and layer teardown. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 9e24ae2f0..bfa7ad5aa 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -3261,3 +3261,11 @@ nanoid 3.3.19. Cesium remains on 1.138.0. Browser QA uses Puppeteer 25.10.0; image-processing tools use Sharp 0.35.4. QA scripts await Puppeteer's asynchronous executable-path lookup before testing or passing the path to Chrome. Supported Node versions remain 24.14.x and 26.x. Use `npm ci` to reproduce the checked-in dependency tree. + +### Traffic city navigation + +Traffic checks the final camera view on arrival and retries a failed road request +while the camera is stationary, backing off from 1.5 to 30 seconds. Failed road +requests report an unavailable source. Leaving the traffic altitude range or +disabling the layer cancels pending work; superseded road and flow requests cannot +release the current request or keep its loading indicator active. diff --git a/scripts/format-scope.json b/scripts/format-scope.json index b1edbccca..155914a9a 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -426,5 +426,7 @@ "src/data/flowTiles.js", "src/layers/traffic/source.test.mjs", "src/layers/bikeshare/source.test.mjs", - "scripts/traffic-fixtures.mjs" + "scripts/traffic-fixtures.mjs", + "src/layers/traffic/navigation.test.mjs", + "scripts/qa-traffic-navigation.mjs" ] diff --git a/scripts/qa-traffic-navigation.mjs b/scripts/qa-traffic-navigation.mjs new file mode 100644 index 000000000..b45daeb2c --- /dev/null +++ b/scripts/qa-traffic-navigation.mjs @@ -0,0 +1,125 @@ +#!/usr/bin/env node +// Deterministic city-navigation exercise; road/flow responses are fixtures. +import assert from 'node:assert/strict'; +import puppeteer from 'puppeteer'; +import { trafficFixtureResponse } from './traffic-fixtures.mjs'; + +const args = process.argv.slice(2); +const url = args[args.indexOf('--url') + 1] || 'http://localhost:4173'; +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); +const browser = await puppeteer.launch({ + headless: true, + protocolTimeout: 120000, + args: [ + '--no-sandbox', + '--disable-dev-shm-usage', + '--use-gl=angle', + '--use-angle=swiftshader', + '--disable-background-timer-throttling', + ], +}); +try { + for (const cctv of [false, true]) { + const page = await browser.newPage(); + const errors = []; + let londonRequests = 0; + page.on('pageerror', (error) => errors.push(error.message)); + page.on('console', (message) => { + if (message.text().includes('[Data:Traffic]')) + console.log(message.text()); + }); + await page.setViewport({ width: 1280, height: 800 }); + await page.setRequestInterception(true); + page.on('request', (request) => { + let response = trafficFixtureResponse(request); + const query = new URLSearchParams(request.postData()).get('data') || ''; + if (query.includes('highway') && query.includes('(51.')) { + londonRequests++; + if (londonRequests === 1) + response = { status: 504, body: 'Temporary road source timeout' }; + } + return response ? request.respond(response) : request.continue(); + }); + await page.goto(`${url}/?welcome=0`, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction(() => window.__godsEyeView?.dataManager, { + timeout: 60000, + }); + await sleep(5000); + await page.keyboard.press('Escape'); + await page.evaluate(async (withCctv) => { + const { viewer, dataManager } = window.__godsEyeView; + const C = await import('/node_modules/.vite/deps/cesium.js'); + for (const id of ['satellites', 'flights', 'military']) + await dataManager.setEnabled(id, false); + viewer.camera.cancelFlight(); + viewer.camera.setView({ + destination: C.Cartesian3.fromDegrees(-97.744, 30.267, 3200), + orientation: { heading: 0, pitch: -Math.PI / 2, roll: 0 }, + }); + await dataManager.setEnabled('traffic', true); + if (withCctv) await dataManager.setEnabled('cctv', true); + }, cctv); + const ready = () => { + const stats = window.__godsEyeView.dataManager.layers + .get('traffic') + .module.getStats(); + return stats.count > 0 && !stats.loading; + }; + try { + await page.waitForFunction(ready, { timeout: 45000 }); + } catch (error) { + console.log( + 'Traffic timeout state', + await page.evaluate(() => { + const { viewer, dataManager } = window.__godsEyeView; + return { + stats: dataManager.layers.get('traffic').module.getStats(), + camera: viewer.camera.positionCartographic, + }; + }), + ); + throw error; + } + await sleep(2500); + await page.evaluate(async () => { + const { viewer } = window.__godsEyeView; + const C = await import('/node_modules/.vite/deps/cesium.js'); + viewer.camera.flyTo({ + destination: C.Cartesian3.fromDegrees(-0.1276, 51.5072, 3200), + orientation: { heading: 0, pitch: -Math.PI / 2, roll: 0 }, + duration: 4, + }); + }); + const deadline = Date.now() + 45000; + while (londonRequests < 2 && Date.now() < deadline) await sleep(500); + assert.ok( + londonRequests >= 2, + 'London must recover its failed request without a toggle', + ); + await page.waitForFunction(ready, { timeout: 45000 }); + const stats = await page.evaluate(() => + window.__godsEyeView.dataManager.layers.get('traffic').module.getStats(), + ); + assert.equal(stats.error, null); + assert.deepEqual(errors, []); + console.log( + 'PASS: Austin → London, first destination request fails then recovers', + { cctv, londonRequests, count: stats.count }, + ); + await page.evaluate(() => + window.__godsEyeView.dataManager.setEnabled('traffic', false), + ); + assert.equal( + await page.evaluate( + () => + window.__godsEyeView.dataManager.layers + .get('traffic') + .module.getStats().loading, + ), + false, + ); + await page.close(); + } +} finally { + await browser.close(); +} diff --git a/src/data/trafficTiming.test.mjs b/src/data/trafficTiming.test.mjs index f7589afb3..1b87deb5b 100644 --- a/src/data/trafficTiming.test.mjs +++ b/src/data/trafficTiming.test.mjs @@ -239,7 +239,7 @@ test('traffic timing pairs real ordering to the scheduling change and guards re- }); trafficLayer.enable(viewer); assert.equal(traffic.getTrafficTimingDiagnostics().marksInstalled, 1); - assert.equal(moveEnd.listenerCount(), 1, 'marksInstalled must represent a live listener'); + assert.equal(moveEnd.listenerCount(), 2, 'one timing listener plus the production arrival check'); assert.equal(performance.getEntriesByName('traffic:stale:mark').length, 0); assert.equal(performance.getEntriesByName('traffic:stale:measure').length, 0); diff --git a/src/layers/traffic/controls.js b/src/layers/traffic/controls.js index 22fe85407..95b3665a3 100644 --- a/src/layers/traffic/controls.js +++ b/src/layers/traffic/controls.js @@ -154,7 +154,7 @@ export function createControls({ state: layerState, services, parts, source }) { lastUpdate: layerState._lastUpdate, loading, mode: feed.mode, - error: feed.error, + error: layerState._roadError || feed.error, flowCoveragePct: layerState._flowCoveragePct, tilesFetched: getFlowSessionStats().tilesFetched, ...(TRAFFIC_TIMING_ENABLED diff --git a/src/layers/traffic/flow.js b/src/layers/traffic/flow.js index 591bd88ce..c12f05f53 100644 --- a/src/layers/traffic/flow.js +++ b/src/layers/traffic/flow.js @@ -131,7 +131,8 @@ export function createFlow({ state: layerState, services, parts, source }) { ); } } finally { - layerState._flowPending -= 1; + if (generation === layerState._loadGeneration) + layerState._flowPending -= 1; } } diff --git a/src/layers/traffic/ingestion.js b/src/layers/traffic/ingestion.js index f4c43ba76..b7d48597f 100644 --- a/src/layers/traffic/ingestion.js +++ b/src/layers/traffic/ingestion.js @@ -158,11 +158,22 @@ export function createIngestion({ // Increment generation to invalidate any in-flight responses from prior calls const generation = ++layerState._loadGeneration; cancelActiveFetch(); + clearTimeout(layerState._retryTimer); + layerState._retryTimer = null; + layerState._flowPending = 0; + layerState._activeFetchAbort = new AbortController(); + const requestSignal = layerState._activeFetchAbort.signal; const clamped = parts.viewport.clampBounds(bounds); // Cache key: fixed-precision bounding-box string for deterministic lookups const cacheKey = `${clamped.south.toFixed(4)},${clamped.west.toFixed(4)},${clamped.north.toFixed(4)},${clamped.east.toFixed(4)}`; + if (cacheKey !== layerState._retryBoundsKey) { + layerState._retryBoundsKey = cacheKey; + layerState._retryDelayMs = 1500; + } + layerState._roadError = null; + // Live mode: warm the flow-tile cache CONCURRENTLY with the Overpass road // fetch — sequential fetches doubled first-paint latency (field-test // round 1). Failures are irrelevant; applyFlowToRoads settles the truth. @@ -172,7 +183,7 @@ export function createIngestion({ layerState._enabled && generation === layerState._loadGeneration ) { - fetchFlowForBounds(clamped, {}).catch(() => { + fetchFlowForBounds(clamped, { signal: requestSignal }).catch(() => { /* warm-up only */ }); } @@ -233,7 +244,6 @@ export function createIngestion({ renderedSomething = true; } else { // Fetch major roads first (smaller payload, faster response) - layerState._activeFetchAbort = new AbortController(); console.log(`[Data:Traffic] Fast fetch major roads [${cacheKey}]`); const majorData = await fetchRoads( clamped.south, @@ -243,7 +253,7 @@ export function createIngestion({ { majorOnly: true, timeoutSec: 12, - signal: layerState._activeFetchAbort.signal, + signal: requestSignal, }, trace, ); @@ -268,7 +278,6 @@ export function createIngestion({ if (altitude > FAST_FETCH_ALTITUDE) return; // Detailed pass: fetch the full road graph (tertiary, residential, etc.) - layerState._activeFetchAbort = new AbortController(); console.log(`[Data:Traffic] Full fetch local roads [${cacheKey}]`); const fullData = await fetchRoads( clamped.south, @@ -278,7 +287,7 @@ export function createIngestion({ { majorOnly: false, timeoutSec: 20, - signal: layerState._activeFetchAbort.signal, + signal: requestSignal, }, trace, ); @@ -299,6 +308,8 @@ export function createIngestion({ renderedSomething = true; } catch (e) { if (e?.name === 'AbortError') return; + if (generation === layerState._loadGeneration && !renderedSomething) + layerState._roadError = 'Road data temporarily unavailable'; console.warn('[Data:Traffic] Fetch error:', e); } finally { if (generation === layerState._loadGeneration) { @@ -310,9 +321,23 @@ export function createIngestion({ if (!renderedSomething) { layerState._lastBounds = prevBounds; layerState._lastViewCenter = prevViewCenter; + if (layerState._enabled) { + layerState._retryTimer = setTimeout(() => { + layerState._retryTimer = null; + parts.viewport.onCameraChanged(); + }, layerState._retryDelayMs); + layerState._retryDelayMs = Math.min( + layerState._retryDelayMs * 2, + 30000, + ); + } + } else { + layerState._retryDelayMs = 1500; } } - layerState._activeFetchAbort = null; + // Keep this generation's controller until superseded or disabled: flow + // can still be running after its paint deadline. An older finally must + // never clear the controller belonging to a newer destination. } } const methods = { diff --git a/src/layers/traffic/lifecycle.js b/src/layers/traffic/lifecycle.js index 175ed8bf2..bb452a984 100644 --- a/src/layers/traffic/lifecycle.js +++ b/src/layers/traffic/lifecycle.js @@ -98,6 +98,11 @@ export function createLifecycle({ // on the camera and leaving it mutated affects every other camera.changed // listener in the app. viewer.camera.changed.addEventListener(parts.viewport.onCameraChanged); + // Always inspect the final view, even when the last flight step is below + // camera.changed's movement threshold. + layerState._arrivalRemover = viewer.camera.moveEnd.addEventListener( + parts.viewport.onCameraChanged, + ); layerState._prevPercentageChanged = viewer.camera.percentageChanged; viewer.camera.percentageChanged = 0.05; @@ -117,7 +122,8 @@ export function createLifecycle({ layerState._enableKickTimer = null; return; } - if (!layerState._fetching) parts.viewport.onCameraChanged(); + if (!layerState._fetching && !layerState._retryTimer) + parts.viewport.onCameraChanged(); }, 1500); }, @@ -133,8 +139,14 @@ export function createLifecycle({ clearTimeout(layerState._fetchTimeout); clearInterval(layerState._enableKickTimer); layerState._enableKickTimer = null; + clearTimeout(layerState._retryTimer); + layerState._retryTimer = null; + layerState._retryDelayMs = 1500; parts.ingestion.cancelActiveFetch(); layerState._loadGeneration++; + layerState._fetching = false; + layerState._flowPending = 0; + layerState._roadError = null; parts.animation.clearDots(); layerState._lastViewCenter = null; // A stale outage from the last session would misreport a fresh enable — @@ -155,6 +167,8 @@ export function createLifecycle({ } viewer.camera.changed.removeEventListener(parts.viewport.onCameraChanged); + layerState._arrivalRemover?.(); + layerState._arrivalRemover = null; // Restore the camera's global percentageChanged so other layers/listeners // keep their expected sensitivity. if (layerState._prevPercentageChanged != null) { diff --git a/src/layers/traffic/navigation.test.mjs b/src/layers/traffic/navigation.test.mjs new file mode 100644 index 000000000..3d594c4e5 --- /dev/null +++ b/src/layers/traffic/navigation.test.mjs @@ -0,0 +1,218 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import * as Cesium from 'cesium'; +import { createTrafficLayer } from './index.js'; + +function deferred() { + let resolve; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +function setup(t, requestRoads) { + t.mock.timers.enable({ apis: ['setTimeout', 'setInterval'] }); + const camera = { + positionCartographic: Cesium.Cartographic.fromDegrees( + -97.744, + 30.267, + 3200, + ), + get positionWC() { + return Cesium.Cartesian3.fromRadians( + this.positionCartographic.longitude, + this.positionCartographic.latitude, + this.positionCartographic.height, + ); + }, + changed: new Cesium.Event(), + moveEnd: new Cesium.Event(), + percentageChanged: 0.5, + computeViewRectangle() { + const { longitude, latitude } = this.positionCartographic; + return new Cesium.Rectangle( + longitude - 0.0002, + latitude - 0.0002, + longitude + 0.0002, + latitude + 0.0002, + ); + }, + pickEllipsoid: () => null, + }; + const viewer = { + camera, + scene: { + canvas: { width: 100, height: 100 }, + preRender: new Cesium.Event(), + primitives: { add: (value) => value, remove: () => true }, + }, + }; + const layer = createTrafficLayer({ + services: { + credits: {}, + render: { holdContinuousRender() {}, releaseContinuousRender() {} }, + }, + source: { + requestRoads, + getStatus: async () => ({ hasKey: false }), + fetchFlowForBounds: async () => [], + getFlowSessionStats: () => ({ tilesFetched: 0 }), + resetFlowTileCache() {}, + }, + }); + layer.init(viewer); + t.after(() => layer.destroy(viewer)); + const move = (lon, lat, height = 3200) => { + camera.positionCartographic = Cesium.Cartographic.fromDegrees( + lon, + lat, + height, + ); + camera.changed.raiseEvent(); + camera.moveEnd.raiseEvent(); + }; + const tick = async (ms) => { + t.mock.timers.tick(ms); + for (let i = 0; i < 30; i++) await Promise.resolve(); + }; + return { layer, viewer, move, tick }; +} +function roads(bounds) { + return { + ok: true, + json: async () => ({ + elements: [ + { + type: 'way', + tags: { highway: 'primary' }, + geometry: [ + { lat: bounds.south, lon: bounds.west }, + { lat: bounds.south + 0.005, lon: bounds.west + 0.005 }, + ], + }, + ], + }), + }; +} + +test('traffic recovers a failed destination request after another city has loaded', async (t) => { + let londonCalls = 0; + const { layer, viewer, move, tick } = setup(t, async (bounds) => { + if (bounds.south > 50 && ++londonCalls === 1) + throw new Error('temporary Overpass outage'); + return roads(bounds); + }); + layer.enable(viewer); + await tick(400); + await tick(2000); + assert.ok(layer.getStats().count > 0); + move(-40, 40, 1000000); + move(-0.1276, 51.5072); + await tick(400); + assert.equal(londonCalls, 1); + for (let i = 0; i < 20; i++) await tick(1500); + assert.ok( + londonCalls >= 2, + 'the stationary destination must retry without a toggle', + ); + assert.ok(layer.getStats().count > 0); + assert.equal(layer.getStats().loading, false); +}); + +test('a superseded road response cannot release the current request controller', async (t) => { + const pending = []; + const { layer, viewer, move, tick } = setup(t, (bounds, { signal }) => { + const result = deferred(); + pending.push({ ...result, bounds, signal }); + return result.promise; + }); + layer.enable(viewer); + await tick(400); + move(-0.1276, 51.5072); + await tick(400); + assert.equal(pending.length, 2); + assert.equal(pending[0].signal.aborted, true); + pending[0].resolve(roads(pending[0].bounds)); + await tick(0); + layer.disable(viewer); + assert.equal( + pending[1].signal.aborted, + true, + 'disable must still cancel the destination request', + ); + pending[1].resolve(roads(pending[1].bounds)); + await tick(0); + assert.equal(layer.getStats().count, 0); + assert.equal(layer.getStats().loading, false); +}); + +test('leaving traffic altitude cancels queued and in-flight work', async (t) => { + const pending = []; + const { layer, viewer, move, tick } = setup(t, (bounds, { signal }) => { + const result = deferred(); + pending.push({ ...result, bounds, signal }); + return result.promise; + }); + layer.enable(viewer); + move(-40, 40, 1000000); + await tick(400); + assert.equal( + pending.length, + 0, + 'a departing city debounce must not fetch above traffic altitude', + ); + move(-0.1276, 51.5072); + await tick(400); + assert.equal(pending.length, 1); + move(-40, 40, 1000000); + assert.equal(pending[0].signal.aborted, true); + pending[0].resolve(roads(pending[0].bounds)); + await tick(0); + assert.equal(layer.getStats().count, 0); + assert.equal(layer.getStats().loading, false); +}); + +test('arrival below the camera change threshold loads the final city and unsubscribes on disable', async (t) => { + const seen = []; + const { layer, viewer, tick } = setup(t, async (bounds) => { + seen.push(bounds); + return roads(bounds); + }); + layer.enable(viewer); + await tick(400); + viewer.camera.positionCartographic = Cesium.Cartographic.fromDegrees( + -0.1276, + 51.5072, + 3200, + ); + viewer.camera.moveEnd.raiseEvent(); + await tick(400); + assert.ok(seen.some((bounds) => bounds.south > 50)); + layer.disable(viewer); + assert.equal(viewer.camera.moveEnd.numberOfListeners, 0); + assert.equal(viewer.camera.changed.numberOfListeners, 0); +}); + +test('parked failures back off and disabling cancels the scheduled retry', async (t) => { + let calls = 0; + const { layer, viewer, tick } = setup(t, async () => { + calls++; + throw new Error('temporary Overpass outage'); + }); + layer.enable(viewer); + await tick(400); + assert.equal(calls, 1); + assert.equal(layer.getStats().loading, false); + assert.equal(layer.getStats().error, 'Road data temporarily unavailable'); + await tick(1500); + await tick(400); + assert.equal(calls, 2); + await tick(1500); + await tick(400); + assert.equal(calls, 2, 'second failure waits longer than the first'); + layer.disable(viewer); + await tick(60000); + assert.equal(calls, 2); + assert.equal(layer.getStats().error, null); +}); diff --git a/src/layers/traffic/state.js b/src/layers/traffic/state.js index c6665a55e..ad240b4bf 100644 --- a/src/layers/traffic/state.js +++ b/src/layers/traffic/state.js @@ -32,6 +32,7 @@ export function createState({ services }) { /** @type {Function|null} Disposer returned by camera.changed event subscription */ layerState._cameraRemover = null; + layerState._arrivalRemover = null; /** @type {ReturnType|null} Debounce timer for camera-change fetch */ @@ -215,6 +216,11 @@ export function createState({ services }) { /** @type {ReturnType|null} Enable-time retry until the first load commits. */ layerState._enableKickTimer = null; + // Failed destinations retry while parked, with a bounded backoff. + layerState._retryTimer = null; + layerState._retryDelayMs = 1500; + layerState._retryBoundsKey = null; + layerState._roadError = null; /** @type {number} 0–100 int — matched roads / roads with any flow candidates */ diff --git a/src/layers/traffic/viewport.js b/src/layers/traffic/viewport.js index 954f705b9..e84a1c045 100644 --- a/src/layers/traffic/viewport.js +++ b/src/layers/traffic/viewport.js @@ -189,6 +189,14 @@ export function createViewport({ state: layerState, services, parts, source }) { // viewport hits the overlap/center-shift skip in step 3 and the dots // (cleared here) never reload (H5). Clearing the gate forces a fresh fetch. if (alt > ACTIVATION_ALTITUDE) { + clearTimeout(layerState._fetchTimeout); + clearTimeout(layerState._retryTimer); + layerState._retryTimer = null; + parts.ingestion.cancelActiveFetch(); + layerState._loadGeneration++; + layerState._fetching = false; + layerState._flowPending = 0; + layerState._roadError = null; parts.animation.clearDots(); layerState._lastBounds = null; layerState._lastViewCenter = null;