Rejected Promise breaks loop despite catch

Question

I am creating a loop to create/update users using functions with built-in promises:

for (const user of usersjson.users) {
 let getuser = getUser(url, okapikey, user[fieldMap.externalSystemId], 
 'externalSystemId'); //Check if user exists on the server
 await getuser
  .then(async (data) => {
    if (data.users.length != 0) { //If user exists in the array
      update = updateUser(url, okapikey, createduser, data.users[0].id);//Create update function
      promises.push(update); //Store function in array
      i++;
    } else {
      create = createNewUser(url, okapikey, createduser);//Create create function 
      promises.push(create); //Store function in array
      i++;
    }
  }).catch((err) => {
    console.error(err);
  });
  if (promises.length == 50 || i == usersjson.users.length) {//Run functions in batches of 50
     await Promise.allSettled(promises)
     .then((responses)=> {
       for (const response of responses) { //For each promise response
         if (response.status == 'fulfilled') { //If fulfilled
           if (response.value.status == 204) {
             console.log(`${response.value.status}: User ${response.value.request.path.substring(7)} was updated.`);
           } else {
             if (response.value.status == 201 && response.value.headers.location) {
               console.log(`${response.value.status}: User ${response.value.headers['location']} was created.`);
             } else {
               console.log(response.value.headers.location);
             }
           }
         } else { //Handle rejections
           console.log(`There was an error with the user:${response.value}`);
         }
       }
     }).catch((err)=> {
       console.log(err);
     });
     promises=[]; //Empty Promise array 
   }
}

async function updateUser(url, token, user, userid)
{
    return new Promise((resolve, reject) => {
        //Create headers for put request
        const options = {
            method: "put",
            headers: {
            'x-okapi-token': token,
            'x-okapi-tenant':'tenant',
            'Content-type':"application/json"
            }
        };
        //Make API get call
        user.id=userid; //Adding the required field ID to the JSON
        axios.put(`${url}/users/${userid}`, JSON.stringify(user), options)
            .then(response => {
              if (response.status == 204) {
                resolve(response);
              } else {
                reject(`Error Code: ${err.response.status}\nError Text: ${err.response.data.errors[0].message}\nError Status: ${err}`);
              }
            }).catch((err) => {
            console.error(`Error Code: ${err.response.status}`);
            if (typeof err.response.data == 'string') {
                console.error(err.response.data);
                reject(`Error Code: ${err.response.status}\nError Text: ${err.response.data.errors[0].message}\nError Status: ${err}`);
            } else if (err.response.data.errors[0].message) {
                console.error(`Error Text: ${err.response.data.errors[0].message}`);
                reject(`Error Code: ${err.response.status}\nError Text: ${err.response.data.errors[0].message}\nError Status: ${err}`);
            } else {
              reject(`Error Code: ${err.response.status}\nError Text: ${err.response.data.errors[0].message}\nError Status: ${err}`);
            }
            console.log(err.response);
            });
    });
};

async function createNewUser (url, token, user) {
    return new Promise((resolve, reject) => {
        //Create headers for put request
        const options = {
            headers: {
            'X-Okapi-token': token,
            'Content-type':"application/json"
            }
        };
        //Make API get call
        axios.post(`${url}/users`, JSON.stringify(user), options)
            .then(response => {
            if (response.status == 201) {
              resolve(response);
            } else {
              reject(`Error Code: ${err.response.status}: ${user.externalSystemId},\nError Text: ${err.response.data.errors[0].message},\nError Status: ${err}`)
            }
            }).catch((err) => {
            console.error(`Error on ${user.externalSystemId}: ${err}`);
            if (err.response.data && typeof err.response.data == 'string') {
                console.error(err.response.data);
                reject(`Error Code: ${err.response.status}: ${user.externalSystemId},\nError Text: ${err.response.data.errors[0].message},\nError Status: ${err}`)
            } else if (err.response.data.errors[0].message) {
                console.error(`Error Text: ${err.response.data.errors[0].message}`);
                reject(`Error Code: ${err.response.status}: ${user.externalSystemId},\nError Text: ${err.response.data.errors[0].message},\nError Status: ${err}`)
            } else {
              reject(`Error Code: ${err.response.status}: ${user.externalSystemId},\nError Text: ${err.response.data.errors[0].message},\nError Status: ${err}`)
            }
            });
    });
};

const getUsers = (url,user,password) =>
{
return new Promise((resolve, reject) => {
  //Create headers for POST request
  const options = {
    method: 'post',
    headers: {
       'Authorization': 'Basic '+Buffer.from(`${user}:${password}`).toString('base64')
    }
  }
  //Make API get call
  axios.get(url, options)
    .then(response => {
      resolve(response.data);
    }).catch((err) => {
      console.error(err);
      reject(err);
    });
});
};

The code and loop works fine when every promise is fulfilled, but once a promise is rejected, the loop breaks. I get the error message, for example:

Error on XXX: Error: Request failed with status code 422 Error Text: User with this username already exists node:internal/process/promises:246 triggerUncaughtException(err, true /* fromPromise */); ^

[UnhandledPromiseRejection: 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(). The promise rejected with the reason "Error Code: 422: XXX, Error Text: User with this username already exists, Error Status: Error: Request failed with status code 422".] { }

Looking at the code and error, I believe this comes from the "createNewUser" function. I'm not sure why the code breaks - I added catches to all the functions, handled rejections, and added catch statements in the main body of the code, but the loop still breaks.

What I need is for the loop to continue as usual even if one function fails (I will later change the log from console.log to an actual log file).

Btw, avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) in your `updateUser`/`createNewUser`/`getUsers` functions! — Bergi, May 24 '21 at 12:38
you misuse/misunderstand `await`. `let getUser = getUser(...); await getUser.then(data => ...)` is the same as `let data = await getUser(...); ...` try not to mix `await` with `.then` in situations like this — Mulan, May 24 '21 at 13:01
@Bergi Since you helped here and the code I implemented was your suggested changes, I thought maybe you could help with a strange issue caused by it. It didn't happen in the previous version, only once I changed the array to functions as you showed me: https://stackoverflow.com/questions/67677866/repeating-values-passed-to-function-in-loop — Nimrod Yanai, May 24 '21 at 21:12

Bergi · Accepted Answer · 2021-05-24T12:35:40.827

update = updateUser(url, okapikey, createduser, data.users[0].id);//Create update function
promises.push(update); //Store function in array

create = createNewUser(url, okapikey, createduser);//Create create function 
promises.push(create); //Store function in array

This is inaccurate. You are not storing functions in that array, you actually call the updateUser/createNewUser functions here and store the resulting promises in the array. Then, your loop goes on to sequentially (because of the await) do more getUser operations before actually calling Promise.allSettled on the promises array. In the meantime, some of the promises might already have been rejected without having any handlers attached to them.

This is basically the same problem as discussed in Waiting for more than one concurrent await operation and Any difference between await Promise.all() and multiple await?.

To fix it, collect actual functions that you can execute later in your array:

let functions = [];
for (const user of usersjson.users) {
  i++;
  try {
    const data = await getUser(url, okapikey, user[fieldMap.externalSystemId], 'externalSystemId');
    if (data.users.length != 0) {
      functions.push(() =>
//                   ^^^^^
        updateUser(url, okapikey, createduser, data.users[0].id)
      ); // Create update function and store it in array
    } else {
      functions.push(() =>
//                   ^^^^^
        createNewUser(url, okapikey, createduser)
      ); // Create create function and store it in array
    }
  } catch(err) {
    console.error(err);
  }
  if (functions.length == 50 || i == usersjson.users.length) { // in batches of 50
    const promises = functions.map(fn => fn()); // Run functions
//  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    const responses = await Promise.allSettled(promises);
    for (const response of responses) {
      if (response.status == 'fulfilled') {
        if (response.value.status == 204) {
          console.log(`${response.value.status}: User ${response.value.request.path.substring(7)} was updated.`);
        } else {
          if (response.value.status == 201 && response.value.headers.location) {
            console.log(`${response.value.status}: User ${response.value.headers['location']} was created.`);
          } else {
            console.log(response.value.headers.location);
          }
        }
      } else {
        console.log(`There was an error with the user:${response.value}`);
      }
    }
    functions = []; // empty functions array 
  }
}

(I've tried to avoid awaiting any .then(…) chains)

Thanks! I'll try and do that right now and see if it works! But before I do, I am a bit confused at your comment about Promise constructor antipatterns. I looked at the answers in the thread you posted but the promises there are much simpler than mine and I can't figure out what is wrong with my functions. I seem to have a reject for every situation or error. — Nimrod Yanai, May 24 '21 at 13:49
@NimrodYanai While you seem to have handled all cases correctly (always calling either `resolve` or `reject`), you can simplify them a lot by just using `return` and `throw` in the `then`/`catch` handlers, without any `new Promise`. — Bergi, May 24 '21 at 13:57
Well, I don't know why, but this caused the functions to stop working. I made the changes as you suggested, and now the update function breaks even though I never touched it. The good news is - the code indeed continues to run even though some of the promises are rejected, so I'm going to call this one solved! Thanks! — Nimrod Yanai, May 24 '21 at 16:42

Rejected Promise breaks loop despite catch

1 Answers1