Skip to content

Conversation

@lpinca
Copy link
Member

@lpincalpinca 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: nodejs#26015
@nodejs-github-bot
Copy link
Collaborator

@lpinca
Copy link
MemberAuthor

This does not fix the issue for readable only streams.

@addaleaxaddaleax added the stream Issues and PRs related to the stream subsystem. label Feb 13, 2019
@addaleax
Copy link
Member

@nodejs/streams

@lpinca
Copy link
MemberAuthor

cc: @nodejs/streams
CI: https://ci.nodejs.org/job/node-test-pull-request/21114/

Copy link
Member

@mcollinamcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 5, 2019
@BridgeAR
Copy link
Member

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

@mcollina
Copy link
Member

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

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

@BridgeAR
Copy link
Member

Landed in 49f1bb9 🎉

@BridgeARBridgeAR closed this Mar 5, 2019
BridgeAR pushed 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#26057Fixes: nodejs#26015 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
@lpincalpinca deleted the gh-26015 branch March 6, 2019 06:37
BridgeAR pushed 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#26057Fixes: nodejs#26015 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed 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: #26057Fixes: #26015 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
@BethGriggsBethGriggs mentioned 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#28000Fixes: nodejs#26015 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed 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: #28000Fixes: #26015 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Sep 19, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.streamIssues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The 'error' event can be emitted more than once when using writable.destroy()

7 participants

@lpinca@nodejs-github-bot@addaleax@BridgeAR@mcollina@jasnell@ZYSzys