Search code examples
javascriptpromisees6-promise

JavaScript promise not firing in order


I have been trying to effectively manage how I build my promises in my Express.js app.

Right now I have the following scenario: During the signup process, a user has an optional Organization Name field. If this is filled in, I need to create the Organization object and then add the _id of it to the other information that I am applying to the user. If there is no Organization name, proceed and update the basic user info.

<-- Right now the user is being updated before the organization is being created. -->

//basic info passed from the signup form
info['first_name'] = req.body.first_name;
info['last_name'] = req.body.last_name;
info['flags.new_user'] = false;

//if organization name is passed, create object and store _id in user object. 
let create_organization = new Promise(function(resolve, reject) {
  if (req.body.organization_name !== "") { //check if name is sent from form
    Organization.create({
      name: req.body.organization_name
    }, function(err, result) {
      console.log(result);
      if (!err) {
        info['local.organization'] = result._id;
        resolve()
      } else {
        reject()
      }
    })
  } else {
    resolve()
  }
});

let update_user = new Promise(function(resolve, reject) {
  User.update({
    _id: req.user._id
  }, info, function(err, result) {
    if (!err) {
      console.log("Updated User!"); < --prints before Organization is created
      resolve();
    } else {
      reject();
    }
  })
});

create_organization
  .then(function() {
    return update_user;
  })
  .then(function() {
    res.redirect('/dash');
  })

Solution

  • Nothing in your code waits for the first promise to settle before proceeding with starting the subsequent work. The work is started as soon as you call User.update, which is done synchronously when you call new Promise with that code in the promise executor.

    Instead, wait to do that until the previous promise resolves. I'd do it by wrapping those functions in reusable promise-enabled wrappers (createOrganization and updateUser):

    // Reusable promise-enabled wrappers
    function createOrganization(name) {
        return new Promise(function(resolve, reject) {
            Organization.create({name: name}, function(err, result) {
                console.log(result);
                if (err) {
                    reject(err);
                } else {
                    resolve(result);
                }
            });
        });
    }
    
    function updateUser(id, info) {
        return new Promise(function(resolve, reject) {
            User.update({_id: id}, info, function(err, result) {
                if (err) {
                    reject(err);
                } else {
                    resolve();
                }
            })
        });
    }
    

    (You may be able to use util.promisify or the promisify npm module to avoid doing that manually.)

    And then:

    //basic info passed from the signup form
    info['first_name'] = req.body.first_name;
    info['last_name'] = req.body.last_name;
    info['flags.new_user'] = false;
    
    //if organization name is passed, create object and store _id in user object. 
    (req.body.organization_name === "" ? Promise.resolve() : createOrganization(req.body.organization_name))
    .then(function() {
        return updateUser(req.user._id, info);
    })
    .catch(function(error) {
        // handle/report error
    });
    

    (I stuck to ES5-level syntax since your code seemed to be doing so...)