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

allow async iterator to be already aborted. #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

martinheidegger
Copy link
Contributor

I noticed that a signal can be aborted when being passed-in. This PR checks if the signal is aborted and throws an error.

index.js Show resolved Hide resolved
@mafintosh
Copy link
Owner

Small comment, looks great.

@martinheidegger in general do you know if people unregister the abort listenener when your resource completes succesfully?

@martinheidegger martinheidegger force-pushed the signal-aborted branch 2 times, most recently from 2cb4ac5 to c04eaa0 Compare December 4, 2020 14:31
@martinheidegger
Copy link
Contributor Author

in general do you know if people unregister the abort listenener when your resource completes succesfully?

I obviously didn't think of that, thanks @mafintosh for pointing this out. Naively I assumed that abortSignals and streams are usually short-lived objects that should be quickly garbage collected but indeed, if a async process (eg. in a cli application starting the server) can be a lot longer-lived it should be removed. - node.js of course removes the listener here - I added a commit that takes care of this!

@martinheidegger
Copy link
Contributor Author

Rebased to recent code updates

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.

None yet

2 participants