Merge pull request #12307 from CesiumGS/skybox-error

Fixes unhandled rejection in Skybox.update #12306
CesiumGS · Nov 14, 2024 · 00eb47d · 00eb47d
2 parents 4dfc79a + ea42875
commit 00eb47d
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 11 deletions.
diff --git a/packages/engine/Source/Scene/SkyBox.js b/packages/engine/Source/Scene/SkyBox.js
@@ -81,6 +81,8 @@ function SkyBox(options) {
 
   this._attributeLocations = undefined;
   this._useHdr = undefined;
+  this._hasError = false;
+  this._error = undefined;
 }
 
 /**
@@ -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,10 +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) {
-        that._cubeMap = that._cubeMap && that._cubeMap.destroy();
-        that._cubeMap = cubeMap;
-      });
+      loadCubeMap(context, this._sources)
+        .then(function (cubeMap) {
+          that._cubeMap = that._cubeMap && that._cubeMap.destroy();
+          that._cubeMap = cubeMap;
+        })
+        .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({

diff --git a/packages/engine/Specs/Scene/SkyBoxSpec.js b/packages/engine/Specs/Scene/SkyBoxSpec.js
@@ -1,6 +1,7 @@
-import { Resource, SceneMode, SkyBox } from "../../index.js";
+import { defined, Resource, SceneMode, SkyBox } from "../../index.js";
 
 import createScene from "../../../../Specs/createScene.js";
+import pollToPromise from "../../../../Specs/pollToPromise.js";
 
 describe(
   "Scene/SkyBox",
@@ -10,26 +11,22 @@ describe(
     let loadedImage;
 
     beforeAll(function () {
-      scene = createScene();
-
       return Resource.fetchImage("./Data/Images/Blue.png").then(
         function (image) {
           loadedImage = image;
         },
       );
     });
 
-    afterAll(function () {
-      scene.destroyForSpecs();
-    });
-
     beforeEach(function () {
+      scene = createScene();
       scene.mode = SceneMode.SCENE3D;
     });
 
     afterEach(function () {
       skyBox = skyBox && skyBox.destroy();
       scene.skyBox = undefined;
+      scene.destroyForSpecs();
     });
 
     it("draws a sky box from Images", function () {
@@ -355,6 +352,32 @@ describe(
         return scene.render();
       }).toThrowDeveloperError();
     });
+
+    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: {
+          positiveX: "./Data/Images/Blue.png",
+          negativeX: "./Data/Images/Blue.png",
+          positiveY: "./Data/Images/Blue.png",
+          negativeY: "./Data/Images/Blue.png",
+          positiveZ: "./Data/Images/Blue.png",
+          negativeZ: "./Data/Images/Blue.png",
+        },
+      });
+
+      scene.frameState.passes.render = true;
+      scene.skyBox = skyBox;
+      skyBox.update(scene.frameState);
+
+      await pollToPromise(() => defined(skyBox._cubeMap) || skyBox._hasError);
+
+      expect(skyBox._hasError).toBeTrue();
+      expect(skyBox._error).toEqual(error);
+      expect(() => skyBox.update(scene.frameState)).toThrow(error);
+    });
   },
   "WebGL",
 );