Cancellation

[benjamingr]

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.

[devsnek]

can you elaborate more on this? in my mind you can "cancel" an iterator using .return/.throw.

[benjamingr]

Sure, I was sitting with @ronag and @Ethan-Arrowhead in a conference and people are asking .map on 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.
``

[zloirock]

AbortSignal is 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.

[devsnek]

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.

[benjamingr]

@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 AbortSignal is 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.

[Jack-Works]

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, Event in the language first.

[benjamingr]

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 AddEventListener operation and use it to add listeners to a signal (with "abort" passed) and probably a CreateSignal and MarkSignalAborted.

That is considerably less work than specifying EventTarget/Event.

[Jack-Works]

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.

[benjamingr]

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 EventTarget in the language. I am suggesting specifying hooks in the language to enable interacting with existing AbortSignals.

[annevk]

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.

[js-choi]

(See also tc39/proposal-cancellation#22 (comment). CC: @rbuckton, @ljharb, @domenic.)

[benjamingr]

@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 .map that 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.

[bakkot]

I feel like the thing missing here is not support for cancellation, but rather support for cleanup. That is, suppose we had a .cleanup method, along the lines of

class 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 return or throw when implementing the iterator protocol directly.

[Edit: I guess finally is 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 ]

[ljharb]

@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.

[domenic]

First, 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 signal option passed to various APIs, like this proposal's map(), or the existing import(). (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

The end result would be, as @annevk says, that various ES-defined APIs would accept signal options, which on the web/Node/Deno would use our existing AbortSignal primitive, 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 signal option 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.

[benjamingr]

@ljharb - I am worried this is getting more and more off-topic but it's still interesting discussion.

Maintainers of the repo - feel free to shut the discussion down and return it to the original discussion:

@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.

We are actually discussing using DOMExceptions for AbortErrors in Node.js rather than our own (to see if that can be unblocked by previous objections) but note that isn't actually a requirement (you can use other errors with AbortController/AbortSignal promise APIs).

In particular my understanding of Domenic's "how this can be done with hooks" does not require AbortError or DOMException.

If however there was a non DOMException AbortError standardized do you believe that would unblock all objections to language level support for the cancellation primitive?

[domenic]

In particular my understanding of Domenic's "how this can be done with hooks" does not require AbortError or DOMException.

Correct, you would use HostGetAbortReason() which would return a host-specific value (usually an "AbortError" DOMException in the web/Deno, and sometimes an "AbortError" DOMException in Node/sometimes a Node-specific AbortError).

[ljharb]

@benjamingr yes, notably, one that is in no way host-defined, including its prototype chain.

[zloirock]

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.

I'm pretty sure that someone will make something like

iterator.forEach(array.push.bind(array))

and in the future it will be impossible.

[ljharb]

If so that would have blocked many language changes; arrow functions are the preferred and more common way of passing callbacks of builtins (ie, the parseInt bug)

[zloirock]

@ljharb

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 .bind is not disabled, enough common and someone most likely will use it.

[tabatkins]

"Someone might use it" isn't a strong argument against; "lots of people are likely to use it" can be.

arr.push.bind(arr) vs x=>arr.push(x) is, I believe, ridiculously weighted towards the latter in terms of usage, suggesting that it would likely be fine.

[zloirock]

@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 .toArray and operations on it, but one developer is enouth.

[ljharb]

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.

[zloirock]

And in too many cases didn't do it because of only one website where it could cause problems. Also Array#flatten, { String, Array }#contains, etc...

However, who cares?

[ljharb]

You've never been able to reliably use bind on functions you didn't author, and that continues to be the case.

[zloirock]

One more time,

How many new additional arguments were added to iteration methods? Moreover, to iteration methods that initially accept only 1 argument?

[zloirock]

However, why am I trying to explain this? That it will break the web, nothing will change personally for me.

[hax]

@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 finally helper?

[benjamingr]

@hax the requirement/ask for this proposal is to reserve the second parameter of methods like map to 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

[hax]

I don't understand how could we reserve the second param for signals, the reducer of reduce() 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?

[benjamingr]

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 .return on it)

[hax]

@benjamingr Thank you for the answer, now I feel I understand the case :-)

[michaelficarra]

Closing in favour of #164.