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.
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);
}
});
}
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));
}();
}
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);
});
}