2

I have a User controller that has a create method that checks the database for email and username uniqueness before creating the user (this is to work-around a bug in the mongodb adpater for SailsJS that doesn't honour the unique attribute flag - version 0.10.5).

The code looks like the following:

User.find({ email: req.body.email }, function (err, user) {
  if(user) {
    return res.badRequest('Unique email constraint. Email is already used.');
  }
});

User.create(req.body).exec(function (err, user) {
// Code to catch and manage err or new user
}

What I expect is that if the email already exists in the database (mongodb), to send a 400 using res.badRequest(), then execution to end.

What happens is that the response is sent, but then control moves to User.create() - execution doesn't end. I suspect that return res.badRequest is returning control back to the calling function (User.findOne), and execution continues from there.

I tried using res.badRequest().end() but that leaves the client hanging (there is no response), and using res.end() after the return res.badRequest() generated 'header send' errors.

How do I have execution of this request end if an existing email is found?

onblur
  • 365
  • 1
  • 5
  • 16

1 Answers1

2

First of all, your findOne is here a find. That's not related to your problem, but it is slightly confusing, and you should ensure you are getting data in the format you expect.

As for finishing the request after marking it bad, I have not used sails, but I was able to end execution in the past by using res.send(). EDIT: after looking at the docs, it seems this is done for you by .badRequest(), so ignore that part.

That said, even THAT is not actually your problem. Your problem is that you start an asynchronous User.find(), and then you immediately start running User.create() (also asynchronously), and so your request doesn't get marked bad until after you have already attempted to create a new user.

What you need to do is one of two things:

  1. Use promises (NOTE: this is how it works for Mongoose; Sails may be different) to only run User.create() after User.find() has completed. e.g;

    var userQuery = User.findOne({ email: req.body.email }).exec();
    userQuery.addBack(function(err, user) {
        if(!!user) res.badRequest('...');
        else create_user();
    });
    
  2. Put your user creation logic inside of your findOne block. e.g.;

    User.findOne({ email: req.body.email }, function(err, user) {
        if (user) { // or perhaps you want if (!err)
            User.create(...);
        } else {
            // handle error
        }
    });
    

Personally, I would advise that you use promises (especially later, when you have long chains of requests happening one on top of the other), but take your pick.

Jordan
  • 166
  • 7
  • 2
    Promises are already built into Waterline (the Sails ORM), currently using [Q](https://github.com/kriskowal/q), so you can do `User.findOne(...).then(...)` and `User.findOne(...).catch(...)`. *But* in most cases, errors are not going to be thrown by queries unless something bad happens at the database level, so even using promises you're always going to be putting all your logic in the "then" handler. Your second suggestion is completely valid, though. – sgress454 Oct 01 '14 at 20:31
  • Thanks, Jordan. I'll try this out - I suspect you're spot-on. – onblur Oct 01 '14 at 20:32
  • @sgress454, I didn't know about Waterline. The above is correct for Mongoose, however -- I should provide that caveat in the answer. – Jordan Oct 01 '14 at 21:07
  • Just tried option 2 out and is now working as intended. Also, I had changed User.find() from .findOne() when messing around to troubleshoot this, but forgot to set it back - it does need to be findOne (though, I guess you could use find() then check the length of the array but findOne works fine). – onblur Oct 02 '14 at 01:32