englercj / resource-loader

A middleware-style generic resource loader built with web games in mind.
http://englercj.github.io/resource-loader/
MIT License
424 stars 77 forks source link

Question re. handling errors on sub-resources in custom middleware #131

Closed jmlee2k closed 5 years ago

jmlee2k commented 5 years ago

First of all, thanks for everyone's work on this project!

We have a custom middleware that we use to load audio sprites (json + mp3/ogg). It works great, I just have a small question re. proper error handling.

The main resource is a json file which contains metadata for the audio sprite, and we load the actual audio data as a XHR_RESPONSE_TYPE.BUFFER subresource:

(the following is our actual code with the irrelevant stuff stripped out)

const audioMiddleware = (origResource:PIXI.loaders.Resource,next:()=>void)=>
{
    const loadOptions = { crossOrigin: origResource.crossOrigin, loadType: PIXI.loaders.Resource.LOAD_TYPE.XHR, xhrType:PIXI.loaders.Resource.XHR_RESPONSE_TYPE.BUFFER, parentResource: origResource};

    loader.add(resource.name+"_audiodata", audioDataPath, loadOptions,  (resource:PIXI.loaders.Resource)=>{
        origResource.audioData = resource.data;
        next();
    });
}

However, if the file can't be found, or there is some other network error, we'd like to know at load-time instead of trying to decode missing/invalid data. We're now checking the xhr status and setting the error property on the subresource when applicable:

    loader.add(resource.name+"_audiodata", audioDataPath, loadOptions, (resource:PIXI.loaders.Resource)=>{
        if (resource.xhr!.status >= 200 && resource.xhr!.status < 300)
            origResource.audioData = resource.data;
        else
            resource.error = new Error("unable to fetch data from "+audioDataPath);

        next();
    });

This works (onError is fired by the loader) but setting the error property doesn't "feel" right. We initially tried to call resource.abort(), but it fails since the resource has already completed loading at that point.

Is this the right way to handle the error? Any information or advice would be most appreciated!

Keep up the good work!

englercj commented 5 years ago

The resource already sets resource.error when the XHR request has a non-success response code. You can just check if resource.error is set, and if so call next() immediately in your middleware instead of adding any subresources.

Maybe that behavior should be different?

jmlee2k commented 5 years ago

I looked into it a bit deeper, and I think I see the problem. We're using the version of resource loader included with Pixi 4.5.2, which has a bug in the code in Resource.prototype._xhrOnLoad:

var status = typeof xhr.status === 'undefined' ? xhr.status : STATUS_OK; // XDR has no `.status`, assume 200.

So for us, status is always 200. I see it's since been fixed: https://github.com/englercj/resource-loader/blob/master/src/Resource.js#L806.

We're planning on moving to Pixi 5 soon, so we'll see what happens then. Thanks for your quick response!

englercj commented 5 years ago

If you upgrade to pixi v4.5.4 at least it should fix this issue as that version uses v2.0.9 of this library (your issue was fixed in v2.0.7). PixiJS v5 uses the latest version of this library so that should be good as well.

Sorry about that!

jmlee2k commented 5 years ago

@englercj, thanks for the info. We've updated to Pixi 4.8.7 in preparation for the move to v5, and now subresources in our middleware have the error property set as expected.

Thanks again!