0

I am learning bluebird and implementing in my code as below:

var getPromiseA = new Promise(function (resolve, reject) {
 executeCommand('command A')
     .then(function (result) {
         var temp = JSON.parse(result);
         for (var i in temp) {
             if (temp[i].abc === 'ABC') {
                 resolve(temp[i]);
             }
         }
     }).catch(function (err) {
         console.log('Error occurred while executing getPromiseA :::');
         reject(err);
     });
  });

 var getPromiseB = new Promise(function (resolve, reject) {
     executeCommand('command B')
         .then(function (result) {
             resolve(JSON.parse(result));
         })
         .catch(function (err) {
             console.log('Error occurred while executing getPromiseB :::');
             reject(err);
         });
 });

 var getPromiseC = new Promise(function (resolve, reject) {
     executeCommand('command C')
         .then(function (result) {
             var temp = JSON.parse(result);
             for (var i in temp) {
                 console.log('in command C::::');
                 if (temp[i].abc != null) {
                     resolve(temp[i]);
                 }
             }
         })
         .catch(function (err) {
             console.log('Error occurred while executing getPromiseC :::');
             reject(err);
         });
 });

 function executeCommand(inputCmd) {
     var commandPromise = new Promise(function (resolve, reject) {
         var response = {}, err = {};
         var command = exec(inputCmd);
         command.stdout.on('data', function (data) {
             response = data;
         });
         command.stderr.on('data', function (data) {
             err = data;
         });
         command.on('close', function (code) {
             console.log('coming in close' + code);
             if (code === 0) {
                 console.log('before resolving in executeCommand');
                 resolve(response);
                 console.log('after resolving in executeCommand' + JSON.stringify(response));

             }
             else {
                 reject(code);
             }
         });
     });
     return commandPromise;
 }

After this I am using Promise.all function to resolve all promises. I am facing the issue while resolving getPromiseC. PromiseC is not going into .then. I am not able to figure out what is the reason.

Output looks like below: For getPromiseA - 1.before resolving in executeCommand 2.after resolving in executeCommand - {data} 3.getPromiseA .then

For getPromiseB - 1.before resolving in executeCommand 2.after resolving in executeCommand - {data} 3.getPromiseB .then

For getPromiseC - 1.before resolving in executeCommand 2.after resolving in executeCommand - {data}

NOT printing 'getPromiseC .then' Control stucks here.But when try executing the function again,it resolves all the promises,i.e, I could see getPromiseC .then is printing in my console.

I am stuck since this issue happens intermittently.And the reload resolves the issue. When I call the function multiple times i see this resolving issue happens randomly.

Can someone help me to learn what is wrong this code?

Vinu
  • 77
  • 2
  • 9
  • Avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! (In `getPromiseA`, `getPromiseB` and `getPromiseC`). That will also take care of the case that you're never calling `resolve` (when `temp` is empty). – Bergi Mar 08 '16 at 18:06
  • @Bergi - Thanks.To remove antipattern,var getPromiseA = Promise.resolve(executeCommand('command A').then(function (result) {var temp = JSON.parse(result);for (var i in temp) {if (temp[i].abc === 'ABC') {Promise.resolve(temp[i]);}}}).catch(function (err) {console.log('Error occurred while executing getPromiseA :::');Promise.reject(err); })); Here I am using Promise.reject to reject the error.is this the right way to do? – Vinu Mar 08 '16 at 18:25
  • No, you need to `return` or `throw` something from the `then` callback. Just creating a promise but doing nothing with it will get it ignored. – Bergi Mar 08 '16 at 18:32

1 Answers1

1

You should avoid the Promise constructor antipattern:

var promiseA = executeCommand('command A').then(function (result) {
    var temp = JSON.parse(result);
    for (var i in temp) {
        if (temp[i].abc === 'ABC') {
            return temp[i];
//          ^^^^^^
        }
    }
}).catch(function (err) {
    console.log('Error occurred while executing getPromiseA :::');
    throw err;
});

var promiseB = executeCommand('command B').then(JSON.parse).catch(function (err) {
    console.log('Error occurred while executing getPromiseB :::');
    throw err;
});

var promiseC = executeCommand('command C').then(function (result) {
    var temp = JSON.parse(result);
    for (var i in temp) {
        console.log('in command C::::');
        if (temp[i].abc != null) {
            return temp[i];
//          ^^^^^^
        }
    }
}).catch(function (err) {
    console.log('Error occurred while executing getPromiseC :::');
    throw err;
});

By that, you will also have eliminated the source of your error: that the promises were never resolved when no appropriate value was found in temp. Your promises now will just resolve with the undefined value that is implicitly returned by the callback functions.

Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Thanks.I am doing exactly what you suggested.I am still facing an intermittent issue here- console goes into then method of promiseB,after that promiseA or promiseC whichever resolves first.But sometimes output is:printing promiseB .then,printing PromiseA .then and PromiseC .then NEVER called.Also another output is printing promiseB .then,printing PromiseC .then and PromiseA .then NEVER called.Note: I am using promise.all after promiseC like this - Promise.all([promiseA, promiseB, promiseC].map(function (promise) { console.log('Resolving allpromises::::');return promise.reflect();})) – Vinu Mar 08 '16 at 19:40
  • If you just don't get the `console.log('in command C::::');` output, that means your loop is not entered (because `temp` has no properties). If the promise never resolves (and also never rejects, as you use `reflect`), this means that `command` never fires a `close` event. Either fix your command, or use `.timeout()`. – Bergi Mar 08 '16 at 19:54
  • My Bad while posting the comment.I moved console.log('in command C::::'); outside for loop and close event returns data and temp has properties too.like this - var promiseC = executeCommand('command C').then(function (result) { console.log('in command C::::'); var temp = JSON.parse(result);for (var i in temp) {... – Vinu Mar 08 '16 at 20:05
  • Also i see that the command fires the close event and give the data. – Vinu Mar 08 '16 at 20:16
  • No,not working yet.You are right about close event not getting called.When I say my output is stuck(not printing what i expected),this is what happens-It resolves any 2 promises(say A,B) then promise C never get called.Or if B,C resolves ,A is not getting resolved.This happens intermittently as well.When I reload it works as expected.Please refer executeCommand(inputCommand) in my initial post,Should I need to change the anti pattern there as well?Or is it some race condition occurs?I am confused. – Vinu Mar 08 '16 at 20:56
  • The use of the `Promise` constructor in `executeCommand` is *not* an antipattern, it's genuinely used to promisify a callback API in there. If `close` is never fired, this is probably `exec`'s fault (or your actual command really is a program that doesn't finish). You might want to ask a new question about what the causes for that might be. – Bergi Mar 08 '16 at 21:02
  • 1
    Thanks Bergi.Appreciate the help.As you suggested finally figured out the issue is due to exec's fault.If I issue multiple exec commands,one process is getting stuck during multiple hits.Hence decided to go for synchronous approach to execute each exec one after another. – Vinu Mar 09 '16 at 17:52