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

change the PingAsync logic #32

Merged
merged 2 commits into from
Dec 9, 2020
Merged

change the PingAsync logic #32

merged 2 commits into from
Dec 9, 2020

Conversation

DHclly
Copy link
Contributor

@DHclly DHclly commented Jun 18, 2019

resolve if can't connect to clam av server ,
PingAsync will throw a exception,it should be return false.

Signed-off-by: DHclly 335121817@qq.com

resolve if can't connect to clam av server ,
PingAsync will throw a  exception,it should be return false.

Signed-off-by: DHclly <335121817@qq.com>
@tekmaven
Copy link
Owner

Hello and thanks for the contribution, it's really appreciated! There are many users of this package and I've done my best to avoid breaking changes to the existing API. Users could depend on this to throw in some cases, so I think we should keep this method as-is and create a new method called TryPingAsync, which adds the try/catch logic. Or you can just wrap your call to PingAsync in a try/catch.

because the method PingAsync if not true ,will throw a exception,so add this
method only return true/false.

Signed-off-by: DHclly <335121817@qq.com>
@DHclly
Copy link
Contributor Author

DHclly commented Jun 24, 2019

Thank you for your reply , I add the method TryPingAsync and add some tip at method PingAsync

@tekmaven
Copy link
Owner

Thanks @DHclly I will incorporate this into the next release shortly.

@douglasg14b
Copy link

@tekmaven When will this be merged? The exception throwing behavior of TryAsync is far from ideal and makes for very verbose and sometimes nested try/catch blocks

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

3 participants