2

This code get's unhandledRejection error and I don't know why.

If the Error is thrown in try/catch, shouldn't it be caught by Catch Expression?

async function main () {
  try {
    await run(throwError)
  } catch (error) {
    console.log('main catch error', error);
  }
}

async function run (callback) {
  return new Promise(async resolve => {
    await throwError()
  });
}

async function throwError () {
  throw new Error('custom error')
}

process.on('unhandledRejection', (reason, promise) => {
  console.log('unhandledRejection - reason', reason, promise);
})

main()
Lxxyx
  • 775
  • 1
  • 7
  • 10
  • Please note, `return new Promise` inside an async function is unneeded. Please remove it? Also there's no need to await `throwError()` - Making those changes seems to also fix your issue – evolutionxbox Aug 19 '22 at 14:39
  • See https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it – T.J. Crowder Aug 19 '22 at 14:43
  • 1
    Does this answer your question? [Try/catch a promise or just catch?](https://stackoverflow.com/questions/70696187/try-catch-a-promise-or-just-catch) – jabaa Aug 19 '22 at 14:43

2 Answers2

1

It's not caught because you're passing an async function into new Promise. An error inside an async function rejects the promise that the function returns. The Promise constructor doesn't do anything a promise returned by the function you pass it (the return value of that function is completely ignored), so the rejection goes unhandled. This is one of the promise anti-patterns: Don't provide a promise to something that won't handle it (like addEventListener on the web, or the Promise constructor, or forEach, ...).

Similarly, there's no reason to use new Promise in your async function at all. async functions already return promises. That's another anti-pattern, sometimes called the explicit promise construction anti-pattern. (But see below if you're wrapping an old-fashioned callback API.)

If you remove the unnecessary new Promise, it works as you expect (I also updated run to call callback rather than ignoring it and calling throwError directly):

async function main() {
    try {
        await run(throwError);
    } catch (error) {
        console.log("main catch error", error);
    }
}

async function run(callback) {
    return await callback();
}

async function throwError() {
    throw new Error("custom error");
}

process.on("unhandledRejection", (reason, promise) => {
    console.log("unhandledRejection - reason", reason, promise);
});

main();

About the return await callback(); in that example: Because whatever you return from an async function is used to settle the function's promise (fulfilling it if you return a non-thenable, resolving it to the thenable if you return one), and that return is at the top level of the function (not inside a try/catch or similar), you could just write return callback();. In fact, you could even remove async from run and do that. But keeping the await makes the async stack trace clearer on some JavaScript engines, more clearly indicates what you're doing, and keeps working correctly even if someone comes along later and puts a try/catch around that code.


In a comment, you said that you used new Promise because you were wrapping around a callback-based API. Here's how you'd do that (see also the answers here):

// Assuming `callback` is a Node.js-style callback API, not an
// `async` function as in the question:
function run(callback) {
    return new Promise((resolve, reject) => {
        callback((err, data) => {
            if (err) {
                reject(err);
            } else {
                resolve(data);
            }
        });
    });
}

but, you don't have to do that yourself if the callback-based API uses the standard Node.js convention as above, there's a promisify function in utils.

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • This manual `new Promise()` call could have been done just for the sake of a generalized MRE. – Nathan Wiles Aug 19 '22 at 14:52
  • @NathanWiles not necessary in *any* case. It's an `async` function. – Pointy Aug 19 '22 at 14:54
  • It is necessary, because removing the `new Promise()` call does not reproduce the error. Keeping it in place was the minimum reproducible example of the issue this person was experiencing. – Nathan Wiles Aug 19 '22 at 14:55
  • @NathanWiles - Maybe, though it's very misleading (and it's really common for folks not to understand `async` functions). The main point about the above, though, is: Don't pass an `async` function to `new Promise`, it doesn't do anything with the returned promise. – T.J. Crowder Aug 19 '22 at 14:55
  • @NathanWiles - You're making an awfully large assumption there, and frankly the OP's `run` doesn't make sense even through that lens. The OP **already** has a promise (just like [the link calls out](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it)): The one from `throwError()`. There's no need whatsoever for `new Promise` in that code. – T.J. Crowder Aug 19 '22 at 14:55
  • Thanks, this answer solved my problem. As for why I use async/await in new Promise, it's because in the actual code, the function entry only supports callback style and not Promise, so I need to run the Promise in callback and call the actual function via async/await, get the result and return – Lxxyx Aug 19 '22 at 15:02
  • @Lxxyx - In that situation, the function you'd pass to `new Promise` shouldn't be an `async` function (it should never be one); if it weren't `async`, you would have seen the error the way you expected to (because an error thrown inside the `new Promise` callback rejects the promise). And if the *only* thing the function was doing was wrapping that API, I'd suggest `run` itself probably shouldn't be `async` since you're explicitly creating a promise as in [the answers here](https://stackoverflow.com/questions/22519784/) about wrapping callback-style APIs in promises. Happy coding! :-) – T.J. Crowder Aug 19 '22 at 15:06
0

You don't want to throw an error in this case, you want to invoke reject:

  return new Promise((resolve, reject) => {
    reject('custom error')
  });

If the error being thrown is out of your control, you can catch it inside the promise implementation and reject in that case.

Nathan Wiles
  • 841
  • 10
  • 30