Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pausing a rate-limited stream doesn't work #642

Open
fmvilas opened this issue Dec 14, 2017 · 2 comments
Open

Pausing a rate-limited stream doesn't work #642

fmvilas opened this issue Dec 14, 2017 · 2 comments
Labels

Comments

@fmvilas
Copy link

fmvilas commented Dec 14, 2017

Hi,

I've been trying the following code:

const highland = require('highland');
const { Writable } = require('stream');

const writable = new Writable({
  objectMode: true,
  write(chunk, encoding, callback) {
    console.log('=>', chunk.toString());
    callback();
  },
});

const stream = highland(['a', 'b', 'c', 'd']).ratelimit(1, 1000);

setTimeout(() => {
  console.log('Pausing...');
  stream.pause();
}, 2000);
setTimeout(() => {
  console.log('Resuming...');
  stream.resume();
}, 6000);
stream.pipe(writable);

But what I've found is that a rate-limited stream doesn't pause. Is it an expected behavior?

Thanks!

@vqvu vqvu added the bug label Dec 14, 2017
@vqvu
Copy link
Collaborator

vqvu commented Dec 14, 2017

This is a bug in 2.x, but it seems complicated to fix, and I don't have a lot of bandwidth to chase this down (the 2.x core has a lot of cruft that makes it hard to understand this kind of behavior). Can you use the 3.0 beta (npm install highland@next) instead?

@fmvilas
Copy link
Author

fmvilas commented Dec 14, 2017

Cool! It works with highland@next. I've been looking at the source code and definitely is a tough problem. From my side is ok to upgrade so no need to spend time fixing old stuff. Feel free to close the issue.

Thanks for your time and work!

vqvu added a commit that referenced this issue Dec 15, 2017
This test was already passing for the 3.0 beta, but it fails in 2.11.1,
so I am adding it here so that we do not regress.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants