Conversation
Enet4
left a comment
There was a problem hiding this comment.
OK, looks nice and simple! Please see the minor suggestions below and we should be able to merge afterwards!
|
@Enet4 would this work ? |
Enet4
left a comment
There was a problem hiding this comment.
Thank you for committing to the PR so far. I have just one more concern and then we can merge.
ul/src/association/client.rs
Outdated
| let conn_result: Result<TcpStream> = if let Some(timeout) = connection_timeout { | ||
| let addresses = ae_address.to_socket_addrs().context(ToAddressSnafu)?; | ||
|
|
||
| let mut res: Result<TcpStream> = NoAddressSnafu {}.fail(); |
There was a problem hiding this comment.
I would appreciate it if you could avoid building this Result::Err every time, because this makes the function pay the cost of constructing a backtrace even when it is not used.
|
As I had to store all the connection errors up to an Ok result, I had to use a Vec. |
|
So this last one fails early by borrowing the |
|
@Enet4 please retract this pull request as the code does not work as intended I am sorry. |
|
Hm, I'm away from a desktop computer, but if you write a pull request within the weekend I'll likely have a moment to try it out and merge. |
A TCP connection od a client connection will hang in case of an invalid address or network error.
This adds the option of a connection timeout, which is different from the already implemented read/write timeout on the TcpStream