Make it easier to debug unhandled stream errors in pipes

[binarykitchen]

We all fear the infamous EPIPE and ECONNRESET errors like these:

Error: write EPIPE
  at errnoException (net.js:904:11)
  at Object.afterWrite (net.js:720:19)

thrown from https://github.com/joyent/node/blob/v0.10.28/lib/stream.js#L94

These are a bitch to debug, see https://gist.github.com/binarykitchen/339305eeadfb1629e57f

I think it would make our lives sweeter if more information were added to that exception before it's thrown.

For example more information about the source and destination. So that it's easier to narrow down the problem.

[trevnorris]

I believe the Error object does contain additional information. Which you should be able to log from the error handler. If not then I'll entertain a PR adding this information to the Error event.

[binarykitchen]

Yeah, I tried that, logging from the error handler. No luck. Currently there is not much useful information in the Error object to debug this.

A PR adding more information to the error event would be great. Especially data about pipes and streams that got lost.

I am no expert here when it comes to streaming and pipes. Maybe the stream wizard @dominictarr can make better suggestions?

[dominictarr]

what sort of information would help?
unfortunatly, this is just one of the challenges of node. Stack traces don't work as you expect,
because the stack stops at the event loop. The way I deal with things like this is to put in logging
statements and move them closer and closer to before the error.
Another idea is to create an error early, say when you create the stream, and then log that error in the error handler, that way you can trace where the stream was created - instead of where the error was emitted.

[binarykitchen]

a pipe identifier with some debug info describing the source and destination would be very useful.

in a kind of debug mode, we could collect i.e. the class names of source and destination when glued together with a pipe.

source.pipe(destination)

and then, very simple and primitive pseudo code:

Stream.prototype.pipe = function(destination) {
    this.pipeIdentifier = this.classname + ' | ' + destination.classname
    this.debugInfo      = 'source: ' + this.toString() + '/n' +
                          'destination: ' + destination.toString()

    // maybe also add the stack?

    // continue, build the pipe as usual
}

yes, the stack stops at the event loop, but if the whole node process is still running in the same reserved memory area, then that information can be theoretically stored without being lost.

when a pipe wants to emit an error that has no destination, then the above data could help us to identify the missing destination.

  function onerror(er) {
    ...
    if (EE.listenerCount(this, 'error') === 0) {
      err.pipeIdentifier = this.pipeIdentifier
      err.debugInfo = this.debugInfo
      throw er; // Unhandled stream error has some useful information that might help us
    }
  }

i am totally aware that i am no expert here and might sound foolish. i bet many others have been through this before. still, i wonder and question all that ...

[dominictarr]

The best way to approach a problem like this, is write a module that adds this, then you can try it out and see if it's helpful, without affecting/breaking anything other people are doing. Then maybe make a pull request into core... or just use it as a module you can add when you need help debugging.

[binarykitchen]

okay, i will try to reproduce this case within tests first and write a module around that.

@dominictarr haven't you tried something similar before?

[dominictarr]

well, I guess I just got used to that thing being difficult, so I look elsewhere for clues

[binarykitchen]

@dominictarr yeah, you could do that. still, this is not a solution. i think @visionmedia latest blog https://medium.com/code-adventures/farewell-node-js-4ba9e7f3e52b nails it where node seems to fail.

i haven't checked the facts yet and still am in the middle of writing such a module to improve nodejs's error handling but it is not looking good ...

[tj]

context on errors definitely belongs in core IMO, even with poor stack traces it's a world of improvement if we get filenames / socket addresses etc in the message. At least then you'd know if an uncaught is from redis, mongo, s3, etc

[binarykitchen]

@visionmedia exactly! that's what i am trying to achieve here. really surprised that error context are still missing in the core after 3 years.

[trevnorris]

Has there been a PR filed for this?

[binarykitchen]

@trevnorris nope, i have too much work elsewhere and am not half-way through yet.

[binarykitchen]

Still working on it ... can anybody tell me where I can find the native Error() implementation of node.js in JS? I searched here https://github.com/joyent/node/tree/v0.10.28/lib but couldn't find the Error class. Thanks for any clues.

[binarykitchen]

@trevnorris @dominictarr @visionmedia Alright folks, here is a very primitive module to add more context to errors thrown in streams: https://github.com/binarykitchen/stream_spy

It is still very primitive but shows what my intentions are. I won't continue without your honest feedback. If you think this is the right direction, I will invest more time into this.

[trevnorris]

@binarykitchen Error, ReferenceError, TypeError, SyntaxError and RangeError are all JS natives. You won't find that in any of Node source.

[binarykitchen]

@trevnorris Okay but can I find any source code about these natives? I can scan C code. I need to know their public attributes and methods.

[OrangeDog]

Their public attributes and methods are detailed here: http://www.ecma-international.org/ecma-262/5.1/#sec-15.11

If you really want to read the source code, then it's in here somewhere: https://github.com/joyent/node/tree/master/deps/v8/src

[binarykitchen]

@orangemocha thanks but sorry, I want the source code

[dominictarr]

@binarykitchen you should probably look at the v8 source. good luck!
It may be easier to just inspect an error property via the node repl.

[binarykitchen]

@dominictarr right, i forgot the node repl - thanks!

stay tuned folks

[binarykitchen]

okay folks, i studied some nodejs source code and realised there isn't much to extract from.

anyway, node repl was useful and i improved few things. the streamspy already helped me with something tricky.

go, check it out at https://www.npmjs.org/package/stream_spy

will add more tests, especially EPIPE + ECONNRESET, in the next days.

i really think nodejs's core team should have a serious look at this @trevnorris

[illegalnumbers]

+1

[jasnell]

@binarykitchen ... any updates on this? do we need to keep this open?

[binarykitchen]

no idea @jasnell - a lot has changed since then

[jasnell]

Ok, going to close this issue for now then. General improvements to error reporting is always an open issue ;-) Can reopen if there's anything specific from this that needs to land in v0.12 or v0.10