Nodejs How To Iterate With a Promise

Question

I'm trying to iterate through an array of AD users and return some user information. I've been looking for a few hours now, or more and haven't been quite able to get my head around the async nature of the activedirectory2 npm package.

I'm getting part of the result I need, however when iterating through the list of usernames, I'm only getting the first one printing out to console.

getADUser.js:

var ActiveDirectory = require('activedirectory2');
var config = require('../../conf/conf-ad.json')
var fileTime = require('./w32FiletimeToEpoch')
var moment = require('moment')
// Find user, return all
var ad = new ActiveDirectory(config);



var getADUser = function (sAMAccountName, opts) {
    return new Promise(function (resolve, reject) {
        ad.findUser(opts, sAMAccountName, function (err, user) {
            if (err) {
                console.log('ERROR: ' + JSON.stringify(err));
                // return;
            }
            if (!user) {
                console.log('User: ' + sAMAccountName + ' not found.');
            } else {
                if (user.userAccountControl == 514) {
                    user.userAccountControl = 'Disabled'
                } else {
                    user.userAccountControl = 'Active'
                }
                if (user.pwdLastSet) {
                    user.pwdLastSet = `${moment(fileTime(user.pwdLastSet))} - ${moment(fileTime(user.pwdLastSet)).fromNow()}`
                }
                if (user.lastLogonTimestamp) {
                    user.lastLogonTimestamp = `${moment(fileTime(user.lastLogonTimestamp))} - ${moment(fileTime(user.lastLogonTimestamp)).fromNow()}`
                }
                if (user.lastLogon) {
                    user.lastLogon = `${moment(fileTime(user.lastLogon))} - ${moment(fileTime(user.lastLogon)).fromNow()}`
                }
                // return;
                // return user.promise();
                // console.log(user)
                // test.push(user)
                resolve(JSON.stringify(user));
            }
        });
    })
}

module.exports = getADUser

checkADCompletions.js:

var checks = ['USERONE', 'USERTWO']

let opts = {
    attributes: ['sAMAccountName', 'userAccountControl']
};
let checkADCompletions = function (userList) {
    let data = []
    return new Promise(function (resolve, reject) {
        return new Promise(function (res, rej) {
            for (let i = 0; i < userList.length; i++) {
                getADUser(userList[i], opts)
                    .then(function (s) {
                        data.push(s)
                    }).then(function () {
                        resolve(data)
                    })
            }
        })
    })
}

checkADCompletions(checks).then(function (d) {
    console.log(d) \\ Only prints the first user details
})

I'm looking at using `.map()` now, but no idea about `Promise.all()`. I'm very new to async... — jERCle, Oct 20 '17 at 04:53
See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all, https://stackoverflow.com/questions/38806097/asynchronously-solution-to-check-data-from-database-kinds-of-loop-cluase/ — guest271314, Oct 20 '17 at 04:57
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! The `new Promise`s in your `checkADCompletions` function don't make any sense. — Bergi, Oct 20 '17 at 05:16

Maria Ines Parnisari · Accepted Answer · 2017-10-23T05:00:09.267

You can use Promise.all like this:

let checkADCompletions = function (userList) {
    var promises = userList.map(function (user) {
        return getADUser(user, opts);
    })

    return Promise.all(promises);
}

You are basically creating an array of promises and then executing them all concurrently.

And then use it like so:

checkADCompletions(checks)
.then(function (responses) {
    console.log(responses); // this will be an array
})
.catch(function (err) {
    // if any of the promises fail, it will enter here.
    // err will be the value of the rejected promise
})

Promise.all will fail even if just one of the checked users fail. So, you need to handle errors nicely, i.e. deal with any possible outcome of ad.findUser:

var getADUser = function (sAMAccountName, opts) {
    return new Promise(function (resolve, reject) {
        ad.findUser(opts, sAMAccountName, function (err, user) {
            if (err || user == null) {
                console.log('ERROR: ' + JSON.stringify(err));
                reject(err);
            }
            if (user.userAccountControl == 514) {
                user.userAccountControl = 'Disabled'
            } else {
                user.userAccountControl = 'Active'
            }
            if (user.pwdLastSet) {
                user.pwdLastSet = `${moment(fileTime(user.pwdLastSet))} - ${moment(fileTime(user.pwdLastSet)).fromNow()}`
            }
            if (user.lastLogonTimestamp) {
                user.lastLogonTimestamp = `${moment(fileTime(user.lastLogonTimestamp))} - ${moment(fileTime(user.lastLogonTimestamp)).fromNow()}`
            }
            if (user.lastLogon) {
                user.lastLogon = `${moment(fileTime(user.lastLogon))} - ${moment(fileTime(user.lastLogon)).fromNow()}`
            }
            resolve(user);
        });
    })
}

Thank you for the detailed explanation! Makes much more sense now :) — jERCle, Oct 21 '17 at 10:41
I've obviously done something wrong because it just returns the first user multiple times. Will keep looking into it. Thanky ou! — jERCle, Oct 23 '17 at 03:44
Yeah, doing it that way runs the same user twice. Quite confusing indeed. — jERCle, Oct 23 '17 at 04:04
Oh, i think it's because the function should have `return getADUser(user, opts);` instead of `return getADUser(userList[i], opts);`. Sorry about that — Maria Ines Parnisari, Oct 23 '17 at 04:59
Nah, I spotted that :) Figured it out in the end though!! :D will post it later — jERCle, Oct 25 '17 at 09:35

score 0 · Answer 2 · answered Oct 20 '17 at 05:43

A simple fix would be to call resolve only when the for loop is finished.

// checkADCompletions.js
var checks = ['USERONE', 'USERTWO']

let opts = {
    attributes: ['sAMAccountName', 'userAccountControl']
};
let checkADCompletions = function (userList) {
    let data = []
    return new Promise(function (resolve, reject) {
        for (let i = 0; i < userList.length; i++) {
            getADUser(userList[i], opts)
                .then(function (s) {
                    data.push(s)
                }).then(function () {
                    if (i === userList.length) {
                        resolve(data)
                    }
                })
            }
        })
    })
}

checkADCompletions(checks).then(function (d) {
    console.log(d)
})

score -1 · Answer 3 · answered Oct 20 '17 at 05:18

-1

When you call resolve, you close out the Promise. You're initiating two Promises and then a for loop, and you call resolve inside the for loop. So the first user gets populated but the for loop does not continue because the Promise has finished executing. Move resolve outside of the loop and you should be good to go.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/resolve

answered Oct 20 '17 at 05:18

user5369036

13
3

No, moving `resolve` outside of the loop doesn't help anything at all. That'll only lead to `data` be empty instead of containing the only the first element. – Bergi Oct 20 '17 at 05:21
Thanks for that @Bergi - This was something I had tried before and was wondering why I was getting that result. – jERCle Oct 21 '17 at 10:43

Nodejs How To Iterate With a Promise

3 Answers3