Search code examples
javascripttypescriptasynchronousasync-awaitpromise

Awaited promise from array not being caught in for loop


I have a function that returns a Promise, I then have another function that creates an array of those promises for later use. I specifically do not want to call the promises inside the array building function as in some cases I would like to call the promises in parallel with Promise.all() and sometimes I want to call the promises sequentially in a for loop (as in this case).

My function that returns a promise looks like this:

export const getQueue = async (
  date: string,
  hour: string,
  queue: Queue
) => {
  const queueDateTime = getDateTimeFromQueueDateHour(date, hour);
  try {

    // do some async work..

    return { queue, queueDateTime };
  } catch (e) {

    // tansforms error to custom error

    if (e instanceof Error) {
      throw new GetQueueError(queue, queueDateTime, e.message);
    } else {
      throw e;
    }
  }
};

Then my promise array building function looks like this (QUEUE_IDS is an imported constant):

export const getAllQueues = (
  date: string,
  hour: string
) => {
  const queuePromises: Promise<{
    queue: string;
    matchCount: number;
    queueDateTime: DateTime;
  }>[] = [];
  for (const queue in QUEUE_IDS) {
    queuePromises.push(
      getQueue(date, hour, queue as Queue)
    );
  }

  return queuePromises;
};

Then my job function that uses the promise array function looks like this:

const matchHistoryJobFn: JobFn<MatchQueueSuccess> = async (jobId, jobKey) => {
  try {

    // ...

    const queues = getAllQueues(
      dateTimeToGetQueueDate(lastMatchIdQueueDate),
      "-1"
    );

    let matchCount = 0;

    for (let i = 0; i < queues.length; i++) {
      try {
        const q = await queues[i];
        matchCount += q.matchCount;
      } catch (e) {
        if (e instanceof GetQueueError) {
          log({
            jobId,
            jobKey,
            status: "error",
            message: e.message,
            currentDateTime: DateTime.utc().toISO(),
            meta: { queueDate: e.queueDateTime.toISO(), queue: e.queue },
          });
        } else {
          log({
            jobId,
            jobKey,
            status: "error",
            message: e as string,
            currentDateTime: DateTime.utc().toISO(),
          });
        }
      }
    }

    // ...

    return {
      jobId,
      meta: { queueDate: lastMatchIdQueueDate.toISO(), matchCount },
    };
  } catch (e) {
    throw handleJobError(e, jobId);
  }
};

Now in my for loop I want to catch the errors on any queues so that the other queues can still be processed. Unfortunately, my program is crashing due to an uncaught exception coming from 'getQueue'. I can't figure out why as not only is the call to await queue[i] wrapped in try catch, but also the entire job function is wrapped in a try catch.

Edit: As a side note, I have run this in VS Code debugger and the error is definitely being emitted from getQueue and not being caught in the for loop.


Solution

  • The question I then have is, if at the point of await is when I receive the error then why would it not be caught in the try catch? Using an array functions has made it work by the way.

    The main issue here is how node detects an unhandled rejection and the fact that your code starts running a promise-based asynchronous operation and does not immediately assign a .catch() handler. So, while your code is sitting at an await, some other promise rejects and you don't have an await and try/catch on that other promise yet, so thus you get the unhandled rejection.

    When you do this:

    const queues = getAllQueues(
      dateTimeToGetQueueDate(lastMatchIdQueueDate),
      "-1"
    );
    

    This calls ALL the getQueue() functions. So, all of those asynchronous operations are underway (in parallel). So, queues is already an array of promises that are attached to live asynchronous operations. At this point, there are no catch handlers for any of these promises that reject.

    And, then you process the queues one at a time using an await. The await will wait for the first promise to fulfill by suspending execution of the getAllQueues() function until that first one fulfills. At that point (or during any of the other await operations in that loop) if any of the other promises (besides the one you are awaiting) rejects, you don't yet have a reject handler for it. You will sometime in the future when you await it with a try/catch around it, but that's too late because nodejs will see that your program is in the event loop (waiting for some await to fulfill), a promise rejected and you don't have a reject handler on it. That, to nodejs, is an unhandled rejection.

    The basic advice to avoid this is don't ever leave a promise sitting in a variable without a .catch() handler on it when you await something or otherwise return back to the event loop.


    If your intention here is to not actually run the asynchronous operations in parallel, then you need to restructure your code to only call the asynchronous operations one at a time. Right now, getQueue() is starting the asynchronous operations and getAllQueues() is starting all of them. There are a number of possible ways to restructure it. You could build an array of functions that return promises and then only call those functions one at a time in a loop, awaiting each one. Or, since the functions are all the same in this case, you can just create a function that only calls getQueue() one at a time, awaiting each one and then returns a promise that resolves with an array of results.