Skip to content

docs: fix stream async iterator sample - #31252

Closed
ronag wants to merge 3 commits into
nodejs:masterfrom
nxtedition:doc-async-iterator-write
Closed

docs: fix stream async iterator sample#31252
ronag wants to merge 3 commits into
nodejs:masterfrom
nxtedition:doc-async-iterator-write

Conversation

@ronag

@ronagronag commented Jan 7, 2020

Copy link
Copy Markdown
Member

The for await loop into writable could cause an unhandled exception
in the case where we are waiting for data from the async iterable and
there is no 'error' handler registered on the writable.

Fixes: #31222

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Jan 7, 2020
@ronag
ronagforce-pushed the doc-async-iterator-write branch 2 times, most recently from 701e364 to ad5e164CompareJanuary 7, 2020 21:44
Comment threaddoc/api/stream.md Outdated
@ronag
ronagforce-pushed the doc-async-iterator-write branch 4 times, most recently from 02a4a94 to 542711cCompareJanuary 7, 2020 21:48
The for await loop into writable loop could cause an unhandled exception
in the case where we are waiting for data from the async iterable and
this no `'error'` handler is registered on the writable.
Fixes: nodejs#31222
@ronag
ronagforce-pushed the doc-async-iterator-write branch from 542711c to a778098CompareJanuary 7, 2020 21:49
@ronag
ronagforce-pushed the doc-async-iterator-write branch from cfe161a to f1d486aCompareJanuary 7, 2020 22:01
Comment threaddoc/api/stream.md
// Handle backpressure on write().
if (!writable.write(chunk))
if (!writable.write(chunk)) {
if (writable.destroyed) return;

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This writable.destroyed check is ugly but I don't see a way around it...

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 10, 2020
Trott pushed a commit that referenced this pull request Jan 10, 2020
The for await loop into writable loop could cause an unhandled exception
in the case where we are waiting for data from the async iterable and
this no `'error'` handler is registered on the writable.
Fixes: #31222
PR-URL: #31252
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott pushed a commit that referenced this pull request Jan 10, 2020
PR-URL: #31252Fixes: #31222
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott

Copy link
Copy Markdown
Member

Landed in e468759...5f76f76.

Thanks for using --fixup on your fixup commit. Being able to use --autosquash while landing is a small-but-meaningful help!

@TrottTrott closed this Jan 10, 2020
MylesBorins pushed a commit that referenced this pull request Jan 16, 2020
The for await loop into writable loop could cause an unhandled exception
in the case where we are waiting for data from the async iterable and
this no `'error'` handler is registered on the writable.
Fixes: #31222
PR-URL: #31252
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 16, 2020
PR-URL: #31252Fixes: #31222
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@codebyterecodebytere mentioned this pull request Jan 16, 2020
codebytere pushed a commit that referenced this pull request Mar 14, 2020
The for await loop into writable loop could cause an unhandled exception
in the case where we are waiting for data from the async iterable and
this no `'error'` handler is registered on the writable.
Fixes: #31222
PR-URL: #31252
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 14, 2020
PR-URL: #31252Fixes: #31222
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
The for await loop into writable loop could cause an unhandled exception
in the case where we are waiting for data from the async iterable and
this no `'error'` handler is registered on the writable.
Fixes: #31222
PR-URL: #31252
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
PR-URL: #31252Fixes: #31222
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@codebyterecodebytere mentioned this pull request Mar 17, 2020
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.docIssues and PRs related to the documentations.streamIssues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: writable streams from async iterators example issue

5 participants

@ronag@Trott@mcollina@jasnell@nodejs-github-bot