Simplify query checks in Node.js+mongoose

Question

I'm writing a really simple RESTful API using Node.js, restify and mongoose but the code is getting complex and long. This is caused by all the checks that I have to do every time I make a query. For example, the next code show the controller for the endpoint POST /users/:user1/follow/:user2.

exports.followUser = function(req, res, next) {
  User.findOne({ username: req.params.user1 }, function(err, user1) {
    if (err) {
      res.status(500);
      res.json({ type: false, data: 'Error occurred: ' + err });
    } else {
      if (user1) {
        User.findOne({ username: req.params.user2 }, function(err, user2) {
          if (err) {
            res.status(500);
            res.json({ type: false, data: 'Error occurred: ' + err });
          } else {
            if (user2) {
              user2.followers.push(user1);
              user2.save();
              res.json({
                type: true,
                data: 'User ' + req.params.user1 + ' is now following user ' + req.params.user2
              });
            } else {
              res.json({
                type: false,
                data: 'User ' + req.params.user2 + ' not found'
              });
            }
          }
        });
      } else {
        res.json({
          type: false,
          data: 'User ' + req.params.user1 + ' not found'
        });
      }
    }
  });
};

How can I simplify this code? As you can see I'm checking the result of each findOne to know if the users received exist and I'm also returning json answers for each error but this is tedious. Is there a way to automatize this?

Answer 1

Promises help make the code a bit more readable, I see two other ways to simplify your code. You'll shave off a few lines and perhaps gain some legibility by handling errors using the built in restify error types

And secondly you could just update user2 instead of fetching him/her and simplify your if statements. Plus you gain an extra error check that you're missing now on the save. I came up with something like this:

exports.followUser = function(req, res, next) {
    User.findOne({username: req.params.user1}, function (err, user1) {

        next.ifError(err);

        if (!user1) 
            return next(new restify.NotFoundError('User ' + req.params.user1 + ' not found'));


        User.update({username: req.params.user2}, {$push: {followers: user1}}, function (err, updated) {

            next.ifError(err);

            if(!updated)
                return next(new restify.NotFoundError('User ' + req.params.user2 + ' not found'));

            return res.send({
                type: true,
                data: 'User ' + req.params.user1 + ' is now following user ' + req.params.user2
            });
            next();

        });

    });
};

if you want to keep your convention of returning a 200 response with extra information when a user isn't found you could create a custom error class:

var restify = require('restify');
var util = require('util');

function MyError(message) {
  restify.RestError.call(this, {
    restCode: 'MyError',
    statusCode: 200,
    message: message,
    constructorOpt: MyError
  });
  this.name = 'MyError';
};
util.inherits(MyError, restify.RestError);

And then create a global handler for that error as per the docs so you can return the json formatted the way you want.

Answer 2

You could use Mongoose's promises to clean it up. Something like:

exports.followUser = function(req, res, next) {
  User.findOne({
    username: req.params.user1
  }).exec().then(function(user1) {
    if (user1 === null) {
      throw new Error('User ' + req.params.user1 + 'not found');
    }

    return User.findOneAndUpdate({
      username: req.params.user2
    }, {$push: {followers: user1}}).exec();
  }).then(function(user2) {
    if (user2 === null) {
      throw new Error('User ' + req.params.user2 + ' not found');
    }

    res.json({
      type: true,
      data: 'User ' + req.params.user1 + ' is now following user ' + req.params.user2
    });
  }, function(err) {
    res.status(500);
    res.json({ type: false, data: 'Error occurred: ' + err });
  });
};