Return null from loadTexture when image fails to load intead of wrongly creating a VideoTexture

Author: vincentfretin

src/systems/material.js

@@ -30,10 +30,14 @@ export var System = registerSystem('material', {
    *
    * @param {string|Element} src - URL or element
    * @param {object} data - Relevant texture properties
-   * @param {function} cb - Callback to pass texture to
+   * @param {function} cb - Callback that receives the texture, or null if loading failed
    */
   loadTexture: function (src, data, cb) {
     this.loadTextureSource(src, function sourceLoaded (source) {
+      if (source === null) {
+        cb(null);
+        return;
+      }
       var texture = createCompatibleTexture(source);
       setTextureProperties(texture, data);
       cb(texture);
@@ -44,15 +48,15 @@ export var System = registerSystem('material', {
    * Determine whether `src` is an image or video. Then try to load the asset, then call back.
    *
    * @param {string|Element} src - URL or element.
-   * @param {function} cb - Callback to pass texture source to.
+   * @param {function} cb - Callback that receives the texture source, or null if loading failed.
    */
   loadTextureSource: function (src, cb) {
     var self = this;
     var sourceCache = this.sourceCache;
 
     var hash = this.hash(src);
     if (sourceCache[hash]) {
-      sourceCache[hash].then(cb);
+      sourceCache[hash].then(cb, function () { cb(null); });
       return;
     }
 
@@ -64,14 +68,15 @@ export var System = registerSystem('material', {
 
     sourceLoaded(new Promise(doSourceLoad));
     function doSourceLoad (resolve, reject) {
-      utils.srcLoader.validateSrc(src, loadImageCb, loadVideoCb);
+      utils.srcLoader.validateSrc(src, loadImageCb, loadVideoCb, notFoundCb);
       function loadImageCb (src) { self.loadImage(src, resolve); }
       function loadVideoCb (src) { self.loadVideo(src, resolve); }
+      function notFoundCb () { reject(new Error('Resource not found: ' + src)); }
     }
 
     function sourceLoaded (sourcePromise) {
       sourceCache[hash] = Promise.resolve(sourcePromise);
-      sourceCache[hash].then(cb);
+      sourceCache[hash].then(cb, function () { cb(null); });
     }
   },
 
@@ -199,8 +204,9 @@ function loadImageUrl (src) {
       resolveSource,
       function () { /* no-op */ },
       function (xhr) {
-        error('`$s` could not be fetched (Error code: %s; Response: %s)', xhr.status,
+        error('`%s` could not be fetched (Error code: %s; Response: %s)', src, xhr.status,
               xhr.statusText);
+        reject(new Error('Failed to load image: ' + src));
       }
     );
 

src/utils/src-loader.js

@@ -13,14 +13,17 @@ var warn = debug('utils:src-loader:warn');
  * @param {string|Element} src - URL or media element.
  * @param {function} isImageCb - callback if texture is an image.
  * @param {function} isVideoCb - callback if texture is a video.
+ * @param {function} isNotFoundCb - optional callback if texture is not found.
  */
-export function validateSrc (src, isImageCb, isVideoCb) {
-  checkIsImage(src, function isAnImageUrl (isImage) {
-    if (isImage) {
+export function validateSrc (src, isImageCb, isVideoCb, isNotFoundCb) {
+  checkIsImage(src, function isAnImageUrl (result) {
+    if (result === true) {
       isImageCb(src);
-      return;
+    } else if (result === false) {
+      isVideoCb(src);
+    } else if (isNotFoundCb) {
+      isNotFoundCb(src);
     }
-    isVideoCb(src);
   });
 }
 
@@ -120,7 +123,7 @@ export function parseUrl (src) {
  * Call back whether `src` is an image.
  *
  * @param {string|Element} src - URL or element that will be tested.
- * @param {function} onResult - Callback with whether `src` is an image.
+ * @param {function} onResult - Callback with true (image), false (video), or null (not found).
  */
 function checkIsImage (src, onResult) {
   var request;
@@ -139,18 +142,21 @@ function checkIsImage (src, onResult) {
       contentType = request.getResponseHeader('Content-Type');
       if (contentType == null) {
         checkIsImageFallback(src, onResult);
+      } else if (contentType.startsWith('image')) {
+        onResult(true);
       } else {
-        if (contentType.startsWith('image')) {
-          onResult(true);
-        } else {
-          onResult(false);
-        }
+        onResult(false);
       }
     } else {
-      checkIsImageFallback(src, onResult);
+      // Non-success status (404, etc.) - resource not found.
+      onResult(null);
     }
     request.abort();
   });
+  request.addEventListener('error', function () {
+    // Network error - resource not found.
+    onResult(null);
+  });
   request.send();
 }
 

tests/systems/material.test.js

@@ -3,6 +3,7 @@ import { entityFactory } from '../helpers.js';
 
 var IMAGE1 = 'base/tests/assets/test.png';
 var IMAGE2 = 'base/tests/assets/test2.png';
+var IMAGE_FAIL = 'base/tests/assets/nonexistent.png';
 var VIDEO1 = 'base/tests/assets/test.mp4';
 var VIDEO2 = 'base/tests/assets/test2.mp4';
 
@@ -122,6 +123,26 @@ suite('material system', function () {
           done();
         });
       });
+
+      test('returns null when image fails to load', function (done) {
+        var system = this.system;
+
+        system.loadTextureSource(IMAGE_FAIL, function (source) {
+          assert.equal(source, null);
+          done();
+        });
+      });
+    });
+
+    suite('loadTexture', function () {
+      test('returns null when image fails to load', function (done) {
+        var system = this.system;
+
+        system.loadTexture(IMAGE_FAIL, {}, function (texture) {
+          assert.equal(texture, null);
+          done();
+        });
+      });
     });
 
     suite('loadVideo', function () {