transform streams: unpipe or ignore future writes after .push(null)

[jonathanong]

ran into the following issue. i'm creating a transform stream that "truncates" the source stream. here's an example:

util.inherits(Truncate, Stream.Transform)

function Truncate() {
  this.count = 0
}

Truncate.prototype._transform = function (doc, enc, cb) {
  if (this.count++ < 10) {
    this.push(doc)
    cb()
  } else {
    this.push(null)
    cb()
  }
}

ideally, once this.push(null) is called, the transform stream should be considered "finished" and ._transform() shouldn't be called anymore. however, this isn't the case as ._transform() will continued to be called, and you'll get cannot push after EOF due to additional .push(null)s. to compensate, you'll have to keep track using some sort of this.ended flag.

implementation might by iffy though if you're piping. not sure if the source stream should unpipe on dest.on('end') or just continue writing and have the transform stream discard the data (which is what you'd do right now, anyways). i'm also not sure if and when the finish event would occur in this case. would also be nice if there were consistent stream.destroy() semantics.

[jasnell]

@jonathanong ... I assume this is still an issue for you since you opened it fairly recently in io.js ... are you able to provide a more complete self-contained test case?

[jasnell]

@trevnorris @chrisdickinson ... what do you think on this one? After doing a .push(null), the transform keeps going an _transform keeps being called. This leads to issues if the custom Transform implementation is not doing it's own state management. What's not clear is whether this is actually a bug or not. Yes, we can update the code to ensure that _transform is not called after push(null) but that would be a change in existing behavior. Thoughts?

[trevnorris]

Not sure what the intended behavior was meant to be. Perhaps we should look at the tests and see if there's anything.

[jasnell]

There are no existing tests in joyent/node/tests/* that I can find that perform similar tasks. From the examples that are there, however, it appears that the assumption is that the Transform subclass is expected to perform it's own state management. That leads me to classify this as Not A Bug but a behavior change. Essentially, there currently does not exist a way of telling the Transform, "Ok, I'm done, don't send me any more _transforms" from within the _transform method itself.

A simple workaround would be to set some internal flag when you're done and ignore subsequent _transform calls but that's a bit sucky because the stream will keep right on going until it's done.