Is this the correct way to write a non-blocking while loop using try-catch, setTimeout and recursion?

Question

I have an endpoint on my server named "/order".

When it gets triggered, I need to send an order over to an API. Sometimes, because some data over ipfs takes too long to download, the order fails within the try-catch block, and I need to resend it.

Since a while loop that tries to resend the order until it is successful would be blocking other requests, I thought I could make one myself using recursion and a setTimeout, to try to send the same request, say, 3 times every 5 minutes and if the third time it fails then it won't try again.

I wanted to know if this was the right way to achieve the non-blocking functionality and if there are some vulnerabilities/things I'm not taking into account:

async function makeOrder(body, tries) {

    if (tries > 0) {
        let order;

        try {
            order = getOrderTemplate(body.shipping_address)

            for (const index in body.line_items) {
                order.items.push(await getItemTemplate(body.line_items[index]))
            }

            await sendOrderToAPI(order)

        } catch (err) {
            setTimeout(function(){
                makeOrder(body, tries - 1)
            }, 180000)
        }
    } else {
        console.log("order n " + body.order_number + " failed")
        //write failed order number to database to deal with it later on
    }
}

When do you want the promise returned by `makeOrder` to fulfill? — Bergi, Apr 10 '22 at 22:38
With `async`/`await`, there is no need for recursion - you can just write a `while` loop if you want to. — Bergi, Apr 10 '22 at 22:39
Do loop it, just call it and if fail setTimeout to repeat request till it can finalize but error handle it. Essentially if you make a request and it fails, call a timeout to repeat the call and define it to cancel at any point. So by your coding it'd seem acceptable if it met your app requirements. — BGPHiJACK, Apr 10 '22 at 22:41
Related: [How to use `setTimeout` with `async`/`await`](https://stackoverflow.com/q/33289726/1048572) — Bergi, Apr 10 '22 at 22:47
hI @Bergi thank you so much for the help! You said with async-await I could just use a while loop but I found that if I use the await inside the while loop, in the case the await is taking a long time it'll never exit the while loop so the loop will block the event loop. — Matteo Barberis, Apr 10 '22 at 23:36
@MatteoBarberis Yes, the loop is taking until the order succeeds, but no this does *not* block the event loop - it's `async`! — Bergi, Apr 11 '22 at 00:09
Hi @Bergi. I tried and you're right, it weirdly doesn't block the event loop but I have no idea why, is it because as soon as I put the await keyboard inside the loop then it knows not to block the event loop? Because if I say made an infinite loop without async inside then it would block other requests and I don't understand the difference — Matteo Barberis, Apr 11 '22 at 07:57
Yes, it's because of `await` - that's telling the engine to suspend the execution of the function and to resume it only when the promise settles — Bergi, Apr 11 '22 at 10:29

CertainPerformance · Accepted Answer · 2022-04-10T22:45:37.447

One significant problem is that, if there's a problem, it'll return a Promise that resolves not when the final API call is finished, but when the initial (failed) API call is finished. This:

        setTimeout(function(){
            makeOrder(body, tries - 1)
        }, 180000)

is not chained properly with the async function outside.

As a result, the following logic will fail:

makeOrder(body, 3)
  .then(() => {
    // All orders are made
  })

Promisify it instead, so that the recursive call can be chained with the outer function easily.

const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
async function makeOrder(body, tries) {
    if (tries > 0) {
        let order;

        try {
            order = getOrderTemplate(body.shipping_address)

            for (const index in body.line_items) {
                order.items.push(await getItemTemplate(body.line_items[index]))
            }

            await sendOrderToAPI(order)

        } catch (err) {
            await delay(180_000);
            return makeOrder(body, tries - 1);
        }
    } else {
        console.log("order n " + body.order_number + " failed")
        //write failed order number to database to deal with it later on
    }
}

Another possible improvement would be to, instead of awaiting inside a loop here:

        for (const index in body.line_items) {
            order.items.push(await getItemTemplate(body.line_items[index]))
        }

to use Promise.all, or a different mechanism that allows for fetching multiple templates at once, instead of having to wait for each one-by-one in serial.

Another potential issue is that makeOrder does not reject when it exceeds the number of tries allowed. That is, the consumer wouldn't be able to do:

makeOrder(body, 3)
  .catch((error) => {
    // implement logic here to handle the error
  })

If you wanted to permit the above, at the very end of makeOrder, throw at the same time you're logging:

} else {
    console.log("order n " + body.order_number + " failed")
    //write failed order number to database to deal with it later on
    throw new Error('Failed');
}

Is this the correct way to write a non-blocking while loop using try-catch, setTimeout and recursion?

1 Answers1