Promises: is .done() executed always even if .catch() is?

charliebrownie picture charliebrownie · Nov 26, 2015 · Viewed 8.8k times · Source

My Promise issue

I am new to Promises and I've been reading the Q Documentation, where it says:

When you get to the end of a chain of promises, you should either return the last promise or end the chain.

I have defined a Promise in my code the Q.Promise way, with the following console.logs to log out an execution trace:

function foo(){
   return Q.Promise(function(resolve, reject) {

    doSomething()
    .then(function() {
      console.log('1');
      return doSomething1();
    })
    .then(function() {
      console.log('2');
      return doSomething2();
    })
    .then(function() {
      console.log('3');
      return doSomething3();
    })
    .catch(function(err) {
      console.log('catch!!');
      reject(err);
    })
    .done(function() {
      console.log('done!!');
      resolve();
    });

  });
}

In case every doSomethingN() executes correctly, everything works as intended and I get the expected trace:

1
2
3
done!!

But in case any of the doSomethingN() fails:

foo() works correctly, because the error function callback is the one that runs whenever a reject(err) occurs:

foo().then(function() { /* */ }, function(err) { /* this runs! */ });

And I get the following trace (ie. when doSomething1() fails):

1
catch!!
done!!

My question

What I thought at first was the following:

Okay, let's handle the chaining success and failure in both: .done() and .catch() methods. If everything goes well .done()'s callback will be executed and the promise will be resolved. In case there's an error at any point, .catch()'s callback will be executed and the promise will be rejected - and because of that, done() won't be executed.

I think I am missing something about how the .done() works... because by having a look at my logging trace, I realized that .done() seems to be executing always - whether there is an error and .catch() is executed or not - and that is what I wasn't expecting.

So, after that, I removed .done()'s callback and now foo():

  • works if there's an error during the chain execution
  • does not work if everything works correctly

What should I reconsider and how could/should I make it work?

Answer

Mariusz Nowak picture Mariusz Nowak · Nov 27, 2015

catch(cb) is just an alias for then(null, cb), and you've actually fixed an error in catch, so flow naturally turned to success result in done.

If you want to just decorate the error in catch, you should rethrow the error afterwards, e.g. proper passthru may look as:

catch(function (err) {
   console.log(err);
   throw err;
});

Still your example doesn't make much sense. You should never use done, when you return a promise. If you want to resolve initialized promise with internally created chain of promises, you should just resolve it as:

resolve(doSomething()
  .then(function() {
    console.log('1');
    return doSomething1();
  })
  ....
  .then(function() {
    console.log('N');
    return doSomethingN();
  }));

There's no need for internal error handling, leave that to consumer of promise which you return.

And other point. If when creating new promise you know it will be resolved with other one, then there's no logical reason to create such promise, just reuse one you planned to resolve with. Such error was also coined as deferred anti-pattern