From ebaa439739ef1f2ee03d8e5a47f597e8b780f36a Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Wed, 11 Nov 2026 12:44:08 +0400 Subject: [PATCH] Tighten the render failure notice's error handling and focus - An error a frame of the inline panel's timer throws no longer reaches the host page; it has been raised as a render error by then. - An error Scene raised and threw on (rethrowRenderErrors) is raised once. - After a lost context, later render errors are only logged, so the notice keeps saying why. - A render error without a value shows no detail. - The globe's tile load progress and the scene's tileset load events are finished too when a listener's error cuts them short. - Focus goes back where it was when the globe draws again, or to the map. - The appearance counter and the guard for a missing viewer are gone. Tests cover every finished event, a thrown timer frame, a single report under rethrowRenderErrors and the focus return; the inline tests share one setup. --- docs/CURRENT-STATE.md | 22 +- scripts/format-scope.json | 1 - src/app/embed.js | 9 +- src/app/renderFailureInlineRetry.test.mjs | 287 ++++++++---------- .../renderFailureInlineSecondLoop.test.mjs | 109 ------- src/ui/renderFailure.js | 87 ++++-- src/ui/renderFailure.test.mjs | 25 +- src/ui/renderFailureEventRetry.test.mjs | 181 +++++++---- src/ui/renderFailureLoopErrors.test.mjs | 58 ++-- src/ui/renderFailureView.test.mjs | 61 +++- src/ui/stores/status.js | 7 +- src/ui/views/panels/RenderFailure.vue | 28 +- 12 files changed, 459 insertions(+), 416 deletions(-) delete mode 100644 src/app/renderFailureInlineSecondLoop.test.mjs diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index e8ec7a60f..0cde22a53 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2077,7 +2077,10 @@ black globe behind controls that still respond. `installRenderFailureNotice()` (`src/ui/renderFailure.js`) listens for Cesium's `renderError`, after which Cesium stops its render loop, and for the canvas' `webglcontextlost`. It logs the error and publishes a notice to the status store, which -`RenderFailure.vue` renders as an alert dialog that takes focus. The +`RenderFailure.vue` renders as an alert dialog that takes focus. When the +globe draws again, focus goes back to the element that had it, or to the map +(`#cesiumContainer`, made focusable with `tabindex="-1"`) if that element is +gone, unless the user has moved focus since. The application scene creates its viewer with `showRenderLoopErrors: false`, so Cesium's own "Rendering has stopped" panel, which offers no way back, no longer appears; `applicationViewerOptions()` and `createApplicationViewer()` keep @@ -2085,15 +2088,22 @@ Cesium's panel by default for other callers. An error thrown outside Scene's own error handling (a `preRender` or `postRender` listener, the clock's `onTick`, a resize) also stops the loop without a `renderError`, so the notice wraps the widget's `render()` and `resize()` and raises such an error as a -render error before Cesium's loop handles it as before. +render error before Cesium's loop handles it as before; one Scene has raised +already and throws on (`rethrowRenderErrors`) is not raised again. A lost +context fails every later frame, and those errors are only logged, so the +notice keeps saying why. The notice sits above floating panels such as the +Québec camera viewer, and its buttons are 44 px tall on touch screens. - **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 the `render()` and `resize()` wrapper - finishes the frame and clock events it cut short (`finishRaise()`) before it - raises the render error: the notice's frame watchers start with the next + finishes the events it cut short (`finishRaise()`) before it raises the + render error. These are the events Cesium raises outside Scene's error + handling (`frameEvents()`): Scene's frame events, the camera's and the + clock's, the globe's tile load progress, and the load events of the scene's + top-level tilesets. So the notice's frame watchers start with the next frame, a listener removed while the loop is stopped, for a layer turned off, is not called again, and one turned off and on again keeps running. - **Reload** after a lost context reloads the page: Cesium cannot draw into a @@ -2106,7 +2116,9 @@ An inline conversation panel never reloads or navigates its host's page, so it offers Retry after a render error only, and nothing after a lost context, where the panel runtime's own notice applies. The panel's timer (`keepPanelRendering()`) draws only while Cesium's render loop is on, so it -stops after a render error and draws again after Retry. +stops after a render error and draws again after Retry. An error a frame of +the timer throws stops there: it has been raised as a render error, and +thrown on it would only reach the host page. A render error marks Cesium's loop stopped, but a frame drawn while the loop ran leaves its next animation frame queued: an error Scene catches itself diff --git a/scripts/format-scope.json b/scripts/format-scope.json index d81419796..386a5a708 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -561,7 +561,6 @@ "src/ui/renderFailureEventRetry.test.mjs", "src/app/renderFailureInlineRetry.test.mjs", "src/ui/renderFailureSecondLoop.test.mjs", - "src/app/renderFailureInlineSecondLoop.test.mjs", "src/layers/cctv/lifecycle.js", "src/layers/cctv/controls.js", "src/layers/cctv/geometry.js", diff --git a/src/app/embed.js b/src/app/embed.js index dba6e9f16..fbb746909 100644 --- a/src/app/embed.js +++ b/src/app/embed.js @@ -117,8 +117,13 @@ export function keepPanelRendering( viewer.isDestroyed?.() ) return; - viewer.resize(); - viewer.render(); + try { + viewer.resize(); + viewer.render(); + } catch { + // Raised as a render error by now (src/ui/renderFailure.js), which + // stops this timer; thrown on, it would only reach the host page. + } }, PANEL_FRAME_MS); return () => { windowRef.clearInterval(timer); diff --git a/src/app/renderFailureInlineRetry.test.mjs b/src/app/renderFailureInlineRetry.test.mjs index e9191582b..2bbc98f1e 100644 --- a/src/app/renderFailureInlineRetry.test.mjs +++ b/src/app/renderFailureInlineRetry.test.mjs @@ -1,93 +1,17 @@ -// Retry in an inline panel whose host stopped animation frames: the panel -// draws from keepPanelRendering's timer, so Retry must make that timer draw -// again, or the notice offers a Retry that does nothing. +// Retry in an inline panel whose host stops animation frames, so the panel +// draws from keepPanelRendering's timer. Drives Cesium's real startRenderLoop +// and useDefaultRenderLoop setter (CesiumWidget.prototype). import test from 'node:test'; import assert from 'node:assert/strict'; import * as Cesium from 'cesium'; import { keepPanelRendering } from './embed.js'; import { installRenderFailureNotice } from '../ui/renderFailure.js'; -test('Retry resumes the inline panel timer after a render error', () => { - let tick = null; - let clock = 0; - // The host reports the panel hidden: animation frames never arrive. - const windowRef = { - requestAnimationFrame: () => 1, - cancelAnimationFrame() {}, - setInterval: (fn) => ((tick = fn), 1), - clearInterval() {}, - }; - const previousRaf = globalThis.requestAnimationFrame; - globalThis.requestAnimationFrame = () => 1; - const previousError = console.error; - console.error = () => {}; - try { - let broken = true; - let drawn = 0; - const scene = { - renderError: new Cesium.Event(), - preUpdate: new Cesium.Event(), - postRender: new Cesium.Event(), - requestRender() {}, - }; - const viewer = { - scene, - canvas: { addEventListener() {}, removeEventListener() {} }, - useDefaultRenderLoop: true, - isDestroyed: () => false, - resize() {}, - // Scene.render: a failing primitive update raises renderError - // (tryAndCatchError), and postRender is still raised. - render() { - drawn += 1; - scene.preUpdate.raiseEvent(scene); - if (broken) scene.renderError.raiseEvent(scene, new Error('boom')); - scene.postRender.raiseEvent(scene); - }, - }; - // CesiumWidget._onRenderError stops the default loop. - scene.renderError.addEventListener(() => { - viewer.useDefaultRenderLoop = false; - }); - const store = { - renderFailure: null, - publishRenderFailure(next) { - this.renderFailure = next; - }, - }; - const uninstall = installRenderFailureNotice(viewer, { - store, - allowReload: false, - locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, - logError() {}, - }); - const stop = keepPanelRendering(viewer, { windowRef, now: () => clock }); - clock = 400; - tick(); - assert.equal(store.renderFailure?.reason, 'render-error'); - assert.equal(typeof store.renderFailure.retry, 'function'); - // The cause goes away and the user presses Retry. - broken = false; - store.renderFailure.retry(); - const before = drawn; - for (let i = 0; i < 5; i += 1) { - clock += 33; - tick(); - } - assert.ok(drawn > before, 'the panel draws again after Retry'); - assert.equal(store.renderFailure, null, 'a clean frame clears the notice'); - stop(); - uninstall(); - } finally { - globalThis.requestAnimationFrame = previousRaf; - console.error = previousError; - } -}); - -test('Retry in a panel whose host stopped frames keeps one render loop', () => { +function setup() { const frames = []; let tick = null; let clock = 0; + // The host reports the panel hidden: its own animation frames never arrive. const windowRef = { requestAnimationFrame: () => 1, cancelAnimationFrame() {}, @@ -98,88 +22,135 @@ test('Retry in a panel whose host stopped frames keeps one render loop', () => { globalThis.requestAnimationFrame = (fn) => frames.push(fn); const previousError = console.error; console.error = () => {}; - try { - let broken = false; - let drawn = 0; - const scene = { - renderError: new Cesium.Event(), - preUpdate: new Cesium.Event(), - postRender: new Cesium.Event(), - requestRender() {}, - initializeFrame() {}, - render() { - drawn += 1; - this.preUpdate.raiseEvent(this); - if (broken) this.renderError.raiseEvent(this, new Error('boom')); - this.postRender.raiseEvent(this); - }, - }; - 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() {}, - }); - // CesiumWidget's own renderError handler, which marks the loop stopped. - 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; - }, - isDestroyed: () => false, - render: () => widget.render(), - resize: () => widget.resize(), - }; - const store = { - renderFailure: null, - publishRenderFailure(next) { - this.renderFailure = next; - }, - }; - const uninstall = installRenderFailureNotice(viewer, { - store, - allowReload: false, - locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, - logError() {}, - }); - const stop = keepPanelRendering(viewer, { windowRef, now: () => clock }); - viewer.useDefaultRenderLoop = true; - // The host stops animation frames with the loop's next one queued. - assert.equal(frames.length, 1); - broken = true; - clock = 400; + const state = { failure: null, drawn: 0 }; + const scene = { + renderError: new Cesium.Event(), + preUpdate: new Cesium.Event(), + postRender: new Cesium.Event(), + requestRender() {}, + initializeFrame() {}, + // Scene.render: a failing primitive update raises renderError + // (tryAndCatchError) and returns; a failing listener throws. + render() { + state.drawn += 1; + this.preUpdate.raiseEvent(this); + if (state.failure === 'caught') + this.renderError.raiseEvent(this, new Error('boom')); + if (state.failure === 'thrown') throw new Error('boom'); + this.postRender.raiseEvent(this); + }, + }; + 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() {}, + }); + // CesiumWidget's own renderError handler, which marks the loop stopped. + 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; + }, + isDestroyed: () => false, + render: () => widget.render(), + resize: () => widget.resize(), + }; + const store = { + renderFailure: null, + publishRenderFailure(next) { + this.renderFailure = next; + }, + }; + const uninstall = installRenderFailureNotice(viewer, { + store, + allowReload: false, + locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, + logError() {}, + }); + const stop = keepPanelRendering(viewer, { windowRef, now: () => clock }); + viewer.useDefaultRenderLoop = true; + const timerFrame = () => { + clock += 400; tick(); - assert.equal(store.renderFailure?.reason, 'render-error'); - broken = false; - store.renderFailure.retry(); - clock += 33; - tick(); - assert.equal(store.renderFailure, null); - assert.equal(frames.length, 1, 'Retry starts no second loop'); - // The host resumes frames: the globe draws once a frame. - drawn = 0; - for (let frame = 1; frame <= 3; frame += 1) + }; + const animationFrames = (n) => { + for (let frame = 1; frame <= n; frame += 1) for (const render of frames.splice(0)) render(frame * 16); - assert.equal(drawn, 3); + }; + const restore = () => { stop(); uninstall(); - } finally { globalThis.requestAnimationFrame = previousRaf; console.error = previousError; + }; + return { frames, state, store, timerFrame, animationFrames, restore }; +} + +for (const failure of ['caught', 'thrown']) { + test(`an error ${failure} in a frame the timer draws: Retry draws again with one render loop`, () => { + const { frames, state, store, timerFrame, animationFrames, restore } = + setup(); + try { + // The host stops animation frames with the loop's next one queued. + assert.equal(frames.length, 1); + state.failure = failure; + timerFrame(); // an error thrown here does not reach the host page + assert.equal(store.renderFailure?.reason, 'render-error'); + assert.equal(typeof store.renderFailure.retry, 'function'); + state.failure = null; + store.renderFailure.retry(); + const before = state.drawn; + timerFrame(); + assert.ok(state.drawn > before, 'the panel draws again after Retry'); + assert.equal( + store.renderFailure, + null, + 'a clean frame clears the notice', + ); + assert.equal(frames.length, 1, 'Retry starts no second loop'); + // The host resumes frames: the globe draws once a frame. + state.drawn = 0; + animationFrames(3); + assert.equal(state.drawn, 3); + } finally { + restore(); + } + }); +} + +test('an error Scene caught in an animation frame right before the host stopped frames: Retry keeps one render loop', () => { + const { frames, state, store, timerFrame, animationFrames, restore } = + setup(); + try { + state.failure = 'caught'; + animationFrames(1); + assert.equal(store.renderFailure?.reason, 'render-error'); + assert.equal(frames.length, 1); + state.failure = null; + store.renderFailure.retry(); + timerFrame(); + assert.equal(store.renderFailure, null); + assert.equal(frames.length, 1, 'Retry starts no second loop'); + state.drawn = 0; + animationFrames(3); + assert.equal(state.drawn, 3); + } finally { + restore(); } }); diff --git a/src/app/renderFailureInlineSecondLoop.test.mjs b/src/app/renderFailureInlineSecondLoop.test.mjs deleted file mode 100644 index 6c450960b..000000000 --- a/src/app/renderFailureInlineSecondLoop.test.mjs +++ /dev/null @@ -1,109 +0,0 @@ -// Retry in an inline panel after an error Scene catches itself in an -// animation frame, when the host stops frames right after it. The loop's next -// animation frame is still queued, so Retry must not start a second loop. -import test from 'node:test'; -import assert from 'node:assert/strict'; -import * as Cesium from 'cesium'; -import { keepPanelRendering } from './embed.js'; -import { installRenderFailureNotice } from '../ui/renderFailure.js'; - -test('Retry in a panel whose host stopped frames right after an error Scene caught in an animation frame keeps one render loop', () => { - const frames = []; - let tick = null; - let clock = 0; - const windowRef = { - requestAnimationFrame: () => 1, - cancelAnimationFrame() {}, - setInterval: (fn) => ((tick = fn), 1), - clearInterval() {}, - }; - const previousRaf = globalThis.requestAnimationFrame; - globalThis.requestAnimationFrame = (fn) => frames.push(fn); - const previousError = console.error; - console.error = () => {}; - try { - let broken = false; - let drawn = 0; - const scene = { - renderError: new Cesium.Event(), - preUpdate: new Cesium.Event(), - postRender: new Cesium.Event(), - requestRender() {}, - initializeFrame() {}, - render() { - drawn += 1; - this.preUpdate.raiseEvent(this); - if (broken) this.renderError.raiseEvent(this, new Error('boom')); - this.postRender.raiseEvent(this); - }, - }; - 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() {}, - }); - // CesiumWidget's own renderError handler, which marks the loop stopped. - 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; - }, - isDestroyed: () => false, - render: () => widget.render(), - resize: () => widget.resize(), - }; - const store = { - renderFailure: null, - publishRenderFailure(next) { - this.renderFailure = next; - }, - }; - const uninstall = installRenderFailureNotice(viewer, { - store, - allowReload: false, - locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, - logError() {}, - }); - const stop = keepPanelRendering(viewer, { windowRef, now: () => clock }); - viewer.useDefaultRenderLoop = true; - // The error comes in an animation frame; Scene catches it, and the loop - // still queues its next frame. Then the host stops animation frames. - broken = true; - for (const render of frames.splice(0)) render(16); - assert.equal(store.renderFailure?.reason, 'render-error'); - assert.equal(frames.length, 1); - broken = false; - clock = 400; - store.renderFailure.retry(); - clock += 33; - tick(); - assert.equal(store.renderFailure, null); - assert.equal(frames.length, 1, 'Retry starts no second loop'); - // The host resumes frames: the globe draws once a frame. - drawn = 0; - for (let frame = 1; frame <= 3; frame += 1) - for (const render of frames.splice(0)) render(frame * 16); - assert.equal(drawn, 3); - stop(); - uninstall(); - } finally { - globalThis.requestAnimationFrame = previousRaf; - console.error = previousError; - } -}); diff --git a/src/ui/renderFailure.js b/src/ui/renderFailure.js index 6a4359bd9..b21e98e45 100644 --- a/src/ui/renderFailure.js +++ b/src/ui/renderFailure.js @@ -8,15 +8,10 @@ import { appRootUrl } from '../share/shareLinkPath.js'; * behind controls that still respond. Cesium stops its render loop after a * render error, and a page whose WebGL context was lost never draws again. * - * The notice is published to the status store, which the scene chrome - * renders. Retry restarts the render loop after an error, and the notice stays - * until a frame renders without one; after a context loss it reloads the page, - * whose address keeps the view. Reset opens the app's root address, without - * the view restored from the address bar. Where the page must not reload - * itself (`allowReload: false`), Reset is left out, and so is Retry after a - * context loss. - * - * Returns a function that removes the listeners and the notice. + * Retry restarts the loop, and the notice stays until a frame renders without + * an error; after a context loss Retry reloads the page. Reset opens the app's + * root address. With `allowReload: false`, Reset is left out, and so is Retry + * after a context loss. Returns a function that removes the notice. */ export function installRenderFailureNotice( viewer, @@ -28,12 +23,13 @@ export function installRenderFailureNotice( console.error('[Render] The globe stopped drawing:', error), } = {}, ) { - const scene = viewer?.scene; - if (!scene?.renderError) return () => {}; + const { scene } = viewer; const canvas = viewer.canvas ?? scene.canvas; let notice = null; - let shown = 0; let contextLost = false; + // The error Scene raised as a render error and, with rethrowRenderErrors, + // throws on: the render() wrapper must not raise it a second time. + let raisedByScene = null; let frameFailed = false; let removeFrameListeners = null; @@ -67,6 +63,7 @@ export function installRenderFailureNotice( if (removeFrameListeners || contextLost) return; const removeStart = scene.preUpdate.addEventListener(() => { frameFailed = false; + raisedByScene = null; }); const removeEnd = scene.postRender.addEventListener(() => { if (!frameFailed) clear(); @@ -84,7 +81,6 @@ export function installRenderFailureNotice( notice = { reason, detail, - shown: notice ? notice.shown : ++shown, retry: contextLost && !allowReload ? null : retry, reset: allowReload ? reset : null, }; @@ -93,9 +89,12 @@ export function installRenderFailureNotice( } const onRenderError = (_scene, error) => { + raisedByScene = { error }; frameFailed = true; logError(error); - present(describeError(error)); + // A lost context fails every frame after it; the notice keeps saying why. + if (contextLost) return; + present(error ? describeError(error) : ''); }; const onContextLost = () => { contextLost = true; @@ -110,23 +109,15 @@ export function installRenderFailureNotice( // render loop, but raises no renderError, and Cesium reports it only in the // panel the app turns off. Raise it as a render error, so Cesium's handler, // an inline panel's timer and this notice all see it. Such an error passes - // here before anything else runs, so finish the raises it cut short first: - // listeners added or removed while the loop is stopped then apply at once, - // in order. + // here before anything else runs, so finish the raises of frameEvents() it + // cut short first: listeners added or removed while the loop is stopped + // then apply at once, in order. const unguard = ['resize', 'render'].map((name) => reportThrown(viewer.cesiumWidget, name, (error) => { - [ - 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); - scene.renderError.raiseEvent(scene, error); + frameEvents(viewer).forEach(finishRaise); + if (!raisedByScene || raisedByScene.error !== error) + scene.renderError.raiseEvent(scene, error); + raisedByScene = null; }), ); canvas?.addEventListener?.('webglcontextlost', onContextLost); @@ -138,6 +129,40 @@ export function installRenderFailureNotice( }; } +/** + * The events Cesium raises in a frame outside Scene's own error handling, so + * that a listener's error cuts their raise short: Scene's frame events, the + * camera's and the clock's, and those raised after the frame renders, the + * globe's tile load progress and the load events of the scene's tilesets. + * Tilesets nested in a collection are not reached. + */ +function frameEvents(viewer) { + const { scene } = viewer; + const events = [ + scene.preUpdate, + scene.postUpdate, + scene.preRender, + scene.postRender, + scene.camera?.changed, + scene.camera?.moveStart, + scene.camera?.moveEnd, + viewer.clock?.onTick, + viewer.clock?.onStop, + scene.globe?.tileLoadProgressEvent, + ]; + const primitives = scene.primitives; + for (let i = 0; i < (primitives?.length ?? 0); i += 1) { + const tileset = primitives.get(i); + events.push( + tileset?.loadProgress, + tileset?.allTilesLoaded, + tileset?.initialTilesLoaded, + tileset?.tileLoad, + ); + } + return events; +} + /** * 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 @@ -162,9 +187,7 @@ function finishRaise(event) { } /** - * Wrap the widget's `name` method so an error it throws is reported and - * thrown on, for the caller to handle as before. Returns a function that - * unwraps it. + * Report an error the widget's `name` method throws, and throw it on. * * A render error marks Cesium's render loop stopped, but a call made while * the loop ran leaves the loop's next animation frame queued: the loop queues diff --git a/src/ui/renderFailure.test.mjs b/src/ui/renderFailure.test.mjs index e513ec8cc..565422226 100644 --- a/src/ui/renderFailure.test.mjs +++ b/src/ui/renderFailure.test.mjs @@ -90,7 +90,6 @@ test('a render error shows a notice that survives the failed frame', (t) => { const notice = f.store.renderFailure; assert.equal(notice.reason, 'render-error'); assert.equal(notice.detail, 'Expected width to be greater than 0'); - assert.equal(notice.shown, 1); assert.equal(typeof notice.retry, 'function'); assert.equal(typeof notice.reset, 'function'); assert.deepEqual(f.logged, [error]); @@ -120,7 +119,7 @@ test('a loop restarted into the same error keeps one notice', (t) => { assert.equal(f.store.published.length, 1); f.frame(new Error('other')); assert.equal(f.store.renderFailure.detail, 'other'); - assert.equal(f.store.renderFailure.shown, first.shown, 'no new appearance'); + assert.ok(!f.store.published.includes(null), 'no new appearance'); }); test('a new failure after recovery is a new appearance', (t) => { @@ -129,7 +128,18 @@ test('a new failure after recovery is a new appearance', (t) => { f.store.renderFailure.retry(); f.frame(); f.frame(new Error('boom')); - assert.equal(f.store.renderFailure.shown, 2); + assert.deepEqual( + f.store.published.map((notice) => notice?.reason ?? null), + ['render-error', null, 'render-error'], + ); +}); + +test('a render error without a value shows no detail', (t) => { + const f = fixture(t); + f.viewer.useDefaultRenderLoop = false; + f.scene.renderError.raise(f.scene, undefined); + assert.equal(f.store.renderFailure.reason, 'render-error'); + assert.equal(f.store.renderFailure.detail, ''); }); test('Reset opens the app root without the restored view', (t) => { @@ -147,7 +157,7 @@ test('a lost context offers a reload and no frame clears it', (t) => { assert.equal(notice.detail, ''); // Cesium's next frame then fails on the dead drawing buffer. f.frame(new Error('Expected width to be greater than 0')); - assert.equal(f.store.renderFailure.reason, 'context-lost'); + assert.equal(f.store.renderFailure, notice, 'the notice keeps saying why'); f.frame(); assert.ok(f.store.renderFailure, 'a lost context never draws again'); f.store.renderFailure.retry(); @@ -175,10 +185,3 @@ test('uninstall removes the listeners and the notice', (t) => { f.frame(new Error('later')); assert.equal(f.store.renderFailure, null); }); - -test('a viewer without a scene installs nothing', () => { - const uninstall = installRenderFailureNotice(null, { - store: { publishRenderFailure: () => assert.fail('nothing to publish') }, - }); - uninstall(); -}); diff --git a/src/ui/renderFailureEventRetry.test.mjs b/src/ui/renderFailureEventRetry.test.mjs index 1ed20b684..d237f4ebf 100644 --- a/src/ui/renderFailureEventRetry.test.mjs +++ b/src/ui/renderFailureEventRetry.test.mjs @@ -136,69 +136,124 @@ test('a throwing postRender listener removed before Retry is not called again', const listenerCount = (event) => [...event._listeners.values()].reduce((sum, scopes) => sum + scopes.size, 0); -test('a listener turned off and on while the loop is stopped runs after Retry', () => { - const { scene, viewer, store, runFrames, restore } = setup(); - try { - let broken = true; - scene.postRender.addEventListener(() => { - if (broken) throw new TypeError('layer listener failed'); - }); - let calls = 0; - const frame = () => { - calls += 1; - }; - let removeFrame = scene.postRender.addEventListener(frame); - scene.requestRender(); - viewer.useDefaultRenderLoop = true; - runFrames(1); - assert.equal(store.renderFailure?.reason, 'render-error'); - removeFrame(); // another layer is turned off - removeFrame = scene.postRender.addEventListener(frame); // and on again - broken = false; - store.renderFailure.retry(); - calls = 0; - scene.requestRender(); - runFrames(3); - assert.ok(calls > 0, 'the re-added listener runs after Retry'); - assert.ok(scene.postRender._listeners.has(frame)); - assert.equal( - scene.postRender.numberOfListeners, - listenerCount(scene.postRender), - ); - } finally { - restore(); - } -}); +// While the loop is stopped, a layer turned off and on again, or a ground +// task in worldAnnotationRenderer that starts, times out and starts again. +const RESUBSCRIBE = { + 'turned off and on': (event, listener) => { + event.removeEventListener(listener); + event.addEventListener(listener); + }, + 'added, removed and added again': (event, listener) => { + event.addEventListener(listener); + event.removeEventListener(listener); + event.addEventListener(listener); + }, +}; -test('a listener added, removed and added again while the loop is stopped runs after Retry', () => { - const { scene, viewer, store, runFrames, restore } = setup(); - try { - let broken = true; - scene.postRender.addEventListener(() => { - if (broken) throw new TypeError('layer listener failed'); +for (const [name, resubscribe] of Object.entries(RESUBSCRIBE)) { + test(`a listener ${name} while the loop is stopped runs after Retry`, () => { + const { scene, viewer, store, runFrames, restore } = setup(); + try { + let broken = true; + scene.postRender.addEventListener(() => { + if (broken) throw new TypeError('layer listener failed'); + }); + let calls = 0; + const listener = () => { + calls += 1; + }; + if (name === 'turned off and on') + scene.postRender.addEventListener(listener); + scene.requestRender(); + viewer.useDefaultRenderLoop = true; + runFrames(1); + assert.equal(store.renderFailure?.reason, 'render-error'); + resubscribe(scene.postRender, listener); + broken = false; + store.renderFailure.retry(); + calls = 0; + scene.requestRender(); + runFrames(3); + assert.ok(calls > 0, 'the re-added listener runs after Retry'); + assert.equal( + scene.postRender.numberOfListeners, + listenerCount(scene.postRender), + ); + } finally { + restore(); + } + }); +} + +// Every event a listener's error can cut short is finished when the error +// surfaces, so a listener added at once is subscribed. +const FRAME_EVENTS = { + 'scene.preUpdate': (v) => v.scene.preUpdate, + 'scene.postUpdate': (v) => v.scene.postUpdate, + 'scene.preRender': (v) => v.scene.preRender, + 'scene.postRender': (v) => v.scene.postRender, + 'camera.changed': (v) => v.scene.camera.changed, + 'camera.moveStart': (v) => v.scene.camera.moveStart, + 'camera.moveEnd': (v) => v.scene.camera.moveEnd, + 'clock.onTick': (v) => v.clock.onTick, + 'clock.onStop': (v) => v.clock.onStop, + 'globe.tileLoadProgressEvent': (v) => v.scene.globe.tileLoadProgressEvent, + 'tileset.loadProgress': (v) => v.scene.primitives.get(0).loadProgress, + 'tileset.allTilesLoaded': (v) => v.scene.primitives.get(0).allTilesLoaded, + 'tileset.initialTilesLoaded': (v) => + v.scene.primitives.get(0).initialTilesLoaded, + 'tileset.tileLoad': (v) => v.scene.primitives.get(0).tileLoad, +}; + +for (const [name, pick] of Object.entries(FRAME_EVENTS)) { + test(`a listener that throws in ${name} leaves it finished`, () => { + const events = () => ({ + changed: new Cesium.Event(), + moveStart: new Cesium.Event(), + moveEnd: new Cesium.Event(), }); - scene.requestRender(); - viewer.useDefaultRenderLoop = true; - runFrames(1); - assert.equal(store.renderFailure?.reason, 'render-error'); - let calls = 0; - const onGroundFrame = () => { - calls += 1; + const tileset = { + loadProgress: new Cesium.Event(), + allTilesLoaded: new Cesium.Event(), + initialTilesLoaded: new Cesium.Event(), + tileLoad: new Cesium.Event(), }; - // worldAnnotationRenderer: a ground task starts, times out, the next starts - scene.postRender.addEventListener(onGroundFrame); - scene.postRender.removeEventListener(onGroundFrame); - scene.postRender.addEventListener(onGroundFrame); - broken = false; - store.renderFailure.retry(); - scene.requestRender(); - runFrames(3); - assert.ok(calls > 0, 'the re-added listener runs after Retry'); - assert.equal( - scene.postRender.numberOfListeners, - listenerCount(scene.postRender), - ); - } finally { - restore(); - } -}); + const viewer = { + scene: { + renderError: new Cesium.Event(), + preUpdate: new Cesium.Event(), + postUpdate: new Cesium.Event(), + preRender: new Cesium.Event(), + postRender: new Cesium.Event(), + camera: events(), + globe: { tileLoadProgressEvent: new Cesium.Event() }, + primitives: { length: 1, get: () => tileset }, + }, + clock: { onTick: new Cesium.Event(), onStop: new Cesium.Event() }, + canvas: { addEventListener() {}, removeEventListener() {} }, + cesiumWidget: { + _renderLoopRunning: false, + render: () => pick(viewer).raiseEvent(), + }, + }; + const store = { publishRenderFailure() {} }; + const uninstall = installRenderFailureNotice(viewer, { + store, + locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, + logError() {}, + }); + try { + const event = pick(viewer); + event.addEventListener(() => { + throw new TypeError('listener failed'); + }); + assert.throws(() => viewer.cesiumWidget.render(), TypeError); + assert.equal(event._invokingListeners, false); + const listener = () => {}; + event.addEventListener(listener); + assert.ok(event._listeners.has(listener)); + } finally { + uninstall(); + } + }); +} diff --git a/src/ui/renderFailureLoopErrors.test.mjs b/src/ui/renderFailureLoopErrors.test.mjs index 4c703a938..399b3dc26 100644 --- a/src/ui/renderFailureLoopErrors.test.mjs +++ b/src/ui/renderFailureLoopErrors.test.mjs @@ -1,7 +1,7 @@ -// An error thrown in the render loop outside Scene's tryAndCatchError -// (a preRender/postRender listener, clock.onTick, resize) must still reach -// the render failure notice. Drives Cesium's real startRenderLoop through the -// CesiumWidget.prototype.useDefaultRenderLoop setter. +// Errors thrown in the render loop outside Scene's tryAndCatchError (a +// preRender/postRender listener, clock.onTick, resize). Drives Cesium's real +// startRenderLoop through the CesiumWidget.prototype.useDefaultRenderLoop +// setter. import test from 'node:test'; import assert from 'node:assert/strict'; import * as Cesium from 'cesium'; @@ -10,7 +10,7 @@ import { installRenderFailureNotice } from './renderFailure.js'; // What src/app/scene.js passes to the viewer (showRenderLoopErrors). const SHOW_RENDER_LOOP_ERRORS = false; -test('an error thrown outside tryAndCatchError still shows the notice', () => { +function runLoop(render) { const frames = []; const previousRaf = globalThis.requestAnimationFrame; globalThis.requestAnimationFrame = (fn) => frames.push(fn); @@ -21,16 +21,8 @@ test('an error thrown outside tryAndCatchError still shows the notice', () => { postRender: new Cesium.Event(), requestRender() {}, initializeFrame() {}, - // Scene.render raises preRender outside tryAndCatchError - // (Scene.js:4439), so a listener's exception propagates. - render() { - this.preUpdate.raiseEvent(this); - throw new TypeError( - "Cannot read properties of undefined (reading 'x')", - ); - }, + render, }; - const panels = []; const widget = Object.create(Cesium.CesiumWidget.prototype); Object.assign(widget, { _scene: scene, @@ -42,7 +34,7 @@ test('an error thrown outside tryAndCatchError still shows the notice', () => { _targetFrameRate: undefined, isDestroyed: () => false, resize() {}, - showErrorPanel: (...args) => panels.push(args), + showErrorPanel() {}, }); const viewer = { scene, @@ -54,8 +46,6 @@ test('an error thrown outside tryAndCatchError still shows the notice', () => { set useDefaultRenderLoop(value) { widget.useDefaultRenderLoop = value; }, - render: () => widget.render(), - resize: () => widget.resize(), }; const store = { renderFailure: null, @@ -69,15 +59,35 @@ test('an error thrown outside tryAndCatchError still shows the notice', () => { locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, logError: (error) => logged.push(error), }); - viewer.useDefaultRenderLoop = true; // starts Cesium's loop + viewer.useDefaultRenderLoop = true; while (frames.length) frames.shift()(16); - // Cesium's catch stopped the loop. - assert.equal(widget.useDefaultRenderLoop, false); - // The user is told, and the error is logged. - assert.equal(store.renderFailure?.reason, 'render-error'); - assert.equal(logged.length, 1); - uninstall(); + return { widget, store, logged, uninstall }; } finally { globalThis.requestAnimationFrame = previousRaf; } +} + +test('an error thrown outside tryAndCatchError still shows the notice', () => { + // Scene.render raises preRender outside tryAndCatchError (Scene.js:4439), + // so a listener's exception propagates. + const { widget, store, logged, uninstall } = runLoop(function () { + this.preUpdate.raiseEvent(this); + throw new TypeError("Cannot read properties of undefined (reading 'x')"); + }); + assert.equal(widget.useDefaultRenderLoop, false); + assert.equal(store.renderFailure?.reason, 'render-error'); + assert.equal(logged.length, 1); + uninstall(); +}); + +test('an error Scene raised and threw on with rethrowRenderErrors is reported once', () => { + const { store, logged, uninstall } = runLoop(function () { + this.preUpdate.raiseEvent(this); + const error = new Error('boom'); + this.renderError.raiseEvent(this, error); + throw error; + }); + assert.equal(store.renderFailure?.detail, 'boom'); + assert.equal(logged.length, 1); + uninstall(); }); diff --git a/src/ui/renderFailureView.test.mjs b/src/ui/renderFailureView.test.mjs index ee28836c6..6b7e602f9 100644 --- a/src/ui/renderFailureView.test.mjs +++ b/src/ui/renderFailureView.test.mjs @@ -30,7 +30,6 @@ test('a render failure shows an alert dialog that moves focus to Retry', async ( store.publishRenderFailure({ reason: 'render-error', detail: 'Expected width to be greater than 0', - shown: 1, retry: () => calls.push('retry'), reset: () => calls.push('reset'), }); @@ -66,7 +65,6 @@ test('a lost context offers Reload, and a panel that cannot reload offers nothin store.publishRenderFailure({ reason: 'context-lost', detail: '', - shown: 1, retry: () => {}, reset: () => {}, }); @@ -80,7 +78,6 @@ test('a lost context offers Reload, and a panel that cannot reload offers nothin store.publishRenderFailure({ reason: 'context-lost', detail: '', - shown: 1, retry: null, reset: null, }); @@ -88,3 +85,61 @@ test('a lost context offers Reload, and a panel that cannot reload offers nothin assert.deepEqual(buttons(), []); assert.equal(notice().querySelector('.render-failure-hint'), null); }); + +const failure = () => ({ + reason: 'render-error', + detail: '', + retry: () => {}, + reset: null, +}); +const settle = async () => { + await nextTick(); + await nextTick(); +}; + +test('focus goes back where it was when the globe draws again', async (t) => { + const store = mount(t); + const search = document.body.appendChild(document.createElement('input')); + search.focus(); + store.publishRenderFailure(failure()); + await settle(); + assert.equal(document.activeElement?.textContent.trim(), 'Retry'); + store.publishRenderFailure(null); + await settle(); + assert.equal(document.activeElement, search); +}); + +test('focus goes to the map when the element it came from is gone', async (t) => { + const store = mount(t); + const search = document.body.appendChild(document.createElement('input')); + search.focus(); + store.publishRenderFailure(failure()); + await settle(); + search.remove(); + store.publishRenderFailure(null); + await settle(); + const map = document.getElementById('cesiumContainer'); + assert.equal(document.activeElement, map); + assert.equal(map.getAttribute('tabindex'), '-1'); +}); + +test('focus the user moved while the notice was up stays put', async (t) => { + const store = mount(t); + store.publishRenderFailure(failure()); + await settle(); + const chat = document.body.appendChild(document.createElement('textarea')); + chat.focus(); + store.publishRenderFailure(null); + await settle(); + assert.equal(document.activeElement, chat); +}); + +test('the globe drawing from the start leaves focus alone', async (t) => { + mount(t); + await settle(); + assert.equal(document.activeElement, document.body); + assert.equal( + document.getElementById('cesiumContainer').hasAttribute('tabindex'), + false, + ); +}); diff --git a/src/ui/stores/status.js b/src/ui/stores/status.js index 1293503fa..d51fefd03 100644 --- a/src/ui/stores/status.js +++ b/src/ui/stores/status.js @@ -35,10 +35,9 @@ export const useStatusStore = defineStore('gev-status', () => { const fps = shallowRef(null); /** - * The globe's render failure, `{reason, detail, shown, retry, reset}`; null - * while the globe draws. `reason` is 'render-error' or 'context-lost', - * `shown` changes each time the notice appears, and `retry` and `reset` are - * null where that action is not offered. + * The globe's render failure, `{reason, detail, retry, reset}`; null while + * the globe draws. `reason` is 'render-error' or 'context-lost', and `retry` + * and `reset` are null where that action is not offered. */ const renderFailure = shallowRef(null); diff --git a/src/ui/views/panels/RenderFailure.vue b/src/ui/views/panels/RenderFailure.vue index f378e7d78..cba25f75a 100644 --- a/src/ui/views/panels/RenderFailure.vue +++ b/src/ui/views/panels/RenderFailure.vue @@ -18,13 +18,33 @@ const MESSAGES = { ), }; // Focus moves to the first action each time the notice appears, so a -// keyboard user can act on it at once. +// keyboard user can act on it at once. When the globe draws again, it goes +// back where it was, or to the map if that is gone, unless the user has moved +// it since. +let focusBefore; watch( - () => failure.value?.shown, + () => Boolean(failure.value), async (shown) => { - if (!shown) return; + if (shown) { + focusBefore = document.activeElement; + await nextTick(); + panel.value?.querySelector('button')?.focus(); + return; + } + if (focusBefore === undefined) return; + const previous = focusBefore; + focusBefore = undefined; await nextTick(); - panel.value?.querySelector('button')?.focus(); + const active = document.activeElement; + if (active && active !== document.body) return; + if (previous?.isConnected && previous !== document.body) { + previous.focus(); + return; + } + const map = document.getElementById('cesiumContainer'); + if (!map) return; + if (!map.hasAttribute('tabindex')) map.tabIndex = -1; + map.focus(); }, { immediate: true }, );