Unhandled Promise rejection despite common understanding

Question

Recently I've stumbled upon an interesting bug. In the essence, the problem boils down to this example:

const waitResolve = (ms) => new Promise((resolve) => {
  setTimeout(() => {
    console.log(`Waited to resolve for ${ms}ms`);
    resolve(ms);
  }, ms);
});

const waitReject = (ms) => new Promise((resolve, reject) => {
  setTimeout(() => {
    console.log(`Waited to reject for ${ms}ms`);
    reject(ms);
  }, ms);
});

const run = async() => {
  const promises = {
    a: [],
    b: [],
    c: [],
  };

  for (let i = 0; i < 5; i += 1) {
    promises.a.push(waitResolve(1e4));
    promises.b.push(waitReject(1e3));
    promises.c.push(waitResolve(1e2));
  }

  try {
    for (const [key, value] of Object.entries(promises)) {
      console.log(`Starting ${key}`);

      try {
        await Promise.all(value);
      } catch (err) {
        console.log(`Caught error in ${key}!`, err);
      }

      console.log(`Finished ${key}`);
    }
  } catch (err) {
    console.log('Caught error in run!', err);
  }
};

run();

Here, despite common understanding that the promises will reside in pending state during and after the for loop and would "really" start to execute only after Promise.all call. It implies that try/catch blocks will catch the rejection produced in waitReject(1e3), but it does not happen (tested in Node.js v18.14.2 and couple of earlier versions).

If the sequence of Promises array pushes would be changed to:

promises.a.push(waitResolve(1e2));
promises.b.push(waitReject(1e3));
promises.c.push(waitResolve(1e4));

The rejection would be caught. Now, I do vaguely understand that it is related to the mIcro and mAcro tasks resolution sequence inside the Event Loop, and the fact that Promises that would have a chance to execute between the ticks would do so.

However, I would really like to hear a proper explanation from someone who has more understanding than I do.

Answer 1

This is just troublesome code given the current implementations of uncaught rejections.

You're allowing a promise to reject WHEN there is no reject handler at the time it rejects and when the system gets back to the microtasks queue. That will trigger an uncaught rejection because at the time you get back to the queue and that promise rejection is now seen by the system, there is no proper reject handler because you haven't yet called Promise.all() for it because the previous await Promise.all() is still awaiting meaning the for loop has been suspended.

This is just how detection of uncaught rejections works.

The system doesn't wait forever to see if at some future point you will then attach a reject handler (which this code will eventually). Instead, it sees that it's back to the micro task loop, a promise has reject and there is CURRENTLY no reject handler for it. It isn't smart enough to know you will eventually attach a reject handler.

If b rejects while you're awaiting a then there will be no reject handler for b when it gets back to the microtasks queue and sees the rejection without a reject handler.

The general design guideline is to not structure your code so that it can get back to the microtasks queue before you've attached your reject handler. The best case is to attach the reject handler immediately as soon as you get the promise. And, certainly, never do it after the asynchronous event has started and then await some other promise because that allows the system to go back to the microtask queue before you've attached the reject handler and opens up this problem.

Or in more simpler terms for this example. Don't create promises without assigning a catch handler and then await them individually in a loop.

It's unclear what real solution to offer here for this particular code because the whole thing seems a bit contrived where you're doing Promise.all() on one promise at a time, but expecting parallel execution of all the operations. The proper implementation for parallel execution would be a single Promise.all() that awaits all the promises, not just some of them. Then, you'd have a reject handler in place for all of them at the same time and before the code had any chance to get back to the queue.