CODEWITHSUNDEEP
javascriptnode.js
Jis0
Jis0

Reputation: 91

Node.js module error handling

I've been struggling last week with error handling, especially error handling in Node.js modules. So first, here is my code:

user.js route

router.post('/register', function(req, res) {
    var newUser = new User({
        firstname: req.body.firstname,
        lastname: req.body.lastname,
        email: req.body.email,
        password: req.body.password,
    });

    User.addUser(newUser, function(err, user) {
        if(err) {
            return next(err)
        } else if(user) {
            res.status(403).send('User already exists');
        } else {
            res.sendStatus(200);
        }
    });
});

user.js module

module.exports.addUser = function(newUser, cb) {
  User.findOne({ email: newUser.email }, function(err, user) {
    if(err) {
      cb(err);
    } else if(user) {
      cb(null, user);
    } else {
      bcrypt.genSalt(10, function(err, salt) {
        if(err) {
          cb(err);
        } else {
          bcrypt.hash(newUser.password, salt, function(err, hash) {
            if(err) {
              cb(err)
            } else {
              newUser.password = hash;

              newUser.save(function(err, newUser) {
                if(err) {
                  cb(err);
                } else {
                  cb(null, false);
                }
              });
            }
          });
        }
      });
    }
  });
}

Everytime if there is error inside user.js module, call callback function and handle error inside user.js route. This works, but that mess inside my module doesn't look good, because there is so many if-else statements..

Is there better approach, or do I have to check everytime if there is error?

Upvotes: 0

Views: 73

Answers (1)

Hammerbot
Hammerbot

Reputation: 16344

You could simplify your code to something like:

module.exports.addUser = function(newUser, cb) {
  User.findOne({ email: newUser.email }, function(err, user) {
    if(err) {
      cb(err);
      return;
    } 

    if(user) {
      cb(null, user);
      return ;
    } 

    bcrypt.genSalt(10, function(err, salt) {
      if(err) {
        cb(err);
        return;
      } 

      bcrypt.hash(newUser.password, salt, function(err, hash) {
        if(err) {
          cb(err)
          return;
        } 

        newUser.password = hash;

        newUser.save(function(err, newUser) {
          if(err) {
            cb(err);
            return;
          } 

          cb(null, false);
        });
      });
    });
  });
}

However, if I were you and as @Scimonster stated in his comment, this is a typical use case for promises that would allow you to write more readable code and avoid the callback hell

Upvotes: 1

Related Questions