From 02a8442e6ea611dc7f9ffc964d4988c02fa59197 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Fri, 6 Nov 2026 14:45:42 +0400 Subject: [PATCH] Let Retry finish a frame event a throwing listener cut short Cesium's Event.raiseEvent leaves an event mid-raise when a listener throws, so listeners added or removed afterwards stay queued. Retry now applies that queue for the frame and clock events: the notice's frame watchers see the next frame, and a listener removed since, such as a layer turned off, is not called again. --- docs/CURRENT-STATE.md | 5 + scripts/format-scope.json | 1 + src/ui/renderFailure.js | 35 +++++++ src/ui/renderFailureEventRetry.test.mjs | 134 ++++++++++++++++++++++++ 4 files changed, 175 insertions(+) create mode 100644 src/ui/renderFailureEventRetry.test.mjs diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index e19c68f8c..583e6d6a6 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1599,6 +1599,11 @@ render error before Cesium's loop handles it as before. - **Retry** after a render error restarts the render loop. The notice stays until a frame starts and ends without a render error (Cesium raises `postRender` after a failed frame too), and a loop that fails again keeps it. + A listener that throws leaves its Cesium event mid-raise, with listeners + added or removed since only queued, so Retry first applies the queue of the + frame and clock events (`finishRaise()`): the notice's frame watchers start + with the next frame, and a listener removed since, for a layer turned off, + is not called again. - **Reload** after a lost context reloads the page: Cesium cannot draw into a context it has lost, and the address keeps the view. - **Reset view** opens the app's root address (`appRootUrl()`), without the diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 973be026c..d5189c793 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -548,6 +548,7 @@ "src/ui/renderFailure.test.mjs", "src/ui/renderFailureView.test.mjs", "src/ui/renderFailureLoopErrors.test.mjs", + "src/ui/renderFailureEventRetry.test.mjs", "src/app/renderFailureInlineRetry.test.mjs", "src/layers/cctv/lifecycle.js", "src/layers/cctv/controls.js", diff --git a/src/ui/renderFailure.js b/src/ui/renderFailure.js index 5785fbc8c..b5d5b2b55 100644 --- a/src/ui/renderFailure.js +++ b/src/ui/renderFailure.js @@ -42,6 +42,17 @@ export function installRenderFailureNotice( locationRef.reload(); return; } + [ + scene.preUpdate, + scene.postUpdate, + scene.preRender, + scene.postRender, + scene.camera?.changed, + scene.camera?.moveStart, + scene.camera?.moveEnd, + viewer.clock?.onTick, + viewer.clock?.onStop, + ].forEach(finishRaise); viewer.useDefaultRenderLoop = true; scene.requestRender?.(); } @@ -124,6 +135,30 @@ export function installRenderFailureNotice( }; } +/** + * Finish the raise a throwing listener cut short. Cesium's `Event.raiseEvent` + * marks the event as raising until every listener returns, and until then it + * only queues the listeners added or removed: the frame watchers the notice + * adds, or the throwing listener of a layer turned off since. Apply the queue + * as the raise would have. + */ +function finishRaise(event) { + if (!event?._invokingListeners) return; + event._invokingListeners = false; + for (const [listener, scopes] of event._toAdd) { + const added = event._listeners.get(listener) ?? new Set(); + scopes.forEach((scope) => added.add(scope)); + event._listeners.set(listener, added); + } + event._toAdd.clear(); + for (const [listener, scopes] of event._toRemove) { + const left = event._listeners.get(listener); + scopes.forEach((scope) => left?.delete(scope)); + if (left?.size === 0) event._listeners.delete(listener); + } + event._toRemove.clear(); +} + /** * Wrap `target[name]` so an error it throws is reported and thrown on, for * the caller to handle as before. Returns a function that unwraps it. diff --git a/src/ui/renderFailureEventRetry.test.mjs b/src/ui/renderFailureEventRetry.test.mjs new file mode 100644 index 000000000..19614ff53 --- /dev/null +++ b/src/ui/renderFailureEventRetry.test.mjs @@ -0,0 +1,134 @@ +// Retry after an error thrown by a scene event listener. Cesium's +// Event.raiseEvent has no try/finally: a throwing listener leaves the event +// with _invokingListeners = true, so listeners added or removed afterwards are +// only queued (_toAdd/_toRemove) until the next raise of that event completes. +// Drives Cesium's real Event and startRenderLoop (CesiumWidget.prototype). +import test from 'node:test'; +import assert from 'node:assert/strict'; +import * as Cesium from 'cesium'; +import { installRenderFailureNotice } from './renderFailure.js'; + +function setup() { + const frames = []; + const previousRaf = globalThis.requestAnimationFrame; + globalThis.requestAnimationFrame = (fn) => frames.push(fn); + // Scene.render as in Scene.js:4369: preUpdate always, preRender/postRender + // only when a frame is rendered (requestRenderMode idle skips them). + const scene = { + renderError: new Cesium.Event(), + preUpdate: new Cesium.Event(), + preRender: new Cesium.Event(), + postRender: new Cesium.Event(), + requestRenderMode: true, + _renderRequested: false, + rendered: 0, + requestRender() { + this._renderRequested = true; + }, + initializeFrame() {}, + render(time) { + this.preUpdate.raiseEvent(this, time); + const shouldRender = !this.requestRenderMode || this._renderRequested; + if (!shouldRender) return; + this._renderRequested = false; + this.preRender.raiseEvent(this, time); + this.rendered += 1; + this.postRender.raiseEvent(this, time); + }, + }; + const widget = Object.create(Cesium.CesiumWidget.prototype); + Object.assign(widget, { + _scene: scene, + _clock: { tick() {} }, + _canRender: true, + _useDefaultRenderLoop: false, + _renderLoopRunning: false, + _showRenderLoopErrors: false, + _targetFrameRate: undefined, + isDestroyed: () => false, + resize() {}, + }); + scene.renderError.addEventListener(() => { + widget._useDefaultRenderLoop = false; + widget._renderLoopRunning = false; + }); + const viewer = { + scene, + cesiumWidget: widget, + canvas: { addEventListener() {}, removeEventListener() {} }, + get useDefaultRenderLoop() { + return widget.useDefaultRenderLoop; + }, + set useDefaultRenderLoop(value) { + widget.useDefaultRenderLoop = value; + }, + }; + const store = { + renderFailure: null, + publishRenderFailure(next) { + this.renderFailure = next; + }, + }; + const uninstall = installRenderFailureNotice(viewer, { + store, + locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, + logError() {}, + }); + const runFrames = (n) => { + for (let i = 0; i < n; i += 1) + for (const fn of frames.splice(0)) fn(16 * (i + 1)); + }; + const restore = () => { + uninstall(); + globalThis.requestAnimationFrame = previousRaf; + }; + return { frames, scene, widget, viewer, store, runFrames, restore }; +} + +for (const eventName of ['postRender', 'preUpdate']) { + test(`idle: a ${eventName} listener that threw once; a successful Retry clears the notice`, () => { + const { scene, viewer, store, runFrames, restore } = setup(); + try { + let broken = true; + scene[eventName].addEventListener(() => { + if (broken) throw new TypeError('listener failed'); + }); + scene.requestRender(); + viewer.useDefaultRenderLoop = true; + runFrames(1); + assert.equal(store.renderFailure?.reason, 'render-error'); + assert.equal(viewer.useDefaultRenderLoop, false); + broken = false; // the cause goes away + store.renderFailure.retry(); + const before = scene.rendered; + runFrames(10); // idle: no camera move, no render request + assert.ok(scene.rendered > before, 'the globe drew again'); + assert.equal(store.renderFailure, null, 'the notice is cleared'); + } finally { + restore(); + } + }); +} + +test('a throwing postRender listener removed before Retry is not called again', () => { + const { scene, viewer, store, runFrames, restore } = setup(); + try { + let calls = 0; + const remove = scene.postRender.addEventListener(() => { + calls += 1; + throw new TypeError('layer listener failed'); + }); + scene.requestRender(); + viewer.useDefaultRenderLoop = true; + runFrames(1); + assert.equal(store.renderFailure?.reason, 'render-error'); + remove(); // the user turns the layer off + store.renderFailure.retry(); + runFrames(5); + assert.equal(calls, 1, 'the removed listener ran again after Retry'); + assert.equal(viewer.useDefaultRenderLoop, true, 'the loop keeps running'); + assert.equal(store.renderFailure, null, 'the notice is cleared'); + } finally { + restore(); + } +});