PR feedback

- Throw during next call to update
- use catch instead of onRejected
- Don't update CHANGES.md for minor bug

CHANGES.md

@@ -1,13 +1,5 @@
 # Change Log
 
-### 1.124.0
-
-#### @cesium/engine
-
-##### Fixes :wrench:
-
-- Handle uncaught promise rejection in `Skybox.update` that could cause tests to fail[#12307](https://github.com/CesiumGS/cesium/pull/12307)
-
 ### 1.123.1 - 2024-11-07
 
 #### @cesium/engine

packages/engine/Source/Scene/SkyBox.js

@@ -81,6 +81,8 @@ function SkyBox(options) {
 
   this._attributeLocations = undefined;
   this._useHdr = undefined;
+  this._hasError = false;
+  this._error = false;
 }
 
 /**
@@ -111,6 +113,15 @@ SkyBox.prototype.update = function (frameState, useHdr) {
     return undefined;
   }
 
+  // Throw any errors that had previously occurred asynchronously so they aren't
+  // ignored when running.  See https://github.com/CesiumGS/cesium/pull/12307
+  if (this._hasError) {
+    const error = this._error;
+    this._hasError = false;
+    this._error = undefined;
+    throw error;
+  }
+
   if (this._sources !== this.sources) {
     this._sources = this.sources;
     const sources = this.sources;
@@ -141,15 +152,18 @@ SkyBox.prototype.update = function (frameState, useHdr) {
 
     if (typeof sources.positiveX === "string") {
       // Given urls for cube-map images.  Load them.
-      loadCubeMap(context, this._sources).then(
-        function (cubeMap) {
+      loadCubeMap(context, this._sources)
+        .then(function (cubeMap) {
           that._cubeMap = that._cubeMap && that._cubeMap.destroy();
           that._cubeMap = cubeMap;
-        },
-        function (error) {
-          console.error("Skybox cube map failed to load", error);
-        },
-      );
+        })
+        .catch((error) => {
+          // Defer throwing the error until the next call to update to prevent
+          // test from failing in `afterAll` if this is rejected after the test
+          // using the Skybox ends.  See https://github.com/CesiumGS/cesium/pull/12307
+          this._hasError = true;
+          this._error = error;
+        });
     } else {
       this._cubeMap = this._cubeMap && this._cubeMap.destroy();
       this._cubeMap = new CubeMap({

packages/engine/Specs/Scene/SkyBoxSpec.js

@@ -356,10 +356,9 @@ describe(
       }).toThrowDeveloperError();
     });
 
-    it("handles error when Resources fail to load", async () => {
-      spyOn(Resource.prototype, "fetchImage").and.rejectWith(
-        "intentional error for test",
-      );
+    it("defers throwing error when Resources fail to load until next call to update", async () => {
+      const error = new Error("intentional error for test");
+      spyOn(Resource.prototype, "fetchImage").and.rejectWith(error);
 
       skyBox = new SkyBox({
         sources: {
@@ -376,9 +375,12 @@ describe(
       scene.skyBox = skyBox;
       skyBox.update(scene.frameState);
 
-      // Flush macro task queue to allow the rejections to be throw in this
-      // function, otherwise the error ends up happening in `afterAll`
-      await new Promise((resolve) => window.setTimeout(resolve, 1));
+      // Flush macro task queue to allow the rejections to be thrown
+      await new Promise((resolve) => window.setTimeout(resolve, 0));
+
+      expect(skyBox._hasError).toBeTrue();
+      expect(skyBox._error).toEqual(error);
+      expect(() => skyBox.update(scene.frameState)).toThrow(error);
     });
   },
   "WebGL",