CODEWITHSUNDEEP
node.jsexpresserror-handling
areller
areller

Reputation: 5238

Handling errors in nodejs

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

Upvotes: 0

Views: 114

Answers (2)

SKY
SKY

Reputation: 240

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);
    }
 });
};

Upvotes: 0

Enrique Fueyo
Enrique Fueyo

Reputation: 3488

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();
        }
    });
});

Upvotes: 2

Related Questions