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 deadline to async module #1022

Merged
merged 1 commit into from
Jul 12, 2021

Conversation

lambdalisue
Copy link
Contributor

@lambdalisue lambdalisue commented Jul 10, 2021

Hi, I'm not sure if you like the idea but I added a timeout function to the async module while simple code with delay(100).then(() => new Error("Timeout")) would cause the following error

AssertionError: Test case is leaking async ops.
Before:
  - dispatched: 0
  - completed: 0
After:
  - dispatched: 4
  - completed: 3

Make sure to await all promises returned from Deno APIs before

Added timeout function call clearTimeout to avoid such kind of error.

Please close this PR if it does not suit the std requirements.

async/timeout.ts Outdated Show resolved Hide resolved
async/timeout.ts Outdated Show resolved Hide resolved
async/README.md Outdated Show resolved Hide resolved
@lambdalisue
Copy link
Contributor Author

I'll squash commits once reviews are completed.

Copy link
Contributor

@wperron wperron left a comment

Choose a reason for hiding this comment

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

There's just a small thing to fix in the docs, otherwise LGTM

async/README.md Outdated
import { deadline } from "https://deno.land/std/async/mod.ts";

const delayedPromise = delay(1000);
// Above throws `DeadlineError` after 10 ms
Copy link
Contributor

Choose a reason for hiding this comment

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

Except it's not above anymore 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. I'll reword.

async/deadline.ts Show resolved Hide resolved
@wperron
Copy link
Contributor

wperron commented Jul 12, 2021

Also, just make sure you rebase, but you don't have to squash, we can do it on merge ourselves

@lambdalisue
Copy link
Contributor Author

Rebase & force pushed 👍

Copy link
Contributor

@wperron wperron left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot for the contribution!

@wperron wperron merged commit 717e976 into denoland:main Jul 12, 2021
@lambdalisue lambdalisue deleted the async-timeout branch July 12, 2021 14:09
@lambdalisue lambdalisue changed the title feat(async): add timeout to async module feat(async): add deadline to async module Jul 15, 2021
lambdalisue added a commit to vim-denops/denops.vim that referenced this pull request Jul 15, 2021
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.

2 participants