In javascript, a function which returns promise and retries the inner async process best practice

Question

I have a function which returns a javascript Promise and inside it there runs some asynchronously code. The async code, needs to be retried for couple of times in cases it fails. I was doing that, till I observed some strange behaviors which made me wonder if I'm doing it right. So I had to change it. Both approaches snipped are down here. I have some idea why the first approach (asyncFunc) doesn't work, I would appreciate if someone could share some technical clarity about it. And for the second approach (ayncFunc_newer) any suggestion on how it can be done better?

var _retryCount = 0;

// this is what I was doing
function asyncFunc () {
 return new Promise(function(fulfill, reject) {
  doAsync()
   .then(fulfill)
   .catch(retry);

  function retry(promiseResult) {
   if(_retryCount < 3) {
    _retryCount++;
    return asyncFunc();
   }
   else {
    reject(promiseResult);
   }
  }
 });
}

// this what I'm doing now
function ayncFunc_newer() {
    return new Promise(function(fulfill, reject) {
        var retryCount = 0;

        doAsync()
            .then(fulfill)
            .catch(onReject);

        function onReject(bmAuthError) {
            if(retryCount < 3) {
                retryCount++;
                logWarning(error);
                
                doAsync()
                 .then(fulfill)
                 .catch(onReject);
            }
            else {
                fulfill(false);
            }
        }
    });                 
};

Where are you incrementing _retryCount ? – juju Jul 24 '15 at 14:22 — juju, Jul 24 '15 at 14:22

score 12 · Accepted Answer · edited May 23 '17 at 12:17

12

Best practice is to avoid the promise constructor anti-pattern. Basically, new Promise exists mostly to wrap non-promise APIs, so if your functions already return promises, then there's usually a way to avoid using it.

If you're doing a low fixed number retries, then your case is as simple as:

function ayncFunc() {
  return doAsync().catch(doAsync).catch(doAsync).catch(doAsync);
};

For a configurable number of retries, you'd expand this to:

var retries = 3;

function ayncFunc() {
  var p = doAsync();
  for (var i = 0; i < retries; i++) {
    p = p.catch(doAsync);
  }
  return p;
};

Or for higher number of retries, you could use a recursive approach:

function ayncFunc() {
  function recurse(i) {
    return doAsync().catch(function(e) {
      if (i < retries) {
        return recurse(++i);
      }
      throw e;
    });
  }
  return recurse(0);
};

var console = { log: msg => div.innerHTML += msg + "<br>" };

function doAsync() {
  console.log("doAsync");
  return Promise.reject("Nope");
}

function ayncFunc() {
  return doAsync().catch(doAsync).catch(doAsync).catch(doAsync);
};

var retries = 3;

function ayncFunc2() {
  var p = doAsync();

  for (var i=0; i < retries; i++) {
    p = p.catch(doAsync);
  }
  return p;
};

function ayncFunc3() {
  function recurse(i) {
    return doAsync().catch(function(e) {
      if (i < retries) {
        return recurse(++i);
      }
      throw e;
    });
  }
  return recurse(0);
};

ayncFunc().catch(function(e) { console.log(e); })
.then(ayncFunc2).catch(function(e) { console.log(e); })
.then(ayncFunc3).catch(function(e) { console.log(e); });

<div id="div"></div>

edited May 23 '17 at 12:17

Community

1
1

answered Jul 25 '15 at 19:00

jib

40,579
17
100
158

I'd rather recommend to use a recursive approach than to construct very long chains. – Bergi Jul 25 '15 at 19:42
@Bergi I doubt the overhead is much since catch handlers are skipped in regular (successful) code flow, but I've added a recursive example for completeness. Thanks for the idea. – jib Jul 25 '15 at 21:52
@jib in the recursive approach at first a promise is returned and in case of exception another promise is returned to the called. Couldn't that confuses the ayncFunc caller? – Abtin Jul 28 '15 at 14:46
@Abtin the `asyncFunc` caller only ever sees one promise, the one immediately returned from `doAsync().catch()`, but that promise soon becomes *resolved to* (or "locked in to" may be a better way to say it, since `reject` was called on it) another pending promise, the one returned from the second call to `recurse()`. The first promise remains *pending* and wont *settle* (be *fulfilled* or *rejected*) until the second promise settles, inheriting its value. This is using the terminology in [states and fates](https://github.com/domenic/promises-unwrapping/blob/master/docs/states-and-fates.md). – jib Jul 28 '15 at 15:52
@jib thanks, I understand that scenario now. Now I'm confused about the ideal scenario where doAsync fulfills on the first try. The asyncFunc caller has doAsync().catch() promise, how does that become fulfilled in this scenario? – Abtin Jul 28 '15 at 16:19
@Abtin - the same way. `.catch(y)` is [just an alias for](http://www.ecma-international.org/ecma-262/6.0/#sec-promise.prototype.catch) `.then(undefined, y)` which behaviorally is equivalent to `.then(x => x, y)`. Fulfillment and rejection work quite symmetrically, and a promise returned from either `.catch` or `.then` is just another link in the promise-chain. – jib Jul 28 '15 at 18:36
In the second example, wouldn't doAsync start to be passed the error as a parameter? Don't these get a lot more complicated if your async work function requires parameters? – Tony Gutierrez Nov 03 '17 at 03:24
@TonyGutierrez Yes, for simplicity I based examples on `doAsync()` in the question. With arguments do `p = p.catch(() => doAsync(...arguments));` – jib Nov 09 '17 at 00:35
Nice practice, thanks ! BTW, would it be possible to set a time delay between each retry ? Something like following: function ayncFunc() { function recurse(i) { return doAsync().catch(function(e) { if (i < retries) { setTimeout(()=>{ return recurse(++i); }, 1000); //return recurse(++i); } throw e; }); } return recurse(0); }; It won't func probably since the result will be returned(whether it's error or not) in the first try ... – mike652638 Apr 16 '20 at 11:27
Ok, inspired by another answer in https://stackoverflow.com/questions/38213668/promise-retry-design-patterns, I came up with a compromising solution like function doAsync(e) { return new Promise(function(resolve, reject) { setTimeout(reject.bind(null, e), t); }); } – mike652638 Apr 16 '20 at 11:36

In javascript, a function which returns promise and retries the inner async process best practice

1 Answers1

Linked

Related