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

stream: ensure writable.destroy() emits error once #26057

Closed
wants to merge 1 commit into from

Conversation

@lpinca
Copy link
Member

lpinca commented Feb 12, 2019

Prevent the 'error' event from being emitted multiple times if
writable.destroy() is called with an error before the _destroy()
callback is called.

Emit the first error, discard all others.

Fixes: #26015
Refs: #20745 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Prevent the `'error'` event from being emitted multiple times if
`writable.destroy()` is called with an error before the `_destroy()`
callback is called.

Emit the first error, discard all others.

Fixes: #26015
@lpinca

This comment has been minimized.

Copy link
Member Author

lpinca commented Feb 12, 2019

This does not fix the issue for readable only streams.

@addaleax addaleax added the stream label Feb 13, 2019
@addaleax

This comment has been minimized.

Copy link
Member

addaleax commented Feb 13, 2019

@lpinca

This comment has been minimized.

Copy link
Member Author

lpinca commented Mar 2, 2019

Copy link
Member

mcollina left a comment

LGTM

@BridgeAR

This comment has been minimized.

Copy link
Member

BridgeAR commented Mar 5, 2019

Should we actually allow calling destroy() more than once in the first place?

@mcollina

This comment has been minimized.

Copy link
Member

mcollina commented Mar 5, 2019

Should we actually allow calling destroy() more than once in the first place?

Yes. This is actually an use case of the API (historically).

@jasnell
jasnell approved these changes Mar 5, 2019
@BridgeAR

This comment has been minimized.

Copy link
Member

BridgeAR commented Mar 5, 2019

Landed in 49f1bb9 🎉

@BridgeAR BridgeAR closed this Mar 5, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 5, 2019
Prevent the `'error'` event from being emitted multiple times if
`writable.destroy()` is called with an error before the `_destroy()`
callback is called.

Emit the first error, discard all others.

PR-URL: nodejs#26057
Fixes: nodejs#26015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@lpinca lpinca deleted the lpinca:gh-26015 branch Mar 6, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 12, 2019
Prevent the `'error'` event from being emitted multiple times if
`writable.destroy()` is called with an error before the `_destroy()`
callback is called.

Emit the first error, discard all others.

PR-URL: nodejs#26057
Fixes: nodejs#26015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs added a commit that referenced this pull request Apr 16, 2019
Prevent the `'error'` event from being emitted multiple times if
`writable.destroy()` is called with an error before the `_destroy()`
callback is called.

Emit the first error, discard all others.

PR-URL: #26057
Fixes: #26015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs referenced this pull request May 1, 2019
lpinca added a commit to lpinca/node that referenced this pull request Jun 1, 2019
Prevent the `'error'` event from being emitted multiple times if
`writable.destroy()` is called with an error before the `_destroy()`
callback is called.

Emit the first error, discard all others.

PR-URL: nodejs#26057
Backport-PR-URL: nodejs#28000
Fixes: nodejs#26015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs added a commit that referenced this pull request Sep 19, 2019
Prevent the `'error'` event from being emitted multiple times if
`writable.destroy()` is called with an error before the `_destroy()`
callback is called.

Emit the first error, discard all others.

PR-URL: #26057
Backport-PR-URL: #28000
Fixes: #26015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs referenced this pull request Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.