Node: async api calls and throwing sync errors

[ronag]

I've noticed this in a few places and I'm not sure if it's in purpose or nobody has though of it.

We have a few places where we provide an async api e.g.foo(options, callback) but we still throw synchronous errors (especially when it comes to INVALID_ARG_TYPE).

So basically in order to safely call such a function I need to do:

try {
 foo({ a: 1 }, err => console.error(err));
} catch (err) {
  console.error(err);
}

which is a bit unfortunate and error prone as I believe most users would not think of the try/catch and just do:

 foo({ a: 1 }, err => console.error(err));

Of course there is no way around this if e.g. the type of callback is not a function (ERR_INVALID_CALLBACK). But otherwise in my opinion we should try to propagate all errors in an async api call through the callback. If not, what is the "rule"? e.g. INVALID_ARG_TYPE errors can be thrown synchronously but nothing else. So we can document it and help users with expectations.

This is not a problem with async/await and Promises since it gets implicitly normalised.

[targos]

I think the general rule is to throw sync errors for programming errors and async errors for runtime errors.

I do not agree that you need to wrap the call in a try/catch to safely call those functions. Your program is not supposed to pass invalid arguments in the first place.
What can you do anyway if the function throws because of an invalid arg type?

[Flarna]

I agree with @targos, errors should be indicated as soon as possible as this eases debugging.

Callbacks are sometimes optional. Users may also forget some parameter and pass a callback at wrong place which can't be indicated async. So even if you could move some errors from sync to async there are always a few leftovers.

[ronag]

Makes sense to me. Thanks for the feedback!