16

Updated, I've now tried explaining the behavior I'm seeing, but it'd still be great to have an answer from a credible source about the unhandledRejection behavor. I've also started a discussion thread on Reddit.


Why do I get an unhandledRejection event (for "error f1") in the following code? That's unexpected, because I handle both rejections in the finally section of main.

I'm seeing the same behavior in Node (v14.13.1) and Chrome (v86.0.4240.75):

window.addEventListener("unhandledrejection", event => {
  console.warn(`unhandledRejection: ${event.reason.message}`);
});

function delay(ms) {
  return new Promise(r => setTimeout(r, ms));
}

async function f1() {
  await delay(100);
  throw new Error("error f1");
}

async function f2() {
  await delay(200);
  throw new Error("error f2");
}

async function main() {
  // start all at once
  const [p1, p2] = [f1(), f2()];
  try {
    await p2;
    // do something after p2 is settled
    await p1;
    // do something after p1 is settled
  }
  finally {
    await p1.catch(e => console.warn(`caught on p1: ${e.message}`));
    await p2.catch(e => console.warn(`caught on p2: ${e.message}`));
  }
}

main().catch(e => console.warn(`caught on main: ${e.message}`));
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • The `finally` clause doesn't do the same thing as the `catch` clause of a `try`. – Pointy Oct 13 '20 at 13:32
  • That is what happens when you `throw` from inside a promise that does an async task, if you replace `new Promise(r => setTimeout(r, ms))` with `new Promise(r => r())` there will be no unhandled rejections. – Titus Oct 13 '20 at 13:36
  • @Pointy, the original problem of why `unhandledRejection` gets fired doesn't have to do anything with whether I use `catch` or `finally` here. I've now explained [the reason for it](https://stackoverflow.com/a/64344718/1768303). – noseratio Oct 14 '20 at 01:41

3 Answers3

14

Ok, answering to myself. I misunderstood how unhandledrejection event actually works.

I'm coming from .NET where a failed Task object can remain unobserved until it gets garbage-collected. Only then UnobservedTaskException will be fired, if the task is still unobserved.

Things are different for JavaScript promises. A rejected Promise that does not have a rejection handler already attached (via then, catch, await or Promise.all/race/allSettle/any), needs one as early as possible, otherwise unhandledrejection event may be fired.

When unhandledrejection will be fired exactly, if ever? This seems to be really implementation-specific. The W3C specs on "Unhandled promise rejections" do not strictly specify when the user agent is to notify about rejected promises.

To stay safe, I'd attach the handler synchronously, before the current function relinquishes the execution control to the caller (by something like return, throw, await, yield).

For example, the following doesn't fire unhandledrejection, because the await continuation handler is attached to p1 synchronously, right after the p1 promise gets created in already rejected state. That makes sense:

window.addEventListener("unhandledrejection", event => {
  console.warn(`unhandledRejection: ${event.reason.message}`);
});

async function main() {
  const p1 = Promise.reject(new Error("Rejected!")); 
  await p1;
}

main().catch(e => console.warn(`caught on main: ${e.message}`));

The following still does not fire unhandledrejection, even though we attach the await handler to p1 asynchronously. I could only speculate, this might be happening because the continuation for the resolved promised is posted as a microtask:

window.addEventListener("unhandledrejection", event => {
  console.warn(`unhandledRejection: ${event.reason.message}`);
});

async function main() {
  const p1 = Promise.reject(new Error("Rejected!")); 
  await Promise.resolve(r => queueMicrotask(r));
  // or we could just do: await Promise.resolve();
  await p1;
}

main().catch(e => console.warn(`caught on main: ${e.message}`));

Node.js (v14.14.0 at the time of posting this) is consistent with the browser behavior.

Now, the following does fire the unhandledrejection event. Again, I could speculate that's because the await continuation handler is now attached to p1 asynchronously and on some later iterations of the event loop, when the task (macrotask) queue is processed:

window.addEventListener("unhandledrejection", event => {
  console.warn(`unhandledRejection: ${event.reason.message}`);
});

async function main() {
  const p1 = Promise.reject(new Error("Rejected!")); 
  await new Promise(r => setTimeout(r, 0));
  await p1;
}

main().catch(e => console.warn(`caught on main: ${e.message}`));

I personally find this whole behavior confusing. I like the .NET approach to observing Task results better. I can think of many cases when I'd really want to keep a reference to a promise and then await it and catch any errors on a later timeline to that of its resolution or rejection.

That said, there is an easy way to get the desired behavior for this example without causing unhandledrejection event:

window.addEventListener("unhandledrejection", event => {
  console.warn(`unhandledRejection: ${event.reason.message}`);
});

async function main() {
  const p1 = Promise.reject(new Error("Rejected!"));
  p1.catch(console.debug); // observe but ignore the error here
  try {
    await new Promise(r => setTimeout(r, 0));
  }
  finally {
    await p1; // throw the error here
  }
}

main().catch(e => console.warn(`caught on main: ${e.message}`));
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • *"...will fire unhandledrejection as soon as it gets rejected"* and *"...on the next event loop tick..."* are two very different things. Separately, even the assertion it'll trigger an unhandled rejection on the next tick is an oversimplification. The thread returning to having nothing but "platform code" on the stack (e.g., between tasks) is part of it, but JavaScript engines have used various approaches to the unhanded rejection issue. I don't think it's as simple as "the next event loop tick" in all cases, and note that this has evolved over time. – T.J. Crowder Oct 14 '20 at 07:00
  • @T.J.Crowder, I totally agree it may not be as simple as "the next tick". I did not want to bring the whole concept of [Tasks, microtasks, queues and schedules](https://jakearchibald.com/2015/tasks-microtasks-queues-and-schedules/) and I have no inside knowledge about how `unhandledrejection` gets scheduled. But suffice to say, it still gets fired before the `await` continuation here: `const p1 = f1(); await delay(0); await p1;` – noseratio Oct 14 '20 at 08:28
  • Right, but as you seem to know, there are a couple of assertions in the answer above that are incorrect. For instance, *"In a nutshell, the following fires unhandledrejection because by the time I do await p1, the p1 promise has already been rejected:"* is **definitely** incorrect. A promise can be rejected prior to the rejection handler being attached, and still not have an unhandled rejection. I'd worry about this answer misleading future readers. – T.J. Crowder Oct 14 '20 at 09:23
  • 2
    So far, I've established this rule for myself: if I don't want to see an `unhandledRejection` event for promise `p`, I should attach a rejection handler to `p` (by either `p.catch()`, `p.then()`, `await p` or `Promise.all/race/allSettle/any`) *before* the function being currently executed relinquishes the execution control to the caller (by either `return`, `throw`, `await` or `yield`). – noseratio Oct 14 '20 at 09:52
  • 1
    Example: `Promise.reject()` returns a promise that is *already* rejected, but `Promise.reject().catch(x => console.error(x));` doesn't result in an unhandled rejection error. I haven't posted an answer because it's a complex topic and I don't have time today to go into it properly. But I thought (with apologies!) it was important to flag up issues in this answer. :-) – T.J. Crowder Oct 14 '20 at 09:54
  • @T.J.Crowder, I agree this might be implementation specific, will amend it, need to dig into specs. I believe it's still safe to follow [my rule](https://stackoverflow.com/questions/64336083/unexpected-unhandledrejection-event-for-promise-which-rejection-does-get-handled/64344718?noredirect=1#comment113791375_64344718) to attach a handler synchronously. That said, both of your fiddles (aren't they the same?) do fire `unhandledRejection` for me — as expected — in both Chrome and Firefox, because a timer callback is a macrotask: https://codepen.io/noseratio/pen/zYBvLJO. Am I missing something? – noseratio Oct 20 '20 at 06:43
  • 1
    They don't fire an unhandled rejection for me using Brave (built on Chromium 86.0.4240.99, which would be V8 v8.6). I don't think you'll find the issuing of unhandled rejections in a specification. It's left to the implementations. Again, why not just "or very soon thereafter"? Since you acknowledge this is implementation-specific, why try to provide detail that may be incorrect? (Side note: A couple of years ago, there was at least one engine that did use GC for this. They had to change it but I don't recall why...) – T.J. Crowder Oct 20 '20 at 07:00
  • 1
    (This is weird: I do get the unhandled rejection in Chrome v86 -- which should *also* have V8 v8.6 in it. Huh.) – T.J. Crowder Oct 20 '20 at 07:02
  • @T.J.Crowder, one thing for sure, it's very impl specific. E.g., for Node only synchronous handlers prevent `unhandledRejection`, but microtask ones: https://runkit.com/noseratio/unhandledrejection. I'm not quite comfy with "or very soon thereafter" because it may leave space for race conditions. E.g., with Node they will soon start tearing down the process if there are unhandled rejections. – noseratio Oct 20 '20 at 07:40
  • 1
    No, it doesn't have to be synchronous in Node: `const p1 = Promise.reject("A"); Promise.resolve().then(() => { p1.catch(x => x); }); setTimeout(() => { console.log("Done"); }, 20000);` (works without keeping the process running as well). (I'm using Node v14.9) I don't have time to continue this back-and-forth. Please, though, be sure that you don't put incorrect information -- assumptions, misunderstandings, implementation-specific things that may change between dot releases, etc. -- in the answer. You don't want to spread incorrect information. – T.J. Crowder Oct 20 '20 at 07:47
2

You should be using try...catch to catch all the errors happening inside your try block:

try {
    await p2;
    // do something after p2 is settled
    await p1;
    // do something after p1 is settled
  }
catch(e) {
  // do something with errors e
}

EDIT:

window.addEventListener("unhandledrejection", event => {
  console.warn(`unhandledRejection: ${event.reason.message}`);
});

function delay(ms) {
  return new Promise(r => setTimeout(r, ms));
}

async function f1() {
  await delay(100);
  throw new Error("error f1");
}

async function main() {
  try {
  const p1 = await f1();
  await delay(200);
  }
  catch(e) {
    console.warn(`caught inside main: ${e.message}`);
  }
}

main().catch(e => console.warn(`caught on main: ${e.message}`));
Agustin Moles
  • 1,364
  • 7
  • 15
  • This code also will also fire `unhandledRejection`, for the same reason I've now explained [here](https://stackoverflow.com/a/64344718/1768303). The original problem doesn't relate to whether I use `catch` or `finally`. – noseratio Oct 14 '20 at 01:38
  • *"But you did what I answered... lol"* - thought you'd say that. Feel free to remove the whole `try/catch` from `main` in my answer and just leave `await p1` there to see the point. The question was about why `unhandledRejection` gets fired, not about how to handle errors. – noseratio Oct 14 '20 at 01:52
  • Does [this code](https://stackoverflow.com/a/64344718/1768303) make a better point, with `catch` gone and `try` brought back? I hope it does illustrate well what I mean. – noseratio Oct 14 '20 at 02:00
  • 1
    Yup got you that's why I deleted it. It should solve it if you move your `try` code block before calling to `p1()`. That's right. Sorry I couldn't help you out before – Agustin Moles Oct 14 '20 at 02:01
  • Yes, the problem is that `p1` gets rejected *before* we try handling that. Whether we do via `try/catch`, `p1.catch()` or `p1.then()` doesn't matter. What matters is that we were do it when the rejection had already happened. – noseratio Oct 14 '20 at 02:05
  • 1
    Edited my answer to show what I was referring to since first, sorry for the confusion. Anyway I think you just figured it out – Agustin Moles Oct 14 '20 at 02:15
0

I do not have a source but I think it works like this: The Promise.reject(new Error("Rejected!")); returns a rejected promise that will error next tick. so:

async function main3() {
    //this wil throw the error next tick
    const p1 = Promise.reject(new Error("Rejected!")); 
    //this will run immediately and attach the await to the promise (so it will not be rejected)
    await p1;
}

Then Promise.resolve will return its result to all .then handler next tick (we don't have them as the wont store the result anywhere) so:

async function main() {
    //this wil throw the error next tick
    const p1 = Promise.reject(new Error("Rejected!")); 
    //this will run immediately (and would give its value next tick)
    await Promise.resolve();
    //then this will run immediately and attach the await to the promise
    await p1;
}

Lastly a setTimeout with 0 delay does not immediately trigger, check: https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop and read the 0 delay section so:

async function main2() {
    //this wil throw the error next tick
    const p1 = Promise.reject(new Error("Rejected!"));  
    //setTimeout does with 0 does run not immediately.
    //because of this the await p1 does not get added before the promise is rejected
    await new Promise(r => setTimeout(r, 0));
    //so this does nothing and the prosime will reject
    await p1;
}
zhulien
  • 5,145
  • 3
  • 22
  • 36
tovernaar123
  • 212
  • 3
  • 12
  • I might be missing something, but does your answer contribute anything on top of [what I've already written about](https://stackoverflow.com/a/64344718/1768303)? Thanks anyway. – noseratio Oct 26 '20 at 06:05
  • well yes as it shows it has nothing to do with putting the await on sync or async, but has more to do with the settimout delay being 0 ( i did supply a source for that) @noseratio ( be sure to read the code comments ) – tovernaar123 Oct 26 '20 at 07:10
  • I used `setTimeout(0)` in my code only to show how the difference between microtask and macrotask queued continuations affect the `unhandledrejection` behavior. Might as well used `requestAnimationFrame`. I understand what's going on, but I'm interested to know why it is designed so. What specs (if any) or other considerations are behind this behavior? IMO, it'd be more logical if `unhandledrejection` was consistently fired for *any asynchronous* continuations, not just for macrotasks. – noseratio Oct 26 '20 at 08:27
  • ah sorry wel i can always be help full @noseratio but yes it does not make a lot of sence – tovernaar123 Oct 26 '20 at 09:41