13

I've been trying to debug my node app to find the source of an error in my log that shows up only as "Error: Can't set headers after they are sent", with no trace information or any context.

As it happens, I think I've now fixed this... I am using connect-timeout and I was continuing processing a callback passed to an asynchronous network operation, which callback would eventually try to do a res.send(), despite req.timedout having been set to 'true' by connect-timeout during the network operation.

BUT I still can't understand why my log wasn't showing trace information for this error. Anywhere that an error is returned in my code I log it to the console with:

console.log(err);

If there is trace information available in the err object, and this seems to be placed in err.stack, shouldn't the above statement dump the whole content of err (including err.stack) to the console log? My understanding is that I wouldn't be losing any information by doing the above, compared e.g. to:

console.log(err.stack);

But posts like this one seem to suggest otherwise (though the linked post has now been updated).

I actually go further, and add some relevant text to help locate the error:

console.log('error in dodgyFunction:', err);

But despite this, I was still only getting "Error: Can't set headers after they are sent", without any of the context I'd put it. Would this be because this console error message is output within an external library (like express)? I thought that external libraries should send errors back to the main code to be dealt with accordingly?

Edit: here's an example of where I put my error and timeout checking, at the top of the callback function passed to the async operation:

var execFile = require('child_process').execFile;
execFile('dodgycommand', options, function(error, stdout, stderr) {
    if (req.timedout) {
        console.log('timeout detected whilst running dodgycommand, so aborting...');
        return;
    }
    if (error) {
        console.log('error running dodgycommand:', error);
        res.sendStatus(400);
        return;
    }

    // ... it's safe to continue ...

}

I basically follow this same pattern throughout.

RJFalconer
  • 10,890
  • 5
  • 51
  • 66
drmrbrewer
  • 11,491
  • 21
  • 85
  • 181
  • Where is your `console.log()` statement placed? – Jyotman Singh Mar 01 '17 at 10:49
  • @JyotmanSingh I've updated my question to give an example of where I put the `console.log()` statement – drmrbrewer Mar 01 '17 at 11:40
  • Good news that you could find the issue! Now, to better help future readers (and to follow Stack Overflow rules), you could remove your EDIT2 message from the question and post it as an answer. After that, mark your answer as the correct answer. – Zanon Mar 02 '17 at 20:30
  • Seems wrong to answer my own question, but I've now done so! – drmrbrewer Mar 02 '17 at 20:37
  • As an aside, you should consider using `console.error` not `console.log` so that it logs to std:err – RJFalconer May 16 '18 at 08:29
  • Some tips: You can use colors to have a better view of: console.log('%c Sample Text', 'color:green;'); Or add some VAR in the text using: console.log(\`Sample ${variable}\`, 'color:green;'); – Gilberto B. Terra Jr. Jan 29 '19 at 11:35

3 Answers3

16

I've just worked out what was going on, and I hope this will help others to avoid this beginner's error.

For some of my error logging I was using something like the following, using string concatenation to construct the error message:

console.log('error in function abc: ' + err + ' whilst doing xyz');

whereas elsewhere I was using something like the following, just passing the pieces of the error message as separate arguments to console.log:

console.log('error in function xyz:', err, 'whilst doing abc');

I now see that these give different results!

The former must stringify err so that it can be concatenated with the other parts of the message, and according to this, in doing so it just uses the message part.

However, in the latter form the err object must be processed by console.log unadulterated, and dumped as a whole.

This explains why I was sometimes not seeing the whole content of the error, as I was expecting, and other times I was.

As for console log messages put there by other libraries, something else to check is that you're not filtering out the 'stack' parts of the log messages in your log viewer... turns out that I was (in order to save on log quota... am using papertrail)... d'oh. I was doing so by filtering out any lines starting with ____at (four spaces followed by 'at'), for example ____at Request.self.callback.

drmrbrewer
  • 11,491
  • 21
  • 85
  • 181
8

I've installed n now and I can confirm the following:

Node 4.0.0

Using console.log(err) prints only the error message.

Node 7.7.0 (latest)

Using console.log(err) prints the error message and the full stack.


I've confirmed that this behavior changed on version 6.0.0. So, if you use an older version, I suggest that you update your Node.js or use console.log(err.stack) instead to print the full stack.

Zanon
  • 29,231
  • 20
  • 113
  • 126
  • 1
    Interesting, very interesting. Thanks. I'm using node `v6.9.4` so I should get the full stack with just `err`, which is good because that's what I already have strewn throughout my code. I suspect that the stackless error message I'm seeing is from outside my code, and just doesn't have any stack to show. – drmrbrewer Mar 01 '17 at 21:18
  • @drmrbrewer if the third party library uses `throw "error message"` instead of creating an object like in `throw new Error("error message")`, you won't be able to find the stack trace. Setting the `err` variable with a string makes it impossible to retrieve the original stack trace. – Zanon Mar 01 '17 at 21:48
  • OK thanks I'm beginning to understand it all a bit better. What has me slightly perplexed is that if I do e.g. a `res.send()` on a `res` whose headers have already been set (already sent) then would it be `express` that is showing the console error? Would `express` not be dumping a full stack trace, or is that considered bad practice? Maybe I should try the 'debug' module referred to by @Paul, as that might expose more of the logging by `express`. – drmrbrewer Mar 01 '17 at 22:03
  • Does anyone know how to print *only the error stack trace* in Node 7+? When I try printing `error.stack` I still get the full error message (when I only want the stack). Interestingly, printing `error.message` will print only the message. It seems as if `error.stack` in the latest node include the error message too now? – GrayedFox Jul 18 '17 at 12:15
7

Your pattern looks generally common, though I'll say that as a rule I don't like it, more on that in a second.

As for your main question, it's really hard to answer it based on what you've provided. If you show the actual code rather than the "I generally follow this pattern", it might help. But it's equally possible that the error was being thrown somewhere that you weren't expecting, and so your console.log wasn't getting called at all.

Seems like you're looking for best practices, so I'll give you what I think is the best I have found so far.

First, don't use console.log for logging. It's not horrible, but you can do much, much better. My favorite is to use morgan as middleware for logging request information, and debug for application logging.

With debug you can set up custom log levels and listen to whatever level you want with whatever level of granularity you want. It's all controlled by setting the DEBUG environment variable, and in production you can redirect to file or whatever other destination you want. Further, many node modules (including Express and Connect) use Debug as their logger under the hood, so by tweaking your DEBUG variable you can see as much or little of their inner logging as you want. very useful for figuring out what went wrong where.

Second, as I said I don't use the pattern you have at all when it comes to routing. I've found it's easy to accidentally send headers more than once if I am not careful, so my middleware always return next() and responses are only sent in actual handlers that I can be sure fire only once. When it comes to error, I always pass next(e) which I can then handle in an error handler function. I also created the praeter library to provide standard errors based on web status codes and a generic error handler.

The pattern looks something like this:

// middleware function to put something on the request object
app.use((req, res, next) => {
  MyModel.doSomething((e, thing) => {
    if (e) return next(e);
    if (!thing) return next(new NotFound()); // NotFound is an error in praeter that equates to a 404. 
    req.thing = thing;
    return next();
  });
});

Then later

// log in here is a reference to my debug configured log object
app.use((err, req, res, next) => {
  log.error(err);
  log.error(err.stack);
  return res.status(err.statusCode || 500).send(err.message)
});

Note this is a simple example of a final error handler. I often have several of these where I might handle different error codes differently, depending on application needs.

Paul
  • 35,689
  • 11
  • 93
  • 122
  • Thanks for this helpful answer. As to the basic question "does `console.log(err)` show `err.stack` as well as everything else in `err`?", what is the answer? It seems to me that logging `err` is better than logging `err.stack` because with the former you get everything? Or maybe `console.log()` checks if what is passed in an error object and only displays the basic message if only `err` is passed? – drmrbrewer Mar 01 '17 at 14:17
  • No, when I do `console.log(new Error('ouch'))` as a test, I get the message and the full stack. When I do `console.log(new Error('ouch').stack) I get the exact same output to the console. As I said, I suspect the log you saw was coming from somewhere that wasn't your code. For a production system, it's not a great idea to always log the full stack (regardless of what logging approach you use), so I would explicitly template log messages in a standard format which only includes just enough information. Using log levels you can log the stack only on the 'verbose' log level. – Paul Mar 01 '17 at 15:28
  • 2
    Actually, I've just spotted something crucial... for some of my error logging I use something like `console.log('error in dodgyFunction: ' + err)` whereas elsewhere I have `console.log('error in anotherDodgyFunction:', err)`. I now see that these give different results! The former must stringify `err` so that it can be concatenated with the first part, and according to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/toString this uses just the `message` part. However, the latter must be processed by `console.log` unadulterated, and dumped as a whole. – drmrbrewer Mar 02 '17 at 19:49
  • So @Paul I'm trying to implement best practice regarding use of `next()` in my middleware (presently I don't)... can I clarify something you said: "so my middleware always return `next()` and responses are only sent in actual handlers". So even after a successful request, you don't do e.g. `sendStatus(200)` in the middleware itself, but instead just `return next()` and have a middleware coming *after* that which assumes that, because it has been reached rather than an error handler, all must be OK and it can send a `200` response? Is it OK to pass info (e.g. a message) via `req`? – drmrbrewer Mar 05 '17 at 19:30
  • Yes, it's ok to pass additional information by attaching new properties to req, this is very common. What I meant about always passing next is that I differentiate between functions hatbi intend to process the request without responding (which I attach with app.use) and those which actually handle the request by responding to the client (which I attach using a router and verbs, e.g. Router.get, router.post, etc – Paul Mar 06 '17 at 01:43
  • Thanks, I've posted a follow up question at http://stackoverflow.com/q/42621331/4070848... I am keen to gather best practice ideas and learn from those who know better, rather than just keep making it up as I going along, as I am doing now! – drmrbrewer Mar 06 '17 at 09:01