Cancellation APIs

[benjamingr]

Hey,

It would be really cool if Deno aligned with the web platform's cancellation primitive AbortSignal/AbortController which Deno already supports but not in most of its APIs.

Myself and a few others have been adding support for AbortSignal to all of the APIs it fits in Node.js to make it easier for people to write universal JavaScript code. For example:

Deno.readTextFile('./someFile', { signal });
Deno.writeTextFile('./someFile', { signal });

And more generally - in Deno's stream abstractions. Would you be open to adding this?

(Sorry for not being too available on Discord with the baby/health stuff/new job :))

[jsejcksn]

I saw there was a PR merged for this feature for addEventListener on EventTarget (#8616) but I'm not sure whether or not it landed in the binary yet, since I can't find any references to a signal property on AddEventListenerOptions the docs or deno types output. (I guess I could also just write a test 🙈)

How do you propose that it would be implemented? Would it cause a sync function to throw (or an async function to reject with) a DOMException for all supported functions?

[benjamingr]

@jsejcksn

I saw there was a PR merged for this feature for addEventListener on EventTarget (#8616) but I'm not sure whether or not it landed in the binary yet, since I can't find any references to a signal property on AddEventListenerOptions the docs or deno types output.

I actually authored that PR (and co-authored that feature in the DOM). If docs/types are missing I am happy to contribute those as well.

How do you propose that it would be implemented? Would it cause a sync function to throw (or an async function to reject with) a DOMException for all supported functions?

Basically, yes.

Go through all APIs that are cancellable/disposable and add support using the WHATWG/dom guidance (add a signal option to the options params and have the promise returned reject with an AbortError). In Node.js we went with "DOMException like" AbortErrors because we vendor some of our code separately - Deno would presumably just use DOMException.

Note there is very little (if at all) value to add this to sync APIs IMO :)

[lucacasonato]

Note there is very little (if at all) value to add this to sync APIs IMO :)

It wouldn't be possible to add to sync APIs anyway, because how would you trigger the abort while the event loop is blocked 😉.

[benjamingr]

It wouldn't be possible to add to sync APIs anyway, because how would you trigger the abort while the event loop is blocked 😉.

I'm glad you asked 😅

Technically (and I know this sounds odd, but this was discussed in the Atomics.waitAsync discussions) you could:

• Make AbortController shareable (this should be doable spec wise if people think it's a good idea, probably not).
• Create a Worker.
• Create an AbortController.
• Share the AbortController with the worker (like SAB) (with a postMessage).
• Do something like setTimeout(() => controller.abort(), 100) in the worker.
• Pass the signal to a Deno.readTextFileSync.

That is not worth the time, effort, maintenance, spec work or mental capacity though IMO :D

[lucacasonato]

I don't think it is possible even if we had a shareable AbortController, because AFAIK the kernel has no way to cancel a synchronous read syscall (without pthread_kill the invoking thread). To make this work we would have to make the "sync read" effectively an async read under the hood, but expose it as a sync read to JS. As you said way more complex than need be 😉.

Regarding async cancellable operations: Generally I am in favor of this. I think it makes sense to be able to cancel certain operations (like async read from a net socket).

[piscisaureus]

because AFAIK the kernel has no way to cancel a synchronous read syscall

Send a signal?
(note how we're full circle now)

[benjamingr]

What would be a good way to have a discussion about this?

It seems like if I start adding APIs to stuff like readTextFile in PRs it would beat the point of having a unified effort towards web-standards cancellation.

I'd at least like the core team (or whoever is in charge) to discuss this first :) I am happy to show up to the meeting to discuss what I'm suggesting based on work in Node and in the DOM spec if that helps.

[benjamingr]

Kind ping

[lucacasonato]

We have AbortSignal in fetch since 1.11.

Generally still in favor of this, but we should discuss which APIs it makes sense for. My current list of initial candidates:

• Deno.readFile
• Deno.readTextFile
• Deno.writeFile
• Deno.writeTextFile

We could start by implementing these to only cancel the internal buffering. This would negate the need to implement cancellation of Deno.read / Deno.write. Canceling Deno.read and Deno.write would be kinda strange as it could lead to inconsistent fd seeker positions in the kernel. Also not sure if Tokio even supports canceling async fs IO.

All of the above APIs would need to get an options bag.

[benjamingr]

@lucacasonato so a good way forward if I want to promote this is to PR adding AbortSignal to stuff like Deno.readFile like I've done for Node's fs.readFile?

[lucacasonato]

Yup, that works for me :-)

We can hash out any details in a PR.

[benjamingr]

Ok, got 30m today going to tackle the next one now that readFile is in

[mfulton26]

I would also like this but for reading bytes on a TCP socket via Deno.Conn from a Deno.connect.

The Reader interface doesn't currently support an abort signal though. Should I open up a separate issue for such or should/can it be part of this one?

e.g. For a simple telnet-like app I want to read incoming bytes but bail after some set amount of time

[benjamingr]

@mfulton26 I'm not deno core I just contribute from time to time to keep up so take my answer with a grain of salt:

It absolutely makes sense for Reader to take AbortSignal and I would open a separate issue asking for that specifically and link it to this one

[jsejcksn]

I think also about the ergonomics of Deno.open and Deno.FsFile — and basically all non-fetch steams.

[mfulton26]

I'd like to see iterateReader's returned AsyncIterableIterator<Uint8Array> support the optional return() method in the async iterator protocol so that when a break or throw is encountered in a for await...of the iterateReader can automatically cancel the underlying read call using try...finally.

We can't create such APIs though without the underlying APIs exposing the functionality. I'm glad this issue is being tracked here. Do we have any ideas on timeline for this? I have written workaround code for now that leaves read calls open until I close the underlying connection. This works but is less than ideal IMO.

[IlyaBoykoAtWork]

I would also like this but for reading bytes on a TCP socket via Deno.Conn from a Deno.connect.

I would also like this for Deno.connect itself!