grncdr / merge-stream

Merge multiple streams into one interleaved stream
MIT License
214 stars 16 forks source link

Sometimes merge does not end #6

Closed heneryville closed 10 years ago

heneryville commented 10 years ago

You're going to hate this bug report, because I'm totally flummoxed as to what's causing it and can't even make a very good reproduction.

After upgrading to version 0.1.5 from 0.1.1 merge stream no longer always ends. I've verified that this is due to the new use of through added in 0.1.2. It appears that some one of my streams is not ending, causing the merge to not end. Here is the trick, it is ending. I can pipe it to ANY OTHER STREAM and get the end event. But inside merge-stream, I don't. Yep. That's it. I've got no idea why specifically inside merge-stream the stream doesn't get ended, but it does everywhere else. Things do work for me on 0.1.1.

I advise that you ignore this issue. I only want to put it here in case anyone else has a similar issue.

rgaskill commented 10 years ago

I am able to reproduce this consistently when the amount of being streamed is large enough. I have been running 0.1.5 for some time copying directories in gulp. When I added a very large directory to the merge-stream it seems to end prematurely with no notification of completion. (i.e. gulp doesn't notify an end to the task) I removed all other directories except the large one and it still fails. I rolled merge-stream back to 0.1.1 and it works fine

This is all I am doing to reproduce

gulp.task('copy', function() {
  merged = merge();

  //src1 is the path to the entire jquery-ui src tree.  I don't think it matters what it is as long as it is large.
  stream1 = gulp.src(src1).pipe(gulp.dest(dest1));
  merged.add(stream1);

  //gulp doesn't show that the task ended but the gulp process ends
  //when the gulp process ends only part of the directory structure is copied 
  //There seem to be no exceptions it just ends
  return merged;       
});
grncdr commented 10 years ago

I am not sure these are the same issue. In the first case the stream doesn't end, but in the second it ends too soon? On Aug 1, 2014 6:44 PM, "Roarke Gaskill" notifications@github.com wrote:

I am able to reproduce this consistently when the amount of being streamed is large enough. I have been running 0.1.5 for some time copying directories in gulp. When I added a very large directory to the merge-stream it seems to end prematurely with no notification of completion. (i.e. gulp doesn't notify an end to the task) I removed all other directories except the large one and it still fails. I rolled merge-stream back to 0.1.1 and it works fine

This is all I am doing to reproduce

gulp.task('copy', function() { merged = merge();

//src1 is the path to the entire jquery-ui src tree. I don't think it matters what it is as long as it is large. stream1 = gulp.src(src1).pipe(gulp.dest(dest1)); merged.add(stream1);

//gulp doesn't show that the task ended but the gulp process ends //when the gulp process ends only part of the directory structure is copied //There seem to be no exceptions it just ends return merged; });

— Reply to this email directly or view it on GitHub https://github.com/grncdr/merge-stream/issues/6#issuecomment-50907065.

rgaskill commented 10 years ago

maybe not.... I guess I was looking at it from the point of view of my gulp task. Gulp logs the start of the task but does not log the end.

grncdr commented 10 years ago

Could you provide a test case that reproduces this without gulp? It seems unlikely that gulp is at fault, but I'd like to isolate the problem if possible.

rgaskill commented 10 years ago

Just add this snippet to your test.js. Uncomment resume() and it will pass.

test('end', function (combined) {

  function addSource (i) {
    if (i === 38) return;
    combined.add(range(i))
    after(6, addSource, i + 1)
  }

  // combined.resume();
  combined.on('end', function () { console.log('ENDED') })

  addSource(-36)
})
grncdr commented 10 years ago

This is expected behaviour for readable streams, if nothing consumes them, they never emit 'end'.

rgaskill commented 10 years ago

I really appreciate your patience with me on this issue. orchestrator/orchestrator#48 was the source of my trouble. Sorry for the wild goose chase I should have dug further into my issue before commenting on this one.

grncdr commented 10 years ago

No worries @rgaskill, I'm glad that the issue got sorted out in the end. Interesting stuff about gulp not necessarily consuming streams, seems like it caught quite a few people off guard.

@heneryville: just checking, but are you consuming the merged stream in your code? You can ensure the merge stream is consumed by calling mergeStream.resume() or piping it to another stream that will be consumed.

heneryville commented 10 years ago

Yes indeed that makes it work. So it looks like the real issue here is in gulp, not in mege-stream. Sorry to bother you all.

grncdr commented 10 years ago

no worries, thanks for reporting the issue either way, I'd way rather have a false positive than miss things entirely. :)