Ensure that end will be queued if paused with data

[jeffbski]

Fix defect where end is not emitted with a buffering stream when it is paused and has data in the buffer when it ends.

The end() method was only invoking _end() if the buffer was empty, however _end() calls the user provided end fn which is where the queue(null) would be called.

So if the stream is paused and there is data in the buffer when end() is called, queue(null) (from the user end fn) never gets invoked.

Change code to always call _end() regardless of buffer length since it will simply queue(null) assuming user provided proper buffer end fn.

[jeffbski]

test provided in pull request.

[dominictarr]

This is wrong, if the stream is paused, don't want it to emit the 'end' until all of the 'data' has gone.

You need to resume() before you can get 'end'. if you just want to get rid of the stream, use destroy()

[jeffbski]

I believe there is a misunderstanding. You are correct in that if the stream is paused, you don't want the end emitted until all the data has gone.

However if there is data in the buffer when stream.end() is called, then end event will never be emitted (even after resume).

The reason is that stream.end() should call _end() which calls the user provided end fn, which for a pausable stream, should do a queue(null).

The way the code is previously, it never calls the _end() (and therefore the queue(null)) if there is data in the buffer when the end() was called.

The queue(null) (from the user provided end fn) needs to be executed, so that an end event will eventually be created on resume.

You can see the behavior via the test I added which simply pauses first, does a few writes and an end, then resumes. The end is never emitted in that scenario without the fix.

I know this is hard to describe since there are so many things named 'end'.

Let me know if this still doesn't make sense.

[jeffbski]

I guess one other thing to remember when looking at this, is that for a buffering stream, the user data and end fn's will be using queue. That is a requirement for being able to buffer.

So the queue(null) needs to occur so that end will eventually be emitted on a resume.

For non-buffering stream, the emits happen immediately since there is no buffer to worry about.

[dominictarr]

Ah, I see. No, I was wrong, you where right. merging!

[dominictarr]

merged into through@1.1.2

(oh by the way, for maximum pull request brownie points, it helps if you commit the tests before the implementation/fix, that way you can run the tests to reproduce by checking out the test commit)

cheers!

[jeffbski]

Excellent!

Yeah, I realized this after I committed that it would have been better to commit the tests first for easier review. Thanks for the tip and fast response.

All the best!