NodeJS Asynchronous

Question

I am looking for ideas/help to improve my code. It's already working, but I am not confident with it and not really proud of it. This is short version of my function -

module.exports.serverlist = async () => {
let promises = [];
const serverlist = [];
serverlist.push({ mon_sid: 'AAA', mon_hostname: 'aaaa.com', mon_port: 80 })
serverlist.push({ mon_sid: 'BBB', mon_hostname: 'bbbb.com', mon_port: 80 })

serverlist.forEach(async (Server) => {
    if (Server.mon_sid.includes('_DB')) {
        // Function home.checkOracleDatabase return promise, same as above functions
        promises.push(home.checkOracleDatabase(Server.mon_hostname, Server.mon_port));
    } else if (Server.mon_sid.includes('_HDB')) {
        promises.push(home.checkHANADatabase(Server.mon_hostname, Server.mon_port));
    } else {
        promises.push(home.checkPort(Server.mon_port, Server.mon_hostname, 1000));
    }
})

for (let i = 0; i < serverlist.length; i++) {
    serverlist[i].status = await promises[i];
}

console.table(serverlist);

What does the code do?: - It asynchronously performing needed availability check of the service.

What is the expectation? - That the code will run asynchronously, at the end of function it will wait for all promises to be resolved, for failed results it will perform the check over again, and to have more control over the promises. At this moment the promises are not really connected somehow with the array of systems, it just base on the order but it can be problematic in future.

If someone can assist/give some advises, I would be more than happy. Also I am not sure how many parallel asynchronous operations NodeJS can perform (or the OS). Currently there are 30 systems on the list, but in future it can be 200-300, I am not sure if it can be handled at once.

Update

I used promise.all - it works fine. However the problem is as I mentioned, I don't have any control over the $promises array. The results as saved in the sequence as they were triggered, and this is how they are being assigned back to serverlist[i].status array from $promises. But I would like to have more control over it, I want to have some index, or something in the $promises array so I can make sure that the results are assigned to PROPER systems (not luckily counting that by the sequence it will be assigned as it should).

Also I would like to extend this function with option to reAttempt failed checks, and for that definitely I need some index in $promises array.

Update 2

After all of your suggestion this is how the code looks for now -

function performChecks(serverlist) {
    const Checks = serverlist.map(Server => {
        if (Server.mon_sid.includes('_DB')) {
            let DB_SID = Server.mon_sid.replace('_DB', '');
            return home.checkOracleDatabase(DB_SID, Server.mon_hostname, Server.mon_port)
        } else if (Server.mon_sid.includes('_HDB')) {
            let DB_SID = Server.mon_sid.replace('_HDB', '');
            return home.checkHANADatabase(DB_SID, Server.mon_hostname, Server.mon_port);
        } else {
            return home.checkPort(Server.mon_port, Server.mon_hostname, 1000)
        }
    })

    return Promise.allSettled(Checks)
}

// Ignore the way that the function is created, it's just for debug purpose
(async function () {
    let checkResults = [];
    let reAttempt = [];
    let reAttemptResults = [];
    const serverlist = [];

serverlist.push({ mon_id: 1, mon_sid: 'AAA', mon_hostname: 'hostname_1', mon_port: 3203 })
serverlist.push({ mon_id: 2, mon_sid: 'BBB', mon_hostname: 'hostname_2', mon_port: 3201 })
serverlist.push({ mon_id: 3, mon_sid: 'CCC', mon_hostname: 'hostname_3', mon_port: 3203 })

    // Perform first check for all servers
    checkResults = await performChecks(serverlist);

    // Combine results from check into serverlist array under status key
    for(let i = 0; i < serverlist.length; i++) {
        serverlist[i]['status'] = checkResults[i].value;
    }

    // Check for failed results and save them under reAttempt variable
    reAttempt = serverlist.filter(Server => Server.status == false);

    // Perform checks again for failed results to make sure that it wasn't temporary netowrk/script issue/lag
    // Additionally performChecks function will accept one more argument in future which will activate additional trace for reAttempt
    reAttemptResults = await performChecks(reAttempt);

    // Combine results from reAttempt checks into reAttempt array
    for(let i = 0; i < reAttempt.length; i++) {
        reAttempt[i]['status'] = reAttemptResults[i].value;
    }

    // Combine reAttempt array with serverlist array so serverlist can have latest updated data
    serverlist.map(x => Object.assign(x, reAttempt.find(y => y.mon_id == x.mon_id)));

    // View the results
    console.table(serverlist);

})();

Yak O&#39;Poe · Accepted Answer

Firstly instead of doing a for each and push promises you can map them and do a Promise all. You need no push. Your function can return directly your promise all call. The caller can await it or use then... Something like this (I didn't test it)

// serverlist declaration
function getList(serverlist) {
const operations = serverlist.map(Server => {
    if (Server.mon_sid.includes('_DB')) {
        return home.checkOracleDatabase(Server.mon_hostname, Server.mon_port);
    } else if (Server.mon_sid.includes('_HDB')) {
        return home.checkHANADatabase(Server.mon_hostname, Server.mon_port);
    } else {
        return home.checkPort(Server.mon_port, Server.mon_hostname, 1000);
    }
});

return Promise.all(operations)
}

const serverlist = [...]
const test = await getList(serverlist)

// test is an array of fulfilled/unfulfilled results of Promise.all

So I would create a function that takes a list of operations(serverList) and returns the promise all like the example above without awaiting for it. The caller would await it or using another promise all of a series of other calls to your function. Potentially you can go deeper like Inception :)

// on the caller you can go even deeper

await Promise.all([getList([...]) , getList([...]) , getList([...]) ])

Considering what you added you can return something more customized like:

    if (Server.mon_sid.includes('_DB')) {
        return home.checkOracleDatabase(Server.mon_hostname, Server.mon_port).then(result => ({result, name: 'Oracle', index:..., date:..., hostname: Server.mon_hostname, port: Server.mon_port}))

In the case above your promise would return a json with the output of your operation as result, plus few additional fields you might want to reuse to organize your data.

NodeJS Asynchronous

Update

Update 2

Answers (2)

Related Questions