-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: AsyncGenerator + Writable + unhandled exception #29397
Labels
doc
Issues and PRs related to the documentations.
good first issue
Issues that are suitable for first-time contributors.
stream
Issues and PRs related to the stream subsystem.
Comments
@mcollina Want a PR? |
Fishrock123
added
doc
Issues and PRs related to the documentations.
stream
Issues and PRs related to the stream subsystem.
good first issue
Issues that are suitable for first-time contributors.
labels
Sep 1, 2019
@ronag Please, either that or let someone new take it. |
+1 |
+1 on a PR. |
I'd like to work on this. I will create a PR with required updates to the doc. |
ckarande
added a commit
to ckarande/node
that referenced
this issue
Sep 3, 2019
Update writable stream code example using async iterator to use safer `finished()` method instead of a `finish` event to avoid uncaught exceptions Fixes: nodejs#29397
3 tasks
targos
pushed a commit
that referenced
this issue
Sep 20, 2019
Update writable stream code example using async iterator to use safer `finished()` method instead of a `finish` event to avoid uncaught exceptions Fixes: #29397 PR-URL: #29425 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
doc
Issues and PRs related to the documentations.
good first issue
Issues that are suitable for first-time contributors.
stream
Issues and PRs related to the stream subsystem.
Given the examples of using async generators with writable streams https://nodejs.org/dist/latest-v12.x/docs/api/stream.html#stream_piping_to_writable_streams_from_async_iterators.
There seems to be an issue in that they can cause an unhandled exception if
'error'
is emitted after'finish'
(which is currently possible in some cases even with core streams).once
will release the'error'
handler on completion ('finish'
). I would personally discourage using it as in the example, i.e.I would replace:
UNSAFE
with
SAFE
or
and
UNSAFE
with
SAFE
Possibly with a note on why
once
is not appropriate in this situation.The text was updated successfully, but these errors were encountered: