Retrieve data from a callback thats in a promise?

Question

I have the following piece of code right now:

const Promise = require('bluebird');
const readFile = Promise.promisify(fs.readFile);
recordPerfMetrics: function(url) {

    var self = this;
    var perf, loadTime, domInteractive, firstPaint;
    var perfData = {};       
    readFile('urls.txt', 'UTF-8').then(function (urls, err) {
        if (err) {
            return console.log(err);
        }

        var urls = urls.split("\n");
        urls.shift();

        urls.forEach(function(url) {     
            console.log(url);   
            self.getStats(url).then(function(data) { 
                data = data[0];
                loadTime = (data.loadEventEnd - data.navigationStart)/1000 + ' sec';
                firstPaint = data.firstPaint;
                domInteractive = (data.domInteractive - data.navigationStart)/1000 + ' sec';

                perfData = {
                    'URL' : url,
                    'firstPaint' : firstPaint,
                    'loadTime' : loadTime,
                    'domInteractive' : domInteractive
                };
                console.log(perfData);
            }).catch(function(error) {
                console.log(error);
            });
        });      

        // console.log(colors.magenta("Starting to record performance metrics for " + url));
        // this.storePerfMetrics();                       
    });    
},

getStats: function(url) {
    return new Promise(function(resolve, reject){
        console.log("Getting data for url: ",url);
        browserPerf(url, function(error, data) {
            console.log("inside browserPerf", url);
            if (!error) {
                resolve(data);
              } else {
                reject(error);
            }
        }, {
            selenium: 'http://localhost:4444/wd/hub',
            browsers: ['chrome']
        });
    });
},

This is basically reading urls from a file and then calling a function browserPerf whose data being returned is in a callback function.

The console.log("Getting data for url: ",url); is in the same order as the urls that are stored in the file,

but the console.log("inside browserPerf", url); is not conjunction as the same and as expected.

I expect the order of the urls to be:

console.log(url);   
console.log("Getting data for url: ",url);
console.log("inside browserPerf", url);

But for reason only the first two are executed in order but the third one is fired randomly after all are being read. Any idea what i am doing wrong here?

Answer 1

Since you are using Bluebird, you can replace your .forEach() loop with Promise.mapSeries() and it will sequentially walk through your array waiting for each async operation to finish before doing the next one. The result will be a promise who's resolved value is an array of results. You also should stop declaring local variables in a higher scope when you have async operations involved. Declare them in the nearest scope practical which, in this case is the scope in which they are used.

const Promise = require('bluebird');
const readFile = Promise.promisify(fs.readFile);

recordPerfMetrics: function() {

    var self = this;
    return readFile('urls.txt', 'UTF-8').then(function (urls) {
        var urls = urls.split("\n");
        urls.shift();

        return Promise.mapSeries(urls, function(url) {     
            console.log(url);   
            return self.getStats(url).then(function(data) { 
                data = data[0];
                let loadTime = (data.loadEventEnd - data.navigationStart)/1000 + ' sec';
                let firstPaint = data.firstPaint;
                let domInteractive = (data.domInteractive - data.navigationStart)/1000 + ' sec';

                let perfData = {
                    'URL' : url,
                    'firstPaint' : firstPaint,
                    'loadTime' : loadTime,
                    'domInteractive' : domInteractive
                };
                console.log(perfData);
                return perfData;
            }).catch(function(error) {
                console.log(error);
                throw error;    // keep the promise rejected
            });
        });      

        // console.log(colors.magenta("Starting to record performance metrics for " + url));
        // this.storePerfMetrics();                       
    });    
},

getStats: function(url) {
    return new Promise(function(resolve, reject){
        console.log("Getting data for url: ",url);
        browserPerf(url, function(error, data) {
            console.log("inside browserPerf", url);
            if (!error) {
                resolve(data);
              } else {
                reject(error);
            }
        }, {
            selenium: 'http://localhost:4444/wd/hub',
            browsers: ['chrome']
        });
    });
},

You would use this like this:

obj.recordPerfMetrics().then(function(results) {
    // process results array here (array of perfData objects)
}).catch(function(err) {
    // error here
});

Summary of changes:

1. Return promise from recordPefMetrics so caller can get data
2. Use Promise.mapSeries() instead of .forEach() for sequential async operations.
3. Return promise from Promise.mapSeries() so it is chained with prior promise.
4. Move variable declarations into local scope so there is no change of different async operations stomping on shared variables.
5. Rethrow .catch() error after logging so the reject propagates
6. return perfData so it becomes the resolved value and is available in results array.