Handling errors in nodejs

Question

i am making a nodejs web api and i have a function that returns a user object that is associated with a given authentication token:

module.exports.getByToken = function (token_value, callback)
{
    mongoose.model('tokens').findOne({ value: token_value }).exec(function (err, token)
    {
        if (err || token == null)
        {
            var error = new Error('couldn\'t find user of the given token');
            callback(error, null);
        }
        else
        {
            mongoose.model('users').find({ _id: token.user }).exec(callback);
        }
    });
};

As you can see, i am passing the error back to the callback instead of throwing it. Am i doing it right?

This function is called from an authentication middleware:

app.use('/api', function (req, res, next)
{
    var token = req.headers.authorization;

    users.getByToken(token, function (err, user)
    {
        if (err || user == null)
        {
            res.status(401).end('Unauthorized');
        }
        else
        {
            app.locals.user = user;
            next();
        }
    });
});

So the idea of passing the error back to the callback works conveniently. But is this the right way to handle errors?

Can it block the main thread? Should i throw the error instead and explicitly catch it in the middleware?

Thanks, Arik

Enrique Fueyo · Accepted Answer · 2015-12-28 13:53:48Z

2

IMO your are doing it the right way. Callbacks should return an error as the first parameter if they are not responsible for handling it. If you want to improve how any possible error is handled you could change your middleware to something like:

app.use('/api', function (req, res, next){
    var token = req.headers.authorization;

    users.getByToken(token, function (err, user){
        if (err){
            res.status(500).end('Something went wrong :('); //token could be valid but you have lost your connection to DB or any other error
        }else if (user == null){
            res.status(401).end('Unauthorized');
        }
        else {
            app.locals.user = user;
            next();
        }
    });
});

answered Dec 28, 2015 at 13:53

Enrique Fueyo

3,4981 gold badge18 silver badges10 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

SKY · Accepted Answer · 2015-12-28 16:48:07Z

It looks to be right. Nothing wrong in it. I would simplify the code or rather make the middleware separate from the routes in the following manner:

app.use('/api',
        auth.checkToken,
        auth.processUser //The function that would do something with the user returned from the middleware if there are no errors
       );

and in another file (where ever you would want all the middleware related to auth to be, say auth/middleware.js) :

module.exports.getByToken = function (req, res, next)
{ var token_value = req.headers.authorization;

  mongoose.model('tokens').findOne({ value: token_value}).exec(function (err, token)
{
    if (err)
    {
        var error = new Error('couldn\'t find user of the given token'); 
     //Log the error, if required
        return res.status(500).send()
    }
    else if(token === null || !token) {
        var error = new Error('couldn\'t find user of the given token'); 
        //Log the error, if required
        return res.status(404).send(error);
    }
    else
    {   //here next refers to a function that accepts error and user as arguments and does some processing.
        mongoose.model('users').find({ _id: token.user }).exec(next);
    }
 });
};

Collectives™ on Stack Overflow

Handling errors in nodejs

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related