-
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
stream: add suport for abort signal in finished() for webstreams #46403
stream: add suport for abort signal in finished() for webstreams #46403
Conversation
Review requested:
|
eb31a9c
to
4312987
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Commit Queue failed- Loading data for nodejs/node/pull/46403 ✔ Done loading data for nodejs/node/pull/46403 ----------------------------------- PR info ------------------------------------ Title stream: add suport for abort signal in finished() for webstreams (#46403) Author Debadree Chatterjee (@debadree25) Branch debadree25:ft/abort-signal-webstreams-finish -> nodejs:main Labels needs-ci Commits 2 - stream: add suport for abort signal in finished() for webstreams - fixup! add tests Committers 1 - Debadree Chatterjee PR-URL: https://github.com/nodejs/node/pull/46403 Refs: https://github.com/nodejs/node/pull/46205 Refs: https://github.com/nodejs/node/pull/37354 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Robert Nagy ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/46403 Refs: https://github.com/nodejs/node/pull/46205 Refs: https://github.com/nodejs/node/pull/37354 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Robert Nagy -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 29 Jan 2023 05:27:07 GMT ✔ Approvals: 3 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274265207 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274266823 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274274475 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-01-30T11:22:45Z: https://ci.nodejs.org/job/node-test-pull-request/49247/ - Querying data for job/node-test-pull-request/49247/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD f84de0ad4c..75080830a4 main -> origin/main ✔ origin/main is now up-to-date main is out of sync with origin/main. Mismatched commits: - 1f708d2037 stream: dont access Object.prototype.type during TransformStream init - 75080830a4 stream: dont access Object.prototype.type during TransformStream init -------------------------------------------------------------------------------- HEAD is now at 75080830a4 stream: dont access Object.prototype.type during TransformStream init ✔ Reset to origin/main - Downloading patch for 46403 From https://github.com/nodejs/node * branch refs/pull/46403/merge -> FETCH_HEAD ✔ Fetched commits as 75080830a40d..43129876236c -------------------------------------------------------------------------------- [main 3cbb426140] stream: add suport for abort signal in finished() for webstreams Author: Debadree Chatterjee Date: Sun Jan 29 09:48:41 2023 +0530 1 file changed, 26 insertions(+), 3 deletions(-) [main 3034793ca3] fixup! add tests Author: Debadree Chatterjee Date: Sun Jan 29 10:52:07 2023 +0530 1 file changed, 90 insertions(+) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)https://github.com/nodejs/node/actions/runs/4077785291 |
Landed in ebcc711 |
Refs: nodejs#46205 PR-URL: nodejs#46403 Refs: nodejs#37354 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
Refs: nodejs#46205 PR-URL: nodejs#46403 Refs: nodejs#37354 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
Refs: nodejs#46205 PR-URL: nodejs#46403 Refs: nodejs#37354 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
In our original PR we hadn't added support for
AbortSignal
when using webstreams withfinished()
hence added the same hereRefs: #46205
Refs: #37354