How to handle errors without setting res headers twice in node.js

723 views Asked by At

I am a little confused with the req, res parameters for node and the best way to handle these when I'm using asynchronous calls. A function I currently have is supposed to add an Item to my DB, update some DB models accordingly, and then send a response saying the updates were successful. However, the functions that this function asynchronously calls can send an erroneous response if an error happens. If this happens, I get the error Can't set headers after they are sent, since I am trying to call res.send twice. Would appreciate someone's help in figuring out a more optimal way to handle errors. Thanks!

the main function:

item.author = req.user._id;
item.description = req.query.description;
item.rating = req.query.rating;

item.save()
    .then(result => {
        // update user's items
        UserController.updateItems(req.user._id, result._id);
        ItemController.updateItemRating(req.query.outingId, req.query.rating, res);
        res.send(result);
    })
    .catch(error => {
        res.send(error);
    });

updateItemRating:

export const updateItemRating = (itemId, rating, res) => {
    Item.findOne({ _id: itemId }).exec((err, item) => {
        if (item === undefined || item === null) {
            return res.status(404).send('Item not found; check item ID');
        }

        Item.findOneAndUpdate(
            { _id: itemId },
            { $set: { rating: rating },
            },
            (error, item) => {
                if (error) {
                    res.status(404).send('Error updating item with new rating');
                }
            });
    });
};

updateItems:

export const updateItems = (userId, itemId) => {
    User.findOneAndUpdate(
        { _id: userId },
        { $push: { items: [itemId] } },
        (err, user) => {
            console.log('user' + user);
            if (err) {
                console.log('got an error in updateItems');
            }
        });
};
2

There are 2 answers

1
Sumeet Kumar Yadav On BEST ANSWER

Function call to updateItems and updateItemRating both are asynchronous . Response send is getting called multiple time , and also not sure which method send is getting called first . To fix you problem I can suggest you to apply below technique :

  1. Callback : You can pass callback as argument which will do res.send and same callback you can call on error or success condition .

    UserController.updateItems(req.user._id, result._id,function(status,message){res.status(status).send(message);});

You can update item rating method like :

export const updateItemRating = (itemId, rating, callback) => {
    Item.findOne({ _id: itemId }).exec((err, item) => {
        if (item === undefined || item === null) {
            callback(404,'Item not found; check item ID');    
        }    
        Item.findOneAndUpdate(
            { _id: itemId },
            { $set: { rating: rating },
            },
            (error, item) => {
                if (error) {
                     callback(404,'Error updating item with new rating'); 
                }else{
                    callback(200);
                }
            });
    });
};
  1. Async Module : You can synchronize you method call using this module.
3
kah608 On

Instead of having your update functions send the result, you should throw() an error, that way your outer function will catch the error and you can use that to return it.

Another way to think about this is that your outer function is handling the success case with a res.send(), so it should also be responsible for the res.send of the error case.

The less your database layer knows about the caller the easier it will be to reuse our code.

Create a custom error type to encapsulate the 404:

function NotFoundError(message) {
  this.message = (message || "");
}
NotFoundError.prototype = new Error();

Then use that in your inner function:

export const updateItemRating = (itemId, rating, res) => {
Item.findOne({ _id: itemId }).exec((err, item) => {
    if (item === undefined || item === null) {
        throw new NotFoundError('Item not found; check item ID');
    }

    Item.findOneAndUpdate(
        { _id: itemId },
        { $set: { rating: rating },
        },
        (error, item) => {
            if (error) {
                throw new NotFoundError('Error updating item with new rating');
            }
        });
    });
};

And your main becomes:

item.save()
.then(result => {
    // update user's items
    UserController.updateItems(req.user._id, result._id);
    ItemController.updateItemRating(req.query.outingId, req.query.rating, res);
    res.send(result);
})
.catch(error => {
    if (error instanceof NotFoundError) {
       res.status(404).send(error.message);
    }
    else {
       res.send(error);
    }
});