ssh: rewrite exit_behavior=Close -> CloseOnCleanExit during connection

ssh connection or host authentication errors would not be displayed
when the config was in its default exit_behavior=Close state.

This commit introduces some plumbing to allow a pane to override
the effective exit_behavior value, so that ssh sessions can now
rewrite it to CloseOnCleanExit while they are in the process
of connecting.

refs: #3941

docs/changelog.md

@@ -107,6 +107,10 @@ As features stabilize some brief notes about them will accumulate here.
 * Pasting large amounts of text in helix caused issues. #3683
 * Wayland: Copying to clipboard was not always successful when triggered by the
   keyboard. Thanks to @osandov! #3929
+* `wezterm ssh` connection errors were not shown; the window would close
+  immediately with the default `exit_behavior` setting. Now ssh sessions change
+  `exit_behavior="Close"` to `exit_behavior="CloseOnCleanExit"` so that error
+  information can be displayed. #3941
 
 #### Updated
 * Bundled harfbuzz to 7.3.0

mux/src/lib.rs

@@ -280,8 +280,8 @@ fn read_from_pane_pty(
     // or in the main mux thread.  If `true`, this thread will terminate.
     let dead = Arc::new(AtomicBool::new(false));
 
-    let pane_id = match pane.upgrade() {
-        Some(pane) => pane.pane_id(),
+    let (pane_id, exit_behavior) = match pane.upgrade() {
+        Some(pane) => (pane.pane_id(), pane.exit_behavior()),
         None => return,
     };
 
@@ -333,7 +333,7 @@ fn read_from_pane_pty(
         }
     }
 
-    match configuration().exit_behavior {
+    match exit_behavior.unwrap_or_else(|| configuration().exit_behavior) {
         ExitBehavior::Hold | ExitBehavior::CloseOnCleanExit => {
             // We don't know if we can unilaterally close
             // this pane right now, so don't!

mux/src/localpane.rs

@@ -42,7 +42,7 @@ enum ProcessState {
     Running {
         child_waiter: Receiver<IoResult<ExitStatus>>,
         pid: Option<u32>,
-        signaller: Box<dyn ChildKiller + Send + Sync>,
+        signaller: Box<dyn ChildKiller + Sync>,
         // Whether we've explicitly killed the child
         killed: bool,
     },
@@ -115,11 +115,17 @@ impl CachedLeaderInfo {
     }
 }
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub enum LocalPaneConnectionState {
+    Connecting,
+    Connected,
+}
+
 pub struct LocalPane {
     pane_id: PaneId,
     terminal: Mutex<Terminal>,
     process: Mutex<ProcessState>,
-    pty: Mutex<Box<dyn MasterPty + Send>>,
+    pty: Mutex<Box<dyn MasterPty>>,
     writer: Mutex<Box<dyn Write + Send>>,
     domain_id: DomainId,
     tmux_domain: Mutex<Option<Arc<TmuxDomainState>>>,
@@ -213,6 +219,24 @@ impl Pane for LocalPane {
         self.terminal.lock().user_vars().clone()
     }
 
+    fn exit_behavior(&self) -> Option<ExitBehavior> {
+        // If we are ssh, and we've not yet fully connected,
+        // then override exit_behavior so that we can show
+        // connection issues
+        let is_ssh_connecting = self
+            .pty
+            .lock()
+            .downcast_mut::<crate::ssh::WrappedSshPty>()
+            .map(|s| s.is_connecting())
+            .unwrap_or(false);
+
+        if is_ssh_connecting {
+            Some(ExitBehavior::CloseOnCleanExit)
+        } else {
+            None
+        }
+    }
+
     fn kill(&self) {
         let mut proc = self.process.lock();
         log::debug!(
@@ -263,9 +287,14 @@ impl Pane for LocalPane {
                             .contains(&status.exit_code()),
                     };
 
-                    match (configuration().exit_behavior, success, killed) {
+                    match (
+                        self.exit_behavior()
+                            .unwrap_or_else(|| configuration().exit_behavior),
+                        success,
+                        killed,
+                    ) {
                         (ExitBehavior::Close, _, _) => *proc = ProcessState::Dead,
-                        (ExitBehavior::CloseOnCleanExit, false, false) => {
+                        (ExitBehavior::CloseOnCleanExit, false, _) => {
                             notify = Some(format!(
                                 "\r\n⚠️  Process {} didn't exit cleanly\r\n{}.\r\n{}=\"CloseOnCleanExit\"\r\n",
                                 self.command_description,
@@ -901,10 +930,10 @@ impl AlertHandler for LocalPaneNotifHandler {
 /// Without this, typing `exit` in `cmd.exe` would keep the pane around
 /// until something else triggered the mux to prune dead processes.
 fn split_child(
-    mut process: Box<dyn Child + Send>,
+    mut process: Box<dyn Child>,
 ) -> (
     Receiver<IoResult<ExitStatus>>,
-    Box<dyn ChildKiller + Send + Sync>,
+    Box<dyn ChildKiller + Sync>,
     Option<u32>,
 ) {
     let pid = process.process_id();
@@ -930,7 +959,7 @@ impl LocalPane {
         pane_id: PaneId,
         mut terminal: Terminal,
         process: Box<dyn Child + Send>,
-        pty: Box<dyn MasterPty + Send>,
+        pty: Box<dyn MasterPty>,
         writer: Box<dyn Write + Send>,
         domain_id: DomainId,
         command_description: String,

mux/src/pane.rs

@@ -1,5 +1,6 @@
 use crate::domain::DomainId;
 use crate::renderable::*;
+use crate::ExitBehavior;
 use async_trait::async_trait;
 use config::keyassignment::{KeyAssignment, ScrollbackEraseMode};
 use downcast_rs::{impl_downcast, Downcast};
@@ -308,6 +309,10 @@ pub trait Pane: Downcast + Send + Sync {
     fn tty_name(&self) -> Option<String> {
         None
     }
+
+    fn exit_behavior(&self) -> Option<ExitBehavior> {
+        None
+    }
 }
 impl_downcast!(Pane);
 

mux/src/ssh.rs

@@ -775,7 +775,7 @@ struct WrappedSshChildKiller {
 }
 
 #[derive(Debug)]
-struct WrappedSshChild {
+pub(crate) struct WrappedSshChild {
     status: Option<AsyncReceiver<ExitStatus>>,
     rx: Receiver<SshChildProcess>,
     exited: Option<ExitStatus>,
@@ -791,7 +791,7 @@ impl WrappedSshChild {
                 }
                 Err(TryRecvError::Empty) => {}
                 Err(err) => {
-                    log::error!("WrappedSshChild err: {:#?}", err);
+                    log::debug!("WrappedSshChild::check_connected err: {:#?}", err);
                     self.exited.replace(ExitStatus::with_exit_code(1));
                 }
             }
@@ -836,7 +836,7 @@ impl portable_pty::Child for WrappedSshChild {
                 }
                 Err(smol::channel::TryRecvError::Empty) => Ok(None),
                 Err(err) => {
-                    log::error!("WrappedSshChild err: {:#?}", err);
+                    log::debug!("WrappedSshChild::try_wait err: {:#?}", err);
                     let status = ExitStatus::with_exit_code(1);
                     self.exited.replace(status.clone());
                     Ok(Some(status))
@@ -858,7 +858,7 @@ impl portable_pty::Child for WrappedSshChild {
                     self.got_child(c);
                 }
                 Err(err) => {
-                    log::error!("WrappedSshChild err: {:#?}", err);
+                    log::debug!("WrappedSshChild err: {:#?}", err);
                     let status = ExitStatus::with_exit_code(1);
                     self.exited.replace(status.clone());
                     return Ok(status);
@@ -926,10 +926,16 @@ impl ChildKiller for WrappedSshChildKiller {
 type BoxedReader = Box<(dyn Read + Send + 'static)>;
 type BoxedWriter = Box<(dyn Write + Send + 'static)>;
 
-struct WrappedSshPty {
+pub(crate) struct WrappedSshPty {
     inner: RefCell<WrappedSshPtyInner>,
 }
 
+impl WrappedSshPty {
+    pub fn is_connecting(&mut self) -> bool {
+        self.inner.borrow_mut().is_connecting()
+    }
+}
+
 enum WrappedSshPtyInner {
     Connecting {
         reader: Option<PtyReader>,
@@ -975,6 +981,14 @@ impl WrappedSshPtyInner {
             _ => Ok(()),
         }
     }
+
+    fn is_connecting(&mut self) -> bool {
+        self.check_connected().ok();
+        match self {
+            Self::Connecting { .. } => true,
+            Self::Connected { .. } => false,
+        }
+    }
 }
 
 impl portable_pty::MasterPty for WrappedSshPty {

pty/src/cmdbuilder.rs

@@ -204,7 +204,7 @@ pub struct CommandBuilder {
 }
 
 impl CommandBuilder {
-    /// Create a new builder instance with argv[0] set to the specified
+    /// Create a new builder instance with argv\[0\] set to the specified
     /// program.
     pub fn new<S: AsRef<OsStr>>(program: S) -> Self {
         Self {

pty/src/lib.rs

@@ -85,7 +85,7 @@ impl Default for PtySize {
 }
 
 /// Represents the master/control end of the pty
-pub trait MasterPty {
+pub trait MasterPty: Downcast + Send {
     /// Inform the kernel and thus the child process that the window resized.
     /// It will update the winsize information maintained by the kernel,
     /// and generate a signal for the child to notice and update its state.
@@ -123,10 +123,11 @@ pub trait MasterPty {
         None
     }
 }
+impl_downcast!(MasterPty);
 
 /// Represents a child process spawned into the pty.
 /// This handle can be used to wait for or terminate that child process.
-pub trait Child: std::fmt::Debug + ChildKiller {
+pub trait Child: std::fmt::Debug + ChildKiller + Downcast + Send {
     /// Poll the child to see if it has completed.
     /// Does not block.
     /// Returns None if the child has not yet terminated,
@@ -143,9 +144,10 @@ pub trait Child: std::fmt::Debug + ChildKiller {
     #[cfg(windows)]
     fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle>;
 }
+impl_downcast!(Child);
 
 /// Represents the ability to signal a Child to terminate
-pub trait ChildKiller: std::fmt::Debug {
+pub trait ChildKiller: std::fmt::Debug + Downcast + Send {
     /// Terminate the child process
     fn kill(&mut self) -> IoResult<()>;
 
@@ -154,6 +156,7 @@ pub trait ChildKiller: std::fmt::Debug {
     /// blocked in `.wait`.
     fn clone_killer(&self) -> Box<dyn ChildKiller + Send + Sync>;
 }
+impl_downcast!(ChildKiller);
 
 /// Represents the slave side of a pty.
 /// Can be used to spawn processes into the pty.