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

[feature] Add an option to suppress CancelledError #28

Open
jonmartz opened this issue May 22, 2022 · 2 comments
Open

[feature] Add an option to suppress CancelledError #28

jonmartz opened this issue May 22, 2022 · 2 comments

Comments

@jonmartz
Copy link

Hello,

In recursive_resolver.py there is a try block that we suspect could be preventing some cancelled programs from actually exiting:

image

If a asyncio.CancelledError is thrown on line 145, it would be supressed by the except clause. Isn't this an issue? Or is it intentional?

In case it's an issue, a possible solution would be to add the following two lines above the except in line 147:

except asyncio.CancelledError:
    raise

Thanks.

@gera2ld
Copy link
Owner

gera2ld commented May 22, 2022

Hi, thanks for reaching out, I think it was intentional.
If we cancel the request on CancelledError due to timeout, we will lose the chance to get the response for the ongoing request and we will need to restart it again later if we still need it.

For example, a query takes a timeout of 3.0s by default, and if it contains 2 requests, each taking 2s, then the query will time out on the second request, and your question is, whether the second request should be canceled.

If the request is canceled, and later the user retries, we still have to resend the request, and it's likely to fail again. But if the request is shielded and it continues to wait for a full timeout (3.0s) as an independent query, it will finally succeed and update the cache. So when the user retries, we can provide the cached response immediately.

BTW, did this behavior cause any problem for you?

@jonmartz
Copy link
Author

Thanks, that makes sense.

Our problem is that a program that should terminate (due to an exception) instead hangs forever, apparently because one or more asyncio task cancellations failed due to try blocks that stop the raise of asyncio.CancelledError.

That being said, we are not sure if specifically async_dns is causing the problem.

@gera2ld gera2ld changed the title Suppressing CancelledError [feature] Add an option to suppress CancelledError Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants