6

I am refactoring my promise chaining codes to async/await style. One of the reasons to do that is I want a single catch block to handle to all error situations (as explained here Understanding promise rejection in node.js)

My question is when I hit a sync error, should I call await Promise.reject or throw error to bail out the process? I know either way will work but I prefer throw error. I already know I got an invalid result why should I await? Using throw to terminate the control flow immediately seems to be a better option.

I am not talking about the promise chain(the whole point of my question), so I don't think the thread JavaScript Promises - reject vs. throw answered my question.

I read the article Error Handling in Node.js I don't think it gives an answer either. But it did say

A given function should deliver operational errors either synchronously (with throw) or asynchronously (with a callback or event emitter), but not both. ... In general, using throw and expecting a caller to use try/catch is pretty rare...

My async function(s) may return Promise.reject. So I am concerned about introducing 2 ways to delivering errors as that article against.

try {
   let result = await aysncFunc().
   if (!isResultValid(result)) { //isResultValid() is sync function 
      await Promise.reject('invalid result')
      //or throw 'invalid result'
   }
   ... //further processing
}
catch (error) {
  ...
}
Qiulang
  • 10,295
  • 11
  • 80
  • 129
  • 1
    `I know either way will work but I prefer throw error` - well, go forth and do that - but are you sure either way will work? – Jaromanda X May 29 '18 at 02:44
  • "but are you sure either way will work?" I am 99% sure. But that is also one of the reasons I asked my question. – Qiulang May 29 '18 at 03:01
  • Of course it is 100% works (as in your code block). I prefer to throw error directly +1, anything you throw in the sync code could be caught. It just more elegant and nature. `await Promise.reject()` is redundant, and potentially could cause confusions for some people. – Leo Li May 29 '18 at 03:15
  • But I think you may hit to a bad direction. Don't use `async/await` just trying to be fashionable. Use it when it is really needed; i.e. async operation, and also use `await Promise.all([...])` if you have two or more independent async operations. – Leo Li May 29 '18 at 03:19
  • Also, another reason for it is performance, but it just marginal in most cases. – Leo Li May 29 '18 at 03:28

1 Answers1

5

It's semantically correct to use throw in promise control flow, this is generally preferable way to bail out of promise chain.

Depending on coding style, await Promise.reject(...) may be used to differentiate between real errors and expected rejections. Rejected promise with string reason is valid but throw 'invalid result' is considered style problem that may be addressed with linter rules, because it's conventional to use Error instances as exceptions.

The reason why it's important is because string exceptions can't be detected with instanceof Error and don't have message property, consistent error logging as console.warn(error.message) will result in obscure undefined entries.

// ok
class Success extends Error {}
try {
  throw new Success('just a friendly notification');
} catch (err) {
  if (!(err instanceof Success)) {
    console.warn(err.message);
    throw err;
  }
}

// more or less
const SUCCESS = 'just a friendly notification';
try {
  await Promise.reject(SUCCESS);
} catch (err) {
  if (err !== SUCCESS)) {
    console.warn(err.message);
    throw err;
  }
}

// not ok
try {
  throw 'exception';
} catch (err) {
  if (typeof err === 'string') {
    console.warn(err);
  } else {
    console.warn(err.message);
  }

  throw err;
}

Since invalid result is actually an error, it's reasonable to make it one:

  throw new TypeError('invalid result');

I am not talking about the promise chain(the whole point of my question), so I don't think the thread JavaScript Promises - reject vs. throw answered my question.

async function is syntactic sugar for promise chain, so all points that are applicable to promises are applicable to async as well.

There may be cases when throwing an error is not the same as rejecting a promise, but they are specific to other promise implementations like AngularJS $q and don't affect ES6 promises. Synchronous error in Promise constructor results in exception, this also isn't applicable to async.

Estus Flask
  • 206,104
  • 70
  • 425
  • 565
  • I added the words "I am not talking about the promise chain ..." just to emphasize that I knew that discussion (in case someone will say it is a duplicated question). Also, I am more concerned about the pros and cons of using one over the other – Qiulang May 29 '18 at 06:38
  • 1
    It's a matter of style, and `throw` is more conventional. I hope I covered pros and cons. It's *It's semantically correct to use throw in promise control flow* vs *Rejected promise with string reason is valid but throw 'invalid result' is considered style problem*. – Estus Flask May 29 '18 at 06:51
  • BTW, the article Error Handling in Node.js did say "A given function should deliver operational errors either synchronously (with throw) or asynchronously (with a callback or event emitter), but not both. " – Qiulang May 29 '18 at 07:07
  • 1
    Since `async` introduces sync-like control flow, it's the first part of the sentence that is applicable. – Estus Flask May 29 '18 at 07:46
  • Does really people `throw` a plain string...?! Come on, it's a really a bad practice. And as I said, for the pros and cons, IMO, there is no pros at all for using `await Promise.reject()` if your know you have as `sync` operation. It just like you have a function, and wrap with another function which just return the function. PS. `Promise.reject()` should also be passed with an `error` object, not a string, though you can do it. – Leo Li May 29 '18 at 22:36
  • @LeoLi They generally don't. But since `async` function is just syntactic sugar, and primitives are commonly used as rejection reasons, that's what we have. `await Promise.reject()` is a syntactic sugar for `.then(() => Promise.reject(...))` which is acceptable way to designate that a rejection isn't really an exception; basically a matter of style. Any way, I personally consider `throw new Success(...)` a cleaner way to achieve same goal. – Estus Flask May 29 '18 at 22:44
  • @estus no, it actually not equivalent. Because you can't not just monkey patching the above code to the same place. And I generally don't like the saying **async/await is a syntactic sugar for promise**, it actually is a generator sugar which return a `Promise`. (think about the `co` module, that's how `async(-> *)/await(-> yield)` implemented) – Leo Li May 30 '18 at 07:58
  • And I have to say a bad/redundant code is not a 'style' at all. think about you have a function named `const hello = () => console.log('hello')` and you have another function as `const sayHello = () => hello()`. That is how stupid `await Promise.reject('some_sync_operation')` was. At the very bottom (in the implementation of `async/await` sugar generator), it still throw the error, which why `catch` can catch it. – Leo Li May 30 '18 at 08:05
  • Any async..await code has a `Promise` counterpart, that's why it's syntactic sugar. `co` and similar attempts were precursors for async. *At the very bottom (in the implementation of async/await sugar generator), it still throw the error, which why catch can catch it* - it could be caught from the outside with `.catch()`. Because it's rejected promise. Not an exception. It's a good thing that you're opinionated towards good practices, but this is primarily a matter of style and there can be more than one opinion on that. – Estus Flask May 30 '18 at 08:18
  • Redundant code is acceptable if it serves the purpose, i.e self-documenting code. A lot of proper code could be written in more obscure but concise way. – Estus Flask May 30 '18 at 08:20