Handling exceptions within recursive promise

Question

I'm trying to both be able to handle a paginated API, as well as do retries if throttled for too many requests. The pagination is handled by recursing if 'nextToken' is present in the response object. I'm hoping to be able to catching a Throttling Exception, and effectively start the whole request over by recursing without passing the token. This is my current code:

function getAllExecHist(execArn) {
  var sfn = new AWS.StepFunctions();
  sfn = Promise.promisifyAll(sfn); 
  execHists = [];
  return new Promise(function(resolve, reject) {
    function getExecHist(nextToken) {
      params = {};
      params.executionArn = execArn;
      if (nextToken !== undefined) {
        params.nextToken = nextToken;
      }
      sfn.getExecutionHistoryAsync(params)
        .then(function(results) {
          execHists = execHists.concat(results.events);
          if (!results.nextToken) {
            resolve(execHists);
          }
          else {
            getExecHist(results.nextToken);
          }
        })
        .catch(function(e) {
          console.log('caught this: ', e);
          console.log('retrying');
          return new Promise(function(res, rej) {
          console.log('Sleeping');
          setTimeout(function() {
            execHists = [];
            res(getExecHist()); 
          }, random(100,10000));
          });         
        })
    }
    getExecHist();
  });
}

The recursion was handling pagination without issue, but since adding the catch, it simply never returns. Any ideas what I'm doing wrong / how to fix?

`getExistHist()` does not return anything so it's unclear what you were trying to do with `res(getExecHist())`. — jfriend00, Mar 09 '17 at 00:23
Avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Mar 09 '17 at 00:38
Your `execHists` and `params` are [accidentally global](http://blog.niftysnippets.org/2008/03/horror-of-implicit-globals.html) — Bergi, Mar 09 '17 at 00:39
Just curious, why would you want to start all over after an error, shouldn't the present results still be valid? — Bergi, Mar 09 '17 at 00:44
Thanks for the pointers. You're right. Ideally I wouldn't start over... I had hoped that for the 'quick and dirty' solution I've been working on, starting all over would if a request gets throttled would be sufficient... That may / may not be true, though. — rumdrums, Mar 09 '17 at 04:39

Matt · Accepted Answer · 2017-03-09T07:51:07.047

1

The AWS SDK supports promises and you can configure Bluebird as it's promise library.

const Promise = require('bluebird');
const AWS = require('aws');
AWS.config.setPromisesDependency(Promise);
const sfn = new AWS.StepFunctions();

Use Promise.delay() instead of setTimeout.

Try and avoid creating new promises if functions are already returning them. Only wrap a promise in new Promise if you have a lot of synchronous code that might throw an error or needs to resolve the promise early.

The following also avoids the extra function and nested scope by passing values between function calls.

function getExecHist(execArn, execHists, nextToken) {
  let params = {};
  params.executionArn = execArn;
  if ( nextToken !== undefined ) params.nextToken = nextToken;
  if ( execHists === undefined ) execHists = [];
  return sfn.getExecutionHistory(params).promise()
    .then(results => {
      execHists = execHists.concat(results.events);
      if (!results.nextToken) return execHists;
      return getExecHist(execArn, execHists, results.nextToken);
    })
    .catch(e => {
      console.log('caught this: ', e);
      console.log('retrying');
      return Promise.delay(random(100,10000))
        .then(() => getExecHist(execArn));
    })
}

Eventually you should be specific about what errors you retry on and include a count or time limit too.

Also note that this is the wrong way to retry a rate limit issue as this starts again from the beginning. A rate limit retry should continue from where it left off, otherwise you are just adding to your rate limit problems.

edited Mar 09 '17 at 07:51

answered Mar 09 '17 at 00:55

Matt

68,711
7
155
158

You should use [`.then(…, …)` instead of `.then(…).catch(…)`](http://stackoverflow.com/q/24662289/1048572) to catch the right errors, and to avoid chaining promises infinitely in the recursion – Bergi Mar 09 '17 at 02:00
Thanks. This code looks beautiful. Point taken on all your comments. I'm likely to never use this code again, so it's admittedly sloppy at the moment. I was hoping you might clarify, though -- is there something specific I've done in my original example that would prevent the function from ever returning? – rumdrums Mar 09 '17 at 04:44
@Bergi "to avoid chaining promises infinitely" Is there a functional difference between the way `fn().catch()` and `fn.then(s,e)` would handle the promise chain? – Matt Mar 09 '17 at 05:14
Yes, there is a functional difference, see the link in my first comment. Regarding the infinite chain, see [these considerations](http://stackoverflow.com/a/29931657/1048572) about memory consumption. – Bergi Mar 09 '17 at 05:26
@Bergi I get the try/catch semantics but not clear on how `then(s, e)` avoids a chain in the recursion? So far about all I've been able to see is that the `.catch` each can fit about 10 million promises in before oom : / – Matt Mar 09 '17 at 07:48
@rumdrums the original code was returning the `new Promise` in the `catch` rather than resolving it and then nothing was being done with the original promise `sfn.getExecutionHistoryAsync()` – Matt Mar 09 '17 at 07:50
@Matt Having only the `then`, whose callback returns the recursive promise, in the last position is basically tailrecursion. If you chain a `catch` to it, you're not building a "deep" but "long" chain which takes linear memory. Admittedly it doesn't really matter here where `getExecHist` will not recurse very often. – Bergi Mar 09 '17 at 14:01
@Bergi Thanks for clarifying. I meant to say previously that recursing with _both_ methods could fit about 10 million promises in before OOM, so I'm not seeing a large memory diff under recent bluebird. – Matt Mar 11 '17 at 03:43

Handling exceptions within recursive promise

1 Answers1