Race condition issue?

[jamesplease]

Hi there – awesome project!

I'm wondering if there's a small bug around this code.

When the if statement matches, an attempt is made to update the user before passing it along to done. However, the call to done immediately after the if statement will likely always resolve before the update to the user completes.

This probably wouldn't affect some applications, because the update call is getting instantiated. But the middleware is likely moving onto to the next middleware a little earlier than expected.

Another consequence of this is that done will get called twice, which I'm pretty sure goes against the contract of how done is intended to be used.

I'm thinking it should instead be:

if (!user.facebook.token) {
  user.facebook.token = token;
  user.facebook.name  = profile.name.givenName + ' ' + profile.name.familyName;
  user.facebook.email = profile.emails[0].value;

  user.save(function(err) {
    if (err)
      throw err;
    return done(null, user);
  });
} else {
  return done(null, user); // user found, return that user
}

This issue likely affects the other services' code, too. What do you think?