CODEWITHSUNDEEP
javascriptnode.jshttpexpresshttp-headers
Nowis
Nowis

Reputation: 590

Express.js: Headers already sent on REST API

I've create a REST API with Node.js and Express.js. On an URL, I got an error message when I call this URL more the once:

send json error: [Error: Can't set headers after they are sent.]

So, I added a try catch to catch the error. But strangely, this work fine. The error is catched and my response is sent:

try {
  res.status(200).json(user);
} catch (e) {
  console.log('send json error:',e);
}

res.on('finish', function() {
  console.log('All is ok, response sent with user datas');
});

I think it's not very clean solution. So I would like to know why this error is throw and how I can write a clean solution for this problem ?

Upvotes: 1

Views: 1041

Answers (1)

Paul
Paul

Reputation: 36319

So from your JSFiddle, I have to say that there's a lot you could do to modularize/cleanup your code. It's very difficult right now to figure out what's happening where. While using the fat arrow notation for closures is handy, it's really hard to debug with when you have a lot of them in sequence or in parallel as they are completely anonymous.

You also don't indicate anywhere what other middleware is firing along that route, which could also be a source of multiple response attempts.

All that said, the first thing to do is very simple: Add a return statement before each call to res.send() or res.end(). Anytime you call one of those, you want your current function to stop doing anything, so be explicit about that, and a lot of this class of problems goes away.

Second, add named functions for every promise handler or callback in your code. This will at least give you cleaner stack traces and make your code more intuitive to other maintainers.

Third, consider grouping those functions into other files in a logical fashion that you can require by name. This will further illuminate your stack trace (by giving you file names and line numbers to look at).

Finally, install and use node-inspector or a similar debugging tool religiously, set breakpoints and step through your code.

// Edit after the OPs comment below

I disagree that the code organization is not the main issue with the post. It is absolutely leading to the main issue. Your comment (which really should have been in the original question) indicates that the closure in question is firing twice. That closure is the event handler for a global object (bad idea) that is firing an event when user data is loaded, but you're pushing the scope of the current handler down into it (the response object), which is a common problem you run into when you organize code the way you have (scope bleed).

Through better modularization / structure, those kinds of problems become more obvious more quickly. What you're suggesting is something of a hack to solve the problem; the real solution is to better understand when things happen in your application and why.

In this case, look to why that user data loaded event is firing more than once. I'm guessing it should not be (at least not in the scope of the same request), but if it should be and that's OK, then use the correct promise chain behavior to handle it only once (whether that's the first time or the last time is up to your needs), and only call the res.json() when you're really ready to (and again, only once).

Upvotes: 1

Related Questions