Report TCP connect failures in the relay window

README.md

@@ -160,7 +160,7 @@ In the relay's window:
 
 | Symptom | Cause | What to do |
 |---|---|---|
-| `Connect TCP` appears to do nothing and the status stays `false` | The connection failed. No error is displayed, and pressing `Connect TCP` again fails silently because the previous thread still exists. | Press `Disconnect TCP` first, then `Connect TCP`. This is the most common first-time snag. |
+| `Connect TCP` fails and the status stays `false` | Nothing is listening at that address, or the address is wrong | Read the reason in the error banner and the event log, fix the address, then press `Connect TCP` again. `Disconnect TCP` first is no longer needed |
 | `:( No Baton Connection` | The plugin is not loaded, or its window is closed | Check `baton_debug.log` in your temp folder (`%TEMP%` on Windows). Every connection attempt is logged there |
 | Data stops part-way through a flight | The "Positional Flight Data" window was closed or hidden (Windows/Linux only) | Reopen it |
 | The relay runs but never picks up the plugin | The socket is already in use. A second relay instance, or a stale `baton.sock`. The relay logs this once, then gives up silently. | `Disconnect IPC`, then `Connect IPC`. Or restart the relay |

relay/src/state.rs

@@ -553,8 +553,14 @@ impl State {
                     stream
                 }
                 Err(e) => {
-                    human_log("TCP", &format!("Connection failed: {}", e));
-                    bail!("Failed to connect to TCP");
+                    let reason = format!("TCP connection failed: {}", e);
+                    human_log("TCP", &reason);
+                    let _ = child_bichannel.set_is_conn_to_endpoint(false);
+                    // Report it so the GUI can show the failure and drop this
+                    // thread, instead of the error only reaching stderr.
+                    let _ = child_bichannel
+                        .send_to_parent(FromTcpThreadMessage::Disconnected(reason.clone()));
+                    bail!(reason);
                 }
             };
 
@@ -653,6 +659,9 @@ impl State {
         });
 
         self.tcp_thread_handle = Some(tcp_thread_handle);
+        // Clear any banner left by an earlier failure, so a successful retry
+        // does not keep showing the old error.
+        self.error_message = None;
         Ok(())
     }
 
@@ -679,6 +688,19 @@ impl State {
         Ok(res?)
     }
 
+    // Drops a TCP thread that has already exited, so tcp_connect is allowed
+    // to start a new one. The join runs detached because the thread returned
+    // an error and its Result is of no use here.
+    pub fn reap_tcp_thread(&mut self) {
+        if let Some(handle) = self.tcp_thread_handle.take() {
+            spawn(move || {
+                let _ = handle.join();
+            });
+        }
+        self.tcp_bichannel = None;
+        self.tcp_connected = false;
+    }
+
     pub fn is_tcp_connected(&self) -> bool {
         if let Some(status) = self
             .tcp_bichannel

relay/src/update.rs

@@ -67,23 +67,30 @@ pub(crate) fn update(state: &mut State, message: Message) -> Task<Message> {
             }
 
             // process TCP messages
-            if let Some(tcp_bichannel) = &state.tcp_bichannel {
-                for msg in tcp_bichannel.received_messages() {
-                    match msg {
-                        FromTcpThreadMessage::Connected => {
-                            state.log_event("TCP connected to iMotions".into());
-                            state.tcp_connected = true;
-                        }
-                        FromTcpThreadMessage::Disconnected(reason) => {
-                            state.log_event(format!("TCP disconnected: {}", reason));
-                            state.tcp_connected = false;
-                        }
-                        FromTcpThreadMessage::Sent(bytes) => {
-                            state.on_tcp_packet_sent(bytes);
-                        }
-                        FromTcpThreadMessage::SendError(err) => {
-                            state.log_event(format!("TCP send error: {}", err));
-                        }
+            let tcp_messages = state
+                .tcp_bichannel
+                .as_ref()
+                .map(|bichannel| bichannel.received_messages())
+                .unwrap_or_default();
+            for msg in tcp_messages {
+                match msg {
+                    FromTcpThreadMessage::Connected => {
+                        state.log_event("TCP connected to iMotions".into());
+                        state.tcp_connected = true;
+                    }
+                    FromTcpThreadMessage::Disconnected(reason) => {
+                        state.log_event(format!("TCP disconnected: {}", reason));
+                        state.error_message = Some(reason);
+                        state.tcp_connected = false;
+                        // The thread has exited. Clear it so Connect TCP works
+                        // again without pressing Disconnect TCP first.
+                        state.reap_tcp_thread();
+                    }
+                    FromTcpThreadMessage::Sent(bytes) => {
+                        state.on_tcp_packet_sent(bytes);
+                    }
+                    FromTcpThreadMessage::SendError(err) => {
+                        state.log_event(format!("TCP send error: {}", err));
                     }
                 }
             }
@@ -346,6 +353,61 @@ mod tests {
         let _ = state.tcp_disconnect();
     }
 
+    // Pumps the Update message the GUI subscription would send, so queued
+    // thread messages are processed.
+    fn pump_update(state: &mut State, ticks: usize) {
+        for _ in 0..ticks {
+            let _ = update(state, Message::Update);
+            sleep(StdDuration::from_millis(10));
+        }
+    }
+
+    // A port nothing listens on, obtained by binding and then dropping.
+    fn closed_port_addr() -> String {
+        let listener = TcpListener::bind("127.0.0.1:0").expect("bind failed");
+        let addr = listener.local_addr().expect("local_addr failed").to_string();
+        drop(listener);
+        addr
+    }
+
+    #[test]
+    fn failed_connect_surfaces_an_error_and_allows_a_retry() {
+        let mut state = State::default();
+        state
+            .tcp_connect(closed_port_addr())
+            .expect("tcp_connect should spawn a thread");
+
+        pump_update(&mut state, 200);
+
+        let error = state.error_message.clone().expect("no error surfaced");
+        assert!(
+            error.starts_with("TCP connection failed:"),
+            "unexpected error: {error}"
+        );
+        assert!(!state.tcp_connected);
+        assert!(state.tcp_thread_handle.is_none(), "dead thread was kept");
+        assert!(
+            state.event_log.iter().any(|e| e.contains("TCP disconnected")),
+            "failure missing from the event log: {:?}",
+            state.event_log
+        );
+
+        // The retry is the point: this used to fail with "TCP thread already
+        // exists" until Disconnect TCP was pressed.
+        let listener = TcpListener::bind("127.0.0.1:0").expect("bind failed");
+        let addr = listener.local_addr().expect("local_addr failed").to_string();
+        state.tcp_connect(addr).expect("retry was rejected");
+        let _ = listener.accept().expect("accept failed");
+        assert!(wait_until(|| state.is_tcp_connected()), "retry never connected");
+        assert!(
+            state.error_message.is_none(),
+            "stale failure banner after a successful retry: {:?}",
+            state.error_message
+        );
+
+        let _ = state.tcp_disconnect();
+    }
+
     #[test]
     fn send_packet_without_tcp_reports_an_error() {
         let mut state = State::default();