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

feat(async): Add abortable to async module. #1939

Merged
merged 3 commits into from
Feb 21, 2022

Conversation

lambdalisue
Copy link
Contributor

Hi.

The abortable is a wrapper function that makes Promise and AsyncIterable cancelable.

For example, in the case of Promise, it looks like this

import { abortable } from "https://deno.land/std/async/mod.ts";
import { delay } from "https://deno.land/std/async/mod.ts";
const p = delay(1000);
const c = new AbortController();
setTimeout(() => c.abort(), 100);
// Below throws `AbortedError` after 100 ms
await abortable(p, c.signal);

and for AsyncIterable as follows

import { abortable } from "https://deno.land/std/async/mod.ts";
import { delay } from "https://deno.land/std/async/mod.ts";
const p = async function* () {
  yield "Hello";
  await delay(1000);
  yield "World";
};
const c = new AbortController();
setTimeout(() => c.abort(), 100);
// Below throws `AbortedError` after 100 ms
// and items become `["Hello"]`
const items: string[] = [];
for await (const item of abortable(p(), c.signal)) {
  items.push(item);
}

I hope you all like it.

@CLAassistant
Copy link

CLAassistant commented Feb 19, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! This seems a reasonable addition to std/async to me.

LGTM

// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
import { deferred } from "./deferred.ts";

export class AbortedError extends Error {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it makes sense to use DOMException with AbortError instead of this custom error:

new DOMException(reason ? `Aborted: ${reason}` : "Aborted", "AbortError")

Copy link
Member

Choose a reason for hiding this comment

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

Yes i think this would be preferable imo. Ideally it would be using error causes, but DOMException doesnt support that yet (whatwg/webidl#969).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall I rewrite it as DOMException with that?

Copy link
Member

Choose a reason for hiding this comment

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

Shall I rewrite it as DOMException with that?

I think that's more preferable (fetch API rejects with that error, for example). Let's do that

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

@lambdalisue LGTM!

@kt3k kt3k merged commit 5e8adcb into denoland:main Feb 21, 2022
@lambdalisue lambdalisue deleted the async-abortable branch February 21, 2022 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants