CODEWITHSUNDEEP
javascriptpromisees6-promise
Sam Munroe
Sam Munroe

Reputation: 1418

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');
  })

Upvotes: 1

Views: 166

Answers (2)

T.J. Crowder
T.J. Crowder

Reputation: 1075875

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...)

Upvotes: 4

raina77ow
raina77ow

Reputation: 106483

See, so called 'executor function', passed into Promise constructor, is invoked immediately. That's why you essentially have a race condition here between two remote procedure calls. To solve this, make the update responsible for promise creation instead:

function updateUser(userId, userInfo) {
  return new Promise(function(resolve, reject) {
    User.update({_id: userId}, userInfo, function(err, result) {
      if (err) { 
        reject(err);
      }
      else {
        resolve(result);
      }
    });
  });
}

... and call this function in then(). By doing this, an executor function will be called only when updateUser is invoked - and that'll be after createOrganization() finished its job.

Upvotes: 1

Related Questions