rvagg / through2

Tiny wrapper around Node streams2 Transform to avoid explicit subclassing noise
MIT License
1.9k stars 106 forks source link

question: do these streams close themselves after idle periods? #70

Closed matthewmueller closed 8 years ago

matthewmueller commented 8 years ago

i'm probably misusing this library, but i'm noticing that my streams are ending after a certain amount of time. would that happen after a certain amount of idle time? or perhaps when the stream has been drained?

Working with a "stream to slack" logger:

function Slack (url, options) {
  let send = Send(url)(options)

  return through(function (chunk, enc, fn) {
    fn(null, chunk)
    let lines = chunk.length ? chunk.toString().split('\n') : ['']
    for (let i = 0, line; line = lines[i]; i++) {
      if ((lines[i].length === 0) && (i == lines.length - 1)) break
      let log = JSON.parse(line)
      send.apply(null, format(log))
    }
  })
}

It could easily be something else too, but I'm noticing that all my loggers using through2 stop working after some time.

juliangruber commented 8 years ago

I don't see a problem in this snippet per se, I just would move the fn(null, chunk) to the end of the function body, the callback should mark the end of a function's lifetime.

If you look at https://github.com/rvagg/through2/blob/master/through2.js you'll see that through2 is just a tiny tiny wrapper around node's core streams, so if there were any logic like this it would live in node core, which I'm not aware of.

A couple things come in mind for the actual problem:

matthewmueller commented 8 years ago

awesome, thanks man for the feedback, i'll give those suggestions a shot. after testing an isolated case, i think it's an issue outside this library.

matthewmueller commented 8 years ago

Alrighty, I figured it out. it was my misunderstanding of Transform streams.

These streams will buffer internally if they don't have a sink, this was mentioned in this PR: https://github.com/rvagg/through2/pull/18#issuecomment-46516072

Might be worth adding this to the documentation here for the n00bs.

Solved it with a simple sink from the gulp team: https://github.com/gulpjs/vinyl-fs/blob/5cf7de1df6fc47886aaa72c1737490069e50ab3b/lib/sink.js#L15-L20

Thanks for your help @juliangruber. setting the { highWaterMark: 1 }, made this issue much easier to debug.

juliangruber commented 8 years ago

glad I could help!

If your transform stream doesn't have a destination, the right approach should be to use a Writable instead. No need for the fake sink then.