connection: Try all resolved TCP addresses [CPP-470] #275
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moves to try all resolved addresses from the user specified host name, rather than just the first one returned. This makes the console behave in line with the old console, which used Python's `socket.create_connection` (https://docs.python.org/3/library/socket.html#socket.create_connection) under the covers to connect to the TCP socket.
samvrlewis
commented
Dec 13, 2021
| if let Some(s) = socket.next() { | ||
| Ok(s) | ||
| } else { | ||
| Err(anyhow!("{}", TCP_CONNECTION_PARSING_FAILURE)) |
There was a problem hiding this comment.
I don't think this line will have ever been triggered, as it looks like to_socket_addr will return an error both if the name can't be resolved to an address or if it can't be parsed - so I've removed this error for now.
notoriaga
reviewed
Dec 13, 2021
| fn open_connection(&self) -> io::Result<TcpStream> { | ||
| let addresses = self.name.to_socket_addrs()?; | ||
|
|
||
| let mut errors = vec![]; |
There was a problem hiding this comment.
This errors vec is not being used anywhere, maybe just log the errors instead? Or you could maybe return them instead of the the last error you have at the bottom
There was a problem hiding this comment.
Oops, good catch - was meaning to add the errors to the returned error if no connection worked, which I've done now.
notoriaga
approved these changes
Dec 13, 2021
silverjam
approved these changes
Dec 13, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moves to try all resolved addresses from the user specified host name,
rather than just the first one returned. This makes the console behave
in line with the old console, which used Python's
socket.create_connection(https://docs.python.org/3/library/socket.html#socket.create_connection)
under the covers to connect to the TCP socket.