Skip to content
New issue

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

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

Already on GitHub? # to your account

stream: How to destroy and stop Readable? #29856

Open
ronag opened this issue Oct 6, 2019 · 1 comment
Open

stream: How to destroy and stop Readable? #29856

ronag opened this issue Oct 6, 2019 · 1 comment
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@ronag
Copy link
Member

ronag commented Oct 6, 2019

I ran into this today:

const { Readable } = require('stream');

const someOtherResource = {
  doStuff () {
    if (this.destroyed) {
      throw new Error('destroyed!')
    }
  },
  destroy () {
    this.destroyed = true
  }
}

const r = new Readable({})

r.on('data', () => {
  someOtherResource.doStuff()
})
someOtherResource.destroy()
r.push('asd')
r.destroy() // or r.destroy(new Error('kaboom'))
r.read()

This will crash with destroyed! since read() will emit already buffered 'data' even though the stream is destroyed.

I can fix my above example by replacing r.destroy() with:

function destroy (r) {
  r.removeAllListeners('data')
  r.destroy()
}

I find this a bit unfortunate.

Questions:

  • Is this expected behaviour?
  • Can we fix it without breaking the ecosystem?
  • If no, can we provide another method to Readable that makes it possible to fully destroy and silence a readable?

In order to fix this in readable we would have to check for destroyed and return early inside Readable.read().

@Fishrock123 Fishrock123 added the stream Issues and PRs related to the stream subsystem. label Oct 7, 2019
@ronag
Copy link
Member Author

ronag commented Oct 8, 2019

@jasnell maybe?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants