Repository navigation
Report TCP connect failures in the relay window - #210
Conversation
A failed TcpStream::connect only reached stderr, and the parent kept the dead thread handle, so every later Connect TCP was rejected with "TCP thread already exists" until Disconnect TCP was pressed. The thread now sends Disconnected with the reason, the update handler shows it in the error banner, and reap_tcp_thread clears the dead handle so a retry works.
Sanjana-Siddani
left a comment
Sanjana-Siddani
left a comment
There was a problem hiding this comment.
Hey, nice fix! Sending the failure back as Disconnected and cleaning up the dead thread makes sense. I checked, and that's the only place the thread sends Disconnected, so it can't drop a thread that's still running. It merges fine with main and all 4 relay tests pass, including your retry test.
One thing before merging: the error banner never gets cleared after a successful retry. I extended your test a bit, and after connecting to the working port the banner still said "TCP connection failed: Connection refused" even though TCP was connected. Could you set state.error_message = None when tcp_connect succeeds in connect_tcp_with_validation_and_save? And maybe add assert!(state.error_message.is_none()) after the retry in the test.
Small thing, not blocking: the README row about the IPC socket still says it "gives up silently". That's fixed in #207, so whichever one merges second can update that line.
Will approve after the banner fix
The banner set from FromTcpThreadMessage::Disconnected stayed on screen after a later connect succeeded, so the window could show "TCP connection failed: Connection refused" while TCP was connected. tcp_connect now clears error_message once the thread spawns, which covers both the Connect TCP button and a saved address picked from the dropdown. The retry test asserts the banner is gone, and it fails without the change.
A failed TcpStream::connect in the TCP thread called human_log and exited, so nothing appeared in the GUI. The parent still held tcp_bichannel and tcp_thread_handle, and the guard in tcp_connect rejected every retry with "TCP thread already exists" until Disconnect TCP was pressed.
The thread now sends FromTcpThreadMessage::Disconnected with the reason before bailing. The update handler puts that reason in the error banner, logs it, and calls the new State::reap_tcp_thread to drop the dead handle and channel, so pressing Connect TCP again just works. Polling of the TCP messages moved out of the if-let borrow so the handler can mutate state.
A test connects to a closed port, pumps the Update message the GUI subscription sends, and asserts the surfaced error, the cleared thread handle, the event-log entry, and that a following connect to a live listener succeeds.
The README troubleshooting row no longer tells people to press Disconnect TCP first.
Closes #179