2

I have a code to update or create new users in my system. It works with a forEach function, and works well. However, I want to take advantage of Node.js and execute several updates/creations at the same time. I have tried to create an array of promises, but I think I am building the promises wrong. This is the code that works fine:

import usersjson from './jsons/users.json';
if (usersjson.users.length) {
    for (const user of usersjson.users) {
        let getuser = getUser(url, user.Id, 'Id');
            getuser.then((data) => {//Make a call to get user by id
                if (data.users.length != 0) { //If user exists
                    let update = updateUser(url, user, data.users[0].id);//update user function
                    update.then((updateresponse) => {
                        console.log(updateresponse);
                        }).catch((err) => {
                            console.log(err);
                        })
                } else { //if user doesn't exist
                    let update = newUser(url, user);//createUser(settings.fieldMap, user)
                    update.then((response) => {
                        console.log(response);
                    }).catch((err) => {
                        console.log(err);
                    });
                }
            }) 
    };
};

Now, I want to make the "update" into promises, so that I can then use promise.all() to send several at the same time. This is what I tried:

import usersjson from './jsons/users.json';
let promises = [];
if (usersjson.users.length) {
    for (const user of usersjson.users) {
        if (promises.length <= 20) {
            let getuser = getUser(url, user.Id, 'Id');
            getuser.then((data) => {//Make a call to get user by id
                if (data.users.length != 0) { //If user exists
                    let update = new promise ((resolve, reject) => {
                        updateUser(url, user, data.users[0].id).then((response) => {
                            resolve(response);
                        }).catch((err) => {
                            console.log(err)
                        });
                    });
                    promises.push(update);
                } else { //if user doesn't exist
                    let update = new promise ((resolve, reject) => {
                        newUser(url, user).then((response) => {
                            resolve(response);
                        }).catch((err) => {
                            console.log(err);
                        })
                    });
                    psomises.push(update);
                }
            });
        } else {
            await promises.all(update)
            .then ((result) => {
                console.log(result);
                promises = [];
            }).catch(err){
                console.log(err);
            }
        };
    };
};

But I get an error message:

(node:22016) UnhandledPromiseRejectionWarning: ReferenceError: promise is not defined at file:///C:/xampp/htdocs/NodeAPI/server.js:70:21 at processTicksAndRejections (internal/process/task_queues.js:97:5) (node:22016) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1) (node:22016) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

My question is, how do I make the forEach run for those 20 users in the array at the same time instead of one by one?

Nimrod Yanai
  • 777
  • 2
  • 8
  • 30
  • Why are you mixing `await` with `then`? – Dai Feb 10 '21 at 16:44
  • 1
    Also, you don't need to use `.forEach` anymore. `for( of )` was added to JavaScript ages ago and it works with any iterable, including all of the types that had a `.forEach` function-property. – Dai Feb 10 '21 at 16:46
  • Because I obviously don’t know what I’m doing? :) I don’t even know why my original code needs to use promises. I was just asked to do it. – Nimrod Yanai Feb 10 '21 at 16:58
  • @Dai I updated the code to work as you suggested and it works fine. Is the "for of" loop asynchronous? Do I still need to have an array of promises if I use it? – Nimrod Yanai Feb 10 '21 at 17:33
  • Not quite. But you've somehow coaxed me into doing it for you now... hold on. – Dai Feb 10 '21 at 17:37
  • 1
    This code is massively more complicated to understand and follow than it needs to be. I'd start by rewriting it in a lot simpler way. I don't know enough about what you're trying to accomplish with this code and what some of the functions you call do to be able to suggest a massively simpler rewrite. – jfriend00 Feb 10 '21 at 18:45
  • 1
    Also, you don't "execute promises". You execute functions and some functions may return a promise which is just an object that helps you monitor an asynchronous operation. Promises don't "execute". The distinction is more than semantics as it's important in understanding what is actually happening with asynchronous operations and promises. – jfriend00 Feb 10 '21 at 18:47
  • @jfriend00 the end code will take an external JSON file and either update or create users in my system based on if the user is already there or not (if it is, it will be updated, if not it will be created). I left out the less important parts of that code out (like comparing the update date between the external JSON and my system, etc.). What about this code is too complicated? I think it's pretty simple and straight forward. – Nimrod Yanai Feb 11 '21 at 14:37
  • @jfriend00 you can see my end goal here: https://stackoverflow.com/questions/66158135/syntaxerror-unexpected-reserved-word-await-node-js-is-correct-version – Nimrod Yanai Feb 11 '21 at 15:55

2 Answers2

3

The easiest way to achieve that is in the foearch method, introduce all the promises in an array and then call them, and wait until everything is done using Promise.all.

let promiseArray = []
array.forEach(x => {
      // Do whatever you need
     promiseArray.push(updateUser(...))
     // Push only the promise without the 'then'
     // You are going to handle the response or 
     // the errors later  

})
const promiseResponses = await Promise.all(promiseArry)
// promiseResponses is an array of responses
// like promiseResponse[0] 
Samuel Urias
  • 92
  • 1
  • 3
  • Thanks! But when I tried to use this method, I get an error "SyntaxError: Unexpected reserved word" for the "await" before the "Promise.all". I can't use "await" in a "forEach" loop, which prevents me from setting up a counter (I want to push the promises about 50 at a time, so after each "push" I check the length of the array and if it's 20, I use the "Promise.all" function. – Nimrod Yanai Feb 11 '21 at 15:07
2
  1. First, move the code flow that deals with each individual user value into its own separate async function.

    • Because of how async functions actually work (as each async function marks the boundary of each async state-machine) it isn't really feasible to do concurrent-async work inside a for statement.
  2. Invoke that function from within the for loop.

  3. Always prefer === over ==.

  4. Use TypeScript. Using async JavaScript without TypeScript is a world of pain because you need to keep track of which functions return Promise<T> vs. those that don't, as well as correctly unwrapping Promise<T> objects.

  5. Always, wherever possible, have your own try/catch statement that wraps the entire body of an async function, otherwise you'll have to deal with inconsistencies with how different JS engines and JS code handles thrown exceptions and objects.

import usersjson from './jsons/users.json';

async function updateUsers() {
    
    const promises = [];

    const users = usersjson;
    for( const user of users ) {
        const promise = updateUser( user ); // <-- Note this is *NOT* awaited here.
        promises.push( promise );
    }

    await Promise.all( promises ); // <-- This is where we await everything.
}

async function updateUser( user ) {
    
    try {
        const data = await getUser( url, user.Id, 'Id' );
        if( data.users.length === 0 ) {
            let createResult = await newUser( url, user );
            console.log( "Created new user %o", createResult  );
        }
        else {
            const updateResult = await updateUser( url, user, data.users[0].id );
            console.log( "Update user %o: Result: %o", user.Id, updateResult );
        }
    }
    catch( err ) {
        console.error( "Failed to update (or create) user with UserId %o: %o", user.userId, err );
    }
}


Dai
  • 141,631
  • 28
  • 261
  • 374
  • So what if I want to do it in blocks? For example 20-50 users at a time? In that case, how can I have the await outside the for loop? – Nimrod Yanai Feb 11 '21 at 16:02
  • Ok, I managed to do this correctly by asyncing the function in which all of this is situated. Thank you so much! – Nimrod Yanai Feb 11 '21 at 16:15