How to retry a Promise resolution N times, with a delay between the attempts?

Question

I want some JavaScript code to take 3 things as parameters:

• A function returning a Promise.
• The maximum number of attempts.
• The delay between each attempt.

What I ended up doing is using a for loop. I did not want to use a recursive function : this way, even if there are 50 attempts the call stack isn't 50 lines longer.

Here is the typescript version of the code:

/**
 * @async
 * @function tryNTimes<T> Tries to resolve a {@link Promise<T>} N times, with a delay between each attempt.
 * @param {Object} options Options for the attempts.
 * @param {() => Promise<T>} options.toTry The {@link Promise<T>} to try to resolve.
 * @param {number} [options.times=5] The maximum number of attempts (must be greater than 0).
 * @param {number} [options.interval=1] The interval of time between each attempt in seconds.
 * @returns {Promise<T>} The resolution of the {@link Promise<T>}.
 */
export async function tryNTimes<T>(
    {
        toTry,
        times = 5,
        interval = 1,
    }:
        {
            toTry: () => Promise<T>,
            times?: number,
            interval?: number,
        }
): Promise<T> {
    if (times < 1) throw new Error(`Bad argument: 'times' must be greater than 0, but ${times} was received.`);
    let attemptCount: number;
    for (attemptCount = 1; attemptCount <= times; attemptCount++) {
        let error: boolean = false;
        const result = await toTry().catch((reason) => {
            error = true;
            return reason;
        });

        if (error) {
            if (attemptCount < times) await delay(interval);
            else return Promise.reject(result);
        }
        else return result;
    }
}

The delay function used above is a promisified timeout:

/**
 * @function delay Delays the execution of an action.
 * @param {number} time The time to wait in seconds.
 * @returns {Promise<void>}
 */
export function delay(time: number): Promise<void> {
    return new Promise<void>((resolve) => setTimeout(resolve, time * 1000));
}

To clarify: the code above works, I'm only wondering if this is a "good" way of doing it, and if not, how I could improve it.

Any suggestion? Thanks in advance for your help.

Answer 1

I did not want to use a recursive function: this way, even if there are 50 attempts the call stack isn't 50 lines longer.

That's not a good excuse. The call stack doesn't overflow from asynchronous calls, and when a recursive solution is more intuitive than an iterative one you should probably go for it.

What I ended up doing is using a for loop. Is this a "good" way of doing it, and if not, how I could improve it?

The for loop is fine. It's a bit weird that it starts at 1 though, 0-based loops are much more idiomatic.

What is not fine however is your weird error handling. That boolean error flag should have no place in your code. Using .catch() is fine, but try/catch would work just as well and should be preferred.

export async function tryNTimes<T>({ toTry, times = 5, interval = 1}) {
    if (times < 1) throw new Error(`Bad argument: 'times' must be greater than 0, but ${times} was received.`);
    let attemptCount = 0
    while (true) {
        try {
            const result = await toTry();
            return result;
        } catch(error) {
            if (++attemptCount >= times) throw error;
        }
        await delay(interval)
    }
}