From d16620a29ff3d8e696c0bada22d32608ca3a21a2 Mon Sep 17 00:00:00 2001 From: dufferdeepu Date: Sun, 11 Oct 2026 17:51:36 +0530 Subject: [PATCH 1/7] fix: guard scene director callbacks --- src/director/clock.js | 8 ++- src/director/clock.test.mjs | 29 ++++++++++ .../interactions/interactions.test.mjs | 55 +++++++++++++++++++ src/director/interactions/session.js | 15 +++-- src/director/playback.js | 17 +++++- src/director/playback.test.mjs | 32 +++++++++++ 6 files changed, 149 insertions(+), 7 deletions(-) diff --git a/src/director/clock.js b/src/director/clock.js index fa83047a3..48b1c19c5 100644 --- a/src/director/clock.js +++ b/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); } diff --git a/src/director/clock.test.mjs b/src/director/clock.test.mjs index c259c10ec..479808fc9 100644 --- a/src/director/clock.test.mjs +++ b/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; + } +}); diff --git a/src/director/interactions/interactions.test.mjs b/src/director/interactions/interactions.test.mjs index fa0aa10ae..496616516 100644 --- a/src/director/interactions/interactions.test.mjs +++ b/src/director/interactions/interactions.test.mjs @@ -178,6 +178,61 @@ 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('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'] }; diff --git a/src/director/interactions/session.js b/src/director/interactions/session.js index 2f305b675..f731492e2 100644 --- a/src/director/interactions/session.js +++ b/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(); } } }, diff --git a/src/director/playback.js b/src/director/playback.js index 4fa97df1a..29759871e 100644 --- a/src/director/playback.js +++ b/src/director/playback.js @@ -53,6 +53,7 @@ export async function playSceneQueue( const cancelled = () => Boolean(token.cancelled || token.signal?.aborted); let activeScene = null; let completedShots = 0; + let runError = null; try { if (!queue.length || cancelled()) { return { @@ -68,8 +69,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 +84,19 @@ export async function playSceneQueue( } if (!cancelled()) await adapter.complete?.(); return { status: cancelled() ? 'cancelled' : 'completed', completedShots }; + } catch (error) { + runError = error; + 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 (!runError) throw error; + } + } } } diff --git a/src/director/playback.test.mjs b/src/director/playback.test.mjs index f1d8f0921..d6c0b6451 100644 --- a/src/director/playback.test.mjs +++ b/src/director/playback.test.mjs @@ -196,3 +196,35 @@ 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 f = fixture({ + travel: async () => { + throw phaseError; + }, + releaseScene: async () => { + throw releaseError; + }, + }); + await assert.rejects( + playSceneQueue(buildPlaybackQueue(scenes, 'a', { single: true }), f), + (error) => error === phaseError, + ); +}); +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']); +}); \ No newline at end of file From d2de6258ae6b5776b11c074c3666d444e3b3cae7 Mon Sep 17 00:00:00 2001 From: dufferdeepu Date: Sat, 17 Oct 2026 14:50:47 +0530 Subject: [PATCH 2/7] docs: document director playback error handling --- changelog.d/1014-director-callback-failures.md | 1 + docs/CURRENT-STATE.md | 4 ++++ src/director/playback.js | 1 + src/director/playback.test.mjs | 1 + 4 files changed, 7 insertions(+) create mode 100644 changelog.d/1014-director-callback-failures.md diff --git a/changelog.d/1014-director-callback-failures.md b/changelog.d/1014-director-callback-failures.md new file mode 100644 index 000000000..65118b7a9 --- /dev/null +++ b/changelog.d/1014-director-callback-failures.md @@ -0,0 +1 @@ +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 (deepeshbind, #1014). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 8cb7b6d17..f40ae5afb 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,5 +1,9 @@ # God's Eye View Current State +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. + Vessel sources can report healthy empty coverage using a current, complete snapshot with zero raw rows, a healthy transport and a positive last-message time. This clears obsolete contacts and settles startup without manufacturing diff --git a/src/director/playback.js b/src/director/playback.js index 29759871e..175534434 100644 --- a/src/director/playback.js +++ b/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. diff --git a/src/director/playback.test.mjs b/src/director/playback.test.mjs index d6c0b6451..73de9b444 100644 --- a/src/director/playback.test.mjs +++ b/src/director/playback.test.mjs @@ -213,6 +213,7 @@ test('a phase failure is preserved when final scene release also throws', async (error) => error === phaseError, ); }); + test('a failing inter-scene release releases the active scene only once', async () => { const releaseError = new Error('release failed'); const released = []; From abf2ce1c9a6fe7422c02469c069b6a539597f835 Mon Sep 17 00:00:00 2001 From: dufferdeepu Date: Sat, 17 Oct 2026 19:40:12 +0530 Subject: [PATCH 3/7] fix: guard scene director callbacks --- src/director/playback.js | 6 +++++- src/director/playback.test.mjs | 34 ++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/director/playback.js b/src/director/playback.js index 175534434..2e243d748 100644 --- a/src/director/playback.js +++ b/src/director/playback.js @@ -95,7 +95,11 @@ export async function playSceneQueue( try { await adapter.releaseScene(toRelease); } catch (error) { - if (!runError) throw error; + if (runError) { + console.warn('[Scenes] Scene release failed:', error); + } else { + throw error; + } } } } diff --git a/src/director/playback.test.mjs b/src/director/playback.test.mjs index 73de9b444..be56079e2 100644 --- a/src/director/playback.test.mjs +++ b/src/director/playback.test.mjs @@ -200,18 +200,28 @@ test('a cleanup failure rejects for the caller to restore its own controls', asy 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 f = fixture({ - travel: async () => { - throw phaseError; - }, - releaseScene: async () => { - throw releaseError; - }, - }); - await assert.rejects( - playSceneQueue(buildPlaybackQueue(scenes, 'a', { single: true }), f), - (error) => error === phaseError, - ); + 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 () => { From 3109ddabb80a5e9c28a9a889b45c5f39cf00ad4f Mon Sep 17 00:00:00 2001 From: dufferdeepu Date: Sun, 18 Oct 2026 13:37:27 +0530 Subject: [PATCH 4/7] add trailing newline to playback test --- src/director/playback.test.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/director/playback.test.mjs b/src/director/playback.test.mjs index be56079e2..0141f2b01 100644 --- a/src/director/playback.test.mjs +++ b/src/director/playback.test.mjs @@ -238,4 +238,4 @@ test('a failing inter-scene release releases the active scene only once', async (error) => error === releaseError, ); assert.deepEqual(released, ['a']); -}); \ No newline at end of file +}); From 76199c07eeda0f88e8cb46474f57453cd3dd5e38 Mon Sep 17 00:00:00 2001 From: Said Turksever Date: Mon, 19 Oct 2026 14:27:25 +0100 Subject: [PATCH 5/7] fix(director): keep a falsy phase error over a release failure runError doubled as the failure flag, so a phase that threw a falsy value was still masked by a final release error (ianblenke's nit). Track failure with a boolean and cover it with a test. Make the changelog fragment a wrapped bullet like the other fragments. Co-Authored-By: Claude Opus 5.5 --- .../1014-director-callback-failures.md | 5 ++++- src/director/playback.js | 6 ++--- src/director/playback.test.mjs | 22 +++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/changelog.d/1014-director-callback-failures.md b/changelog.d/1014-director-callback-failures.md index 65118b7a9..f3877eca4 100644 --- a/changelog.d/1014-director-callback-failures.md +++ b/changelog.d/1014-director-callback-failures.md @@ -1 +1,4 @@ -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 (deepeshbind, #1014). +- 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). diff --git a/src/director/playback.js b/src/director/playback.js index 2e243d748..3c060b2bb 100644 --- a/src/director/playback.js +++ b/src/director/playback.js @@ -54,7 +54,7 @@ export async function playSceneQueue( const cancelled = () => Boolean(token.cancelled || token.signal?.aborted); let activeScene = null; let completedShots = 0; - let runError = null; + let runFailed = false; try { if (!queue.length || cancelled()) { return { @@ -86,7 +86,7 @@ export async function playSceneQueue( if (!cancelled()) await adapter.complete?.(); return { status: cancelled() ? 'cancelled' : 'completed', completedShots }; } catch (error) { - runError = error; + runFailed = true; throw error; } finally { if (releaseOnFinish && activeScene) { @@ -95,7 +95,7 @@ export async function playSceneQueue( try { await adapter.releaseScene(toRelease); } catch (error) { - if (runError) { + if (runFailed) { console.warn('[Scenes] Scene release failed:', error); } else { throw error; diff --git a/src/director/playback.test.mjs b/src/director/playback.test.mjs index 0141f2b01..820e8186e 100644 --- a/src/director/playback.test.mjs +++ b/src/director/playback.test.mjs @@ -239,3 +239,25 @@ test('a failing inter-scene release releases the active scene only once', async ); 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; + } +}); From 6a5374435fec4940a780d98960d0a501c630db53 Mon Sep 17 00:00:00 2001 From: Said Turksever Date: Mon, 19 Oct 2026 14:52:46 +0100 Subject: [PATCH 6/7] docs(current-state): move the #1014 note into its topical section Four open PRs prepended their notes at the top of CURRENT-STATE.md, so merging any one of them made the others conflict. Place this note in its own section instead. Co-Authored-By: Claude Opus 5.5 --- docs/CURRENT-STATE.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 2fd0aaa0c..055e5f7d2 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,9 +1,5 @@ # God's Eye View Current State -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. - Shared distance checks use `src/geoDistance.js` with `{ lat, lon }` points and the IUGG mean spherical Earth radius, 6371.0088 km. `greatCircleKm` is canonical; `greatCircleMeters` is its exact metre conversion. Missing, @@ -986,6 +982,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 From 811eb7835d2fccb1a30f2fefa333d1f2d25a298b Mon Sep 17 00:00:00 2001 From: Said Turksever Date: Wed, 28 Oct 2026 14:08:49 +0000 Subject: [PATCH 7/7] test(director): cover observer failures in session activate and clear --- .../interactions/interactions.test.mjs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/director/interactions/interactions.test.mjs b/src/director/interactions/interactions.test.mjs index 496616516..6c13cecef 100644 --- a/src/director/interactions/interactions.test.mjs +++ b/src/director/interactions/interactions.test.mjs @@ -233,6 +233,37 @@ test('a throwing changed observer does not break session dispatch, activation, o } }); +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'] };