You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
stream: 'end' is no longer emitted after 'error' #20334
(I've copied this more or less from the original issue for better visibility)
With node v10 I noticed that 'end' events are no longer emitted after 'error' events for streams, which caused breakage at least with my projects and modules. I tracked it down to #20104 which was merged only 5 days ago and never landed in a v9 release. As of this writing, it's not even marked as semver-major which I think is a mistake, as it can clearly break userland.
Secondly, there is no indication in the streams documentation that this new behavior is the intended behavior.
With all of these things considered, I think the offending commit should be reverted. At the very least it should be semver-major and delayed for some time to give end users a chance to somehow deal with the change in behavior. Perhaps streams should instead emit 'close' after 'error' instead (stream implementors could opt out of this behavior via constructor options)? Something like that would allow for a much smoother transition.
/cc @mcollina ... In hindsight, looking it over again, this really should have been semver-major. +1 to reverting in 10.x, keeping it in 11.x, and marking it semver-major. The change itself is the correct thing to do so reverting it in master would not be the right thing.
Activity
targos commentedon Apr 26, 2018
@nodejs/streams
jasnell commentedon Apr 26, 2018
/cc @mcollina ... In hindsight, looking it over again, this really should have been semver-major. +1 to reverting in 10.x, keeping it in 11.x, and marking it semver-major. The change itself is the correct thing to do so reverting it in master would not be the right thing.
mcollina commentedon Apr 26, 2018
@mscdex the PR was landed as minor because a) it didn’t break known modules and b) reacting to end after error means an hard-to-understand state.
Did this break any module specifically?
I’m ok to revert in 10.x.x.
stream: prevent 'end' to be emitted after 'error'
Revert "stream: prevent 'end' to be emitted after 'error'"
Revert "stream: prevent 'end' to be emitted after 'error'"
Revert "stream: prevent 'end' to be emitted after 'error'"
fix: Add Node 10 support (#5769)
Revert "stream: prevent 'end' to be emitted after 'error'"