Making calls to functions that return promises cleaner

Question

I have two called functions that return promises and I would like to make it cleaner by possibly having on catch statement to catch the errors.

I think I am doing it the right way although the code does call the functions in the correct order asynchronously.

These are my calls:

// The text is hopefully the email address and the pin number
fb.verifyEmailPin(text).then(function(reply){

    // Set the new state to 'get_city'
    fb.setState(FB_ID).then(function(result){  


    }).catch(function(v) {
      // Rejection
        // If there was an error then prompt the user to enter again
    }); // setState

}).catch(function(err){

});// verifyEmailPin

And this the actual function - for setState, I haven't yet written the code for the verifyEmailPin function but it follows the same structure as the set state in terms of passing back resolve or reject.

/*
* Function : setState
* Purpose  : Set the state to what every is send in on the parameter
*/
exports.setState = function(fbid,newstate){

  var success = 'Y';
  return new Promise((resolve, reject) => {

    client.hmset(fbid, { 'state': newstate });

    // Check that we have set it ok
    client.hmget(fbid,'state',function(err,reply){

      if (err || reply != newstate) {
           return reject(err);
      }

        return resolve(success);

    });


  }).catch(function(v) {

  });

 }

*"the code does call the functions in the correct order asynchronously."* - So what is the problem? You don't seem to have asked a question. — nnnnnn, Aug 11 '16 at 20:38

hsan · Accepted Answer · 2016-08-12T19:24:03.930

2

You can use only one .catch at the end. And to have less indentation you could chain the .then. If you do something asynchronous inside on of then then-functions make sure to return a promise otherwise the following thens will not wait for its completion. On synchronous operations (e.g. somePromise.then(JSON.parse).then(...)) no promise is needed.

Here's a short example:

function promiseTest(x) {
    Promise.resolve(x).then(function(a) { // instead of Promise.resolve do something asynchronous, e.g. an ajax call that returns a promise
        if (typeof x != "number") throw "NaN";
        return a*2;
    }).then(function(a) {
        console.log(a);
    }).catch(function(err) {
        console.error("error in promise:", err);
    })
}
promiseTest(1); //logs 2 to the console
promiseTest("a"); // shows error message in the console

If you want to run several asynchronous operations in parallel and wait for all of them to finish you can use Promise.all by supplying it with an array of promises.

Promise.all([doSomethingAsyncAndReturnPromise(), somethingElseAsync()]).then(function results) {
    // results[0] contains the result from doSomethingAsyncAndReturnPromise
    // results[1] contains the result from somethingElseAsync
});

edited Aug 12 '16 at 19:24

answered Aug 11 '16 at 20:41

hsan

1,560
1
9
12

Chaining like this is really the biggest benefit I feel you get out of promises. Callbacks are consolidated into a single, synchronous control flow and you can do all of your error handling in one place if you like. – Squirrel Aug 11 '16 at 20:51
I think it would be better if you chained `promiseTest(1)` and `promiseTest("a")` as well, rather than have them run in parallel like this. – jib Aug 12 '16 at 03:34
@jib That would not work since `promiseTest` does not return a promise. Also, there is nothing wrong with having multiple promises in parallel (`Promise.all` is really neat) especially if they in no way depend on one another and don't interfere with stuff going on in the other promises. And since this is really just an open-brower-console-and-test kind of example you can just call the function manually as many times as you want to experiment with the behavior... – hsan Aug 12 '16 at 16:19
It's a pity the answer leaves out addressing one of the common pitfalls which is to forget to return promises. If you want it in parallel, you should actually use `Promise.all` IMHO, otherwise errors often don't get propagated properly (yes you catch them in parallel in this particular instance, but that overlooks the general problem). – jib Aug 12 '16 at 18:54
@jib Edited the answer to include returning promises and `Promise.all`. Do you think that covers it? I disagree however about always using `.all` for parallel promises. If there is no follow-up to all promises being resolved there is no need to handle that case and errors should then be handled directly by the promise/chain responsible for it. Happy to change my opinion on this, I'm just not convinced at the moment. – hsan Aug 12 '16 at 19:20
@hsan Thanks, actually I was just hoping you'd add a `return` before your first `Promise.resolve`, i.e. `function promiseTest(x) { return Promise.resolve(x).then(...`. That way, if someone wants to run them in sequence then they can do so with `promiseTest(1).then(() => promiseTest("a"))` and it'll work as expected (not run in parallel like now). [Return all promises](http://stackoverflow.com/a/37084467/918910) :) – jib Aug 12 '16 at 21:02

Making calls to functions that return promises cleaner

1 Answers1