Why does Promise.race() fulfill with an unfulfilled promise?

Question

I am running a sequence of functions on a page in Puppeteer. If one of those functions throws I retry the whole sequence a couple of times because the systems I'm using are not reliable. It's easier than handling all possible exceptions.

Sometimes instead of throwing, one of those steps will just hang so I want to have a timeout for safety, and retry as if an exception happened.

If I don't wrap my sequence in the below asyncCallWithTimeout() everything works correctly and resolves to what I want (an array). However, when I do use that function I'm left with an unresolved function that I passed as asyncPromise.

For the life of me, I can't figure out why Promise.race() would resolve with an unresolved asyncPromise that I passed. What am I missing?

Here's the function to time out other async functions when they run too long:

async function asyncCallWithTimeout(asyncPromise, timeLimit) {
  let timeoutHandle;

  const timeoutPromise = new Promise((_resolve, reject) => {
    timeoutHandle = setTimeout(
      () => reject(new Error('Async call timeout limit reached')),
      timeLimit
    );
  });

  return Promise.race([asyncPromise, timeoutPromise]).then((result) => {
    clearTimeout(timeoutHandle); // .then() looks to be executed every time even if timeout is set to 1 ms
    return result;
  });
};

const timer = ms => new Promise(r => setTimeout(r, ms));

const shouldResolve = asyncCallWithTimeout(timer(1000), 2000).then(console.log).catch(console.error);
const shouldReject = asyncCallWithTimeout(timer(2000), 1000).then(console.log).catch(console.error);

Here's the retry function in case it's relevant:

async function retry(promiseFactory, retryCount) {
    try {
      return await promiseFactory();
    } catch (error) {
      if (retryCount <= 0) {
        throw error;
      }
      return await retry(promiseFactory, retryCount - 1);
    }
};

I'm using it this way:

async function checkT(browser, url) {
    const tUrl = `https://example.com/?url=${url}`;
    return retry(async () => {
        const browserPage = await browser.newPage();
        try {
            return asyncCallWithTimeout(async () => { // works if this is commented
                await browserPage.goto(tUrl);
                await doSomething(browserPage);
                await waitForSomething(browserPage);
                const results = await getResults(browserPage);
                await browserPage.close();
                return results;
            }, 10000); // works if this is commented
        } catch (error) {
            await browserPage.close();
            throw error;
        }
    }, 3);
};

And then I call it elsewhere like so (using p-queue which is irrelevant):

for (const page of pages) {
    queue.add(() => checkT(browser, page.url)
        .then(async (results) => {
            // results should be an array but is a function and not iterable hence fails below
            for (const result of results.data) {
                // do something with the result
            }
        })
    );
};

I added a runnable snippet to test your issue, but it seems that it is working properly. The promise with the smaller timeout is the one that becomes the final value in both examples. Can you modify that snippet to reproduce your issue? — CRice, Jan 17 '22 at 20:35
Your `asyncCallWithTimeout` expects a promise but you're passing an `async function` instead. Also you're missing the `await` in the `try` block, by immediately `return`ing the promise a rejection won't cause the `catch` clause to run. Btw I'd recommend to use `finally` for the `browserPage.close()` anyway — Bergi, Jan 17 '22 at 20:49
Thanks, @Bergi. Could you explain the missing `await` and why the immediate `return` won't ever trigger `catch`? I'd love to read more but don't know what to search for. About `finally` I assume this is just a personal preference of yours? — Kuba Serafinowski, Jan 17 '22 at 22:45
See [here](https://stackoverflow.com/q/43353087/1048572) and [there](https://stackoverflow.com/q/38708550/1048572) for `return await`. As for `finally`, no, it's objectively better. It expresses your intentions more clearly, it avoid duplicating the cleanup code, and it won't try to close the browser page again if the timeout occurs during the `await browserPage.close();` or if an error happens in there. — Bergi, Jan 17 '22 at 23:05

score 1 · Accepted Answer · answered Jan 17 '22 at 20:38

You are passing a function as the first argument of asyncCallWithTimeout, but I'm assuming you wanted to pass a promise given both the argument name and how you're using it.

When you called Promise.race, asyncPromise is still a function:

Promise.race([asyncPromise, timeoutPromise])

To get this to work you should call it:

            return asyncCallWithTimeout((async () => {
                await browserPage.goto(tUrl);
                await doSomething(browserPage);
                await waitForSomething(browserPage);
                const results = await getResults(browserPage);
                await browserPage.close();
                return results;
            })(), 10000);
//            ^
//            |
// You arrow function called here, 
// so this should return a promise 
// to the first argument of `asyncCallWithTimeout`

Notice that the function is wrapped in parenthesis and then called here. Obviously, this is not the only way to call the function, you can give it a name and call it, then pass in the returned promise variable, but I'm hoping you get my point.

Yes, that was it! Thanks, can you point me to some documentation I can read to understand why it's not executing? I thought an async function would be wrapped in a promise by default but now I see it's the return value that is wrapped in the promise. — Kuba Serafinowski, Jan 17 '22 at 22:26

Why does Promise.race() fulfill with an unfulfilled promise?

1 Answers1