Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Emits finish dispite still consuming #2

Open
piglovesyou opened this issue Mar 5, 2018 · 3 comments
Open

Emits finish dispite still consuming #2

piglovesyou opened this issue Mar 5, 2018 · 3 comments

Comments

@piglovesyou
Copy link

piglovesyou commented Mar 5, 2018

The following code simply increments number asynchronously 1000 times. I assumed the parallel transform emits a finish event after all data consumption but is didn't.

const expect = 1000;
let actual = 0;
const r = createReadable(expect);

class MyTransform extends ParallelStream {
  constructor() {
    super({objectMode: true, maxParallel: 32, highWaterMark: 32});
  }

  _parallelTransform(data, enc, callback) {
    setTimeout(() => {
      console.log(++actual);
      callback();
    }, Math.random() * 100);
  }
}

const t = new MyTransform();
t.on('finish', () => {
  assert.strictEqual(actual, expect); // AssertionError [ERR_ASSERTION]: 985 === 1000
});

r.pipe(t);

function createReadable(size) {
  let i = 0;
  return new stream.Readable({
    read: function () {
      this.push(i < size ? i++ : null);
    },
    objectMode: true,
  });
}

This is not pleasant especially when we use libs such as pump that calls a callback function depending on finish event.

It seems that it emits finish after all writable data is buffered instead of consumed.

This problem occurs also in parallel-stream module.

Also, when we do readable.pipe(parallelTransform).pipe(writable), writable emits finish earlier than expected.

It is likely that we have to dig more in private methods of Node Stream.

@piglovesyou
Copy link
Author

I hesitated but I post the similar content to parallel-transform too since both have the same problem and whichever solves it I'd be happy. Please tell me if it's not appropriate.

@Tapppi
Copy link

Tapppi commented Jul 18, 2018

I answered pretty comprehensively in the issue in parallel-transform.

EDIT: not working as it should

@Tapppi
Copy link

Tapppi commented Jul 19, 2018

Basically, this is very hard unless using node >8, or comparable readable-stream, because that's when _final was added, and that fixes the problem of writable side thinking the transform is done before it should. The only difference is that you can use final as it is intended, instead of having to use end event or pump or something similar.

Since this module uses Node core stream instead of readable-stream, which is generally unrecommended, and is just a "modernised" and inheritance based version of parallel-transform I'll leave it as an exercise to someone else to port my pr mafintosh/parallel-transform#6 for this lib.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants