CODEWITHSUNDEEP
javascriptnode.jsexpresspromise
rrain
rrain

Reputation: 61

Promises out of order

I have a promise nested inside of another promise looking like this:

 adb.screencap(list, id).then(function(missing) {
     adb.getScreens(list, id).then(function () {
         res.render('screencap', {output: "Screens captured."})
     })
 })

Where adb.screencap and adb.getScreens is: 

adb.screencap = function(list, id){
    return new Promise(function (resolve, reject) {
        var array = new Array();
        var promises = new Array();

        for (var i = 0; i < list.length; i++) {
            var cmd = "adb -s " + list[i] + " shell screencap /sdcard/" + list[i] + "-" + id + ".png";
            var missing = new Array();

            console.log("== " + cmd);
            promises.push(adb.openArray(i, array, list, cmd, missing));
        }
        Promise.all(promises).then(function (missing) {
            console.log(("resolve"));
            resolve(missing);
        })
    })
}
adb.getScreens = function(list, id){
    return new Promise(function (resolve, reject){
        for (var i = 0; i < list.length; i++) {
            var cmd = 'adb -s ' + list[i] + ' pull /sdcard/' + list[i] + "-" + id + ".png /home/IdeaProjects/DeviceServer/public/files/" + list[i] + "-" + id + ".png";
            exec(cmd, function (err, stdout, stderr) {
                console.log(stdout);
                console.log(cmd);
            });
        }
        resolve();
    })
}

Is there any reason why the adb.getScreens promise is completing before the adb.screencap promise?

Upvotes: 0

Views: 111

Answers (3)

jfriend00
jfriend00

Reputation: 707376

You are resolving getScreens() before any of the exec() calls inside it have completed. They are async. They finish sometime later, but you are calling resolve() right after you've started them all. So, you end up trying to render before any of the results you need to render are available.

Instead, you need to promisify exec() itself and then collect those promises in an array and use Promise.all() to see when all of them are done. Only then will getScreens() actually be done.

There are lots of other issues here too. screencap() is using an anti-pattern. There's no need to create your own promise here. You can just return the promise from Promise.all() and the use of this anti-pattern causes you to lose any errors thrown by any of the promises.

Here's a recommendation:

// promisify exec
function execP(cmd) {
    return new Promise(function(resolve, reject) {
        exec(cmd, function(err, stdout, stderr) {
            if (err) return reject(err);
            console.log(cmd);
            console.log(stdout);
            console.log(stderr);
            resolve({stdout: stdout, stderr: stderr});
        });
    });
}

adb.getScreens = function(list, id){
    return Promise.all(list.map(function(item) {
       var cmd = 'adb -s ' + item + ' pull /sdcard/' + item + "-" + id + ".png /home/IdeaProjects/DeviceServer/public/files/" + item + "-" + id + ".png";
       return execP(cmd);
    }));
}

Now .getScreens() returns a promise that will be resolved only when all the exec() calls are done (so your output files will be available and ready for your redner).

This configures .getScreens() to resolve to an array of objects that contains the stdout, stderr info from all your exec calls. It doesn't appear you are attempting to use that output anyway, but that's how this is configured.

Upvotes: 1

Shilly
Shilly

Reputation: 8589

I'm still trying to figure out what's going wrong, since it seems rather weird this occurs. It's probably somewhere inside the openArray() method or in the exec() function.

For what it's worth, you can simplify some things, as Felix suggest:

// If the getScreens handler doesn't need the 'missing' returned from screencap
adb.screencap(list, id).then(function(missing) {
    return adb.getScreens(list, id);
}).then(function ( gottenScreens ) {
    res.render('screencap', {output: "Screens captured."})
});

// You can just return the Promise.all here instead of adding another promise
// just to resolve the outer one.
adb.screencap = function(list, id) {
    var array = new Array();
    var promises = new Array();
    for (var i = 0; i < list.length; i++) {
        var cmd = "adb -s " + list[i] + " shell screencap /sdcard/" + list[i] + "-" + id + ".png";
        var missing = new Array();

        console.log("== " + cmd);
        // is this some async operation or are you just creating all the commands here?
        // if so, it does't make sense to use a promise here.
        promises.push(adb.openArray(i, array, list, cmd, missing));
    }       
    return Promise.all(promises);
}

Upvotes: 1

Pankaj
Pankaj

Reputation: 966

Does your adb.openArray return promise? Then only Promise.all will work.

And you should resolve the promise of getScreens inside the callback if exec. It is outside and does not wait for exec to finish.

Upvotes: 0

Related Questions