Use promise then() instead of catch() with callbacks

[feross]

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[x] Bug fix
[ ] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)

Addresses my review here: #1419 (review)

Is there anything you'd like reviewers to focus on?

[Borewit]

Although I think the bigger improvements for readability are

• inline variables string=${x} instead of concatenations like "string=" + x
• getting rid of redundant return
• and function (x) to x =>

[feross]

@Borewit These changes weren't about readability but about correctness. The .catch will catch more than just the promise rejections from the tasks array. It will also catch any exceptions thrown from the handler in .then() which means that if the cb function throws an exception, then we'll catch it here. Instead we should use .then(success, fail) for correct behavior.

[Borewit]

I honestly don't see the difference Ferros.
I think in either cases it will catch all.

The catch() method returns a Promise and deals with rejected cases only. It behaves the same as calling Promise.prototype.then(undefined, onRejected)
source: developer.mozilla.org

The catch method on a promise is pretty simple because it is an alias for then(null, errorCallback). source: codingame.com

[feross]

@Borewit There's a difference. See https://stackoverflow.com/a/33278420/292185 Let me know if it still doesn't make sense and I'll try to explain further.

[Borewit]

Ah yes, you are right Feross! I completely missed the point.

catch(errorCallback)` is an alias

for

then(null, errorCallback)

where the alias is appended and not instead of the error handler of the previous then (should say the promise it is calling).

[codealchemist]

This has an interesting behavior:

const p1 = new Promise((resolve, reject) => {
  throw(new Error('Promise 1 THROWN'))
})

const p2 = new Promise((resolve, reject) => {
  setTimeout(() => {
    throw(new Error('Promise 2 THROWN'))
  })
})

p1
  .then(null, message => console.log('P1 failed:', message))
  .catch(message => console.log('P1 catched:', message))

p2
  .then(null, message => console.log('P2 failed:', message))
  .catch(message => console.log('P2 catched:', message))

You can run it here.
If the code fails synchronously, the failure callback of then will be called.
If there's no failure callback it will naturally flow thru catch.
If it fails asynchronously, it won't be caught by neither the failure callback nor the catch.