Allow optional custom onFinish handler for Readable constructor.

[vqvu]

Handle the issue raised in #490 in a disciplined way (by punting to the user). We simply allow an extra argument when wrapping a Readable that handles the onFinish logic for us..

[ronag]

I would like to suggest to take this one step further. Especially considering how common this issue is. Can't we simply check for instanceof http.IncomingMessage and automatically apply onFinished?

[vqvu]

That would cause the browserify build to pull in the http package, which I don't want to do, as it bumps up the size of our minified JS from 58.21 kB to 102.59 kB in exchange for just a bit of convenience.

[ronag]

@vqvu: I see. Isn't there other ways to check for http.IncomingMessage? e.g. if (stream.socket && stream.httpVersion).

Note, I'm actually fine with the proposed solution. I just think that this is quite a big and common "gotcha".

[vqvu]

That's certainly possible, but I think adding special handling for IncomingMessage is out of scope for the library.

Agreed that this is something of a gotcha. I'm happy to add something to the docs about this though (even though that's not as foolproof). I mention IncomingMessage specifically in the new docs for the stream constructor, and I'm open to suggestions for documentation improvements to make these kinds of issues more visible.

P.S. I'll leave this issue open for a few days so the other contributors can weight in if they so choose. Then I'll merge it and cut a release for you.

[ronag]

@vqvu: Could you merge into 3.0.0 branch? That is what I'm currently using.

[vqvu]

Yeah, I can do that this weekend. I just tried and it's a bit complicated.