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

events: allow use of AbortController with on #34912

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 24, 2020

Allows use of an AbortController with events.on(). This builds on #34911, which should land first.

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

const i = setInterval(() => ee.emit('foo', 'bar'));

async function foo() {
  for await (const f of events.on(ee, 'foo', { signal: ac.signal }))
    console.log(f);
}

foo().catch((error) => {
  if (error.name === 'AbortError')
    console.error('Iteration was aborted!');
  else
    console.error('There was an error:', error.message);
}).finally(() => {
  clearInterval(i);
});

process.nextTick(() => ac.abort());
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 Issues and PRs related to the events subsystem / EventEmitter. label Aug 24, 2020
@jasnell jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 24, 2020
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

good work!

@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 24, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 24, 2020
@nodejs-github-bot

This comment has been minimized.

@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 24, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 24, 2020
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

This is cool 👍

lib/events.js Show resolved Hide resolved
test/parallel/test-event-on-async-iterator.js Show resolved Hide resolved
@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 26, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 26, 2020
@github-actions
Copy link
Contributor

Commit Queue failed
- Loading data for nodejs/node/pull/34912
✔  Done loading data for nodejs/node/pull/34912
----------------------------------- PR info ------------------------------------
Title      events: allow use of AbortController with on (#34912)
Author     James M Snell  (@jasnell)
Branch     jasnell:abortable-on -> nodejs:master
Labels     events, semver-minor
Commits    7
 - events: allow use of AbortController with once
 - fixup! events: allow use of AbortController with once
 - fixup! fixup! events: allow use of AbortController with once
 - fixup! fixup! fixup! events: allow use of AbortController with once
 - events: allow use of AbortController with on
 - fixup! events: allow use of AbortController with on
 - fixup! fixup! events: allow use of AbortController with on
Committers 1
 - James M Snell 
PR-URL: https://github.com/nodejs/node/pull/34912
Reviewed-By: Matteo Collina 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Denys Otrishko 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/34912
Reviewed-By: Matteo Collina 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Denys Otrishko 
--------------------------------------------------------------------------------
   ℹ  Last Full PR CI on 2020-08-24T23:10:20Z: https://ci.nodejs.org/job/node-test-pull-request/32923/
- Querying data of job/node-test-pull-request/32923/
✔  Build data downloaded
- Querying failures of job/node-test-commit/40437/
✔  Data downloaded
   ✖  3 failure(s) on the last Jenkins CI run
   ℹ  This PR was created on Mon, 24 Aug 2020 20:52:55 GMT
   ✔  Approvals: 3
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/34912#pullrequestreview-473854472
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/34912#pullrequestreview-474479754
   ✔  - Denys Otrishko (@lundibundi): https://github.com/nodejs/node/pull/34912#pullrequestreview-474674079
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

@github-actions github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Aug 26, 2020
@jasnell
Copy link
Member Author

jasnell commented Aug 27, 2020

@mcollina ... #34911 needs to land before this one

@lundibundi
Copy link
Member

Also, commit-queue currently doesn't support PRs with more than just 1 commit (#34770). At least until we integrate #34770 (comment) (which is hopefully soon).

@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2020
@nodejs-github-bot
Copy link
Collaborator

lib/events.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

@jasnell
Copy link
Member Author

jasnell commented Sep 2, 2020

Landed in df1023b

@jasnell jasnell closed this Sep 2, 2020
jasnell added a commit that referenced this pull request Sep 2, 2020
Signed-off-by: James M Snell <[email protected]>

PR-URL: #34912
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
Signed-off-by: James M Snell <[email protected]>

PR-URL: nodejs#34912
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
addaleax added a commit to addaleax/node that referenced this pull request Mar 29, 2021
This was missed in the original PR.

Refs: nodejs#34912
jasnell pushed a commit that referenced this pull request Apr 1, 2021
This was missed in the original PR.

Refs: #34912

PR-URL: #37965
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 4, 2021
This was missed in the original PR.

Refs: #34912

PR-URL: #37965
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@targos targos added backport-open-v14.x and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. dont-land-on-v14.x labels Apr 24, 2021
targos pushed a commit to targos/node that referenced this pull request Apr 24, 2021
Signed-off-by: James M Snell <[email protected]>

PR-URL: nodejs#34912
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Apr 26, 2021
Signed-off-by: James M Snell <[email protected]>

PR-URL: nodejs#34912
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Apr 30, 2021
Signed-off-by: James M Snell <[email protected]>

PR-URL: nodejs#34912
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
targos pushed a commit that referenced this pull request Apr 30, 2021
Signed-off-by: James M Snell <[email protected]>

PR-URL: #34912
Backport-PR-URL: #38386
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
targos pushed a commit that referenced this pull request May 30, 2021
This was missed in the original PR.

Refs: #34912

PR-URL: #37965
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
This was missed in the original PR.

Refs: #34912

PR-URL: #37965
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Jun 11, 2021
This was missed in the original PR.

Refs: #34912

PR-URL: #37965
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
events Issues and PRs related to the events subsystem / EventEmitter. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants