Best way to avoid "Cannot set headers after they are sent" error

Question

I've read a few of the responses to this question on S.O. (here) and here, and I'm wondering if this is the best way to do this, or if there is a more universally-accepted best practice for this. So if I have the following code, for example:

app.delete('/something/:somethingId', function (req, res) {
    for (var i = 0; i < thingList.length; i++) {
        if (thingList[i].id === req.params.somethingId) {
            thingList.splice(i, 1);
            res.send(thingList[i]);
        }
    }
    res.send("Nothing with that ID found");
});

This errors with a "Cannot set headers after they are set" I believe because res.send() is asynchronous and this code will try to run the outer res.send() while the inner one has already begun the send process. However, I can't put the outer res.send() in an else block because that will kick out of my loop before it should. So is adding a return statement like so:

...
for (var i = 0; i < thingList.length; i++) {
    if (thingList[i].id === req.params.somethingId) {
        thingList.splice(i, 1);
        return res.send(thingList[i]);
    }
}
res.send("Nothing with that ID found");
...

or, alternatively:

...
res.send(thingList[i]);
return;
...

the best options I have? I'm teaching this to some people and wanted to make sure I had it right before giving them a solution that might be considered hacky.

lucasjackson · Accepted Answer · 2016-06-07T21:00:38.903

2

Returning callbacks in Node is a common convention -- so yes, that is one option.

Another simple option is to store your match in a variable if found and break from your loop.

var match;
for (var i = 0; i < thingList.length; i++) {
    if (thingList[i].id === req.params.somethingId) {
        match = thingList[i];
        break;
    }
}
if(match !== undefined) {
    res.send(match);
} else {
    res.send("Nothing with that ID found");
}

edited Jun 07 '16 at 21:00

answered Jun 07 '16 at 20:34

lucasjackson

1,515
1
11
21

Awesome, thank you. I think I like the return option best, and it's good to hear it's not a weird/uncommon thing to do. As a side note, I had forgotten to fix my reference of the array name from "bountyList" to "thingList" inside the for loop condition, so you may want to change that in your answer to avoid any future confusion now that I fixed mine. – bobbyz Jun 07 '16 at 20:59

Best way to avoid "Cannot set headers after they are sent" error

1 Answers1