Merge branch 'main' into fix/i18n-layer-errors-in-data

changelog.d/1014-director-callback-failures.md

@@ -0,0 +1,4 @@
+- Director clock listeners, interaction observers and scene release failures
+  no longer leave the director inconsistent: each scene is released once, and
+  a phase error is kept (with the release failure logged) when cleanup also
+  fails (deepeshbind, #1014).

docs/CURRENT-STATE.md

@@ -1144,6 +1144,10 @@ Shot selection updates the highlight without replacing the row, preserving
 native double-click rename. Replacing rows revokes their old listeners. Disposal stops controls immediately;
 late file and failed-action completions cannot update removed presentation.
 
+Director clock listeners, interaction observers and scene release failures no
+longer leave the director inconsistent, and a phase error is kept when cleanup
+also fails.
+
 ## Cockpit component ownership
 
 Cockpit presentation is separated from its camera/controller behavior. Existing

src/director/clock.js

@@ -124,7 +124,13 @@ class PlaybackClock {
   subscribe(listener) {
     if (this._destroyed || typeof listener !== 'function') return () => {};
     this._sceneClockListeners.add(listener);
-    if (this._sceneClockSnapshot) listener({ ...this._sceneClockSnapshot });
+    if (this._sceneClockSnapshot) {
+      try {
+        listener({ ...this._sceneClockSnapshot });
+      } catch (error) {
+        console.warn('[Scenes] Scene clock listener failed:', error);
+      }
+    }
     return () => this._sceneClockListeners.delete(listener);
   }
 

src/director/clock.test.mjs

@@ -171,3 +171,32 @@ test('a subscriber can Stop or replace the initial clock without the old start a
   });
   assert.equal(f.timers.size, 0);
 });
+
+test('a subscriber throwing on immediate snapshot does not crash subscribe and receives unsubscribe handle', () => {
+  const f = fixture();
+  f.clock.publish(scene, shot, 1);
+  let calls = 0;
+  const warnings = [];
+  const origWarn = console.warn;
+  console.warn = (...args) => warnings.push(args);
+  try {
+    const unsubscribe = f.clock.subscribe(() => {
+      calls++;
+      throw new Error('listener failure');
+    });
+    assert.equal(typeof unsubscribe, 'function');
+    assert.equal(calls, 1);
+    assert.equal(warnings.length, 1);
+    assert.equal(warnings[0][0], '[Scenes] Scene clock listener failed:');
+    f.clock.publish(scene, shot, 2);
+    assert.equal(calls, 2);
+    assert.equal(warnings.length, 2);
+    unsubscribe();
+    f.clock.publish(scene, shot, 3);
+    assert.equal(calls, 2);
+    assert.equal(warnings.length, 2);
+    f.clock.destroy();
+  } finally {
+    console.warn = origWarn;
+  }
+});

src/director/interactions/interactions.test.mjs

@@ -178,6 +178,92 @@ test('synchronous stop before execution prevents any side effect; rejection unlo
   assert.equal(calls, 2);
 });
 
+test('a throwing changed observer does not break session dispatch, activation, or clearing', async () => {
+  let executed = 0;
+  let throwOnBusy = true;
+  let throwOnEnd = true;
+  const warnings = [];
+  const origWarn = console.warn;
+  console.warn = (...args) => warnings.push(args);
+  try {
+    const session = createInteractionSession({
+      execute: () => {
+        executed++;
+        return true;
+      },
+      changed: (state) => {
+        if (throwOnBusy && state.busy) throw new Error('start failed');
+        if (throwOnEnd && !state.busy && state.selected === 'a') {
+          throw new Error('end failed');
+        }
+      },
+    });
+    session.activate([{ id: 'a' }]);
+    const result = await session.dispatch('a');
+    assert.equal(result, true);
+    assert.equal(executed, 1);
+    assert.deepEqual(session.getState(), {
+      active: true,
+      busy: false,
+      selected: 'a',
+      count: 1,
+    });
+    assert.equal(warnings.length, 2);
+    assert.equal(
+      warnings[0][0],
+      '[Scenes] Interaction session observer failed:',
+    );
+    assert.equal(
+      warnings[1][0],
+      '[Scenes] Interaction session observer failed:',
+    );
+    throwOnBusy = false;
+    throwOnEnd = false;
+    assert.equal(await session.dispatch('a'), true);
+    assert.equal(executed, 2);
+    session.clear();
+    assert.deepEqual(session.getState(), {
+      active: false,
+      busy: false,
+      selected: null,
+      count: 0,
+    });
+  } finally {
+    console.warn = origWarn;
+  }
+});
+
+test('an observer that always throws cannot stop activate or clear', () => {
+  const warnings = [];
+  const origWarn = console.warn;
+  console.warn = (...args) => warnings.push(args);
+  try {
+    const session = createInteractionSession({
+      execute: () => true,
+      changed: () => {
+        throw new Error('observer failed');
+      },
+    });
+    session.activate([{ id: 'a' }, { id: 'b' }]);
+    assert.deepEqual(session.getState(), {
+      active: true,
+      busy: false,
+      selected: null,
+      count: 2,
+    });
+    session.clear();
+    assert.deepEqual(session.getState(), {
+      active: false,
+      busy: false,
+      selected: null,
+      count: 0,
+    });
+    assert.equal(warnings.length, 3);
+  } finally {
+    console.warn = origWarn;
+  }
+});
+
 test('settled pack shots cannot take the same-shot seek shortcut after Stop released geometry', async () => {
   const { SceneDirector } = await import('../../scenes/director.js');
   const shot = { id: 'shot', dataPackIds: ['pack'] };

src/director/interactions/session.js

@@ -6,6 +6,13 @@ export function createInteractionSession({ execute, changed = () => {} }) {
     selected = null;
   let actions = new Map();
   const state = () => ({ active, busy, selected, count: actions.size });
+  function notify() {
+    try {
+      changed(state());
+    } catch (error) {
+      console.warn('[Scenes] Interaction session observer failed:', error);
+    }
+  }
   function clear() {
     active = false;
     busy = false;
@@ -13,7 +20,7 @@ export function createInteractionSession({ execute, changed = () => {} }) {
     actions.clear();
     controller?.abort();
     controller = null;
-    changed(state());
+    notify();
   }
   return {
     clear,
@@ -22,7 +29,7 @@ export function createInteractionSession({ execute, changed = () => {} }) {
       clear();
       actions = new Map(items.map((item) => [item.id, item]));
       active = !!actions.size;
-      changed(state());
+      notify();
     },
     async dispatch(id) {
       const item = actions.get(id);
@@ -31,7 +38,7 @@ export function createInteractionSession({ execute, changed = () => {} }) {
       controller = current;
       busy = true;
       selected = id;
-      changed(state());
+      notify();
       let abort;
       try {
         const cancelled = new Promise((resolve) => {
@@ -53,7 +60,7 @@ export function createInteractionSession({ execute, changed = () => {} }) {
         if (controller === current) {
           busy = false;
           controller = null;
-          changed(state());
+          notify();
         }
       }
     },

src/director/playback.js

@@ -35,6 +35,7 @@ export function buildPlaybackQueue(
  * releaseScene(scene, token?) leaves a previously loaded scene before playback
  * (false refuses that initial handoff). Between scenes and during final cleanup
  * it receives no cancelled token, so resource release can finish after Stop.
+ * The first error wins when the final release also fails.
  * An empty/already-cancelled queue acquires and releases no resources.
  *
  * @param {Array<{scene:Object, shot:Object}>} queue Normalized, fixed shot queue.
@@ -53,6 +54,7 @@ export async function playSceneQueue(
   const cancelled = () => Boolean(token.cancelled || token.signal?.aborted);
   let activeScene = null;
   let completedShots = 0;
+  let runFailed = false;
   try {
     if (!queue.length || cancelled()) {
       return {
@@ -68,8 +70,9 @@ export async function playSceneQueue(
     for (let index = 0; index < queue.length && !cancelled(); index++) {
       const { scene, shot } = queue[index];
       if (activeScene && activeScene.id !== scene.id) {
-        await adapter.releaseScene(activeScene);
+        const toRelease = activeScene;
         activeScene = null;
+        await adapter.releaseScene(toRelease);
         if (cancelled()) break;
       }
       activeScene = scene;
@@ -82,8 +85,23 @@ export async function playSceneQueue(
     }
     if (!cancelled()) await adapter.complete?.();
     return { status: cancelled() ? 'cancelled' : 'completed', completedShots };
+  } catch (error) {
+    runFailed = true;
+    throw error;
   } finally {
-    if (releaseOnFinish && activeScene) await adapter.releaseScene(activeScene);
+    if (releaseOnFinish && activeScene) {
+      const toRelease = activeScene;
+      activeScene = null;
+      try {
+        await adapter.releaseScene(toRelease);
+      } catch (error) {
+        if (runFailed) {
+          console.warn('[Scenes] Scene release failed:', error);
+        } else {
+          throw error;
+        }
+      }
+    }
   }
 }
 

src/director/playback.test.mjs

@@ -196,3 +196,68 @@ test('a cleanup failure rejects for the caller to restore its own controls', asy
     (error) => error === failure,
   );
 });
+
+test('a phase failure is preserved when final scene release also throws', async () => {
+  const phaseError = new Error('travel failed');
+  const releaseError = new Error('release failed');
+  const warnings = [];
+  const origWarn = console.warn;
+  console.warn = (...args) => warnings.push(args);
+  try {
+    const f = fixture({
+      travel: async () => {
+        throw phaseError;
+      },
+      releaseScene: async () => {
+        throw releaseError;
+      },
+    });
+    await assert.rejects(
+      playSceneQueue(buildPlaybackQueue(scenes, 'a', { single: true }), f),
+      (error) => error === phaseError,
+    );
+    assert.equal(warnings.length, 1);
+    assert.equal(warnings[0][0], '[Scenes] Scene release failed:');
+    assert.equal(warnings[0][1], releaseError);
+  } finally {
+    console.warn = origWarn;
+  }
+});
+
+test('a failing inter-scene release releases the active scene only once', async () => {
+  const releaseError = new Error('release failed');
+  const released = [];
+  const f = fixture({
+    releaseScene: async (scene) => {
+      released.push(scene.id);
+      throw releaseError;
+    },
+  });
+  await assert.rejects(
+    playSceneQueue(buildPlaybackQueue(scenes, 'a'), f),
+    (error) => error === releaseError,
+  );
+  assert.deepEqual(released, ['a']);
+});
+
+test('a falsy phase failure is still preserved over a release failure', async () => {
+  const releaseError = new Error('release failed');
+  const origWarn = console.warn;
+  console.warn = () => {};
+  try {
+    const f = fixture({
+      travel: async () => {
+        throw undefined;
+      },
+      releaseScene: async () => {
+        throw releaseError;
+      },
+    });
+    await assert.rejects(
+      playSceneQueue(buildPlaybackQueue(scenes, 'a', { single: true }), f),
+      (error) => error === undefined,
+    );
+  } finally {
+    console.warn = origWarn;
+  }
+});