datasources/base.js ImageLoader.load - Error callback broken

[bitroost]

The imageLoader onProgress-callback is missing from the ImageLoader.load-call so the error callback never triggers in case of a tile that is missing. From the Three.js ImageLoader documentation:

// load a image resource
// instantiate a loader
const loader = new THREE.ImageLoader();

loader.load(
    // resource URL
    'textures/skyboxsun25degtest.png',

    // onLoad callback
    function ( image ) {
        // use the image, e.g. draw part of it on a canvas
        const canvas = document.createElement( 'canvas' );
        const context = canvas.getContext( '2d' );
        context.drawImage( image, 100, 100 );
    },

    // onProgress callback currently not supported
    undefined,

    // onError callback
    function () {
        console.error( 'An error happened.' );
    }
);

To Reproduce The issue becomes noticeable when using a custom datasource with a lot of requests returning 204 for 'Tile does not exist' using Tileserver-GL (https://github.com/maptiler/tileserver-gl/pull/278). What happens is that the request errors do not get deleted from this.fetching, pile up there and eventually trigger throttling which, in turn, after 32 errors, blocks the request of any new tile.

datasources/base.js #95-100

    // Throttle downloads
    let downloadIndices = Object.values( this.fetching );
    if ( downloadIndices.length > 32 ) {
      log( 'throttling...' );
      return;
    }

Expected behavior With the error callback working, the errors get deleted from this.fetching and do not trigger throttling, but in case of 'missing tiles' will try again and retrigger the error.

Proposed fix I think errors should be stored in an object similar to this.fetching

    this.fetching = {};
    this.failed = {};

populated on error and then checked against at the beginning of fetchIfNeeded():

  fetchIfNeeded( quadkey ) {
    if ( this.lookup[ quadkey ] !== undefined ||
       this.fetching[ quadkey ] !== undefined ||
       this.failed[ quadkey ] ) {
      // Have data, download in progress, or tried and failed: skip
      return;
    }

    // Throttle downloads
    let downloadIndices = Object.values( this.fetching );
    if ( downloadIndices.length > 32 ) {
      log( 'throttling...' );
      return;
    }

    let newIndex = this.findNewIndex( quadkey );

    // Mark as download in progress
    this.fetching[ quadkey ] = newIndex;

    // Actually fetch data
    let url = this.urlForTile( ...tilebelt.quadkeyToTile( quadkey ) );
    ImageLoader.load( url, ( image ) => {
      // Image loaded OK
      this.imgCache[ quadkey ] = image;
      insertIntoTextureArray( this.textureArray, newIndex, image );

      // Remove download and mark image location in lookup
      delete this.fetching[ quadkey ];
      this.lookup[ quadkey ] = newIndex;

      // TODO remove, just for demo page
      if ( this.useFloat ) {
        let el = document.getElementById( 'elevation-tile-count' );
        if ( el ) {
          let n = parseInt( el.innerHTML );
          if ( isNaN( n ) ) { n = 0 }

          el.innerHTML = ++n;
        }
      }

      this.updateIndirectionTexture();
      this.notifyUpdate();
    },
    undefined,
    ( err ) => {
      if ( err.name == 'InvalidStateError' ) {
        let newIndex = this.findNewIndex( quadkey );
        this.failed[ quadkey ] = newIndex;
        log( 'InvalidStateError / HTTP 204', quadkey );
      } else {
        console.error( 'Failed to get image', quadkey );
      }

      delete this.fetching[ quadkey ];
    } );
  }

Http 204 produces 'InvalidStateError' which should not trigger a console.error but everything else still does.