-
Notifications
You must be signed in to change notification settings - Fork 205
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
Timeout : return an error compatible with os.IsTimeout() #118
base: master
Are you sure you want to change the base?
Conversation
The actual behavior is "return 0 without error", that can also be understood as "end of stream". Network connections and file reads have their own timeout errors that can be detected with |
This is exactly the issue i'm running into. I'm using |
@maitredede do you have time to rebase this? |
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 case Timeout:; return "Timeout"
should also be added before default:
(bcaab3f#diff-ad4fb56affb62e9f5024c32ea8a857376661813e1bc22b7dfef64f21ff16c34fR176)
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.
Done
@maitredede i you don't have time, i think i can take over with a new PR (keeping you as author of course). |
… be checked with os.IsTimeout()
Hello @quite, I have rebased/updated 😃 |
@maitredede great! I hope @cmaglie will get some time to look over this and merge it |
This would be very useful to have as I'm testing various types of connections and all of them have timeouts except for my serial ones and this would fix it. |
@cmaglie sorry to ping you as you said, on another issue, that there is no timeline for adding timeouts in a possible 2.0 version several months ago. |
Like
*net.Conn.Read()
or*os.File.Read()
when a timeout occurs, return an error that can be checked withos.IsTimeout()