X Tutup
The Wayback Machine - https://web.archive.org/web/20220423174240/https://github.com/nodejs/node/pull/34911
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

Jump to bottom

events: allow use of AbortController with once #34911

Closed
wants to merge 1 commit into from
Closed

events: allow use of AbortController with once #34911

wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 24, 2020

Allows an AbortSignal to be passed in to events.once() to cancel
waiting on an event.

const ee = new EventEmitter();
const ac = new AbortController();

const p = events.once(ee, 'foo', { signal: ac.signal });

process.nextTick(() => ac.abort());

// p is rejected with an AbortError
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-bot nodejs-github-bot added the events label Aug 24, 2020
@jasnell jasnell added semver-minor request-ci labels Aug 24, 2020
@github-actions github-actions bot removed the request-ci label Aug 24, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
lundibundi
lundibundi reviewed Aug 24, 2020
View changes
lib/events.js Outdated Show resolved Hide resolved
lib/events.js Outdated Show resolved Hide resolved
lib/events.js Outdated Show resolved Hide resolved
test/parallel/test-events-once.js Show resolved Hide resolved
@jasnell jasnell mentioned this pull request Aug 24, 2020
Closed
4 tasks
@jasnell jasnell requested a review from lundibundi Aug 24, 2020
@jasnell jasnell added the request-ci label Aug 24, 2020
@github-actions github-actions bot removed the request-ci label Aug 24, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Aug 24, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/32922/

lundibundi
lundibundi approved these changes Aug 25, 2020
View changes
lib/events.js Outdated Show resolved Hide resolved
lib/events.js Outdated Show resolved Hide resolved
lib/internal/validators.js Outdated Show resolved Hide resolved
test/parallel/test-events-once.js Show resolved Hide resolved
@benjamingr
Copy link
Member

@benjamingr benjamingr commented Aug 25, 2020

The actual code looks good to me - I am not sure what the use case is - to unsubscribe from lots of events with AbortSignal at once?

@jasnell
Copy link
Member Author

@jasnell jasnell commented Aug 25, 2020

Mostly to allow propagation of an AbortSignal through a complex operation. Imagine, for instance, having an async operation that you want to be abortable, and within that is an await on an event or series of events. Currently, there is no way to bail out of that without a potentially destructive action on the emitter (emitting an error).

mcollina
mcollina approved these changes Aug 27, 2020
View changes
Copy link
Member

@mcollina mcollina left a comment

lgtm

@jasnell
events: allow use of AbortController with once
90a703e 
Allows an AbortSignal to be passed in to events.once() to cancel
waiting on an event.

Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell jasnell added the request-ci label Aug 27, 2020
@github-actions github-actions bot removed the request-ci label Aug 27, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Aug 27, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/32958/

lundibundi
lundibundi reviewed Aug 28, 2020
View changes
lib/events.js Show resolved Hide resolved
lib/events.js Show resolved Hide resolved
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Aug 28, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/32962/

@jasnell jasnell added the commit-queue label Aug 31, 2020
@github-actions github-actions bot removed the commit-queue label Aug 31, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Aug 31, 2020

Landed in 883fc77

nodejs-github-bot pushed a commit that referenced this issue Aug 31, 2020
@jasnell @nodejs-github-bot
events: allow use of AbortController with once
883fc77 
Allows an AbortSignal to be passed in to events.once() to cancel
waiting on an event.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #34911
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@github-actions github-actions bot closed this Aug 31, 2020
@richardlau richardlau added dont-land-on-v12.x dont-land-on-v14.x labels Sep 1, 2020
@richardlau
Copy link
Member

@richardlau richardlau commented Sep 1, 2020

Added dont-land labels as this depends on AbortController (#33527) which is semver-major.

@richardlau richardlau mentioned this pull request Sep 1, 2020
Merged
2 tasks
@lundibundi lundibundi mentioned this pull request Sep 16, 2020
Closed
4 tasks
@targos targos added backport-open-v14.x and removed dont-land-on-v14.x labels Apr 24, 2021
targos pushed a commit to targos/node that referenced this issue Apr 24, 2021
@jasnell @targos
events: allow use of AbortController with once
f9e0c71 
Allows an AbortSignal to be passed in to events.once() to cancel
waiting on an event.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#34911
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@targos targos mentioned this pull request Apr 24, 2021
Closed
targos pushed a commit to targos/node that referenced this issue Apr 26, 2021
@jasnell @targos
events: allow use of AbortController with once
7f46ec7 
Allows an AbortSignal to be passed in to events.once() to cancel
waiting on an event.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#34911
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit to targos/node that referenced this issue Apr 30, 2021
@jasnell @targos
events: allow use of AbortController with once
268686a 
Allows an AbortSignal to be passed in to events.once() to cancel
waiting on an event.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#34911
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit that referenced this issue Apr 30, 2021
@jasnell @targos
events: allow use of AbortController with once
1fefb5c 
Allows an AbortSignal to be passed in to events.once() to cancel
waiting on an event.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #34911
Backport-PR-URL: #38386
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@targos targos added backported-to-v14.x and removed backport-open-v14.x labels Apr 30, 2021
@danielleadams danielleadams mentioned this pull request May 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamingr benjamingr

@lundibundi lundibundi

@mcollina mcollina

Assignees
No one assigned
Labels
backported-to-v14.x dont-land-on-v12.x events semver-minor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@jasnell @nodejs-github-bot @benjamingr @richardlau @mcollina @lundibundi @targos
X Tutup