Provide a facility for easily recognizing that a module failed to load

[kfranqueiro]

There are cases when we should be able to tell if a module fails to load, as well as if it succeeds.

For example, in this code in request.ts, the promise will never settle whatsoever if the module fails to load.

Failures involving invalid URLs resulting in 404s should be detectable by adding an error event listener to the script element being injected. However, errors resulting from syntax errors would only be detectable from whatever calls the factory function, and I don't think we would want to try/catch that, which would likely leave a timeout as the only way of detecting a failure in that regard.

EDIT: Some failures (i.e. syntax errors, preventing the script from even being run) might be detectable by listening for the script element's load event and checking if some basic housekeeping hasn't been done by then (since define itself should have at least been called by then, IIUC).

Meanwhile, as for the actual reporting mechanism, I'm not sure which direction we'd want to go with that. Dojo 1's loader has an entire event system that IIRC is pretty obtuse to use for this particular purpose. OTOH, curl actually returns promise-like objects from its require calls, but I don't know whether we would want to go in that direction.

[rcgill]

Dojo 1's loader has an entire event system that IIRC is pretty obtuse to use for this particular purpose.

require.on("error", function(e){
if(e.message=="scriptError"){
//do something
}
});

https://github.com/dojo/dojo/blob/master/dojo.js#L365

[bryanforbes]

I think the "obtuse" comment was more describing how users have to filter out events they don't care about, rather than just saying, "for this require call, tell me when an error happens". For instance, I've wrapped require in a promise before and the rejection path can be tricky:

function getModules(moduleIds) {
    return new Promise(function (resolve, reject) {
        var handle;
        var moduleUrls = {};

        for (var i = 0; i < moduleIds.length; i++) {
            moduleUrls[require.toUrl(moduleIds[i])] = moduleIds[i];
        }

        handle = require.on('error', function (error) {
            // TODO: handle plugins correctly
            if (error.message === 'scriptError') {
                var moduleUrl = error.info[0].slice(0, -3);
                if (moduleUrl in moduleUrls) {
                    handle && handle.remove();
                    handle = moduleIds = null;

                    var reportedError = new Error('Couldn\'t load ' + moduleUrls[moduleUrl] + ' from ' + error.info[0]);
                    reportedError.url = error.info[0];
                    reportedError.originalError = error.info[1];
                    reject(reportedError);
                }
            }
        });

        require(moduleIds, function () {
            handle && handle.remove();
            handle = moduleIds = null;
            var modules = Array.prototype.slice.call(arguments, 0);
            resolve(modules);
        });
    });
}

Even this only covers errors for the modules explicitly requested. @kfranqueiro can correct me, but the filtering out of events is what he meant by "obtuse".

[rcgill]

Agreed.

Also require.on is not part of the AMD spec. Indeed, though the problem is solvable with the current design and implementation of the dojo loader (or, even, a trimmed down loader w/out all the cruft from legacy and plugin code), the solution would not work with other loaders.

That said, I still think incorporating an event API into the loader is the correct design as it separates concerns brightly and can be compiled out without effort when not used.

[csnover]

This is also a super sucky thing for Intern at the moment; probably my number 1 most desired feature for the loader is to get error handling for require calls. I glanced over this stuff late last week, it seems like it will be tricky due to the way intermediate dependencies are loaded. The loader design is elegant in reducing duplication for the two different types of calls but it makes it rather difficult to associate dependency load failures with their original require calls.

[rcgill]

If you are going to use either the dojo v1.0 as a code base (or the minimized version I published a couple of years back), you could add a property to the module object maintained by the loader which lists the modules that depend on the given module (so, now the module object has two lists...(1) the list of modules the given module depends on, and (2) the list of modules that depend on the given module). This would be a trivial extension (in algorithm time and space, and implementation effort). And with this information, you should be able to solve the problem at hand.

[kitsonk]

RequireJS takes an errback after the callback. I am not sure if that pattern is any less clunky. It also supports a overriding onError. The only pattern that RequireJS seems to apply is the ability to 'undef' a failed module and try to reload that module from somewhere else (e.g. you can load it off the CDN, so I will load elsewhere).

The ES6 module loader follows ES6 Promises and resolves non-loading via a rejection/catch.

@csnover give a good use case (which if I am reading correctly is "I want to be able to provide additional diagnostic information when I cannot load a dependency") and there is the RequireJS use case "I want to load from somewhere else". Are there any further use cases, as that might help determine what the best approach/API would be?

[kfranqueiro]

Any use cases for dojo/core#35 (including core/request and ostensibly crypto) will want this.

[csnover]

@kitsonk Long term the best API will be to have the require function return a Promise like all async functions, but in the medium-term a separate errback is better since it keeps the size and complexity of the loader down while maintaining compatibility.

[kfranqueiro]

It occurs to me (while thinking on #16) that ostensibly we'd also need a way for AMD plugins to signal errors so that the loader could properly report them. The plugin API only currently involves a single callback for success, doesn't it?

[edhager]

I would think the plugin would just throw an error. The loader would catch the error, add some info about where the error occurred and call the callback function.

[kfranqueiro]

Except it's an asynchronous process, right? I would think that just throwing an error would not be catchable. (Edit: redacted the "core API" comment, confused this with the text plugin issue :))

[kitsonk]

Calling the callback function on error seems strange, assuming the loader could even catch it. Just looked at RequireJS, and the onLoad function is also decorated with onLoad.error() which the plugin can use, normalize is a sync return value (so could throw).