diff --git a/changelog.d/1275-forecast-issue-fix.md b/changelog.d/1275-forecast-issue-fix.md new file mode 100644 index 000000000..2013bc4f2 --- /dev/null +++ b/changelog.d/1275-forecast-issue-fix.md @@ -0,0 +1,3 @@ +- Preserve enable during first-load replacement (asturksever, #1275). +- Release disabled weather camera listeners and make teardown resource leaks fail the browser probe (asturksever, #1275). +- Keep visible-page weather teardown probes focused through the browser testing API (asturksever, #1275). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 182c26ff0..c9864809e 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -9,10 +9,7 @@ broader suffix variants. Follow-ups retain boundary provenance, and cancelled lookups preserve the previous analyst cohort and boundary source. Navigation search keeps a successful geocode, with no search or view-based region recovery. -Share links and structured views accept only own keys in their style registries. -Inherited names such as `constructor` and `__proto__` fall back to the existing -normal/null defaults without interrupting other restored fields. Programmatic -visual-style calls ignore unknown or non-string values before mutating state. +Wind and weather parameter changes may supersede a first-load request without rejecting the layer’s enable transaction. Obsolete responses cannot publish data; a replacement request remains visible through the loading state. Explicit caller cancellation, disable, and destroy still reject the obsolete update. Source failures continue to report their existing unavailable state. Shared distance checks use `src/geoDistance.js` with `{ lat, lon }` points and the IUGG mean spherical Earth radius, 6371.0088 km. `greatCircleKm` is @@ -275,6 +272,7 @@ catalog construction owns each instance. Wind animates one forecast without advancing forecast time. Separate Weather observation layers provide radar and satellite history; none claims measured cloud volume. +Weather imagery releases its camera move listener on disable and registers it once on re-enable. Disabling also clears active imagery work. Weather imagery drapes onto the globe basemap. On photorealistic 3D Tiles each observed product is instead a raised, translucent shell: one rectangle over the product bounds at a fixed height (global infrared 5.5 km, regional infrared diff --git a/scripts/qa-weather-teardown.mjs b/scripts/qa-weather-teardown.mjs index b64d1d59c..bf7b00269 100644 --- a/scripts/qa-weather-teardown.mjs +++ b/scripts/qa-weather-teardown.mjs @@ -61,6 +61,10 @@ const browser = await puppeteer.launch({ const boot = async () => { const page = await browser.newPage(); + // This probe measures active rendering; an OS-minimized window must not + // silently exercise the application's intentional hidden-page pause. + const focus = await page.createCDPSession(); + await focus.send('Emulation.setFocusEmulationEnabled', { enabled: true }); await page.setViewport({ width: 1400, height: 920, deviceScaleFactor: 1 }); const errors = []; page.on('pageerror', (e) => @@ -141,7 +145,7 @@ const snapshot = (page) => ), }, weather, - windCells: wind.gpu?.pathCount ?? wind.paths ?? null, + windCells: wind.gpu?.pathCount ?? wind.paths ?? 0, enabled: [...dm.layers].filter(([, e]) => e.enabled).map(([id]) => id), }; }); @@ -220,7 +224,7 @@ try { const baseline = await snapshot(a.page); console.log('baseline', JSON.stringify(baseline)); for (const id of WEATHER) await setLayer(a.page, id, true); - await a.page + const weatherReady = await a.page .waitForFunction( () => ['weather-radar', 'weather-satellite', 'weather-lightning'].every( @@ -233,7 +237,13 @@ try { ), { timeout: 120_000 }, ) - .catch(() => console.log(' !! weather not ready')); + .then( + () => true, + () => false, + ); + results.weatherReady = weatherReady; + if (!weatherReady) + console.log(' !! weather not ready — feature coverage incomplete'); await sleep(2000); // exercise: history playback, wind reading, storm selection, product switch await a.page.evaluate(() => { @@ -331,11 +341,25 @@ try { 'errors (control run):', results.errorsB.length ? results.errorsB : 'none', ); + // Resource ownership is an acceptance check; provider availability remains + // separately reported diagnostic evidence in both runs. + if ( + !weatherReady || + Object.keys(leak).length || + [...a.errors, ...b.errors].some((e) => e.startsWith('pageerror:')) + ) { + process.exitCode = 1; + } +} catch (error) { + results.failure = { name: error.name, message: error.message }; + throw error; +} finally { + // Preserve completed resource evidence even if a later provider or browser + // phase aborts. A partial run still exits nonzero. writeFileSync( path.join(outDir, 'results.json'), JSON.stringify(results, null, 2), ); console.log(`wrote ${path.join(outDir, 'results.json')}`); -} finally { await browser.close(); } diff --git a/src/layers/weather/index.js b/src/layers/weather/index.js index db62ea98a..757764fe6 100644 --- a/src/layers/weather/index.js +++ b/src/layers/weather/index.js @@ -296,10 +296,6 @@ export function createWeatherLayer({ motion?.addEventListener?.('change', onVisibility); documentRef?.addEventListener?.('visibilitychange', onVisibility); registerClock(); - removeCamera = viewer.camera.moveEnd?.addEventListener(() => { - checkHost(); - if (enabled) notify(); - }); }, attachShellServices(services) { imageryHost = @@ -315,10 +311,18 @@ export function createWeatherLayer({ }, enable() { enabled = true; + if (!removeCamera) { + removeCamera = viewer.camera.moveEnd?.addEventListener(() => { + checkHost(); + notify(); + }); + } registerClock(); }, disable() { enabled = false; + removeCamera?.(); + removeCamera = null; unregisterClock?.(); unregisterClock = null; ++generation; @@ -357,7 +361,7 @@ export function createWeatherLayer({ signal: controller.signal, }); if (!enabled || controller.signal.aborted || request !== controller) - return false; + return enabled && !signal?.aborted && request !== controller; if (snapshot.unavailable) { error = msg( 'Weather source unavailable; previous observation retained', @@ -376,7 +380,9 @@ export function createWeatherLayer({ ) await show(snapshot.latest, controller.signal); return ( - !controller.signal.aborted && request === controller && enabled + enabled && + !signal?.aborted && + (!controller.signal.aborted || request !== controller) ); } const time = @@ -388,9 +394,14 @@ export function createWeatherLayer({ (satellite && rendering.getDiagnostics().infrared !== infrared) ) await show(time, controller.signal); - return !controller.signal.aborted && request === controller && enabled; + return ( + enabled && + !signal?.aborted && + (!controller.signal.aborted || request !== controller) + ); } catch (cause) { - if (controller.signal.aborted || request !== controller) return false; + if (controller.signal.aborted || request !== controller) + return enabled && !signal?.aborted && request !== controller; error = cause?.message || msg('Weather unavailable'); stop(); return true; diff --git a/src/layers/weather/weather.test.mjs b/src/layers/weather/weather.test.mjs index 1a4ba5177..486d42520 100644 --- a/src/layers/weather/weather.test.mjs +++ b/src/layers/weather/weather.test.mjs @@ -694,7 +694,11 @@ test('superseding a pending stage preserves the newer manifest loading state', a await flush(); const newer = h.layer.update(); await flush(); - assert.equal(await old, false); + assert.equal( + await old, + true, + 'replacement is accepted by the enable lifecycle', + ); assert.equal( h.layer.getStats().loading, true, @@ -2311,3 +2315,44 @@ for (const product of ['radar', 'clouds-regional', 'lightning']) { assert.equal(h.viewer.camera.moveEnd.size, 0); }); } + +test('a product replacement accepts supersession but never paints the obsolete first response', async () => { + const pending = []; + const h = layerHarness({ + id: 'weather-satellite', + feed: { + getSnapshot: (args) => { + const task = deferred(); + pending.push({ ...args, ...task }); + return task.promise; + }, + }, + }); + const first = h.layer.update(); + h.layer.setParams({ product: 'clouds' }); + assert.equal(pending.length, 2); + pending[0].resolve({ ...snapshot, product: 'clouds-regional' }); + assert.equal(await first, true); + assert.equal(h.stages.length, 0); + pending[1].resolve({ ...snapshot, product: 'clouds' }); + await flush(); + h.stages.at(-1).finish(); + await flush(); + assert.equal(h.layer.getParams().product, 'clouds'); + h.layer.destroy(); +}); + +test('disabled weather releases its camera listener and re-enabling binds only once', () => { + const h = layerHarness(); + h.layer.disable(); + assert.equal(h.moveEnd.size, 0, 'disabled layers own no active camera work'); + h.layer.enable(); + h.layer.enable(); + assert.equal(h.moveEnd.size, 1); + h.layer.disable(); + assert.equal(h.moveEnd.size, 0); + h.layer.enable(); + assert.equal(h.moveEnd.size, 1); + h.layer.destroy(); + assert.equal(h.moveEnd.size, 0); +}); diff --git a/src/layers/wind/index.js b/src/layers/wind/index.js index c76e2501c..aa6161f06 100644 --- a/src/layers/wind/index.js +++ b/src/layers/wind/index.js @@ -180,7 +180,7 @@ export function createWindLayer({ overlay: requestedScalar(), }); if (!enabled || controller.signal.aborted || request !== controller) - return false; + return enabled && !signal?.aborted && request !== controller; manifest = snapshot; error = null; if (sampledPosition) sample(sampledPosition); @@ -194,7 +194,8 @@ export function createWindLayer({ } return true; } catch (cause) { - if (controller.signal.aborted || request !== controller) return false; + if (controller.signal.aborted || request !== controller) + return enabled && !signal?.aborted && request !== controller; error = cause?.message || msg('Wind source unavailable'); return true; } finally { diff --git a/src/layers/wind/index.test.mjs b/src/layers/wind/index.test.mjs index ac843f62a..3b5e09697 100644 --- a/src/layers/wind/index.test.mjs +++ b/src/layers/wind/index.test.mjs @@ -412,3 +412,36 @@ test('wind unit chips appear only alongside a speed legend, including canvas tra for (const overlay of ['pressure', 'temperature', 'none']) { layer.setParams({ overlay }); assert.deepEqual(unitChips(), []); } layer.destroy(); }); + +test('a parameter replacement during manager enable does not roll the layer back', async () => { + const { LayerLifecycle } = await import('../../data/lifecycle.js'); + const pending = []; + const rendering = Object.fromEntries( + ['attach', 'start', 'stop', 'clear', 'destroy', 'setField'].map((name) => [ + name, + () => {}, + ]), + ); + const layer = createWindLayer({ + feed: { + getSnapshot: (args) => + new Promise((resolve) => pending.push({ ...args, resolve })), + }, + createRendering: () => rendering, + }); + const manager = new LayerLifecycle({ container: {} }); + manager.register(layer); + const enabled = manager.setEnabled('wind', true); + for (let i = 0; i < 20 && !pending.length; i++) await Promise.resolve(); + layer.setParams({ model: 'ifs' }); + await Promise.resolve(); + pending[0].resolve(snapshot('gfs')); + assert.equal(await enabled, true); + assert.equal(manager.isEnabled('wind'), true); + assert.equal(layer.getStats().loading, true); + pending[1].resolve(snapshot('ifs')); + for (let i = 0; i < 10; i++) await Promise.resolve(); + assert.equal(layer.getStats().model, 'IFS'); + assert.equal(layer.getStats().count, 4); + await manager.destroyAll(); +});