connect_tcp() is inconsistent between Tokio and async-std

[abonander]

While triaging #2808 I realized that we have inconsistent behavior between Tokio and async-std when opening a TCP connection, in the case that a domain name resolves to multiple IP addresses:

The Tokio socket implementation will actually try them all in order as we just forward to TcpStream::connect(): https://github.com/launchbadge/sqlx/blob/main/sqlx-core/src/net/socket/mod.rs#L197

However, the async-std implementation is a little funky because async_std::net::TcpStream doesn't have the same set of calls we expect (try_read, try_write, poll_ready()) and so we have to go down one level and use async-io directly. But Async<TcpStream>::connect() doesn't use ToSocketAddrs so I had to implement that part manually, and I didn't realize the behavior as-written was inconsistent.

This is an easy fix, though:

sqlx/sqlx-core/src/net/socket/mod.rs

Lines 208 to 212 in 58cb18a

	let socket_addr = (host, port)
	.to_socket_addrs()
	.await?
	.next()
	.expect("BUG: to_socket_addrs() should have returned at least one result");

That code should iterate through the addresses returned by (host, port).to_socket_addrs().await? and try to connect to each one in turn, returning the first that succeeds or the last error otherwise. If it resolves to zero addresses, it should also return an error instead of panicking (what actually happens if the hostname doesn't resolve to anything? I don't actually know).

It'd be pretty complex to set up a regression test for it, though it'd be nice to at least manually test it.

[anupj]

@abonander so something like this would work?

    #[cfg(feature = "_rt-async-std")]
    {
        use async_io::Async;
        use async_std::net::ToSocketAddrs;
        use std::net::TcpStream;

        let mut last_err = None;

        // Loop through all the Socket Addresses that the hostname resolves to
        for socket_addr in (host, port).to_socket_addrs().await? {
            match Async::<TcpStream>::connect(socket_addr).await {
                Ok(stream) => return Ok(with_socket.with_socket(stream)),
                Err(e) => last_err = Some(e),
            }
        }

        // If we reach this point, it means we failed to connect to any of the addresses.
        // Return the last error we encountered, or a custom error if the hostname didn't resolve to any address.
        match last_err {
            Some(err) => return Err(err.into()),
            None => {
                return Err(io::Error::new(
                    io::ErrorKind::AddrNotAvailable,
                    "Hostname did not resolve to any addresses",
                )
                .into())
            }
        }
    }

I tried this on my fork but the tests fail with the following (unrelated) message:

thread 'it_does_not_stop_stream_after_decoding_error' panicked at 'at least one of the `runtime-*` features must be enabled', tests/any/any.rs:47:1

[abonander]

@anupj you have to enable the runtime-async-std feature.

[anupj]

@abonander I am now following the tests README.md instructions and I get this error message when I run ./x.py.

failures:

---- it_opens_with_extension stdout ----
Error: error returned from database: (code: 0) dlopen(ipaddr.dylib, 0x000A): tried: 'ipaddr.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OSipaddr.dylib' (no such file), '/usr/lib/ipaddr.dylib' (no such file, not in dyld cache), 'ipaddr.dylib'

[abonander]

There's some fixes to x.py in #2754 and it looks like that error is one of them. I've reopened it to merge.

[abonander]

If you rebase, x.py should be working now.

[anupj]

Thank you @abonander and thanks for merging my PR 👍🏽