hold pane open when spawn fails

Show the error message in the pane itself when a spawn attempt
fails, regardless of the exit_behavior setting.

refs: #3950
refs: #3928

docs/changelog.md

@@ -115,6 +115,10 @@ As features stabilize some brief notes about them will accumulate here.
   override `exit_behavior="CloseOnCleanExit"` while connecting so that error
   information can be displayed. #3941
 * Divide by zero panic with lots of splits and resizing panes. #3921
+* Spawn failures were not shown; the window would close immediately
+  with the default `exit_behavior` setting. Now local commands override
+  `exit_behavior="CloseOnCleanExit"` if the command fails to spawn, and
+  a more detailed error message is shown explaining what failed. #3928 #3950
 
 #### Updated
 * Bundled harfbuzz to 8.0.0

mux/src/domain.rs

@@ -16,7 +16,7 @@ use config::keyassignment::{SpawnCommand, SpawnTabDomain};
 use config::{configuration, ExecDomain, SerialDomain, ValueOrFunc, WslDomain};
 use downcast_rs::{impl_downcast, Downcast};
 use parking_lot::Mutex;
-use portable_pty::{native_pty_system, CommandBuilder, PtySystem};
+use portable_pty::{native_pty_system, CommandBuilder, ExitStatus, MasterPty, PtySize, PtySystem};
 use std::collections::HashMap;
 use std::ffi::OsString;
 use std::io::Write;
@@ -501,6 +501,75 @@ impl std::io::Write for WriterWrapper {
     }
 }
 
+/// Wraps the underlying pty; we use this as a marker for when
+/// the spawn attempt failed in order to hold the pane open
+pub(crate) struct FailedSpawnPty {
+    inner: Mutex<Box<dyn MasterPty>>,
+}
+
+impl portable_pty::MasterPty for FailedSpawnPty {
+    fn resize(&self, new_size: PtySize) -> anyhow::Result<()> {
+        self.inner.lock().resize(new_size)
+    }
+    fn get_size(&self) -> anyhow::Result<PtySize> {
+        self.inner.lock().get_size()
+    }
+    fn try_clone_reader(&self) -> anyhow::Result<Box<(dyn std::io::Read + Send + 'static)>> {
+        self.inner.lock().try_clone_reader()
+    }
+    fn take_writer(&self) -> anyhow::Result<Box<(dyn std::io::Write + Send + 'static)>> {
+        self.inner.lock().take_writer()
+    }
+
+    #[cfg(unix)]
+    fn process_group_leader(&self) -> Option<i32> {
+        None
+    }
+
+    #[cfg(unix)]
+    fn as_raw_fd(&self) -> Option<std::os::fd::RawFd> {
+        None
+    }
+
+    #[cfg(unix)]
+    fn tty_name(&self) -> Option<std::path::PathBuf> {
+        None
+    }
+}
+
+/// A fake child process for the case where the spawn attempt
+/// failed. It reports as immediately terminated.
+#[derive(Debug)]
+pub(crate) struct FailedProcessSpawn {}
+
+impl portable_pty::Child for FailedProcessSpawn {
+    fn try_wait(&mut self) -> std::io::Result<Option<ExitStatus>> {
+        Ok(Some(ExitStatus::with_exit_code(1)))
+    }
+
+    fn wait(&mut self) -> std::io::Result<ExitStatus> {
+        Ok(ExitStatus::with_exit_code(1))
+    }
+
+    fn process_id(&self) -> Option<u32> {
+        None
+    }
+
+    #[cfg(windows)]
+    fn as_raw_handle(&self) -> Option<std::os::windows::io::RawHandle> {
+        None
+    }
+}
+
+impl portable_pty::ChildKiller for FailedProcessSpawn {
+    fn kill(&mut self) -> std::io::Result<()> {
+        Ok(())
+    }
+    fn clone_killer(&self) -> Box<dyn portable_pty::ChildKiller + Send + Sync> {
+        Box::new(FailedProcessSpawn {})
+    }
+}
+
 #[async_trait(?Send)]
 impl Domain for LocalDomain {
     async fn spawn_pane(
@@ -510,7 +579,10 @@ impl Domain for LocalDomain {
         command_dir: Option<String>,
     ) -> anyhow::Result<Arc<dyn Pane>> {
         let pane_id = alloc_pane_id();
-        let cmd = self.build_command(command, command_dir, pane_id).await?;
+        let cmd = self
+            .build_command(command, command_dir, pane_id)
+            .await
+            .context("build_command")?;
         let pair = self
             .pty_system
             .lock()
@@ -528,10 +600,8 @@ impl Domain for LocalDomain {
             },
             self.name
         );
-        let child = pair.slave.spawn_command(cmd)?;
-        log::trace!("spawned: {:?}", child);
-
-        let writer = WriterWrapper::new(pair.master.take_writer()?);
+        let child_result = pair.slave.spawn_command(cmd);
+        let mut writer = WriterWrapper::new(pair.master.take_writer()?);
 
         let mut terminal = wezterm_term::Terminal::new(
             size,
@@ -544,15 +614,34 @@ impl Domain for LocalDomain {
             terminal.enable_conpty_quirks();
         }
 
-        let pane: Arc<dyn Pane> = Arc::new(LocalPane::new(
-            pane_id,
-            terminal,
-            child,
-            pair.master,
-            Box::new(writer),
-            self.id,
-            command_description,
-        ));
+        let pane: Arc<dyn Pane> = match child_result {
+            Ok(child) => Arc::new(LocalPane::new(
+                pane_id,
+                terminal,
+                child,
+                pair.master,
+                Box::new(writer),
+                self.id,
+                command_description,
+            )),
+            Err(err) => {
+                // Show the error to the user in the new pane
+                write!(writer, "{err:#}").ok();
+
+                // and return a dummy pane that has exited
+                Arc::new(LocalPane::new(
+                    pane_id,
+                    terminal,
+                    Box::new(FailedProcessSpawn {}),
+                    Box::new(FailedSpawnPty {
+                        inner: Mutex::new(pair.master),
+                    }),
+                    Box::new(writer),
+                    self.id,
+                    command_description,
+                ))
+            }
+        };
 
         let mux = Mux::get();
         mux.add_pane(&pane)?;

mux/src/localpane.rs

@@ -223,14 +223,14 @@ impl Pane for LocalPane {
         // 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()
+        let mut pty = self.pty.lock();
+        let is_ssh_connecting = pty
             .downcast_mut::<crate::ssh::WrappedSshPty>()
             .map(|s| s.is_connecting())
             .unwrap_or(false);
+        let is_failed_spawn = pty.is::<crate::domain::FailedSpawnPty>();
 
-        if is_ssh_connecting {
+        if is_ssh_connecting || is_failed_spawn {
             Some(ExitBehavior::CloseOnCleanExit)
         } else {
             None

pty/src/cmdbuilder.rs

@@ -415,30 +415,54 @@ impl CommandBuilder {
         if exe_path.is_relative() {
             let cwd: &Path = cwd.as_ref();
             let abs_path = cwd.join(exe_path);
-            if abs_path.exists() {
+
+            let mut errors = vec![];
+
+            if access(&abs_path, AccessFlags::X_OK).is_ok() {
                 return Ok(abs_path.into_os_string());
             }
-
-            if let Some(path) = self.resolve_path() {
-                for path in std::env::split_paths(&path) {
-                    let candidate = path.join(&exe);
-                    if access(&candidate, AccessFlags::X_OK).is_ok() {
-                        return Ok(candidate.into_os_string());
+            if access(&abs_path, AccessFlags::F_OK).is_ok() {
+                errors.push(format!(
+                    "{} exists but is not executable",
+                    abs_path.display()
+                ));
+            } else {
+                if let Some(path) = self.resolve_path() {
+                    for path in std::env::split_paths(&path) {
+                        let candidate = path.join(&exe);
+                        if access(&candidate, AccessFlags::X_OK).is_ok() {
+                            return Ok(candidate.into_os_string());
+                        }
+                        if access(&candidate, AccessFlags::F_OK).is_ok() {
+                            errors.push(format!(
+                                "{} exists but is not executable",
+                                candidate.display()
+                            ));
+                        }
                     }
+                    errors.push(format!("No viable candidates found in PATH {path:?}"));
+                } else {
+                    errors.push("Unable to resolve the PATH".to_string());
                 }
             }
             anyhow::bail!(
-                "Unable to spawn {} because it doesn't exist on the filesystem \
-                and was not found in PATH",
-                exe_path.display()
+                "Unable to spawn {} because:\n{}",
+                exe_path.display(),
+                errors.join(".\n")
             );
         } else {
             if let Err(err) = access(exe_path, AccessFlags::X_OK) {
-                anyhow::bail!(
-                    "Unable to spawn {} because it doesn't exist on the filesystem \
-                    or is not executable ({err:#})",
-                    exe_path.display()
-                );
+                if access(exe_path, AccessFlags::F_OK).is_ok() {
+                    anyhow::bail!(
+                        "Unable to spawn {} because it is not executable ({err:#})",
+                        exe_path.display()
+                    );
+                } else {
+                    anyhow::bail!(
+                        "Unable to spawn {} because it doesn't exist on the filesystem ({err:#})",
+                        exe_path.display()
+                    );
+                }
             }
 
             Ok(exe.to_owned())