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

Session Proposal: Support Cancellation (AbortController) in Node.js core #273

Closed
4 of 11 tasks
benjamingr opened this issue May 3, 2020 · 33 comments
Closed
4 of 11 tasks
Assignees
Labels
Austin 2020 Collaborator Summit Project Working Session Track for proposals that support a specific project's needs Session Proposal A session proposal for the Collaboration Summit

Comments

@benjamingr
Copy link

benjamingr commented May 3, 2020

Proposal

Topic of the session Landing AbortController and probably also EventTarget in Node Core.

Related to the fetch in core topic from last year (cc @MylesBorins @mcollina)

Related to other parties interested in cancellation or that I recall having said interesting things to me about it (cc @benlesh @littledan @jakearchibald @devsnek @ljharb @BridgeAR @nodejs/open-standards @felixfbecker )

Type of the session

  • Collaborate
  • Workshop
  • Talk

Timezone of facilitator GMT+2 but I will make myself flexible to other participants.

Estimated duration of the session 2 hours

Follow-up / Set-up sessions (if any) probably background in nodejs/node#19393 https://docs.google.com/document/d/1tn_-0S_FG_sla81wFohi8Sc8YI5PJiTopEzSA7UaLKM/edit and others.

Participants will be able to contribute and we want opinions even if you were not involved at all until this point.

Level

  • Beginner
  • Intermediate
  • Advanced

Pre-requisite knowledge

To best collaborate people should be familiar with AbortController EventTarget and EventEmitter.

Describe the session

Do we want to include AbortController in core? Does that mean we also have to include EventTarget?

How should core handle cancellation? What should the user stories be?

Session facilitator(s), Github handle(s) and timezone(s)

Benjamin Gruenbaum - @benjamingr - GMT + 2

I am fine with anyone else facilitating this although I am also fine with facilitating this myself.
I would be happy to attend the promises session as well.

Additional context (optional)

This section is for contributors handling Collaborator Summit logistics

Track

Triage

  • Track chosen
  • Scheduled
@benjamingr
Copy link
Author

Note: @littledan pointed out that it would be very useful to have some DOM people in the meeting to provide feedback from that perspective and I agree. If anyone can invite some relvant parties that would be helpful.

I forgot to ping a bunch of people who are really smart and have a lot to contribute on the topic - as usual - I hope I am forgiven and they still show up for the session :]

@MylesBorins
Copy link
Contributor

MylesBorins commented May 3, 2020 via email

@devsnek
Copy link

devsnek commented May 3, 2020

@MylesBorins the focus of the entire collab summit is technical writing?

@benjamingr
Copy link
Author

I'm a bit confused, the collab summit is the one space we get together and discuss these issues. If there is another more appropriate space or if in particular this year this session isn't a good fit that's fine.

Of course: this is a session proposal, I promise to still collaborate in good faith in the project if this session proposal gets rejected though this is like... the one time a year we take part of these
sort of discussions as a project.

@ljharb
Copy link
Member

ljharb commented May 3, 2020

I think it would be a very bad idea for language-level cancellation (there remains a proposal) if there was further proliferation of cancellation mechanisms in platforms, beyond fetch in browsers.

@MylesBorins
Copy link
Contributor

MylesBorins commented May 3, 2020 via email

@benjamingr
Copy link
Author

@ljharb that's a good point - Maybe @rbuckton could attend this and help provide feedback from the TC39 proposal PoV?

@devsnek
Copy link

devsnek commented May 3, 2020

I'll also be there and I'm happy to wear my delegate hat if we can't get one of the champions of that proposal.

@ptomato
Copy link

ptomato commented May 4, 2020

Hi, lately I have been working on a proof of concept of what it would look like for various core APIs to support AbortSignal, over in nodejs/node#31971. I'd be interested in participating in this session.

@benjamingr
Copy link
Author

@ptomato that's really cool and having you participate would be awesome.

Also cc @martinheidegger

@benjamingr
Copy link
Author

benjamingr commented May 7, 2020

I started making a list (this is a group effort) of differences between EventEmitter and EventTarget :] https://docs.google.com/document/d/1NFARs04-4U_2y6Ssw9Lqu1GMXBUM981-NO9PLJWifTI/edit#

Edit: Turns out there was a ton I didn't know about EventTarget that I learned. Super happy with the result and with the contributions. I'm sure there are a lot of things missing so anyone who feels like taking a look that would be appreciated.

@benjamingr
Copy link
Author

FWIW @jasnell opened nodejs/node#33527 :]

@benjamingr
Copy link
Author

Hey, just a status update:

  • James's PR for EventTarget/Event landed. (As experimental).
  • James's PR for AbortController is in the TSC agenda to be discussed by the TSC.
  • I have made a bunch of PRs to try to get the Node EventTarget to pass the WPT and match the spec. (Help welcome)
  • I have started making a list for AbortController APIs we might want to discuss (for the summit).

The idea is to land a close-to-spec AbortController in core as experimental and play with it.

What can you do to help in the meantime?:

  • Check master or the AbortController branch and play with EventTarget and AbortController and review the AbortController PR as well as the EventTarget PR.
  • Help with WPT tests and APIs.

This is a good opportunity for people who want to contribute to Node but are not experienced with the code base.

EventTarget/Event is one fine (internal/event_target.js), it's entirely in JavaScript and it's isolated (you just node --expose-interals to access it). I made a few PRs like nodejs/node#33612 for compatibility. More (and WPTs) welcome.

@mcollina
Copy link
Member

I propose we rename this to "Support Cancellation (AbortController) in Node.js core".

@benjamingr
Copy link
Author

@mcollina that sounds good to me, can I go ahead and rename this (governance wise) or since this is already a session proposal is there some other process I should follow?

@mcollina
Copy link
Member

Go for it.

@benjamingr benjamingr changed the title Session Proposal: AbortController in core Session Proposal: Support Cancellation (AbortController) in Node.js core May 29, 2020
@jorydotcom
Copy link
Collaborator

Hi @benjamingr - tentatively looking to schedule this session for 10 a.m. GMT-5 on June 25 (8 a.m. GMT-7 for @ptomato, midnight for @martinheidegger, and 5 p.m. GMT+2 for yourself). Can you confirm this works with your availability?

Also let me know if there is any other information you'd like to share with participants beyond what you've shared in this issue.

@jorydotcom jorydotcom added the Project Working Session Track for proposals that support a specific project's needs label Jun 5, 2020
@ptomato
Copy link

ptomato commented Jun 5, 2020

That time works for me, thanks. I have nothing to add at the moment but I hope to post more progress on this thread and nodejs/node#31971 by the time the session occurs.

@benjamingr
Copy link
Author

@jorydotcom works for me. Would be great to get a calendar invite so I'm sure I don't mix up timezones with daylight savings time.

@jorydotcom
Copy link
Collaborator

definitely... will send something out after we get the rest of the schedule set. thanks @benjamingr @ptomato

@martinheidegger
Copy link

I will try to attend, midnight bears always the chance of me falling unintentionally asleep. (thanks @jorydotcom for thinking of me)

@jorydotcom
Copy link
Collaborator

Per @benjamingr 's request this session now starts at 10:30 CST - the agenda has been updated as well!

@ptomato
Copy link

ptomato commented Jun 23, 2020

In preparation for Thursday, I wrote up some thoughts on how cancellation could work with Node.js APIs here: https://gist.github.com/ptomato/10f3d35e6692a7febf917f4ec905dc58

(Other existing reading material is @benjamingr's list of APIs that could use AbortController, from nodejs/node#33528 (comment))

@benjamingr
Copy link
Author

Suggested agenda (just a suggestion, I don't feel strongly about this being the only things to discuss).

Major themes:

  • AbortController as a standard cross-platform cancellation primitive:
    • Historical context on how we got to AbortController, where browsers use it and what browsers are building on top of it (cancellable promises, third state semantics, cancel tokens etc).
    • AbortController has seen userland adoption on both server-side and client-side libraries - what feedback have we been getting? Maybe talk to some library authors (cc @benlesh ) regarding why they want us to standardize it in core.
    • Does adding support for AbortController hurt or help future standardization efforts? What can we do to help our community and standards bodies with cancellation?
  • Technical considerations:
    • AbortController landed in Node (yay?) what does that mean and how can I help?
    • How do async APIs look like with AbortController support? (Show @jasnell 's pr with timers/promises and the doc and possibly go over @ptomato 's doc)
    • What should Node's recommendations to library authors be in the meantime?
    • Should Node provide additional tooling for debugging cancellation?

@ljharb
Copy link
Member

ljharb commented Jun 25, 2020

in TC39 future proposals are probably going to be compatible with AbortController. The TC39 work is complementary to AbortController work and focuses on composing cancellations.

can someone elaborate on this comment? AbortController was a reason given to block Promise cancellation in the past, so I'm not confident there's a way to make them compatible - I'm very concerned that spreading AbortController will prevent Promise cancellation from ever becoming a thing.

@benjamingr
Copy link
Author

@ljharb first of all - take the notes with a grain of salt :] I didn't make an attempt to make an accurate summary since the talks are recorded (I think they will be made available soon'ish), so it makes sense the notes aren't great (sorry!)

Basically I raised compatibility with future TC39 work as a concern and I think @littledan mentioned that when the current proposal was discussed in TC39 and compatibility with AbortController cancellation was a requirement.

I mentioned the fact that a future proposal might be incompatible with AbortController even though TC39 made it a prerequisite for the current proposal and asked if anyone is concerned by that and the people in the meeting were not since AbortController is already a DOM API (so we are not inventing new problems).

I'm very concerned that spreading AbortController will prevent Promise cancellation from ever becoming a thing.

I thought TC39 shut down promise cancellation because of "action at a distance" concerns ~3-4 years ago. I would be very interested in knowing if that decision has changed since I am personally in favor of exploring that direction again and seeing if it can be done safely. The reason AbortController was created in the first place is that TC39 shut down cancellable promises (before that fetch waited due to the different API).

@ljharb
Copy link
Member

ljharb commented Jun 25, 2020

As far as I recall, Promise cancellation wasn't "shut down" in TC39 at all, the proposal was withdrawn, and fetch got AbortController instead.

@devsnek
Copy link

devsnek commented Jun 25, 2020

@ljharb that comment was about the cancellation protocol proposal, not promise cancellation.

@jorydotcom
Copy link
Collaborator

jorydotcom commented Jul 3, 2020

Notes from the session room

@benjamingr do you recall if this session was recorded?

@benjamingr
Copy link
Author

@jorydotcom this session was recorded through Zoom (cloud).

I think you sent the wrong notes (these are the CI stability room notes), the right document is likely this one ?

@jorydotcom
Copy link
Collaborator

ah thank you @benjamingr, sorry about that. fixed.

@WaleedAshraf
Copy link
Member

Closing this issue as no further action is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Austin 2020 Collaborator Summit Project Working Session Track for proposals that support a specific project's needs Session Proposal A session proposal for the Collaboration Summit
Projects
None yet
Development

No branches or pull requests