-
Notifications
You must be signed in to change notification settings - Fork 3
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
Detect and handle connection lost errors #50
base: main
Are you sure you want to change the base?
Detect and handle connection lost errors #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @e-nastasia. I'm sorry for delay.
Thank you for pointing it out.
Could you check some comments?
Err(_) => continue, | ||
Err(e) => match e { | ||
Error::MaxRetriesExceeded => { | ||
error!("Reached MaxRetriesExceeded"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use lowercase for error messages?
ref: rust-lang/api-guidelines#79
break; | ||
} | ||
Error::ConnectionClosed => { | ||
error!("Reached ConnectionClosed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can't access this error because it isn't returned to the main task.
Are you assuming that this pull request is a first step to fix a connection lost problem?
Hello @johnmanjiro13!
Thank you for the work you've put into creating this crate! I am happy it exists and I find it very useful.
I am having one issue with it though: when my binary loses connection to fluent, there's no way for me to catch this situation and handle it because the connection errors aren't propagated from the crate.
So I've created this pull request to address the issue: with these changes crate users will be able to get access to the connection lost errors so they can do something about it.