When is .then(success, fail) considered an antipattern for promises?

Question

I had a look at the bluebird promise FAQ, in which it mentions that .then(success, fail) is an antipattern. I don't quite understand its explanation as for the try and catch. What's wrong with the following?

some_promise_call()
.then(function(res) { logger.log(res) }, function(err) { logger.log(err) })

It seems that the example is suggesting the following to be the correct way.

some_promise_call()
.then(function(res) { logger.log(res) })
.catch(function(err) { logger.log(err) })

What's the difference?

Franctireur

then().catch() is more readable, as you don’t need to looking for comma and investigate is this callback for success or fail branch. – Indue

@KrzysztofSafjanowski - devastated by 'looks better' argument. Totally wrong! – Wheelock

@AndreyPopov where you see „looks better”?. Please read next answer and what is more readable .then(function(res) { logger.log(res) }, function(err) { logger.log(err) }) or .then(function(res) { logger.log(res) }).catch( function(err) { logger.log(err) }) – Indue

NOTE: When you are using .catch, you do not know which step caused the problem - inside the last then or somewhere else up the promise chain. So it does have its own disadvantage. – Monomial

I always add function names to the promise .then() params to make it readable i.e. some_promise_call() .then(function fulfilled(res) { logger.log(res) }, function rejected(err) { logger.log(err) }) – Diaphysis

Answer 1

What's the difference?

The .then() call will return a promise that will be rejected in case the callback throws an error. This means, when your success logger fails, the error would be passed to the following .catch() callback, but not to the fail callback that goes alongside success.

Here's a control flow diagram:

To express it in synchronous code:

// some_promise_call().then(logger.log, logger.log)
then: {
    try {
        var results = some_call();
    } catch(e) {
        logger.log(e);
        break then;
    } // else
        logger.log(results);
}

The second log (which is like the first argument to .then()) will only be executed in the case that no exception happened. The labelled block and the break statement feel a bit odd, this is actually what python has try-except-else for (recommended reading!).

// some_promise_call().then(logger.log).catch(logger.log)
try {
    var results = some_call();
    logger.log(results);
} catch(e) {
    logger.log(e);
}

The catch logger will also handle exceptions from the success logger call.

So much for the difference.

I don't quite understand its explanation as for the try and catch

The argument is that usually, you want to catch errors in every step of the processing and that you shouldn't use it in chains. The expectation is that you only have one final handler which handles all errors - while, when you use the "antipattern", errors in some of the then-callbacks are not handled.

However, this pattern is actually very useful: When you want to handle errors that happened in exactly this step, and you want to do something entirely different when no error happened - i.e. when the error is unrecoverable. Be aware that this is branching your control flow. Of course, this is sometimes desired.

---

What's wrong with the following?

some_promise_call()
.then(function(res) { logger.log(res) }, function(err) { logger.log(err) })

That you had to repeat your callback. You rather want

some_promise_call()
   .catch(function(e) {
       return e; // it's OK, we'll just log it
   })
   .done(function(res) {
       logger.log(res);
   });

You also might consider using .finally() for this.

Muddlehead

this is the most helpful explanation I've read in a few days (and I've read a lot). I can't explain how grateful I am! :) I think you should stress out more the difference between the two, that .catch will catch errors even inside the success function.. Personally, I find this extremely wrong as you end up with one error-entry point, which can get multiple errors from multiple actions, but this is my problem. Anyways - thanks for the info! Don't you have some online communication tool you are willing to share so I can ask few things more? :P – Wheelock

@AndreyPopov You can come to chat if you want – Muddlehead

I hope this is giving you some more upvotes here. Definitely one of the best explanations of an important Promise mechanic on this site. – Lichfield

.done() isn't part of the standard, is it? At least MDN doesn't list that method. It would be helpful. – Algebraist

@Algebraist Indeed. done is a Bluebird thing that was basically deprecated by then+unhandled-rejection detection. – Muddlehead

just a note from a colorblind: the diagrams make no sense :) – Hormonal

Answer 2

The two aren't quite identical. The difference is that the first example won't catch an exception that's thrown in your success handler. So if your method should only ever return resolved promises, as is often the case, you need a trailing catch handler (or yet another then with an empty success parameter). Sure, it may be that your then handler doesn't do anything that might potentially fail, in which case using one 2-parameter then could be fine.

But I believe the point of the text you linked to is that then is mostly useful versus callbacks in its ability to chain a bunch of asynchronous steps, and when you actually do this, the 2-parameter form of then subtly doesn't behave quite as expected, for the above reason. It's particularly counterintuitive when used mid-chain.

As someone who's done a lot of complex async stuff and bumped into corners like this more than I care to admit, I really recommend avoiding this anti-pattern and going with the separate handler approach.

Megasporangium

Answer 3

By looking at advantages and disadvantages of both we can make a calculated guess as to which is appropriate for the situation. These are the two main approaches to implementing promises. Both have it's pluses and minus

Catch Approach

some_promise_call()
.then(function(res) { logger.log(res) })
.catch(function(err) { logger.log(err) })

Advantages

1. All errors are handled by one catch block.
2. Even catches any exception in the then block.
3. Chaining of multiple success callbacks

Disadvantages

1. In case of chaining it becomes difficult to show different error messages.

Success/Error Approach

some_promise_call()
.then(function success(res) { logger.log(res) },
      function error(err) { logger.log(err) })

Advantages

1. You get fine grained error control.
2. You can have common error handling function for various categories of errors like db error, 500 error etc.

Disavantages

1. You will still need another catch if you wish to handler errors thrown by the success callback

Artamas

For someone who needs to debug production issues using just a log file I prefer the Success/Error Approach as it gives the ability to create a causal error chain that can be logged at the exit boundaries of you app. – Diaphysis

question. lets say I make an async call that does one of a few things: 1) returns successfully (2xx statuscode), 2) returns unsuccessfully (4xx or 5xx code) but not rejected per se, 3) or doesn't return at all (internet connection is down). For case #1, the success callback in the .then is hit. For case #2, the error callback in the .then is hit. For case #3, the .catch is called. This is correct analysis, right? Case #2 is most tricky bc technically a 4xx or 5xx isn't a rejection, it still successfully returns. Thus, we need to handle it within the .then. ....Is my understanding correct? – Anora

"For case #2, the error callback in the .then is hit. For case #3, the .catch is called. This is correct analysis, right?" - That's how fetch works – Artamas

Answer 4

Simple explain:

In ES2018

When the catch method is called with argument onRejected, the following steps are taken:

1. Let promise be the this value.
2. Return ? Invoke(promise, "then", « undefined, onRejected »).

that means:

promise.then(f1).catch(f2)

equals

promise.then(f1).then(undefiend, f2)

Cornice

Answer 5

Using .then().catch() lets you enable Promise Chaining which is required to fulfil a workflow. You may need to read some information from database then you want to pass it to an async API then you want to manipulate the response. You may want to push the response back into the database. Handling all these workflows with your concept is doable but very hard to manage. The better solution will be then().then().then().then().catch() which receives all errors in just once catch and lets you keep the maintainability of the code.

Donothingism

Answer 6

Using then() and catch() helps chain success and failure handler on the promise.catch() works on promise returned by then(). It handles,

1. If promise was rejected. See #3 in the picture
2. If error occurred in success handler of then(), between line numbers 4 to 7 below. See #2.a in the picture (Failure callback on then() does not handle this.)
3. If error occurred in failure handler of then(), line number 8 below. See #3.b in the picture.

1. let promiseRef: Promise = this. aTimetakingTask (false); 2. promiseRef 3. .then( 4. (result) => { 5. /* successfully, resolved promise. 6. Work on data here */ 7. }, 8. (error) => console.log(error) 9. ) 10. .catch( (e) => { 11. /* successfully, resolved promise. 12. Work on data here */ 13. });

Note: Many times, failure handler might not be defined if catch() is written already. EDIT: reject() result in invoking catch() only if the error handler in then() is not defined. Notice #3 in the picture to the catch(). It is invoked when handler in line# 8 and 9 are not defined.

It makes sense because promise returned by then() does not have an error if a callback is taking care of it.

Few

The arrow from the number 3 to the catch callback seems wrong. – Muddlehead

Thanks! With an error callback defined in then(), it is not invoked (line #8 and #9 in code snippet). #3 invokes one of the two arrows. It makes sense because promise returned by then() does not have an error if a callback is taking care of it. Edited the answer! – Few

Answer 7

Instead of words, good example. Following code (if first promise resolved):

Promise.resolve()
.then
(
  () => { throw new Error('Error occurs'); },
  err => console.log('This error is caught:', err)
);

is identical to:

Promise.resolve()
.catch
(
  err => console.log('This error is caught:', err)
)
.then
(
  () => { throw new Error('Error occurs'); }
)

But with rejected first promise, this is not identical:

Promise.reject()
.then
(
  () => { throw new Error('Error occurs'); },
  err => console.log('This error is caught:', err)
);

Promise.reject()
.catch
(
  err => console.log('This error is caught:', err)
)
.then
(
  () => { throw new Error('Error occurs'); }
)

Mak

This doesn't make sense, can you please remove this answer? It's misleading and distracting from the correct answer. – Baiel

@AndyRay, this doesn't make sense in real application, but it makes sense to understand the work of the promises. – Mak

I think this code does need some words so we can understand what this is trying to tell us. How are they identical and how are they not identical? – Airdrop