Pausing internally gets overridden by downstream streams #11

Open
ForbesLindesay opened this Issue Apr 6, 2013 · 6 comments

Comments

Projects
None yet
2 participants
@ForbesLindesay

e.g.

source.pipe(through(function (chunk) {
  var self = this;
  console.log('start');
  self.pause();
  setTimeout(function () {
    console.log('end');
    self.resume();
  }, 1000);
})
.pipe(process.stdout);

if you stream chunks from source, will print:

start
start
start
start
end
end
end
end

because the resumes from the process.stdout stream break the pauses within the through stream.

@ForbesLindesay

This comment has been minimized.

Show comment Hide comment
@ForbesLindesay

ForbesLindesay Apr 6, 2013

Can be worked around with code like:

function throttle(data, end) {
  var inProgress = false;
  var finished = false;
  var res = through(function (chunk) {
    var self = Object.create(this);
    self.pause = function () {
      inProgress = true;
      res.pause();
    }
    self.resume = function () {
      inProgress = false;
      if (finished) end.call(self);
      else res.resume();
    }
    data.call(self, chunk);
  }, function () {
    if (inProgress) return finished = true;
    else return end.call(this);
  });
  var resume = res.resume.bind(res);
  res.resume = function () {
    if (!inProgress) return resume();
  }
  return res;
}

Can be worked around with code like:

function throttle(data, end) {
  var inProgress = false;
  var finished = false;
  var res = through(function (chunk) {
    var self = Object.create(this);
    self.pause = function () {
      inProgress = true;
      res.pause();
    }
    self.resume = function () {
      inProgress = false;
      if (finished) end.call(self);
      else res.resume();
    }
    data.call(self, chunk);
  }, function () {
    if (inProgress) return finished = true;
    else return end.call(this);
  });
  var resume = res.resume.bind(res);
  res.resume = function () {
    if (!inProgress) return resume();
  }
  return res;
}
@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Apr 6, 2013

Owner

cool, that may be desired is some use-cases, depending on how the dest emits 'resume'.

You should publish this as an npm module!

Owner

dominictarr commented Apr 6, 2013

cool, that may be desired is some use-cases, depending on how the dest emits 'resume'.

You should publish this as an npm module!

@ForbesLindesay

This comment has been minimized.

Show comment Hide comment
@ForbesLindesay

ForbesLindesay Apr 6, 2013

Will do, was just thinking perhaps this should be part of how the core of through works?

Will do, was just thinking perhaps this should be part of how the core of through works?

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Apr 7, 2013

Owner

what version of node are you using?

Owner

dominictarr commented Apr 7, 2013

what version of node are you using?

@ForbesLindesay

This comment has been minimized.

Show comment Hide comment
@ForbesLindesay

ForbesLindesay Apr 7, 2013

0.8, I'm looking to update to 0.10 soon, but I need to find time to re-test my apps.

0.8, I'm looking to update to 0.10 soon, but I need to find time to re-test my apps.

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Apr 7, 2013

Owner

Ah, so it must be that stdout is emitting resume too often.
If stdout says it can handle data, then that is it's business though.

Pause is just for stopping the flow if the dest has more than it can handle.
If you are trying to do something else, like just block output...
That should be a different module.

It might be better to add another method hardPause which totally prevents output...

Owner

dominictarr commented Apr 7, 2013

Ah, so it must be that stdout is emitting resume too often.
If stdout says it can handle data, then that is it's business though.

Pause is just for stopping the flow if the dest has more than it can handle.
If you are trying to do something else, like just block output...
That should be a different module.

It might be better to add another method hardPause which totally prevents output...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment