From d86047fe8f1fa6727c2a21c68681cc257f79ba21 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Tue, 3 Nov 2026 19:45:34 +0400 Subject: [PATCH] Release the shell from a candidate engine that fails after it attached The shell's scene part detaches it too, so a candidate failing in a later part (a layer's drawing, Cockpit clouds) leaves the shell free for the working engine instead of refusing every later attach. StyleManager.attachScene releases what it built when it throws, and one detach runs at a time. The controls phase awaits its parts, and a failed attachment no longer forces handoff off for the parts it releases. scripts/qa-app-once.mjs now also rolls back from a Navara candidate whose layer attachment fails. --- scripts/package-boundaries.json | 1 + scripts/qa-app-once.mjs | 40 ++++++++++ src/app/controls.js | 28 ++----- src/app/controls.test.mjs | 2 +- src/app/controlsSceneFailure.test.mjs | 60 ++++++++++++++ src/app/sceneHost.js | 6 +- src/app/shellPart.js | 31 ++++++++ src/app/tools.js | 2 +- src/cameraHandoff.test.mjs | 4 +- src/loadingFeedback.test.mjs | 2 +- src/ui/applicationShell.js | 108 +++++++++++++++----------- src/ui/shellSceneSwap.test.mjs | 21 +++++ 12 files changed, 229 insertions(+), 76 deletions(-) create mode 100644 src/app/controlsSceneFailure.test.mjs create mode 100644 src/app/shellPart.js 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')); +});