basket.js icon indicating copy to clipboard operation
basket.js copied to clipboard

Ability to inspect which scripts failed

Open foxx opened this issue 9 years ago • 8 comments

It would be nice to have the ability to inspect which scripts failed to load on the error() handler.

This would allow me to determine if the failed script was in the "important" list or not, and send the error to a collection service where needed. For example, jquery would be a fatal error where as a third party tracking library might not be.

Currently it just seems to show this in console;

Error
    at XMLHttpRequest.RSVP.Promise.d.onreadystatechange (https://cdnjs.cloudflare.com/ajax/libs/basket.js/0.5.2/basket.full.min.js:10:12467)

Ideally it would be good to be able to do something like;

basket.require(
    { url: '//example.com', key: 'jquery' },
    { url: '//example.com', key: 'other' })
    .then(function(){}, function(err) {
        console.log(err.failed) // ['jquery']
    );

foxx avatar Aug 20 '15 14:08 foxx

I have been playing with basket.js lately, as I find the idea behind it fascinating... I do think it would be useful to have an array of the resources that failed to be retrieved.

Looking at the code, I believe that the behavior described in this ticket is caused by the fact that internally basket.js uses RSVP.all(promises) which gets rejected immediately if any promise in the array is rejected. This doesn't allow us to keep track of the scripts that failed to load.

A potential enhancement would be to modify the require method to accept an optional trackFailures argument, and if it's truthy then basket will use RSVP.allSettled(promises) (instead of RSVP.all(promises)) which fulfills (or rejects) with an array of the promises' result states, allowing us to keep track of the failures in a more elegant way.

I wouldn't mind working on a PR that implements this solution If the project's core members believe that this would be useful.

In the meantime, you can have a workaround by doing something like this:

// define one valid URL and two invalid ones
var urls = [
        'https://badurl1/x.js',
        'https://cdnjs.cloudflare.com/ajax/libs/jquery/3.0.0-alpha1/jquery.min.js',
        'https://badurl2/y.js'
    ],
    failedUrls = [],
    i;

// closure that lets us deal with each url individually
var getResource = function (url) {
    basket.require({url: url}).then(function () {}, function (err) {
        // deal with the failures here
        // for this example we just push the failed url to an array to be checked later
        failedUrls.push(url);
    });  
};

// get the resources
for (i = 0; i < urls.length; i++) {
    getResource(urls[i]);
}

// wait a little bit for all the 'requires' to be fullfilled or rejected
// and check the 'failedUrls' array to see which ones failed
setTimeout(function () {
    console.log(failedUrls) // expect: ["https://badurl1/x.js", "https://badurl2/y.js"]
}, 500);

Cheers!

reicolina avatar Aug 25 '15 04:08 reicolina

I'm also happy to help out with PR, can contribute 2 hours if needed. Thoughts @sindresorhus / @addyosmani ?

foxx avatar Aug 25 '15 10:08 foxx

As an extension to the answer given by @reinaldo13, it might also be nice to have failure reason. For example;

basket.require(
    { url: '//example.com', key: 'jquery' },
    { url: '//example.com', key: 'other' })
    .then(function(){}, function(err) {
        console.log(err.failed) // [ ['jquery', 'timeout', Error()],  ]
    );

Or alternatively, you could even add nice chaining;

basket.require(
    { url: '//example.com', key: 'jquery' },
    { url: '//example.com', key: 'other' })
    .then(function(){
    })
    .fail(function(name, reason, err){
    });

foxx avatar Aug 25 '15 10:08 foxx

this feature would make basket.js amazing! we often end up using teamviewer for this, which is a pain.

hadifarnoud avatar Sep 03 '15 10:09 hadifarnoud

@sindresorhus / @addyosmani - Can you spare 5 minutes to review this before commit time for a PR?

foxx avatar Sep 03 '15 12:09 foxx

@sindresorhus I'm personally fine with the community extending the inspection capabilities of basket. We'll need to review how large an impact this has on source as per any PR, but otherwise curious what this would look like, @foxx :+1:

addyosmani avatar Sep 06 '15 23:09 addyosmani

Thanks for the update and apologies for the delay, I'll get a PR submitted this week

foxx avatar Sep 14 '15 13:09 foxx

What would be the most suitable way of turning on "trackFailures" ?

An options object passed after the array of needed resources, so it only applies to those resources:

basket.require([
    { url: '//example.com' },
    { url: '//example.com' }
], { trackFailures: true });

Or a global config option basket.trackFailures = true ?

MarkusPint avatar Sep 22 '15 08:09 MarkusPint