Finish an interrupted frame event as soon as its error surfaces

Retry used to apply the queued listener changes of a Cesium event a
throwing listener left mid-raise, adds first and removes second. A layer
turned off and on while the notice was up landed in both queues and was
removed. The render and resize wrapper now finishes those events before
it raises the render error, so later adds and removes apply at once and
in order.

docs/CURRENT-STATE.md

@@ -1600,10 +1600,11 @@ render error before Cesium's loop handles it as before.
   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.
+  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
+  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
   context it has lost, and the address keeps the view.
 - **Reset view** opens the app's root address (`appRootUrl()`), without the

src/ui/renderFailure.js

@@ -42,17 +42,6 @@ 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?.();
   }
@@ -120,11 +109,25 @@ export function installRenderFailureNotice(
   // postRender listener, the clock's onTick, a resize) also stops Cesium's
   // 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.
+  // 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.
   const unguard = ['resize', 'render'].map((name) =>
-    reportThrown(viewer.cesiumWidget, name, (error) =>
-      scene.renderError.raiseEvent(scene, error),
-    ),
+    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);
+    }),
   );
   canvas?.addEventListener?.('webglcontextlost', onContextLost);
   return () => {
@@ -138,9 +141,8 @@ 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.
+ * only queues the listeners added or removed. Apply the queue as the raise
+ * would have, so listeners added or removed from here on take effect at once.
  */
 function finishRaise(event) {
   if (!event?._invokingListeners) return;

src/ui/renderFailureEventRetry.test.mjs

@@ -132,3 +132,73 @@ test('a throwing postRender listener removed before Retry is not called again',
     restore();
   }
 });
+
+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();
+  }
+});
+
+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');
+    });
+    scene.requestRender();
+    viewer.useDefaultRenderLoop = true;
+    runFrames(1);
+    assert.equal(store.renderFailure?.reason, 'render-error');
+    let calls = 0;
+    const onGroundFrame = () => {
+      calls += 1;
+    };
+    // 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();
+  }
+});