Mongoose promise & error handling

Question

I search for rails-like way of writing NodeJS code and it's more then hard to find the right way. The code quickly become bloated and unreadable. I would like my code to be as clean as possible.

Example (common ExpressJS middleware API controller):

var User = require('../models/user'); // mongoose model

exports.updateUser = function(req, res, next) {
  User.passwordToSecretWithBcrypt(req.body.password, function(err, secret) {
    if (err) return next(err);

    User.findById(req.user.id).exec(function(err, user) {
      if (err) return next(err);
      if (!user) return next(new Error('no user found'));

      if (req.body.name) user.name = req.body.name;
      if (req.body.email) user.email = req.body.email;
      if (secret) user.secret = secret;
      user.save(function(err, user) {
        if (err) return next(err);
        if (!user) return next(new Error('user not saved'));

        res.json(user);
      });
    });
  });
};

The main question is the error handling. A simple record update in the example above has 5 error handling statements. That's crazy in my opinion and you never know exactly what kind of error you'll get so it's very hard to construct the JSON error response that is shown to a user.

Is there a better way for handling errors?

I also tried to use promises but I couldn't find a best-practice example that would show me the whole example as a chain of the find, save, create and delete Mongoose methods.

How can I convert the example above to use promises?

Would promises fix the error handling? If so, how (question1)?

Answer 1

Interesting question,

I will not comment on making this more rails-like, I have no clue there. But I can observe that you in essence do 3 things: so you could perhaps group your error handling into 3 locations.

Furthermore, I want to point out that your code handles some error situations silently, that's never a good idea.

Other than that the code is very straightforward and easy enough to follow.

For my counter proposal I would start with a function that takes a request and checks for mandatory fields:

function validateMandatoryRequestFields( req , fields ){
  for( var i = 0 ; i < fields.length ; i++){
    if( !req.body[fields[i]] ){
      return fields[i] + ' was not provided';
    }
  }
  return '';
}

So that your function can do something like this in the beginning:

exports.updateUser = function(req, res, next) {
  //Do we have all we need ?
  var mandatoryFields = [ 'password', 'name' , 'email' ],
    errorMessage = validateMandatoryRequestFields( req , mandatoryFields );
  //Do we have what we need ?
  if(errorMessage){
    return next(errorMessage);
  }

You should always make sure that the request has what you need upfront.

Then, for callbacks I tend to mangle the err handling and bogus data handling into 1 if statement with 1 return statement:

  //Encrypt password
  User.passwordToSecretWithBcrypt(req.body.password, function(err, secret) {
    if (err || !secret ) {
      return next(err || new Error('Password encryption failed') );
    }

If you keep applying this to all of the code, you end up with something like this:

function validateMandatoryRequestFields( req , fields ){
  for( var i = 0 ; i < fields.length ; i++){
    if( !req.body[fields[i]] ){
      return fields[i] + ' was not provided';
    }
  }
  return '';
}    

exports.updateUser = function(req, res, next) {
  //Do we have all we need ?
  var mandatoryFields = [ 'password', 'name' , 'email' ],
    errorMessage = validateMandatoryRequestFields( req , mandatoryFields );
  //Do we have what we need ?
  if(errorMessage){
    return next(errorMessage);
  }
  //Encrypt password
  User.passwordToSecretWithBcrypt(req.body.password, function(err, secret) {
    if (err || !secret ) {
      return next(err || new Error('Password encryption failed') );
    }
    //Find user
    User.findById(req.user.id).exec(function(err, user) {
      if (err || !user){
        return next(err || new Error('no user found'));
      }
      //Update user
      user.save(function(err, user) {
        if (err || !user) {
          return next(err || new Error('user not saved') );
        }
        res.json(user);
      });
    });
  });
};