1

I am really hoping someone can help me with this issue despite my inability (confidentiality issues) to share too much information. Basically, I wrote this function to retry failed promises:

function retryOnFailure(func) {
  return func().then(function(resp) {
    return resp
  }, function(err) {
    return setTimeout(function() {
      return retryOnFailure(func)
    }, 5000)
  }
)}

And then I have this code to utilize it:

function getStuff() {
  return $.get('/some/endpoint')
}

retryOnFailure(getStuff).then(function(res) { console.log(res) })

Now the recursive retry function works fine, but my then in the last line of code doesn't fire in the event that an error case is reached in retryOnFailure. I know that the reason is because each recursive call creates a new promise so the promise that I'm listening to in my last line is not the same one that gets resolved, but I'm having difficulty figuring out how to remedy this. I want this function to be a generic retry function and as such don't want to handle the actual success in the method, but I'm not seeing any other way. Any help would be greatly appreciated!

sunny-mittal
  • 499
  • 5
  • 12

3 Answers3

1
return setTimeout(function() {
    return retryOnFailure(func)
}, 5000)

looks like you're trying to do the right thing - return the next result from your .catch callback. However, setTimeout does not return a promise, so retryOnFailure() will be resolved immediately (with the timeout cancellation id).

Instead, you need to make sure that for all asynchronous functions that you use a promise is produced, so let's start by promisifying setTimeout:

function delay(ms) {
  return new Promise(function(resolve, reject) {
    setTimeout(resolve, ms);
  });
}

Now we can use that to chain the retry after a delay, and get a promise from it:

function retryOnFailure(func) {
  return func().then(null, function(err) {
    return delay(5000).then(function() {
      return retryOnFailure(func);
    });
  });
}

Notice that instead of .then(null, …) you can also use .catch(…).

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
0

This is the first way that came to mind, there is probably a more elegant solution:

function retryOnFailure(func, onSuccess) {
    return func().then(function(resp) {
        onSuccess(resp);
    }, function(err) {
           return setTimeout(function() {
               return retryOnFailure(func, onSuccess)
           }, 5000)
    }
)}

retryOnFailure(getStuff, function(res) { console.log(res) });
dave
  • 62,300
  • 5
  • 72
  • 93
  • Oh wow...I don't know how I could have overlooked such a simple solution! Thank you so much :) I was really hoping to have something I could chain with `then` for consistency across the codebase for my team but this is a totally acceptable solution. Cheers! – sunny-mittal Aug 13 '15 at 00:26
  • `return func().then(onSuccess, ...)` - instantly more elegant :-). Also the innermost return is superfluous. – Roamer-1888 Aug 13 '15 at 08:17
  • Errrm... isn't avoiding promises completely, and using callbacks instead, totally *not* the way to solve this question? :) @Bergi's solution, to add promise-based delays, would seem to be a better way to go. – iCollect.it Ltd Sep 01 '15 at 16:39
  • @TrueBlueAussie as I said, I'm sure there is a more elegant solution, was just sharing one way that works. – dave Sep 01 '15 at 16:43
  • Lots of things *work*, but this would completely change the calling code (back to a traditional callback model). I'm just saying this should not be the accepted answer, as it will lead users back to the dark-side of non-promise programming, when the original was leading to the promise'd land :). – iCollect.it Ltd Sep 01 '15 at 16:46
  • Then write a better answer? – dave Sep 01 '15 at 16:48
0

The problem is your call to setTimeout, it breaks the flow of your promise. Try something like this instead:

function retryOnFailure(func) {
  return new Promise(function(resolve) {
    func().then(function(resp) {
      resolve(resp);
    }).catch(function(err) {
      setTimeout(function() {
        retryOnFailure(func).then(resolve);
      }, 5000)
    });
  });
};

retryOnFailure(getStuff).then(function(res) { console.log(res) });

Note how I've created a new promise inside the function which never throws an error, but instead will simply attempt to process the func() and recurse itself on failure.

It also may be a good idea to put some kind of retry-limit as well.

Good luck!

Lochemage
  • 3,974
  • 11
  • 11