Repository: osa1/tiny (language: rust)

Issue:
tiny fails to connect with "other os error"
I configured `tiny`, following the description in the `README.md`, but `tiny` fails to connect to any server (tried servers: `irc.mozilla.org, chat.freenode.net`) with the following message:
```
Connection error: other os error. Will try to reconnect in 30 seconds.
Disconnected. Will try to reconnect in 30 seconds.
```
I don't really know how to debug this.
I tried connecting with either TLS enabled or disabled, both don't work.
Possible related: my network has no outgoing IPv6 routes/internet acces, only IPv4, but uses IPv6 internally.


Discussion / hints:
Can you paste your tinyrc.yml file, without the passwords? 

> I tried connecting with either TLS enabled or disabled, both don't work.

Did you update the port after changing TLS settings? For plain TCP port 6665 should work, for TLS you need 6697. 

> Possible related: my network has no outgoing IPv6 routes/internet acces, only IPv4, but uses IPv6 internally.

Interesting. Does this mean you can only connect to IPv4 addresses? Currently we use the first address that the DNS query returns, perhaps in your case that's an IPv6 address that you can't use, but maybe the query returns more addresses (some of which are IPv4). I'm not sure how to reproduce this issue locally, it'd be helpful if you could try printing all addresses here: https://github.com/osa1/tiny/blob/master/libtiny_client/src/lib.rs#L405-L411 Simply adding a line like

```
for other_addr in addr_iter { println!("{:?}", other_addr); }
```

Should do it.
1.
```yaml
# Servers to auto-connect
servers:
    - addr: irc.mozilla.org
      port: 6667
      #port: 6697
      #tls: true
      hostname: 'ytrizja.duckdns.org'
      realname: Erik Z
      nicks: [ zseri, zseri2 ]

      # Channels to automatically join
      join:
          - '#rust'

      # Three authentication methods: pass, sasl, and nickserv_ident
      # These are optional and you probably only need one of these, delete
      # others.

      # Server or nick password
      # pass: 'hunter2'

      # SASL authentication
      #sasl:
      #  username: 'tiny_user'
      #  password: 'hunter2'

      # Identify nick by sending a message to NickServ:
      # (useful when `pass` or `sasl` fields above are not used)
      #nickserv_ident: 'hunter2'
    - addr: chat.freenode.net
      port: 6667
      #port: 6697
      #tls: true
      hostname: 'ytrizja.duckdns.org'
      realname: Erik Z
      nicks: [ zseri, zseri2 ]

      join:
          - '#esoteric'

      #sasl:
        #username: 'zseri'
        #password: ''

# Defaults used when connecting to servers via the /connect command
defaults:
    nicks: [ zseri, zseri2 ]
    hostname: 'ytrizja.duckdns.org'
    realname: Erik Z
    join: []
    tls: false

# Where to put log files
log_dir: '/home/z/tiny_logs'

# Color theme based on 256 colors (if supported). Colors can be defined as color
# indices (0-255) or with their names.
#
# Accepted color names are:
# default (0), black (0), maroon (1), green (2), olive (3), navy (4),
# purple (5), teal (6), silver (7), gray (8), red (9), lime (10),
# yellow (11), blue (12), magenta (13), cyan (14), white (15)
#
# Attributes can be combined (e.g [bold, underline]), and valid values are bold
# and underline
colors:
    nick: [ 1, 2, 3, 4, 5, 6, 7, 9, 10, 11, 12, 13, 14 ]

    clear:
        fg: default
        bg: default

    user_msg:
        fg: black
        bg: default

    err_msg:
        fg: black
        bg: maroon
        attrs: [bold]

    topic:
        fg: cyan
        bg: default
        attrs: [bold]

    cursor:
        fg: black
        bg: default

    join:
        fg: lime
        bg: default
        attrs: [bold]

    part:
        fg: maroon
        bg: default
        attrs: [bold]

    nick_change:
        fg: lime
        bg: default
        attrs: [bold]

    faded:
        fg: 242
        bg: default

    exit_dialogue:
        fg: default
        bg: navy

    highlight:
        fg: red
        bg: default
        attrs: [bold]

    completion:
        fg: 84
        bg: default

    timestamp:
        fg: 242
        bg: default

    tab_active:
        fg: default
        bg: default
        attrs: [bold]

    tab_normal:
        fg: gray
        bg: default

    tab_new_msg:
        fg: purple
        bg: default

    tab_highlight:
        fg: red
        bg: default
        attrs: [bold]

```
2. yes

changing the port doesn't help/doesn't change any output.
3. yes
Code:
```rust
        };

        println!("main addr = {:?}", addr);
        for other_addr in addr_iter { println!("addr = {:?}", other_addr); }

        //
```
Log:
```
Any mentions to you will be listed here.
main addr = V6([2001:948:7:7::140]:6697)
mentions irc.mozilla.org chat.freenode.n
addr = V6([2001:5a0:3604:1:64:86:243:181]:6697)
addr = V6([2a01:270:0:666f::1]:6697)
addr = V4(185.30.166.37:6697)
addr = V4(204.225.96.251:6697)
addr = V4(149.56.134.238:6697)
Any mentions to you will be listed here.                                                                                                                                                                            addr = V4(94.125.182.252:6697)
Use `/connect <server>` to connect to a server                                                                                                                                                                                                    addr = V4(38.229.70.22:6697)
addr = V4(63.245.208.110:6697)
mentions irc.mozilla.org chat.freenode.net
```
Thanks.. OK so as I suspected the address resolves to some V6 addressed and some V4 addresses, and I guess you can only use V4 addresses. I wonder what's a good way to implement this. Some ideas:

- On connection error (maybe depending on the error type) try other addresses until one of them works.
- Let users specify whether to use IPv6 or not, use IPv4 by default.

Anyone have any other ideas?
Another idea would be to use IPv4 always for now.
I think "Let users specify whether to use IPv6 or not" is a good idea. Maybe introduce an config field `force_ipv` which is either `no|4|6` (| = xor).
Here's another idea: pick one IPv4 and one IPv6 address from the list, and concurrently try to connect to both. Close the other one when one connects.
> Here's another idea: pick one IPv4 and one IPv6 address from the list, and concurrently try to connect to both. Close the other one when one connects.

I like this idea because it requires no help from the user, it just works.

I quickly implemented a PoC; @zserik can you try installing tiny again with this command

```
cargo install --git https://github.com/osa1/tiny --branch ipv_selection --force
```

and try again? Let me know how it works.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9082ce43..231afafe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# Unreleased
+
+- When a domain name resolves to multiple IP addresses tiny now tries connecting
+  to the rest of the addresses when one fails (#144).
+
 # 2019/10/05: 0.5.0
 
 Starting with this release tiny is no longer distributed on crates.io. Please
diff --git a/libtiny_client/src/lib.rs b/libtiny_client/src/lib.rs
index 55278305..b8e1bea2 100644
--- a/libtiny_client/src/lib.rs
+++ b/libtiny_client/src/lib.rs
@@ -74,19 +74,27 @@ pub struct SASLAuth {
 }
 
 /// IRC client events. Returned by `Client` to the users via a channel.
+///
+/// Note that Client only returns when it can't resolve the domain name. In all other cases (no
+/// matter what the error is) it continues, in case of a connection error either by trying another
+/// IP address of the same domain, or by waiting `RECONNECT_SECS` seconds and then trying again.
+/// The latter happens after sending a `Disconnected` event.
 #[derive(Debug)]
 pub enum Event {
-    /// Client trying to connect
-    Connecting,
+    /// Client resolving domain name
+    ResolvingHost,
+    /// Domain name resolved, client trying to connect to the given IP address
+    Connecting(SocketAddr),
     /// TCP connection established *and* the introduction sequence with the IRC server started.
     Connected,
-    /// Disconnected from the server. Usually sent right after an `Event::IoErr`.
+    /// Disconnected from the server. Usually sent right after an `Event::IoErr`. Client tries to
+    /// reconnect after `RECONNECT_SECS` seconds after sending this event.
     Disconnected,
     /// An IO error happened.
     IoErr(std::io::Error),
     /// A TLS error happened
     TlsErr(native_tls::Error),
-    /// Client couldn't resolve host address.
+    /// Client couldn't resolve host address. The client stops after sending this event.
     CantResolveAddr,
     /// Nick changed.
     NickChange(String),
@@ -360,77 +368,65 @@ async fn main_loop(
         // trailing "\r\n") and the task directly sends them to the server.
         let (mut snd_msg, mut rcv_msg) = mpsc::channel::<String>(100);
 
-        snd_ev.send(Event::Connecting).await.unwrap();
-
         //
         // Resolve IP address
         //
 
+        snd_ev.send(Event::ResolvingHost).await.unwrap();
+
         let serv_name = server_info.addr.clone();
 
         debug!("Resolving address");
 
         let serv_name_clone = serv_name.clone();
 
-        let mut addr_iter =
-            match resolve_addr(serv_name_clone, port, &mut rcv_cmd, &mut snd_ev).await {
-                Done(addr_iter) => {
-                    debug!("resolve_addr: done");
-                    addr_iter
-                }
-                TryWithPort(new_port) => {
-                    debug!("resolve_addr: try new port");
-                    port = new_port;
-                    wait = false;
-                    continue;
-                }
-                TryReconnect => {
-                    debug!("resolve_addr: try again");
-                    wait = false;
-                    continue;
-                }
-                TryAfterDelay => {
-                    debug!("resolve_addr: try after delay");
-                    wait = true;
-                    continue;
-                }
-                Return => {
-                    debug!("resolve_addr: return");
-                    return;
-                }
-            };
-
-        debug!("Address resolved");
-
-        let addr = match addr_iter.next() {
-            None => {
-                snd_ev.send(Event::CantResolveAddr).await.unwrap();
-                break;
+        let addr_iter = match resolve_addr(serv_name_clone, port, &mut rcv_cmd, &mut snd_ev).await {
+            Done(addr_iter) => {
+                debug!("resolve_addr: done");
+                addr_iter
+            }
+            TryWithPort(new_port) => {
+                debug!("resolve_addr: try new port");
+                port = new_port;
+                wait = false;
+                continue;
+            }
+            TryReconnect => {
+                debug!("resolve_addr: try again");
+                wait = false;
+                continue;
+            }
+            TryAfterDelay => {
+                debug!("resolve_addr: try after delay");
+                wait = true;
+                continue;
+            }
+            Return => {
+                debug!("resolve_addr: return");
+                return;
             }
-            Some(addr) => addr,
         };
 
+        let addrs = addr_iter.collect::<Vec<_>>();
+
+        if addrs.is_empty() {
+            snd_ev.send(Event::CantResolveAddr).await.unwrap();
+            break; // returns
+        }
+
+        debug!("Address resolved: {:?}", addrs);
+
         //
         // Establish TCP connection to the server
         //
 
-        debug!("Establishing connection ...");
-
-        // TODO: This step can block too, but hopefully not too much?
-        let mb_stream = if server_info.tls {
-            Stream::new_tls(addr, &serv_name).await
-        } else {
-            Stream::new_tcp(addr).await
-        };
-
-        let stream = match mb_stream {
-            Ok(stream) => stream,
-            Err(err) => {
-                snd_ev.send(Event::from(err)).await.unwrap();
+        let stream = match try_connect(addrs, &serv_name, server_info.tls, &mut snd_ev).await {
+            None => {
                 snd_ev.send(Event::Disconnected).await.unwrap();
                 wait = true;
                 continue;
             }
+            Some(stream) => stream,
         };
 
         let (mut read_half, mut write_half) = tokio::io::split(stream);
@@ -642,3 +638,29 @@ async fn resolve_addr(
         }
     }
 }
+
+async fn try_connect(
+    addrs: Vec<SocketAddr>,
+    serv_name: &str,
+    use_tls: bool,
+    snd_ev: &mut mpsc::Sender<Event>,
+) -> Option<Stream> {
+    for addr in addrs {
+        snd_ev.send(Event::Connecting(addr)).await.unwrap();
+        let mb_stream = if use_tls {
+            Stream::new_tls(addr, &serv_name).await
+        } else {
+            Stream::new_tcp(addr).await
+        };
+        match mb_stream {
+            Err(err) => {
+                snd_ev.send(Event::from(err)).await.unwrap();
+            }
+            Ok(stream) => {
+                return Some(stream);
+            }
+        }
+    }
+
+    None
+}
diff --git a/src/conn.rs b/src/conn.rs
index f5a40390..735a1f30 100644
--- a/src/conn.rs
+++ b/src/conn.rs
@@ -26,9 +26,17 @@ pub(crate) async fn task(
 fn handle_conn_ev(ui: &dyn UI, client: &Client, ev: libtiny_client::Event) -> bool {
     use libtiny_client::Event::*;
     match ev {
-        Connecting => {
+        ResolvingHost => {
             ui.add_client_msg(
-                "Connecting...",
+                "Resolving host...",
+                &MsgTarget::AllServTabs {
+                    serv: client.get_serv_name(),
+                },
+            );
+        }
+        Connecting(sock_addr) => {
+            ui.add_client_msg(
+                &format!("Connecting to {}", sock_addr),
                 &MsgTarget::AllServTabs {
                     serv: client.get_serv_name(),
                 },
@@ -57,11 +65,7 @@ fn handle_conn_ev(ui: &dyn UI, client: &Client, ev: libtiny_client::Event) -> bo
         }
         IoErr(err) => {
             ui.add_err_msg(
-                &format!(
-                    "Connection error: {}. Will try to reconnect in {} seconds.",
-                    err.description(),
-                    libtiny_client::RECONNECT_SECS
-                ),
+                &format!("Connection error: {}", err.description()),
                 time::now(),
                 &MsgTarget::AllServTabs {
                     serv: client.get_serv_name(),
@@ -70,11 +74,7 @@ fn handle_conn_ev(ui: &dyn UI, client: &Client, ev: libtiny_client::Event) -> bo
         }
         TlsErr(err) => {
             ui.add_err_msg(
-                &format!(
-                    "TLS error: {}. Will try to reconnect in {} seconds.",
-                    err.description(),
-                    libtiny_client::RECONNECT_SECS
-                ),
+                &format!("TLS error: {}", err.description()),
                 time::now(),
                 &MsgTarget::AllServTabs {
                     serv: client.get_serv_name(),


Test patch: