Repository navigation
Cancellation #162
Description
Activity
can you elaborate more on this? in my mind you can "cancel" an iterator using .return/.throw.
Reacted by Denis PushkarevSure, I was sitting with @ronag and @Ethan-Arrowhead in a conference and people are asking
.mapon streams and iterables.The use case we found is something like this:
const iterable = obtainGeneratorOfUrls().map(async (url, { signal }) => { await fetch(url, { signal }); // cancel the request }); // fetch first two urls iterable.next(); iterable.next(); // close the generator, usually break from the loop iterable.return(); // I want this to abort ongoing `fetch` requests. ``
AbortSignalis not a part of ECMAScript. It's useful, but adding something like that is another big concern. I'm not sure that it should be solved in the scope of this proposal.that is indeed an interesting use case and it seems well motivated to me. I'm not sure it could be neatly solved within the scope of this proposal though.
@zloirock yes, that is a reasonable concern. It has been discussed a bunch before. There have been talks (in the cancellation proposal's protocol version and elsewhere) about specifying a way for the spec to interact with cancellable things (like AbortSignal). I think this can and will (eventually) be solved and the spec will be able to interact with
AbortSignals.Note that my understanding is that Chrome's opinion is that
AbortSignalis the cross-platform cancellation primitive we have and that any proposal to TC39 regarding cancellation should use it.
@devsnek the ask here isn't for this proposal to enable cancellation but to provide a future-proofing guarantee so that when this issue is resolved in TC39 a signal can be passed eventually.
Note that my understanding is that Chrome's opinion is that AbortSignal is the cross-platform cancellation primitive we have and that any proposal to TC39 regarding cancellation should use it.
And we need to have
EventTarget,Eventin the language first.And we need to have EventTarget, Event in the language first.
I might be getting off-topic, but I am happy to discuss what it would include although I am hardly an expert. When talking to DOM people I think it should be sufficient to specify an
AddEventListeneroperation and use it to add listeners to a signal (with "abort" passed) and probably aCreateSignalandMarkSignalAborted.That is considerably less work than specifying
EventTarget/Event.I might be getting off-topic, but I am happy to discuss what it would include although I am hardly an expert. When talking to DOM people I think it should be sufficient to specify an AddEventListener operation and use it to add listeners to a signal (with "abort" passed) and probably a CreateSignal and MarkSignalAborted.
If you specify it in an incompatible way, it won't be added to the language.
If you specify it in an incompatible way, it won't be added to the language.
I am not sure what you mean by "an incompatible way" so I think I'm probably not explaining myself well.
I am not suggesting specifying a primitive
EventTargetin the language. I am suggesting specifying hooks in the language to enable interacting with existingAbortSignals.I think what @benjamingr is suggesting is that the aborting primitive would be host-defined. That could be a reasonable way forward here. I suspect that the web, Deno, and Node.js would all pick the same primitive, but other hosts might not.
Reacted by Benjamin Gruenbaum and HE Shi-Jun(See also tc39/proposal-cancellation#22 (comment). CC: @rbuckton, @ljharb, @domenic.)
@js-choi thanks. I'd like to emphasize that I am very interested (both personally and as a Node.js member) in this proposal regardless of the language getting cancellation before/after.
My ask here is much much smaller in scope and is only future compatibility for eventually supporting cancellation (e.g. pass a second argument to
.mapthat is an empty object that may contain a signal or whatever primitive is decided on in the future).I am confident that can be done in an iterative approach (this proposal lands, a future proposal adds this capability) and I am only asking for that reasonable extension to be taken into account.
Reacted by J. S. ChoiI feel like the thing missing here is not support for cancellation, but rather support for cleanup. That is, suppose we had a
.cleanupmethod, along the lines ofclass Iterator { #cleanup = null; cleanup(thunk) { let old = this.#cleanup; this.#cleanup = () => { old?.call(this); thunk.call(this); return this; }; } return(arg) { this.#cleanup?.(); return { value: arg, done: true }; } throw(e) { this.#cleanup?.(); throw e; } }
Then you could do
let controller = new AbortController(); const iterable = obtainGeneratorOfUrls().map(async url => { await fetch(url, { signal: controller.signal }); }).cleanup(() => { controller.abort(); }); iterable.next(); iterable.next(); iterable.return();
But you could also do any other kind of cleanup you might need to do - release resources, etc - just as you'd do in a manually implemented
returnorthrowwhen implementing the iterator protocol directly.[Edit: I guess
finallyis probably a reasonable name for this, actually? Except that it doesn't run when the underlying iterator is exhausted, which is kind of surprising.][edit x2: opened #164 ]
@benjamingr the problem is that chrome’s previously stated position is also that anything in the language using cancellation must have an AbortError that inherits from a DOMException, the latter of which doesn't belong in the language. If that changes, I’d be much more optimistic.
Having the primitive be host-defined would be a step backwards; we’re trying to have fewer things be host-defined, not more.
Reacted by Benjamin GruenbaumFirst, I'll second that I don't think cancelation support is a "v1" feature for proposal-iterator-helpers, i.e. it should not block stage advancement or anything like that. As we can see from some of the misunderstandings in this thread, it's a complex topic that needs its own discussion.
It'd be good to ensure that the proposal is forward-compatible with cancelation. That seems relatively easy; e.g., the options argument sketch shown seems like something that could be added in a future proposal with no real compat concerns. So the proposal is probably already good from that perspective, although it might be good for the champions to double-check and keep that desire in mind through any future revisions that might happen. I don't think you need to explicitly pass an empty object as the second param to
map(), personally, but maybe that's the real thing that needs discussing.Finally, to expand on what @benjamingr and @annevk are saying, and some of the things mentioned in tc39/proposal-cancellation#22: the idea would be something like this.
- Introduce a concept like "host-defined abort signal" into the ES spec.
- Add HostAddAbortSteps(hostDefinedAbortSignal, algorithmSteps) host hook
- Add HostSignalAbort(hostDefinedAbortSignal) host hook
- Add HostValidateAbortSignal(hostDefinedAbortSignal) host hook
- Add HostGetAbortReason(hostDefinedAbortSignal) host hook
- Actually integrate with the proposals. Stuff like:
- Call HostValidateAbortSignal() on the
signaloption passed to various APIs, like this proposal'smap(), or the existingimport(). (This causes early failures if something incorrect is passed as that parameter.) - Make
iterable.return()trigger HostSignalAbort() on the stored host-defined abort signal - Make
import()use HostAddAbortSteps() host hook to do stuff like rejecting the returned promise with the result of HostGetAbortReason(), and probably removing stuff from the module map
- Call HostValidateAbortSignal() on the
The end result would be, as @annevk says, that various ES-defined APIs would accept
signaloptions, which on the web/Node/Deno would use our existingAbortSignalprimitive, but other hosts would be free to define their own, via their own host hook definitions.You can imagine variants of this, e.g. if you don't want to tie yourself to the
signaloption style, you could instead have something like HostExtractAbortSignal(optionsObject), and then you could rename all the host hooks to be about "cancel tokens" or whatever name since the exact API is now up to the hosts.Reacted by Benjamin Gruenbaum5 remaining items
If so that would have blocked many language changes
How many new additional arguments were added to iteration methods? Moreover, to iteration methods that initially accept only 1 argument?
arrow functions are the preferred
But
.bindis not disabled, enough common and someone most likely will use it."Someone might use it" isn't a strong argument against; "lots of people are likely to use it" can be.
arr.push.bind(arr)vsx=>arr.push(x)is, I believe, ridiculously weighted towards the latter in terms of usage, suggesting that it would likely be fine.Reacted by Jordan Harband@tabatkins even if one of the millions of developers will use
iterator.forEach(array.push.bind(array))and will publish a website that uses it (and someone definitely will do it since it enough common pattern), the addition of one more argument to those methods will break the web. Sure, most will prefer.toArrayand operations on it, but one developer is enouth.Breaking the web isn't a binary state; one developer, one library, one site is definitely not necessarily enough. We have made many changes that technically "break the web" but don't actually do so.
You've never been able to reliably use
bindon functions you didn't author, and that continues to be the case.One more time,
How many new additional arguments were added to iteration methods? Moreover, to iteration methods that initially accept only 1 argument?
However, why am I trying to explain this? That it will break the web, nothing will change personally for me.
@benjamingr I'm not sure I understand the use case, is the requirement passing the signal (like this use case: tc39/proposal-function.sent#10 ) or
finallyhelper?@hax the requirement/ask for this proposal is to reserve the second parameter of methods like
mapto allow future extensions for signals.The actual use case is what .NET refers to "deep cancellation" - like:
const urlRequests = AsyncIterator.from(urls).map((url, { signal }) => fetch(url, { signal })); const request = urlRequests.next(); // get the next url response urlRequests.return(); // close the iteration - this should abort the currently running url request
Reacted by Robert NagyI don't understand how could we reserve the second param for signals, the
reducerofreduce()already have two arguments. And I also don't understand the code sample, where is the signal coming from? Is it auto generated by the AsyncIterator, or created in some other place and injected by some other method?I don't understand how could we reserve the second param for signals, the reducer of reduce() already have two arguments.
In those cases it'd be the third parameter - like what Node does.
where is the signal coming from? Is it auto generated by the AsyncIterator, or created in some other place and injected by some other method?
It is auto-generated by the iterator and cancellation happens when the generator is disposed (by calling
.returnon it)Reacted by Robert Nagy and HE Shi-Jun@benjamingr Thank you for the answer, now I feel I understand the case :-)
Reacted by Benjamin GruenbaumClosing in favour of #164.
Hey, @ronag and myself are looking at implementing this proposal (or a similar API) on Node.js streams and one thing we are missing is cancellation support.
Given the current state of things I think it would be useful to accept an options object as the second parameter (so we can pass an AbortSignal).
If this is complicated right now, it would be great if the spec was future-proofed for when cancellation has language level-support/integration.
Also - this is a good example of a language-level API that would benefit from cancellation at the language level.