Deprecate isAsync for validators

[lorixx]

Do you want to request a feature or report a bug?

Report a bug.

What is the current behavior?

I have some simple code that is trying to save a user into db:

/**
 * Create user
 */

exports.create = function (req, res) {
  const user = new User(req.body);
  user.provider = 'local';

  user.save(function(err, user) {
    req.logIn(user, err => {
      if (err) req.flash('info', 'Sorry! We are not able to log you in!');
      return res.redirect('/');
    });
  });
};

However, the callback is never called, I am on Nodejs v8.9.4, MongoDB 4.0 and Mongoose 5.2.2

If the current behavior is a bug, please provide the steps to reproduce.

I uploaded the full code in my github repo here: https://github.com/lorixx/node-express-mongoose-demo

The issue is reproducible by just registering a new user after running the web app.

See the above line 369 is NEVER getting called at all.

Instead, after I step-in during debugging, we get into this Kareem codepath then nothing else happen after I step-out.

We can reproduce this by signing up a new user here from the above screen, the request is hang and never got completed.

What is the expected behavior?

The user should be saved successfully with the right callback.

Please mention your node.js, mongoose and MongoDB version.

I am on Nodejs v8.9.4, MongoDB 4.0 and Mongoose 5.2.2.
The initial startup connection to MongoDB also works fine, see the printout below from console:

/usr/local/bin/node --inspect-brk=24906 server.js 
Debugger listening on ws://127.0.0.1:24906/3377d318-65f1-4c0c-b2d2-194a648f4c11
Debugger attached.
Express app started on port 3000

Thanks!

[lineus]

Hi @lorixx 4 of your 5 validators have isAsync: true but aren't async. I checked out your code and removed isAsync: true from all of your validators except the 2nd email validator and was able to successfully create a user.

Here are the relevant logs

Express app started on port 3000
  router dispatching POST /users +11s
  router compression  : /users +1ms
  router corsMiddleware  : /users +1ms
  router serveStatic  : /users +1ms
  router logger  : /users +0ms
  router <anonymous>  : /users +2ms
  router jsonParser  : /users +0ms
  body-parser:json content-type "application/x-www-form-urlencoded" +0ms
  body-parser:json skip parsing +1ms
  router urlencodedParser  : /users +0ms
  body-parser:urlencoded content-type "application/x-www-form-urlencoded" +0ms
  body-parser:urlencoded content-encoding "identity" +1ms
  body-parser:urlencoded read body +0ms
  body-parser:urlencoded parse body +13ms
  body-parser:urlencoded parse extended urlencoding +0ms
  router multerMiddleware  : /users +2ms
  router methodOverride  : /users +1ms
  router cookieParser  : /users +0ms
  router _cookieSession  : /users +0ms
  router session  : /users +1ms
  cookie-session parse eyJmbGFzaCI6e30sImNzcmZTZWNyZXQiOiJoRXQxeXZtTWRseUhaM2ZZbUxVMDFodm0iLCJyZXR1cm5UbyI6Ii9hcnRpY2xlcy9uZXciLCJwYXNzcG9ydCI6e319 +11s
  router initialize  : /users +2ms
  router authenticate  : /users +0ms
  router <anonymous>  : /users +1ms
  router <anonymous>  : /users +0ms
  router csrf  : /users +1ms
  router <anonymous>  : /users +0ms
Mongoose: users.find({ email: 'lksjdlfkj@lskdjf.sll' }, { fields: {} })
Mongoose: users.insertOne({ name: 'lskdjfls lsjflskjf', email: 'lksjdlfkj@lskdjf.sll', username: 'sldkfjlskjfdlskdjflskj', provider: 'local', hashed_password: 'b09732ac963e02918f7cbb666be0e788ed18cdec', salt: '1416256198917', authToken: '', _id: ObjectId("5b45dd94b09d3b1d8cafb942"), __v: 0 })
  cookie-session save eyJmbGFzaCI6e30sImNzcmZTZWNyZXQiOiJoRXQxeXZtTWRseUhaM2ZZbUxVMDFodm0iLCJyZXR1cm5UbyI6Ii9hcnRpY2xlcy9uZXciLCJwYXNzcG9ydCI6eyJ1c2VyIjoiNWI0NWRkOTRiMDlkM2IxZDhjYWZiOTQyIn19 +46ms
  compression no compression: size below threshold +45ms
  morgan log request +4ms
POST /users 302 68.711 ms - 46

[lineus]

Including a diff for clarity

6700>: git diff -w app/models/user.js
diff --git a/app/models/user.js b/app/models/user.js
index 6154339..1bfce4c 100644
--- a/app/models/user.js
+++ b/app/models/user.js
@@ -59,7 +59,6 @@ UserSchema
 // the below 5 validations only apply if you are signing up traditionally

 UserSchema.path('name').validate({
-  isAsync: true,
   validator: function (name) {
     if (this.skipValidation()) return true;
     return name.length;
@@ -68,7 +67,6 @@ UserSchema.path('name').validate({
 });

 UserSchema.path('email').validate({
-  isAsync: true,
   validator: function (email) {
     if (this.skipValidation()) return true;
     return email.length;
@@ -95,7 +93,6 @@ UserSchema.path('email').validate({
 });

 UserSchema.path('username').validate({
-  isAsync: true,
   validator: function (username) {
     if (this.skipValidation()) return true;
     return username.length;
@@ -104,7 +101,6 @@ UserSchema.path('username').validate({
 });

 UserSchema.path('hashed_password').validate({
-  isAsync: true,
   validator: function (hashed_password) {
     if (this.skipValidation()) return true;
     return hashed_password.length && this._password.length;
6700>:

[lorixx]

Thank you so much @lineus ! Do you know why the async did not work the way I expected? I tried update the controllers/user.js in the controller level, but it still doesn't work with the async either:

exports.create = async(function* (req, res) {
  const user = new User(req.body);
  user.provider = 'local';

  user.save(function(err, user) {
    req.logIn(user, err => {
      if (err) req.flash('info', 'Sorry! We are not able to log you in!');
      return res.redirect('/');
    });
  });
});

Thanks again for your kind reply!

[lineus]

You're welcome @lorixx ! The isAsync property on the object passed into .validate() on the schema paths refers to whether or not the validator function is async. The 4 validate objects that I changed had validator functions that neither returned a promise or a callback, meaning that the validator functions themselves are synchronous operations.

[lorixx]

@lineus Thanks! Now I understand why!!

Actually from this wiki: http://mongoosejs.com/docs/validation.html#async-custom-validators
It explained about how this isAsync should be used, I guess my original thought was that the validate function will be executed asynchronously. Guess I am wrong.

On a follow-up note thou, I wish we can have a more explicit error to inform this misuse.
Thank you Kev!!

[vkarpov15]

In general, we should really deprecate isAsync. Mongoose supports returning promises from middleware without isAsync, isAsync is just for callbacks.