Possible fix for aframevr#5032: Don't hang forever on asset loading e…

…rror (aframevr#5033)

* Better handling of asset loading errors.

* Simple test for asset loading error.

* Ensure all assets loaded even when one is in error.  Plus test cases.

* fix typos

* Remove trailing spaces to pass linting checks.

* Fix linting errors arising from merge

* Fix up merge

* Fix test script that fails due to inadequate mocking of canvas.

* Revert "Fix test script that fails due to inadequate mocking of canvas."

This reverts commit 84a0d63.

* Remove example - this is now tested in UT.

src/core/a-node.js

@@ -2,7 +2,6 @@
 var utils = require('../utils/');
 
 var warn = utils.debug('core:a-node:warn');
-var error = utils.debug('core:a-node:error');
 
 var knownTags = {
   'a-scene': true,
@@ -123,20 +122,27 @@ class ANode extends HTMLElement {
     // Wait for children to load (if any), then load.
     children = this.getChildren();
     childrenLoaded = children.filter(childFilter).map(function (child) {
-      return new Promise(function waitForLoaded (resolve) {
+      return new Promise(function waitForLoaded (resolve, reject) {
         if (child.hasLoaded) { return resolve(); }
         child.addEventListener('loaded', resolve);
+        child.addEventListener('error', reject);
       });
     });
 
-    Promise.all(childrenLoaded).then(function emitLoaded () {
+    Promise.allSettled(childrenLoaded).then(function emitLoaded (results) {
+      results.forEach(function checkResultForError (result) {
+        if (result.status === 'rejected') {
+          // An "error" event has already been fired by THREE.js loader,
+          // so we don't need to fire another one.
+          // A warning explaining the consequences of the error is sufficient.
+          warn('Rendering scene with errors on node: ', result.reason.target);
+        }
+      });
+
       self.hasLoaded = true;
       if (cb) { cb(); }
       self.setupMutationObserver();
-
       self.emit('loaded', undefined, false);
-    }).catch(function (err) {
-      error('Failure loading node: ', err);
     });
   }
 

tests/core/a-assets.test.js

@@ -25,6 +25,24 @@ suite('a-assets', function () {
     THREE.Cache.files = {};
   });
 
+  test('loads even if one asset fails to load', function (done) {
+    var el = this.el;
+    var scene = this.scene;
+    var assetItem = document.createElement('a-asset-item');
+    assetItem.setAttribute('src', 'invalid-filename');
+    assetItem.addEventListener('error', function (evt) {
+      assert.ok(evt.detail.xhr !== undefined);
+      // Stop propagation of the event so it doesn't trigger
+      // mocha unhandled exception logic.
+      evt.stopPropagation();
+    });
+    scene.addEventListener('loaded', function () {
+      done();
+    });
+    el.appendChild(assetItem);
+    document.body.appendChild(scene);
+  });
+
   test('throws error if not in a-scene', function () {
     var div = document.createElement('div');
     var assets = document.createElement('a-assets');
@@ -235,6 +253,7 @@ suite('a-asset-item', function () {
   });
 
   test('emits progress event', function (done) {
+    THREE.Cache.remove(XHR_SRC);
     var assetItem = document.createElement('a-asset-item');
     assetItem.setAttribute('src', XHR_SRC);
     assetItem.addEventListener('progress', function (evt) {
@@ -262,6 +281,33 @@ suite('a-asset-item', function () {
     document.body.appendChild(this.sceneEl);
   });
 
+  test('waits for valid assets to load, even when some assets are invalid', function (done) {
+    var scene = this.sceneEl;
+    var assetItem1 = document.createElement('a-asset-item');
+    assetItem1.setAttribute('src', 'doesntexist');
+    var assetItem2 = document.createElement('a-asset-item');
+    assetItem2.setAttribute('src', XHR_SRC);
+
+    // Remove cache data to not load from it.
+    THREE.Cache.remove(XHR_SRC);
+
+    assetItem1.addEventListener('error', function (evt) {
+      assert.ok(evt.detail.xhr !== undefined);
+      evt.stopPropagation();
+    });
+    // To pass the test, we must get the 'loaded' event on asset 2 first,
+    // and only then on the scene.
+    assetItem2.addEventListener('loaded', function () {
+      scene.addEventListener('loaded', function () {
+        done();
+      });
+    });
+
+    this.assetsEl.appendChild(assetItem1);
+    this.assetsEl.appendChild(assetItem2);
+    document.body.appendChild(this.sceneEl);
+  });
+
   test('loads as text without responseType attribute', function (done) {
     var assetItem = document.createElement('a-asset-item');
     // Remove cache data to not load from it.