Detach every layer from a scene even when one fails, and let in-flight attachments stop first

src/data/lifecycle.js

@@ -107,6 +107,12 @@ export class LayerLifecycle {
     this._registrationDispositions = null;
     this._allowQaRegistration = allowQaRegistration === true;
     this._qaLayerIds = new Set();
+    // The bound scene: its signal reaches every attachRenderer() call and is
+    // aborted when the scene detaches. Asynchronous attachments still running
+    // are kept here so the detach releases them only after they stopped.
+    this._sceneController = null;
+    this._sceneDetached = false;
+    this._pendingAttachments = new Set();
   }
 
   /** The second argument of every layer module call: `{ signal, engine }`. */
@@ -125,20 +131,71 @@ export class LayerLifecycle {
    * Release the scene only. Models with an explicit renderer lifetime keep
    * their entry, polling interval, visibility intents and in-flight source.
    * Legacy modules use their ordinary teardown until they gain that contract.
+   *
+   * Every layer is released even when another one fails, and the manager is
+   * always left unbound, so the next bindCatalog() can attach a scene. The
+   * failures are reported together. A layer whose release failed keeps it
+   * pending: a repeated detachScene(), or the next bindCatalog(), retries it.
+   * Periodic refreshes due while no scene is bound wait for the next one.
    */
   async detachScene() {
-    for (const [id, entry] of [...this.layers]) {
-      await entry.toggleChain.catch(() => {});
-      if (entry.module.attachRenderer && entry.module.detachRenderer) {
-        if (entry.initialized) await entry.module.detachRenderer(this.viewer);
-      } else if (!(await this.destroyLayer(id))) {
-        throw new Error(`Cannot detach layer ${id}`);
+    // Attachments still in flight stop at their next checkpoint; their
+    // partial work is released below like a finished attachment.
+    this._sceneController?.abort();
+    this._sceneController = null;
+    this._sceneDetached = true;
+    const viewer = this.viewer;
+    const errors = [];
+    const droppedQaLayers = [];
+    try {
+      await Promise.allSettled([...this._pendingAttachments]);
+      for (const [id, entry] of [...this.layers]) {
+        try {
+          await entry.toggleChain.catch(() => {});
+          if (entry.module.attachRenderer && entry.module.detachRenderer) {
+            await this._detachEntryRenderer(entry, viewer);
+          } else {
+            const qaLayer = this._qaLayerIds.has(id);
+            if (!(await this.destroyLayer(id))) {
+              // The next catalog brings a fresh module for this id. A stale
+              // entry would refuse it, so the failed one is dropped here.
+              this._stopUpdateLoop(entry);
+              this.layers.delete(id);
+              this._qaLayerIds.delete(id);
+              throw new Error(`Cannot detach layer ${id}`);
+            }
+            if (qaLayer) droppedQaLayers.push(id);
+          }
+        } catch (error) {
+          errors.push(error);
+        }
       }
+    } finally {
+      this.viewer = null;
+      this.engine = null;
+      this._registrationsFinalized = false;
+      this._registrationDispositions = null;
     }
-    this.viewer = null;
-    this.engine = null;
-    this._registrationsFinalized = false;
-    this._registrationDispositions = null;
+    if (droppedQaLayers.length)
+      console.warn(
+        `[Data] QA layers do not survive a scene switch; register them again: ${droppedQaLayers.join(', ')}`,
+      );
+    if (errors.length) {
+      for (const error of errors)
+        console.warn('[Data] scene detach error:', error);
+      throw new AggregateError(errors, 'Data layers could not detach');
+    }
+  }
+
+  /** Release one retained layer's renderer, keeping a failure for a retry. */
+  async _detachEntryRenderer(entry, viewer) {
+    const pending = entry.pendingSceneDetach;
+    if (!pending && (!entry.initialized || entry.sceneDetached)) return;
+    const target = pending ? pending.viewer : viewer;
+    entry.pendingSceneDetach = { viewer: target };
+    await entry.module.detachRenderer(target);
+    entry.pendingSceneDetach = null;
+    entry.sceneDetached = true;
   }
 
   /** Bind the next catalog only after the preceding scene released its entries. */
@@ -147,6 +204,12 @@ export class LayerLifecycle {
     this.viewer = viewer;
     this.engine = engine ?? engineFromViewer(viewer) ?? null;
     this._catalogSourceAvailability = catalog.getSourceAvailability;
+    const scene = new AbortController();
+    this._sceneController = scene;
+    this._sceneDetached = false;
+    // An attachment that is still running when the scene detaches sees this
+    // signal aborted and must stop touching the scene.
+    const context = { ...this._moduleContext(scene.signal), ...options };
     const attachments = [];
     for (const layer of catalog.layers) {
       const entry = this.layers.get(layer.id);
@@ -155,11 +218,7 @@ export class LayerLifecycle {
         if (entry.module !== layer)
           throw new Error(`Layer owner changed: ${layer.id}`);
         if (entry.initialized) {
-          const ready = layer.attachRenderer(viewer, {
-            ...this._moduleContext(),
-            ...options,
-          });
-          if (ready?.then) attachments.push(ready);
+          attachments.push(this._attachEntryRenderer(entry, viewer, context));
         }
       }
     }
@@ -171,7 +230,50 @@ export class LayerLifecycle {
       this.layers.set(id, entry);
     }
     this.finalizeRegistrations(catalog.metadata);
-    if (attachments.length) return Promise.all(attachments);
+    const pending = attachments.filter((ready) => ready?.then);
+    const settled = Promise.allSettled(pending).then(() => {
+      if (this._sceneController === scene) this._runDeferredRefreshes();
+    });
+    if (pending.length) return settled.then(() => Promise.all(pending));
+  }
+
+  /**
+   * Attach one retained layer to the bound scene. A release that failed on
+   * the previous scene is retried first. Asynchronous attachments are tracked
+   * so detachScene() releases them only after they stopped.
+   */
+  _attachEntryRenderer(entry, viewer, context) {
+    let ready;
+    try {
+      const pending = entry.pendingSceneDetach;
+      entry.sceneDetached = false;
+      if (pending) {
+        ready = Promise.resolve()
+          .then(() => entry.module.detachRenderer(pending.viewer))
+          .then(() => {
+            entry.pendingSceneDetach = null;
+            if (context.signal.aborted) return undefined;
+            return entry.module.attachRenderer(viewer, context);
+          });
+      } else ready = entry.module.attachRenderer(viewer, context);
+    } catch (error) {
+      ready = Promise.reject(error);
+    }
+    if (!ready?.then) return ready;
+    const tracked = Promise.resolve(ready);
+    this._pendingAttachments.add(tracked);
+    const untrack = () => this._pendingAttachments.delete(tracked);
+    tracked.then(untrack, untrack);
+    return tracked;
+  }
+
+  /** Run the periodic refreshes that came due while no scene was bound. */
+  _runDeferredRefreshes() {
+    for (const [layerId, entry] of this.layers) {
+      if (!entry.refreshDeferred) continue;
+      entry.refreshDeferred = false;
+      void this._runPeriodicUpdate(layerId, entry);
+    }
   }
 
   /** Register a synthetic layer after sealing in an explicitly dev-enabled manager. */
@@ -251,6 +353,12 @@ export class LayerLifecycle {
       // interval is armed after the user already turned the layer off, and a
       // subsequent enable arms a SECOND interval → 2× poll → OpenSky 429 (M1).
       toggleChain: Promise.resolve(),
+      // Scene handoff (detachScene/bindCatalog): whether this entry's
+      // renderer is released, a release that failed and is retried, and a
+      // periodic refresh that came due while no scene was bound.
+      sceneDetached: false,
+      pendingSceneDetach: null,
+      refreshDeferred: false,
     });
   }
 
@@ -346,6 +454,11 @@ export class LayerLifecycle {
   }
 
   async _runPeriodicUpdate(layerId, entry, { signal = null } = {}) {
+    if (this._sceneDetached) {
+      // No renderer to draw into: refresh as soon as the next scene binds.
+      if (entry.enabled && !entry.destroying) entry.refreshDeferred = true;
+      return false;
+    }
     if (
       !entry.enabled ||
       entry.lifecycleState !== 'enabled' ||

src/data/lifecycleScene.test.mjs

@@ -0,0 +1,304 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { LayerLifecycle } from './lifecycle.js';
+
+// A layer with a renderer lifetime: it outlives scenes through
+// detachRenderer()/attachRenderer(). Hooks replace the default behaviour.
+function sceneLayer(id, hooks = {}) {
+  const calls = [];
+  const module = {
+    id,
+    name: id,
+    icon: '',
+    source: 'test',
+    init(viewer, context) {
+      calls.push('init');
+      module.attachRenderer(viewer, context);
+      return true;
+    },
+    attachRenderer(viewer, context) {
+      calls.push(['attach', viewer]);
+      return hooks.attach?.(viewer, context);
+    },
+    detachRenderer(viewer) {
+      calls.push(['detach', viewer]);
+      return hooks.detach?.(viewer);
+    },
+    enable: () => true,
+    update(viewer) {
+      calls.push(['update', viewer]);
+      return true;
+    },
+    disable: () => true,
+    destroy: () => true,
+  };
+  return { module, calls };
+}
+
+function catalog(...modules) {
+  return {
+    layers: modules,
+    metadata: modules.map(({ id }) => ({
+      id,
+      disposition: 'enabled-only',
+      token: id,
+    })),
+  };
+}
+
+const engine = (name) => ({ kind: 'test', name });
+const firstViewer = { name: 'first viewer' };
+const secondViewer = { name: 'second viewer' };
+
+async function boundManager(...layers) {
+  const manager = new LayerLifecycle(firstViewer, {
+    engine: engine('first'),
+  });
+  const modules = layers.map(({ module }) => module);
+  await manager.bindCatalog(firstViewer, engine('first'), catalog(...modules));
+  for (const { module } of layers)
+    assert.equal(
+      await manager.setEnabled(module.id, true, { origin: 'user' }),
+      true,
+    );
+  return { manager, modules };
+}
+
+// Detach failures are also logged; keep the test output to the assertions.
+async function quietly(run) {
+  const warn = console.warn;
+  console.warn = () => {};
+  try {
+    return await run();
+  } finally {
+    console.warn = warn;
+  }
+}
+
+const detaches = (calls) => calls.filter((call) => call[0] === 'detach');
+const attaches = (calls) => calls.filter((call) => call[0] === 'attach');
+
+test('one failing detachRenderer leaves the others released and the manager unbound', async () => {
+  let failures = 1;
+  const broken = sceneLayer('broken', {
+    detach() {
+      if (failures-- > 0) throw new Error('release failed');
+    },
+  });
+  const after = sceneLayer('after');
+  const { manager, modules } = await boundManager(broken, after);
+
+  await assert.rejects(
+    quietly(() => manager.detachScene()),
+    (error) => {
+      assert.ok(error instanceof AggregateError);
+      assert.deepEqual(
+        error.errors.map(({ message }) => message),
+        ['release failed'],
+      );
+      return true;
+    },
+  );
+  assert.deepEqual(detaches(after.calls), [['detach', firstViewer]]);
+  assert.equal(manager.viewer, null);
+  assert.equal(manager.engine, null);
+  assert.equal(manager.registrationsFinalized, false);
+
+  // A repeated detach retries only the failed release, on its own viewer.
+  await manager.detachScene();
+  assert.deepEqual(detaches(broken.calls), [
+    ['detach', firstViewer],
+    ['detach', firstViewer],
+  ]);
+  assert.equal(detaches(after.calls).length, 1);
+
+  // The next scene binds and both retained layers attach to it.
+  await manager.bindCatalog(
+    secondViewer,
+    engine('second'),
+    catalog(...modules),
+  );
+  assert.equal(manager.viewer, secondViewer);
+  assert.deepEqual(attaches(broken.calls).at(-1), ['attach', secondViewer]);
+  assert.deepEqual(attaches(after.calls).at(-1), ['attach', secondViewer]);
+  assert.equal(manager.isEnabled('broken'), true);
+});
+
+test('bindCatalog after a failed detach retries the release before attaching', async () => {
+  let failures = 1;
+  const broken = sceneLayer('broken', {
+    detach() {
+      if (failures-- > 0) throw new Error('release failed');
+    },
+  });
+  const { manager, modules } = await boundManager(broken);
+  await assert.rejects(
+    quietly(() => manager.detachScene()),
+    AggregateError,
+  );
+
+  await manager.bindCatalog(
+    secondViewer,
+    engine('second'),
+    catalog(...modules),
+  );
+  assert.deepEqual(broken.calls.slice(-3), [
+    ['detach', firstViewer],
+    ['detach', firstViewer],
+    ['attach', secondViewer],
+  ]);
+  // The scene is settled: the next detach releases the second viewer once.
+  await manager.detachScene();
+  assert.deepEqual(broken.calls.at(-1), ['detach', secondViewer]);
+});
+
+test('an asynchronous attach detached mid-flight stops before the release', async () => {
+  let resume;
+  let attachSignal;
+  let attachFinished = false;
+  let sawAbortBeforeFinishing = null;
+  const slow = sceneLayer('slow', {
+    async attach(viewer, { signal }) {
+      if (viewer !== secondViewer) return;
+      attachSignal = signal;
+      await new Promise((resolve) => {
+        resume = resolve;
+      });
+      sawAbortBeforeFinishing = signal.aborted;
+      attachFinished = true;
+    },
+    detach() {
+      assert.equal(
+        attachFinished || attachSignal === undefined,
+        true,
+        'the release waits for the attachment to stop',
+      );
+    },
+  });
+  const { manager, modules } = await boundManager(slow);
+  await manager.detachScene();
+
+  const bound = manager.bindCatalog(
+    secondViewer,
+    engine('second'),
+    catalog(...modules),
+  );
+  assert.ok(bound?.then, 'an asynchronous attachment is awaited');
+  assert.equal(attachSignal.aborted, false);
+
+  const detached = manager.detachScene();
+  assert.equal(attachSignal.aborted, true);
+  let released = false;
+  void detached.then(() => {
+    released = true;
+  });
+  await new Promise((resolve) => setImmediate(resolve));
+  assert.equal(released, false, 'the detach waits for the attachment');
+  assert.equal(detaches(slow.calls).length, 1);
+
+  resume();
+  await detached;
+  await bound;
+  assert.equal(sawAbortBeforeFinishing, true);
+  assert.deepEqual(detaches(slow.calls).at(-1), ['detach', secondViewer]);
+});
+
+test('periodic refreshes due while no scene is bound run after the next bind', async () => {
+  const layer = sceneLayer('feed');
+  const { manager, modules } = await boundManager(layer);
+  const entry = manager.layers.get('feed');
+  const updates = () =>
+    layer.calls.filter((call) => call[0] === 'update').length;
+  const before = updates();
+
+  await manager.detachScene();
+  assert.equal(await manager._runPeriodicUpdate('feed', entry), false);
+  assert.equal(updates(), before, 'no refresh against a detached scene');
+  assert.equal(entry.refreshDeferred, true);
+
+  await manager.bindCatalog(
+    secondViewer,
+    engine('second'),
+    catalog(...modules),
+  );
+  await new Promise((resolve) => setImmediate(resolve));
+  assert.equal(updates(), before + 1);
+  assert.deepEqual(layer.calls.at(-1), ['update', secondViewer]);
+  assert.equal(entry.refreshDeferred, false);
+});
+
+test('a legacy layer that cannot be destroyed is dropped so the next catalog binds', async () => {
+  const legacy = {
+    id: 'legacy',
+    name: 'legacy',
+    icon: '',
+    source: 'test',
+    init: () => true,
+    enable: () => true,
+    update: () => true,
+    disable() {
+      throw new Error('disable failed');
+    },
+    destroy: () => true,
+  };
+  const retained = sceneLayer('retained');
+  const manager = new LayerLifecycle(firstViewer, { engine: engine('first') });
+  await manager.bindCatalog(
+    firstViewer,
+    engine('first'),
+    catalog(legacy, retained.module),
+  );
+  await manager.setEnabled('legacy', true, { origin: 'user' });
+  await manager.setEnabled('retained', true, { origin: 'user' });
+  const warn = console.warn;
+  console.warn = () => {};
+  try {
+    await assert.rejects(manager.detachScene(), /could not detach/);
+  } finally {
+    console.warn = warn;
+  }
+  assert.equal(manager.layers.has('legacy'), false);
+  assert.deepEqual(detaches(retained.calls), [['detach', firstViewer]]);
+
+  const fresh = { ...legacy, disable: () => true };
+  await manager.bindCatalog(
+    secondViewer,
+    engine('second'),
+    catalog(fresh, retained.module),
+  );
+  assert.equal(manager.layers.get('legacy').module, fresh);
+});
+
+test('QA layers are reported when a scene switch drops them', async () => {
+  const retained = sceneLayer('retained');
+  const manager = new LayerLifecycle(firstViewer, {
+    engine: engine('first'),
+    allowQaRegistration: true,
+  });
+  await manager.bindCatalog(
+    firstViewer,
+    engine('first'),
+    catalog(retained.module),
+  );
+  manager.registerForQa({
+    id: 'qa-probe',
+    name: 'QA',
+    icon: '',
+    source: 'test',
+    init: () => true,
+    enable: () => true,
+    update: () => true,
+    disable: () => true,
+    destroy: () => true,
+  });
+  const warnings = [];
+  const warn = console.warn;
+  console.warn = (...args) => warnings.push(args.join(' '));
+  try {
+    await manager.detachScene();
+  } finally {
+    console.warn = warn;
+  }
+  assert.equal(manager.layers.has('qa-probe'), false);
+  assert.ok(warnings.some((line) => line.includes('qa-probe')));
+});