Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

NodeJS best practices: Errors for flow control?

In Node.js, should I use errors for flow control, or should I use them more like exceptions?

I'm writing an authentication controller and some unit tests in Sails.js, and currently, my registration method checks to see if a user with the same username exists. If a user already exists with the username, my model method calls its callback arguments with a new Error object, like so:

Model:

exists: function (options, cb) {
    User.findOne({
        where: { username: typeof options === 'Object' && options.username ? options.username : options },
    }).exec(function (err, user) {
        if (err) return cb(err);
        if (user) return cb(new Error("A user with that username already exists."));
        cb(null, !!user);
    });
},

Controller:

User.exists(req.body.user.username, function (err, exists) {
  if (err) {
    console.log("error: ", err);
    return res.status(409).json({
      message: err
    });      
  }

  User.create(req.user).then(function (data) {
    res.status(201).json({
      user: data
    });
  });
});

Is this best practice? I'm not sure if node conventions favor errors for exceptional cases, or for flow control. I'm thinking I should rewrite this, but I want to know conventions before I do so. I think I've seen some examples written this way in Sails. Thanks!

like image 378
jedd.ahyoung Avatar asked Sep 29 '22 06:09

jedd.ahyoung


1 Answers

The above answer is good for Express, but in Sails controllers you should not be calling next; best practice is to always return a response. In most example Sails code you won't even see next as an argument to a controller action function. Also note that Sails comes with some default response methods built right into the res object, such as res.serverError and res.badRequest, as well as res.negotiate which will attempt to route the error to the appropriate handler for you based on the status code. So your example could be tweaked as:

Model:

exists: function (options, cb) {
    User.findOne({
        where: { username: typeof options === 'Object' && options.username ? options.username : options },
    }).exec(function (err, user) {
        // res.negotiate will default to a 500 server error
        if (err) return cb(err);
        // res.negotiate will just output the status code and error object
        // as JSON for codes between 400 and 500, unless you 
        // provide a custom view as api/responses/badRequest.ejs
        if (user) return cb({
          status: 409, 
          message: "A user with that username already exists."
        });
        cb(null, !!user);
    });
},

Controller:

User.exists(req.body.user.username, function (err, exists) {
  // Let Sails handle those errors for you
  if (err) {return res.negotiate(err);}

  User.create(req.user).then(function (data) {
    res.status(201).json({
      user: data
    });
  });
});
like image 54
sgress454 Avatar answered Oct 01 '22 19:10

sgress454