diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index f40cf571f..9a0525970 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -2662,6 +2662,7 @@ "src/app/tools.js", "src/app/sessionVoice.js", "src/app/sceneParts.js", + "src/app/shellPart.js", "src/app/viewer.js", "src/sky/starrySky.js", "src/sky/stars.js", diff --git a/scripts/qa-app-once.mjs b/scripts/qa-app-once.mjs index 85c62bcf0..69bb1051a 100644 --- a/scripts/qa-app-once.mjs +++ b/scripts/qa-app-once.mjs @@ -320,6 +320,46 @@ try { 'ids changed after the rollback', idDifference(boot.ids, final.ids), ); + + // A candidate failing after the shell attached to it: a layer's + // drawing refuses to attach on Navara. The shell lets go of the failed + // globe and Cesium attaches again; the next switch works. + await page.evaluate(() => { + window.__qaDataRefused = false; + const module = + window.__godsEyeView.dataManager.layers.get('earthquakes').module; + const attach = module.attachRenderer; + module.attachRenderer = function () { + module.attachRenderer = attach; + window.__qaDataRefused = true; + return Promise.reject(new Error('QA data attach failure')); + }; + }); + const beforeDataFailure = await page.evaluate(() => history.length); + await requestEngine(page, 'navara'); + await page.waitForFunction(() => window.__qaDataRefused === true, { + timeout: 60_000, + }); + await waitReady(page, 'cesium'); + const afterDataFailure = await readSession(page); + report.dataFailure = { + ids: idDifference(boot.ids, afterDataFailure.ids), + counters: afterDataFailure.counters, + history: (await page.evaluate(() => history.length)) - beforeDataFailure, + }; + check( + Object.keys(report.dataFailure.ids).length === 0 && + report.dataFailure.history === 0, + 'a candidate failing in the data part changed the session', + report.dataFailure, + ); + await switchEngine(page, 'navara'); + await switchEngine(page, 'cesium'); + check( + Object.keys(idDifference(boot.ids, (await readSession(page)).ids)) + .length === 0, + 'ids changed after the data-part failure', + ); } report.errors = errors; check(errors.length === 0, 'page errors', errors); diff --git a/src/app/controls.js b/src/app/controls.js index fe18c9875..957e39a99 100644 --- a/src/app/controls.js +++ b/src/app/controls.js @@ -4,6 +4,7 @@ import { flyToAustin } from '../camera.js'; import { initCockpitCloudEffects } from '../cockpitCloudEffects.js'; import { t } from '../i18n/index.js'; import { attachScenePart, currentSceneOf } from './sceneParts.js'; +import { shellScenePart } from './shellPart.js'; /** * Construct the existing controls and camera presentation. `initialShareCode` @@ -15,7 +16,7 @@ import { attachScenePart, currentSceneOf } from './sceneParts.js'; * engines it attaches to each later one (StyleManager.attachScene) with that * engine's catalog, and the Cockpit cloud effects follow the viewer. */ -export function createApplicationControls({ +export async function createApplicationControls({ scene, loaderStatus, Controls = StyleManager, @@ -59,28 +60,9 @@ export function createApplicationControls({ defer(() => styleManager.hud.destroy()); defer(() => styleManager.dispose()); // The shell was built on this globe; later globes attach to it. - let builtOn = currentSceneOf(scene); - attachScenePart( + await attachScenePart( scene, - { - name: 'shell', - attachScene(next) { - if (next === builtOn) { - builtOn = null; - return; - } - builtOn = null; - styleManager.attachScene({ - viewer: next.viewer, - engine: next.engine, - mapStackController: next.mapStackController, - services: catalogControlServices(next.catalog ?? catalog), - engineSwitch: true, - }); - }, - // The shell leaves its globe in 'shell-release' (the tools phase), - // before the data layers detach, as its teardown always did. - }, + shellScenePart(styleManager, { builtOn: currentSceneOf(scene), catalog }), { defer }, ); // The previous multi-canvas weather compositor remains disabled. Cockpit @@ -88,7 +70,7 @@ export function createApplicationControls({ // Cesium fog or post-process stages and is fully stopped in map mode. const weatherEffects = null; let cockpitCloudEffects = null; - attachScenePart( + await attachScenePart( scene, { name: 'cockpit-clouds', diff --git a/src/app/controls.test.mjs b/src/app/controls.test.mjs index 24a6b5a8e..98ed58d2f 100644 --- a/src/app/controls.test.mjs +++ b/src/app/controls.test.mjs @@ -18,7 +18,7 @@ test('the controls pass the scene engine to the shell', () => { assert.match(controls, /new Controls\(viewer, \{[\s\S]*?\n\s+engine,\n/); // A later globe reaches the shell with its own engine. assert.match( - controls, + read('./shellPart.js'), /styleManager\.attachScene\(\{\s+viewer: next\.viewer,\s+engine: next\.engine,/, ); }); diff --git a/src/app/controlsSceneFailure.test.mjs b/src/app/controlsSceneFailure.test.mjs new file mode 100644 index 000000000..9e992fb72 --- /dev/null +++ b/src/app/controlsSceneFailure.test.mjs @@ -0,0 +1,60 @@ +// A candidate engine that fails after the shell attached to it (a later +// part: data, Cockpit clouds) must leave the shell detached, so the working +// engine attaches again. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { shellScenePart } from './shellPart.js'; +import { createSceneHost } from './sceneHost.js'; + +/** A shell double with the real attach rule: one globe at a time. */ +class FakeShell { + constructor(engine) { + this.engine = engine; + this.attaches = []; + } + attachScene({ engine }) { + if (this.engine) + throw new Error('The shell is attached to a globe already'); + this.engine = engine; + this.attaches.push(engine.kind); + } + async detachScene() { + this.engine = null; + } +} + +test('a candidate failing in a later part leaves the shell ready for the working engine', async () => { + const sceneHost = createSceneHost(); + const catalog = { get: (id) => ({ id }), unavailableMetadata: [] }; + const cesium = { + kind: 'cesium', + viewer: null, + engine: { kind: 'cesium' }, + catalog, + }; + await sceneHost.attach(cesium); + const styleManager = new FakeShell(cesium.engine); + await sceneHost.register( + shellScenePart(styleManager, { builtOn: cesium, catalog }), + ); + // The data part: it fails on Navara. + await sceneHost.register({ + name: 'data', + attachScene(next) { + if (next.kind === 'navara') throw new Error('layer attachment failed'); + }, + }); + await sceneHost.detach({ handoff: true }); + assert.equal(styleManager.engine, null); + const navara = { + kind: 'navara', + viewer: null, + engine: { kind: 'navara' }, + catalog, + }; + await assert.rejects(sceneHost.attach(navara), /layer attachment failed/); + assert.equal(styleManager.engine, null, 'the failed candidate let go'); + await sceneHost.attach(cesium); + assert.equal(styleManager.engine.kind, 'cesium'); + assert.deepEqual(styleManager.attaches, ['navara', 'cesium']); +}); diff --git a/src/app/sceneHost.js b/src/app/sceneHost.js index b56842c31..e3d1c65dd 100644 --- a/src/app/sceneHost.js +++ b/src/app/sceneHost.js @@ -129,10 +129,8 @@ export function createSceneHost({ onError = null } = {}) { for (const part of [...parts]) await attachPart(part, target, current); } catch (error) { - const errors = await release(current, { - handoff: false, - failed: true, - }); + // The parts keep their own handoff choice; `failed` names why. + const errors = await release(current, { failed: true }); attachment = null; scene = null; state = 'detached'; diff --git a/src/app/shellPart.js b/src/app/shellPart.js new file mode 100644 index 000000000..bfa4cc86a --- /dev/null +++ b/src/app/shellPart.js @@ -0,0 +1,31 @@ +import { catalogControlServices } from './catalog.js'; + +/** + * The shell's scene part: the shell was built on `builtOn`, and each later + * globe attaches to it with that globe's catalog. A switch detaches the + * shell in 'shell-release' (the tools phase), before the data layers, as its + * teardown always did; this part's detach covers an attachment that failed + * before that part attached (a later part of a candidate engine) and is a + * no-op once the shell has let go. + */ +export function shellScenePart(styleManager, { builtOn, catalog }) { + let first = builtOn; + return { + name: 'shell', + attachScene(next) { + if (next === first) { + first = null; + return; + } + first = null; + styleManager.attachScene({ + viewer: next.viewer, + engine: next.engine, + mapStackController: next.mapStackController, + services: catalogControlServices(next.catalog ?? catalog), + engineSwitch: true, + }); + }, + detachScene: (options) => styleManager.detachScene(options), + }; +} diff --git a/src/app/tools.js b/src/app/tools.js index fc6a0f782..604359638 100644 --- a/src/app/tools.js +++ b/src/app/tools.js @@ -113,7 +113,7 @@ export async function createApplicationTools({ scene, { name: 'shell-release', - detachScene: () => styleManager.detachScene?.(), + detachScene: (options) => styleManager.detachScene?.(options), }, { defer }, ); diff --git a/src/cameraHandoff.test.mjs b/src/cameraHandoff.test.mjs index 87454e61a..b4a594412 100644 --- a/src/cameraHandoff.test.mjs +++ b/src/cameraHandoff.test.mjs @@ -306,7 +306,7 @@ test('newer navigation, reset, Cockpit, and teardown share one generation', () = 'this._navigation.stop();', ], 'dispose invalidation'); // An engine switch retires the outgoing globe's navigation the same way. - const detach = body(ui, /async detachScene\(\{[\s\S]*?\} = \{\}\) \{([\s\S]*?)\n \}/, 'detach'); + const detach = body(ui, /async _detachScene\(\{[\s\S]*?\} = \{\}\) \{([\s\S]*?)\n \}/, 'detach'); ordered(detach, [ 'this._navigation.setScene(', 'this._layerBindings.stop();', @@ -321,7 +321,7 @@ test('teardown synchronously closes immediate camera entry points', () => { 'this._disposed = true;', 'await this.detachScene(', ], 'synchronous teardown barrier'); - const detach = body(ui, /async detachScene\(\{[\s\S]*?\} = \{\}\) \{([\s\S]*?)\n \}/, 'detach'); + const detach = body(ui, /async _detachScene\(\{[\s\S]*?\} = \{\}\) \{([\s\S]*?)\n \}/, 'detach'); ordered(detach, [ 'this._layerBindings.stop();', 'await this._contextControls.restoreForDisposal();', diff --git a/src/loadingFeedback.test.mjs b/src/loadingFeedback.test.mjs index 4740ac333..a67cade9a 100644 --- a/src/loadingFeedback.test.mjs +++ b/src/loadingFeedback.test.mjs @@ -212,7 +212,7 @@ test('universal notice lifecycle clears on dispose and uses the one top-center l assert.match(dispose, /this\._feedback\.clearLoadingStatus\(\);/); // The globe-bound parts, share restoration included, go with the scene. assert.match(dispose, /await this\.detachScene\(/); - const detachStart = ui.indexOf(' async detachScene({'); + const detachStart = ui.indexOf(' async _detachScene({'); const detach = ui.slice(detachStart, ui.indexOf('\n }\n', detachStart)); assert.match(detach, /this\._shareRestoration\.destroy\(\)/); assert.match(ShareRestoration.prototype.destroy.toString(), /this\._shareTrackingNoticeGeneration \+= 1;/); diff --git a/src/ui/applicationShell.js b/src/ui/applicationShell.js index d15e0be1d..53f3130db 100644 --- a/src/ui/applicationShell.js +++ b/src/ui/applicationShell.js @@ -708,49 +708,61 @@ export class StyleManager extends ShellFacade { if (this._disposed) return; if (this._scene.engine) throw new Error('The shell is attached to a globe already'); - const previousServices = this.services; - this.services = { ...this.services, ...services }; - this._scene = { viewer, engine, mapStackController }; - const { - flightsLayer, - militaryFlightsLayer, - satellitesLayer, - aisLiveVesselsLayer, - militaryAwarenessLayer, - rocketLaunchesLayer, - } = this.services; - this._navigation.tracking = { - flightsLayer, - militaryFlightsLayer, - satellitesLayer, - aisLiveVesselsLayer, - militaryAwarenessLayer, - rocketLaunchesLayer, - }; - this._navigation.setScene({ viewer, engine }); - this._locationNavigation.attachScene({ viewer, engine }); - this.hud.attachEngine(engine, { viewer }); - this.shareLinkManager.attachEngine(engine); - this._displayBindings.attachViewer(viewer); - this._buildSceneControls(); - this._buildSceneCockpit(); - this._shareRestoration.attachLinks(this.shareLinkManager); - this._initSceneOverlays(); - this._initMapStackControl(); - this._refreshLayerControls(previousServices); - this._initCameraOrientationControls(); - this._initHudScene(); - this._initCockpitDisplayPortal(); - this._applyGlobalPostDefaults(); - this._startAnimationLoop(); - this._publishActiveStyle(); - this._shareRestoration.start({ - engineSwitch, - retainScene: Boolean(this._layerSession?.handoffLayers), - }); - this._layerBindings.observeCamera(); - this._layoutRightPanels(); - this._syncCctvPanelViewport(); + // An attachment that fails part-way releases what it built: the shell + // stays detached, ready for the working engine. + try { + const previousServices = this.services; + this.services = { ...this.services, ...services }; + this._scene = { viewer, engine, mapStackController }; + const { + flightsLayer, + militaryFlightsLayer, + satellitesLayer, + aisLiveVesselsLayer, + militaryAwarenessLayer, + rocketLaunchesLayer, + } = this.services; + this._navigation.tracking = { + flightsLayer, + militaryFlightsLayer, + satellitesLayer, + aisLiveVesselsLayer, + militaryAwarenessLayer, + rocketLaunchesLayer, + }; + this._navigation.setScene({ viewer, engine }); + this._locationNavigation.attachScene({ viewer, engine }); + this.hud.attachEngine(engine, { viewer }); + this.shareLinkManager.attachEngine(engine); + this._displayBindings.attachViewer(viewer); + this._buildSceneControls(); + this._buildSceneCockpit(); + this._shareRestoration.attachLinks(this.shareLinkManager); + this._initSceneOverlays(); + this._initMapStackControl(); + this._refreshLayerControls(previousServices); + this._initCameraOrientationControls(); + this._initHudScene(); + this._initCockpitDisplayPortal(); + this._applyGlobalPostDefaults(); + this._startAnimationLoop(); + this._publishActiveStyle(); + this._shareRestoration.start({ + engineSwitch, + retainScene: Boolean(this._layerSession?.handoffLayers), + }); + this._layerBindings.observeCamera(); + this._layoutRightPanels(); + this._syncCctvPanelViewport(); + } catch (error) { + void this.detachScene().catch((cleanupError) => + console.error( + 'The shell could not leave a failed globe:', + cleanupError, + ), + ); + throw error; + } } /** @@ -759,7 +771,15 @@ export class StyleManager extends ShellFacade { * the open panels stay. `handoff` keeps the selection and a retainable * Context mode for the next globe. */ - async detachScene({ + detachScene(options = {}) { + // One detach at a time: a second request joins the one under way. + this._sceneDetach ??= this._detachScene(options).finally(() => { + this._sceneDetach = null; + }); + return this._sceneDetach; + } + + async _detachScene({ handoff = Boolean(this._layerSession?.handoffLayers), } = {}) { if (!this._scene.engine && !this._scene.viewer) return; diff --git a/src/ui/shellSceneSwap.test.mjs b/src/ui/shellSceneSwap.test.mjs index c7dafb6d7..69018f2ad 100644 --- a/src/ui/shellSceneSwap.test.mjs +++ b/src/ui/shellSceneSwap.test.mjs @@ -195,3 +195,24 @@ test('a Context transition in flight is restored before the globe goes', async ( assert.equal(session.context._contextModeGeneration, 5); assert.equal(session.context._contextModeChanging, false); }); + +test('an attachment that throws part-way leaves the shell detached and attachable', async () => { + const { shell, log } = fakeShell(); + await shell.detachScene({ handoff: true }); + shell._initSceneOverlays = () => { + throw new Error('overlay host failed'); + }; + assert.throws( + () => shell.attachScene({ engine: { kind: 'navara' } }), + /overlay host failed/, + ); + // The shell releases what it built and is detached when that settles. + await shell.detachScene(); + assert.equal(shell.engine, null); + log.length = 0; + shell._initSceneOverlays = () => log.push('_initSceneOverlays'); + const engine = { kind: 'cesium' }; + shell.attachScene({ engine }); + assert.equal(shell.engine, engine); + assert.ok(log.includes('shareRestoration.start')); +});