-
Notifications
You must be signed in to change notification settings - Fork 5
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
Abort assumes close event #12
Comments
skeggse
pushed a commit
to mixmaxhq/mongoist
that referenced
this issue
May 30, 2018
Despite (some) documentation to the contrary, some folks assume that destroying a stream will emit a close event. This Cursor does not emit a close event, which causes some interoperability issues. Streams should probably emit the close event despite not being required to do so by the streams2 spec. See also pull-stream/stream-to-pull-stream#12.
I think it's a far better idea for streams to emit close consistently. Even before I went to pull-streams, I argued that node streams should consider close to be mandatory. All core streams have close, and so do streams created via through/through2 and the base classes in node. I'd consider that a defacto standard. |
Agreed, thanks for the feedback! |
saintedlama
pushed a commit
to mongoist/mongoist
that referenced
this issue
Jun 4, 2018
Despite (some) documentation to the contrary, some folks assume that destroying a stream will emit a close event. This Cursor does not emit a close event, which causes some interoperability issues. Streams should probably emit the close event despite not being required to do so by the streams2 spec. See also pull-stream/stream-to-pull-stream#12.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When you abort a readable stream using stream-to-pull-stream, we assume the underlying Node-style stream will emit an
close
event. This is not always the case, and not required by the docs. I do think streams should emit theclose
event in this case, though, given the newdestroy
docs claim that it will emit theclose
event.I'm just bringing this up in case you have a good idea on how to fix this. I'm attempting to fix this issue upstream too (by having the stream in question emit the
close
event).The text was updated successfully, but these errors were encountered: