Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to inspect which scripts failed #152

Open
foxx opened this issue Aug 20, 2015 · 8 comments
Open

Ability to inspect which scripts failed #152

foxx opened this issue Aug 20, 2015 · 8 comments

Comments

@foxx
Copy link

foxx commented Aug 20, 2015

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']
    );
@reicolina
Copy link

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!

@foxx
Copy link
Author

foxx commented Aug 25, 2015

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

@foxx
Copy link
Author

foxx commented Aug 25, 2015

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){
    });

@hadifarnoud
Copy link

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

@foxx
Copy link
Author

foxx commented Sep 3, 2015

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

@addyosmani
Copy link
Owner

@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 👍

@foxx
Copy link
Author

foxx commented Sep 14, 2015

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

@MarkusPint
Copy link

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants