Javascript: Function that retries with setTimeout

Question

I have a function downloadItem that may fail for network reasons, I want to be able to retry it a few times before actually rejecting that item. The retries need to be with a timeout since if there is a network issue there is no point in retrying immediately.

Here is what I have so far:

function downloadItemWithRetryAndTimeout(url, retry, failedReason) {
    return new Promise(function(resolve, reject) {
        try {
            if (retry < 0 && failedReason != null) reject(failedReason);

            downloadItem(url);
            resolve();
        } catch (e) {
            setTimeout(function() {
                downloadItemWithRetryAndTimeout(url, retry - 1, e);
            }, 1000);
        }
    });
}

Obviously this will fail since the second (and on) time I call downloadItemWithRetryAndTimeout I don't return a promise as required.

How do I make it work correctly with that second promise?

P.S. incase it matters the code is running in NodeJS.

maybe using promise does not fit your scenario. I don't get why people suddenly use promises for every single async case. — webduvet
– webduvet, Commented Aug 10, 2015 at 14:31
I don't have to use a promise, you have any solution without a promise? — AlexD
– AlexD, Commented Aug 10, 2015 at 14:38
@webduvet: because that's what promises are made for: async cases with a single result. And it does fit this scenario very well. — Bergi
– Bergi, Commented Aug 10, 2015 at 16:29
@AlexD: Is your downloadItem function really synchronous? I think it should return a promise. — Bergi
– Bergi, Commented Aug 10, 2015 at 16:29
@alexd with Bluebird you can use Promise.delay and Promise.try — aarosil
– aarosil, Commented Aug 11, 2015 at 8:04

Ori Drori · Accepted Answer · 2015-08-10 18:05:03Z

5

I've got two ideas:

Move the promise out side of the iterated function downloadItemWithRetryAndTimeout - now resolve() will available to all iterations:

function downloadWrapper(url, retry) {
    return new Promise(function (resolve, reject) {
        function downloadItemWithRetryAndTimeout(url, retry, failedReason) {

            try {
                if (retry < 0 && failedReason != null)
                    reject(failedReason);

                downloadItem(url);
                resolve();
            } catch (e) {
                setTimeout(function () {
                    downloadItemWithRetryAndTimeout(url, retry - 1, e);
                }, 1000);
            }

        }

        downloadItemWithRetryAndTimeout(url, retry, null);
    });
}

This solution works, but it's an anti pattern as it breaks the promise chain: As each iteration returns a promise, just resolve the promise, and use .then to resolve the previous promise, and so on:

function downloadItemWithRetryAndTimeout(url, retry, failedReason) {
    return new Promise(function (resolve, reject) {
        try {
            if (retry < 0 && failedReason != null)
                reject(failedReason);

            downloadItem(url);
            resolve();
        } catch (e) {
            setTimeout(function () {
                downloadItemWithRetryAndTimeout(url, retry - 1, e).then(function () {
                    resolve();
                });
            }, 1000);
        }
    });
}

edited Aug 10, 2015 at 18:05

answered Aug 10, 2015 at 14:38

Ori Drori

196k32 gold badges243 silver badges233 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

AlexD Over a year ago

I really like your first solution! I've changed it a bit since inside the promise after declaring the inner function you have to also call it once. I'll try it now

Ori Drori Over a year ago

Sloppy me - forgot to run the function - fixed.

Bergi Over a year ago

Please avoid the promise constructor antipattern

Ori Drori Over a year ago

@Bergi - thanks. Didn't think that it falls to the same deferred trap.

score 4 · Accepted Answer · 2016-03-27 15:04:18Z

4

There is no need to create new promises to handle this. Assuming downloadItem is synchronous and returns a promise, simply return the result of calling it, along with a catch to call downloadItemWithRetryAndTimeout recursively.

function wait(n) { return new Promise(resolve => setTimeout(resolve, n)); }

function downloadItemWithRetryAndTimeout(url, retry) {
  if (retry < 0) return Promise.reject();

  return downloadItem(url) . 
    catch(() => wait(1000) . 
      then(() => downloadItemWithRetryAndTimeout(url, retry - 1)
  );
}

Some might find the following slightly cleaner:

function downloadItemWithRetryAndTimeout(url, retry) {
  return function download() {
    return --retry < 0 ? Promise.reject() :
      downloadItem(url) . catch(() => wait(1000) . then(download));
  }();
}

edited Mar 27, 2016 at 15:04

answered Mar 27, 2016 at 14:49

user663031

2 Comments

Benjamin Gruenbaum Over a year ago

It'd make more sense to do it through a general retry method const retry = (fn, retries = 3) => fn().catch(e => retries <= 0? Promise.reject(e) : retry(fn, retries - 1)) and compose it with timeouts: const delay = ms => new Promise(r => setTimeout(r, ms)) and const delayError = (fn, ms) => fn().catch(e => delay(ms).then(y => Promise.reject(e))). Then the code becomes const downloadWithRetryAndTimeout = retry(delayError(download, 1000)) or something like that.

user663031 Over a year ago

@BenjaminGruenbaum Thanks for this elegant decomposition.

Stel Abrego · Accepted Answer · 2019-09-06 14:17:49Z

@BenjaminGruenbaum comment on @user663031 is fantastic but there's a slight error because this:

const delayError = (fn, ms) => fn().catch(e => delay(ms).then(y => Promise.reject(e)))

should actually be:

const delayError = (fn, ms) => () => fn().catch(e => delay(ms).then(y => Promise.reject(e)))

so it will return a function, not a promise. It's a tricky error that's hard to solve so I'm posting on here in case anyone needs it. Here's the whole thing:

const retry = (fn, retries = 3) => fn().catch(e => retries <= 0 ? Promise.reject(e) : retry(fn, retries - 1))
const delay = ms => new Promise(resolve => setTimeout(resolve, ms))
const delayError = (fn, ms) => () => fn().catch(e => delay(ms).then(y => Promise.reject(e)))
retry(delayError(download, 1000))

Rovak · Accepted Answer · 2015-08-10 14:38:25Z

1

function downloadItemWithRetryAndTimeout(url, retry) {
    return new Promise(function(resolve, reject) {
        var tryDownload = function(attempts) {
            try {
                downloadItem(url);
                resolve();
            } catch (e) {
                if (attempts == 0)  {
                    reject(e);
                } else {
                    setTimeout(function() {
                        tryDownload(attempts - 1);
                    }, 1000);
                }
            }
        };
        tryDownload(retry);
    });
}

answered Aug 10, 2015 at 14:38

Rovak

7881 gold badge6 silver badges13 bronze badges

Collectives™ on Stack Overflow

Javascript: Function that retries with setTimeout

4 Answers 4

4 Comments

2 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

4 Comments

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related