Javascript Recursive Promise

Question

I'm trying to create a recursive function using Promises, but can't quite seem to get it right. I have working code without using promises, but it uses counters and global variables etc. and doesn't feel quite right, so I'm attempting a rewrite and also creating a module for reuse.

Essentially, the function is supposed to be getting a user from Active Directory and then recursively finding any direct reports and their direct reports and so on.

I've played with lots of versions of the function, this is the current one:

function loadReports(personEmail, list) {
    return new Promise((resolve, reject) => {
        getAccessTokenPromise()
            .then(access_token => {
                list.push(personEmail);
                return makeRequest(personEmail, access_token);
            }).then(result => {
                if (result.value.length > 0) {
                    Promise.all(result.value.map(person => {
                        loadReports(person.userPrincipalName, list);
                    })).then(resolve());
                } else {
                    resolve();
                }
            })
            .catch(e => reject(e));
    });
}

The getAccessTokenPromise function performs a request for an access token and returns a promise for that. The makeRequest function again just makes an https request for the user and their reports and returns a json object with the results as a Promise.

Any thoughts greatly received. Many thanks. D.

Answer 1

To make recursion work with promises, you generally want to chain all the recursive promises to their callers. To do that, you MUST return any promises from your .then() handlers so that they are chained to the originals. This also tends to eliminate your promise anti-pattern of wrapping an existing promise with a manually created promise which is fraught with problems. Here's one way of doing that:

function loadReports(personEmail, list) {
    return getAccessTokenPromise().then(access_token => {
        list.push(personEmail);
        return makeRequest(personEmail, access_token);
    }).then(result => {
        if (result.value.length > 0) {
            return Promise.all(result.value.map(person => {
                return loadReports(person.userPrincipalName, list);
            }));
        }
    });
}

Changes made:

1. Add return before getAccessTokenPromise() so we're returning the initial promise. This also lets us eliminate the new Promise() and all the manual reject and resolve that was involved in the anti-pattern.

2. Add return before the recursive loadReports(). This has to be done to allow the .map() to collect that promise before it passed it to Promise.all().

3. Add return before the Promise.all() so it is chained to the original promise chain.

---

You will have to make sure that you can never get any sort of circularity in the database data (an error in the DB that creates a circular list of reports). A reports to B, B reports to C, C reports to A because if you did have this, then this code would go on forever and never complete (probably eventually exhausting some system resource).

If this were my code, I might create a Set of all people visited in the database as I go and refuse to call loadReports() on any person who we'd already visited before. This would make it safe from circularity. You would probably also want to log() if you saw that condition because it would probably be a database error/corruption.