diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fc235aad..0ad781cda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * feat: allow toggling mouse mode at runtime (https://github.com/zellij-org/zellij/pull/1883) * fix: display status bar properly if limited to only 1 line (https://github.com/zellij-org/zellij/pull/1875) * feat: allow starting command panes suspended (https://github.com/zellij-org/zellij/pull/1887) +* debugging: Remove calls to unwrap in `zellij_server::os_input_output` (https://github.com/zellij-org/zellij/pull/1895) ## [0.32.0] - 2022-10-25 diff --git a/zellij-server/src/lib.rs b/zellij-server/src/lib.rs index 147fe91fb..dc28a9aea 100644 --- a/zellij-server/src/lib.rs +++ b/zellij-server/src/lib.rs @@ -135,7 +135,7 @@ impl Drop for SessionMetaData { macro_rules! remove_client { ($client_id:expr, $os_input:expr, $session_state:expr) => { - $os_input.remove_client($client_id); + $os_input.remove_client($client_id).unwrap(); $session_state.write().unwrap().remove_client($client_id); }; } @@ -260,7 +260,7 @@ pub fn start_server(mut os_input: Box, socket_path: PathBuf) { Ok(stream) => { let mut os_input = os_input.clone(); let client_id = session_state.write().unwrap().new_client(); - let receiver = os_input.new_client(client_id, stream); + let receiver = os_input.new_client(client_id, stream).unwrap(); let session_data = session_data.clone(); let session_state = session_state.clone(); let to_server = to_server.clone(); diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs index 02f53dbe5..322cfb9fd 100644 --- a/zellij-server/src/os_input_output.rs +++ b/zellij-server/src/os_input_output.rs @@ -11,6 +11,7 @@ use std::path::PathBuf; use std::process::{Child, Command}; use std::sync::{Arc, Mutex}; +use zellij_utils::errors::prelude::*; use zellij_utils::{async_std, interprocess, libc, nix, signal_hook}; use async_std::fs::File as AsyncFile; @@ -61,11 +62,20 @@ fn set_terminal_size_using_fd(fd: RawFd, columns: u16, rows: u16) { /// Handle some signals for the child process. This will loop until the child /// process exits. -fn handle_command_exit(mut child: Child) -> Option { +fn handle_command_exit(mut child: Child) -> Result> { + let id = child.id(); + let err_context = || { + format!( + "failed to handle signals and command exit for child process pid {}", + id + ) + }; + // returns the exit status, if any let mut should_exit = false; let mut attempts = 3; - let mut signals = signal_hook::iterator::Signals::new(&[SIGINT, SIGTERM]).unwrap(); + let mut signals = + signal_hook::iterator::Signals::new(&[SIGINT, SIGTERM]).with_context(err_context)?; 'handle_exit: loop { // test whether the child process has exited match child.try_wait() { @@ -73,7 +83,7 @@ fn handle_command_exit(mut child: Child) -> Option { // if the child process has exited, break outside of the loop // and exit this function // TODO: handle errors? - break 'handle_exit status.code(); + break 'handle_exit Ok(status.code()); }, Ok(None) => { ::std::thread::sleep(::std::time::Duration::from_millis(10)); @@ -90,12 +100,13 @@ fn handle_command_exit(mut child: Child) -> Option { } else if attempts > 0 { // let's try nicely first... attempts -= 1; - kill(Pid::from_raw(child.id() as i32), Some(Signal::SIGTERM)).unwrap(); + kill(Pid::from_raw(child.id() as i32), Some(Signal::SIGTERM)) + .with_context(err_context)?; continue; } else { // when I say whoa, I mean WHOA! let _ = child.kill(); - break 'handle_exit None; + break 'handle_exit Ok(None); } } } @@ -132,7 +143,14 @@ fn handle_openpty( cmd: RunCommand, quit_cb: Box, RunCommand) + Send>, // u32 is the exit status terminal_id: u32, -) -> Result<(RawFd, RawFd), SpawnTerminalError> { +) -> Result<(RawFd, RawFd)> { + let err_context = |cmd: &RunCommand| { + format!( + "failed to open PTY for command '{}'", + cmd.command.to_string_lossy().to_string() + ) + }; + // primary side of pty and child fd let pid_primary = open_pty_res.master; let pid_secondary = open_pty_res.slave; @@ -167,15 +185,21 @@ fn handle_openpty( let child_id = child.id(); std::thread::spawn(move || { - child.wait().unwrap(); - let exit_status = handle_command_exit(child); + child.wait().with_context(|| err_context(&cmd)).fatal(); + let exit_status = handle_command_exit(child) + .with_context(|| err_context(&cmd)) + .fatal(); let _ = nix::unistd::close(pid_secondary); quit_cb(PaneId::Terminal(terminal_id), exit_status, cmd); }); Ok((pid_primary, child_id as RawFd)) } else { - Err(SpawnTerminalError::CommandNotFound(terminal_id)) + Err(ZellijError::CommandNotFound { + terminal_id, + command: cmd.command.to_string_lossy().to_string(), + }) + .with_context(|| err_context(&cmd)) } } @@ -188,7 +212,9 @@ fn handle_terminal( orig_termios: termios::Termios, quit_cb: Box, RunCommand) + Send>, terminal_id: u32, -) -> Result<(RawFd, RawFd), SpawnTerminalError> { +) -> Result<(RawFd, RawFd)> { + let err_context = || "failed to spawn child terminal".to_string(); + // Create a pipe to allow the child the communicate the shell's pid to its // parent. match openpty(None, Some(&orig_termios)) { @@ -196,11 +222,12 @@ fn handle_terminal( Err(e) => match failover_cmd { Some(failover_cmd) => { handle_terminal(failover_cmd, None, orig_termios, quit_cb, terminal_id) + .with_context(err_context) }, - None => { - log::error!("Failed to start pty: {:?}", e); - Err(SpawnTerminalError::FailedToStartPty) - }, + None => Err::<(i32, i32), _>(e) + .context("failed to start pty") + .with_context(err_context) + .to_log(), }, } } @@ -241,7 +268,7 @@ fn spawn_terminal( quit_cb: Box, RunCommand) + Send>, // u32 is the exit_status default_editor: Option, terminal_id: u32, -) -> Result<(RawFd, RawFd), SpawnTerminalError> { +) -> Result<(RawFd, RawFd)> { // returns the terminal_id, the primary fd and the // secondary fd let mut failover_cmd_args = None; @@ -296,40 +323,6 @@ fn spawn_terminal( handle_terminal(cmd, failover_cmd, orig_termios, quit_cb, terminal_id) } -#[derive(Debug, Clone, Copy)] -pub enum SpawnTerminalError { - CommandNotFound(u32), // u32 is the terminal id - NoEditorFound, - NoMoreTerminalIds, - FailedToStartPty, - GenericSpawnError(&'static str), -} - -impl std::fmt::Display for SpawnTerminalError { - fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - match self { - SpawnTerminalError::CommandNotFound(terminal_id) => { - write!(f, "Command not found for terminal_id: {}", terminal_id) - }, - SpawnTerminalError::NoEditorFound => { - write!( - f, - "No Editor found, consider setting a path to one in $EDITOR or $VISUAL" - ) - }, - SpawnTerminalError::NoMoreTerminalIds => { - write!(f, "No more terminal ids left to allocate.") - }, - SpawnTerminalError::FailedToStartPty => { - write!(f, "Failed to start pty") - }, - SpawnTerminalError::GenericSpawnError(msg) => { - write!(f, "{}", msg) - }, - } - } -} - #[derive(Clone)] pub struct ServerOsInputOutput { orig_termios: Arc>, @@ -372,7 +365,7 @@ impl AsyncReader for RawFdAsyncReader { /// The `ServerOsApi` trait represents an abstract interface to the features of an operating system that /// Zellij server requires. pub trait ServerOsApi: Send + Sync { - fn set_terminal_size_using_terminal_id(&self, id: u32, cols: u16, rows: u16); + fn set_terminal_size_using_terminal_id(&self, id: u32, cols: u16, rows: u16) -> Result<()>; /// Spawn a new terminal, with a terminal action. The returned tuple contains the master file /// descriptor of the forked pseudo terminal and a [ChildId] struct containing process id's for /// the forked child process. @@ -381,54 +374,61 @@ pub trait ServerOsApi: Send + Sync { terminal_action: TerminalAction, quit_cb: Box, RunCommand) + Send>, // u32 is the exit status default_editor: Option, - ) -> Result<(u32, RawFd, RawFd), SpawnTerminalError>; + ) -> Result<(u32, RawFd, RawFd)>; // reserves a terminal id without actually opening a terminal - fn reserve_terminal_id(&self) -> Result { + fn reserve_terminal_id(&self) -> Result { unimplemented!() } /// Read bytes from the standard output of the virtual terminal referred to by `fd`. - fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result; + fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result; /// Creates an `AsyncReader` that can be used to read from `fd` in an async context fn async_file_reader(&self, fd: RawFd) -> Box; /// Write bytes to the standard input of the virtual terminal referred to by `fd`. - fn write_to_tty_stdin(&self, terminal_id: u32, buf: &[u8]) -> Result; + fn write_to_tty_stdin(&self, terminal_id: u32, buf: &[u8]) -> Result; /// Wait until all output written to the object referred to by `fd` has been transmitted. - fn tcdrain(&self, terminal_id: u32) -> Result<(), nix::Error>; + fn tcdrain(&self, terminal_id: u32) -> Result<()>; /// Terminate the process with process ID `pid`. (SIGTERM) - fn kill(&self, pid: Pid) -> Result<(), nix::Error>; + fn kill(&self, pid: Pid) -> Result<()>; /// Terminate the process with process ID `pid`. (SIGKILL) - fn force_kill(&self, pid: Pid) -> Result<(), nix::Error>; + fn force_kill(&self, pid: Pid) -> Result<()>; /// Returns a [`Box`] pointer to this [`ServerOsApi`] struct. fn box_clone(&self) -> Box; - fn send_to_client( - &self, - client_id: ClientId, - msg: ServerToClientMsg, - ) -> Result<(), &'static str>; + fn send_to_client(&self, client_id: ClientId, msg: ServerToClientMsg) -> Result<()>; fn new_client( &mut self, client_id: ClientId, stream: LocalSocketStream, - ) -> IpcReceiverWithContext; - fn remove_client(&mut self, client_id: ClientId); + ) -> Result>; + fn remove_client(&mut self, client_id: ClientId) -> Result<()>; fn load_palette(&self) -> Palette; /// Returns the current working directory for a given pid fn get_cwd(&self, pid: Pid) -> Option; /// Writes the given buffer to a string - fn write_to_file(&mut self, buf: String, file: Option); + fn write_to_file(&mut self, buf: String, file: Option) -> Result<()>; fn re_run_command_in_terminal( &self, terminal_id: u32, run_command: RunCommand, quit_cb: Box, RunCommand) + Send>, // u32 is the exit status - ) -> Result<(RawFd, RawFd), SpawnTerminalError>; - fn clear_terminal_id(&self, terminal_id: u32); + ) -> Result<(RawFd, RawFd)>; + fn clear_terminal_id(&self, terminal_id: u32) -> Result<()>; } impl ServerOsApi for ServerOsInputOutput { - fn set_terminal_size_using_terminal_id(&self, id: u32, cols: u16, rows: u16) { - match self.terminal_id_to_raw_fd.lock().unwrap().get(&id) { + fn set_terminal_size_using_terminal_id(&self, id: u32, cols: u16, rows: u16) -> Result<()> { + match self + .terminal_id_to_raw_fd + .lock() + .to_anyhow() + .with_context(|| { + format!( + "failed to set terminal id {} to size ({}, {})", + id, rows, cols + ) + })? + .get(&id) + { Some(Some(fd)) => { if cols > 0 && rows > 0 { set_terminal_size_using_fd(*fd, cols, rows); @@ -438,20 +438,28 @@ impl ServerOsApi for ServerOsInputOutput { log::error!("Failed to find terminal fd for id: {id}, so cannot resize terminal"); }, } + Ok(()) } fn spawn_terminal( &self, terminal_action: TerminalAction, quit_cb: Box, RunCommand) + Send>, // u32 is the exit status default_editor: Option, - ) -> Result<(u32, RawFd, RawFd), SpawnTerminalError> { - let orig_termios = self.orig_termios.lock().unwrap(); + ) -> Result<(u32, RawFd, RawFd)> { + let err_context = || "failed to spawn terminal".to_string(); + + let orig_termios = self + .orig_termios + .lock() + .to_anyhow() + .with_context(err_context)?; let mut terminal_id = None; { let current_ids: HashSet = self .terminal_id_to_raw_fd .lock() - .unwrap() + .to_anyhow() + .with_context(err_context)? .keys() .copied() .collect(); @@ -467,35 +475,38 @@ impl ServerOsApi for ServerOsInputOutput { Some(terminal_id) => { self.terminal_id_to_raw_fd .lock() - .unwrap() + .to_anyhow() + .with_context(err_context)? .insert(terminal_id, None); - match spawn_terminal( + spawn_terminal( terminal_action, orig_termios.clone(), quit_cb, default_editor, terminal_id, - ) { - Ok((pid_primary, pid_secondary)) => { - self.terminal_id_to_raw_fd - .lock() - .unwrap() - .insert(terminal_id, Some(pid_primary)); - Ok((terminal_id, pid_primary, pid_secondary)) - }, - Err(e) => Err(e), - } + ) + .and_then(|(pid_primary, pid_secondary)| { + self.terminal_id_to_raw_fd + .lock() + .to_anyhow()? + .insert(terminal_id, Some(pid_primary)); + Ok((terminal_id, pid_primary, pid_secondary)) + }) + .with_context(err_context) }, - None => Err(SpawnTerminalError::NoMoreTerminalIds), + None => Err(anyhow!("no more terminal IDs left to allocate")), } } - fn reserve_terminal_id(&self) -> Result { + fn reserve_terminal_id(&self) -> Result { + let err_context = || "failed to reserve a terminal ID".to_string(); + let mut terminal_id = None; { let current_ids: HashSet = self .terminal_id_to_raw_fd .lock() - .unwrap() + .to_anyhow() + .with_context(err_context)? .keys() .copied() .collect(); @@ -511,83 +522,106 @@ impl ServerOsApi for ServerOsInputOutput { Some(terminal_id) => { self.terminal_id_to_raw_fd .lock() - .unwrap() + .to_anyhow() + .with_context(err_context)? .insert(terminal_id, None); Ok(terminal_id) }, - None => Err(SpawnTerminalError::NoMoreTerminalIds), + None => Err(anyhow!("no more terminal IDs available")), } } - fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result { - unistd::read(fd, buf) + fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result { + unistd::read(fd, buf).with_context(|| format!("failed to read stdout of raw FD {}", fd)) } fn async_file_reader(&self, fd: RawFd) -> Box { Box::new(RawFdAsyncReader::new(fd)) } - fn write_to_tty_stdin(&self, terminal_id: u32, buf: &[u8]) -> Result { - match self.terminal_id_to_raw_fd.lock().unwrap().get(&terminal_id) { - Some(Some(fd)) => unistd::write(*fd, buf), - _ => { - // TODO: propagate this error - log::error!("Failed to write to terminal with {terminal_id} - could not find its file descriptor"); - Ok(0) - }, + fn write_to_tty_stdin(&self, terminal_id: u32, buf: &[u8]) -> Result { + let err_context = || format!("failed to write to stdin of TTY ID {}", terminal_id); + + match self + .terminal_id_to_raw_fd + .lock() + .to_anyhow() + .with_context(err_context)? + .get(&terminal_id) + { + Some(Some(fd)) => unistd::write(*fd, buf).with_context(err_context), + _ => Err(anyhow!("could not find raw file descriptor")).with_context(err_context), } } - fn tcdrain(&self, terminal_id: u32) -> Result<(), nix::Error> { - match self.terminal_id_to_raw_fd.lock().unwrap().get(&terminal_id) { - Some(Some(fd)) => termios::tcdrain(*fd), - _ => { - // TODO: propagate this error - log::error!("Failed to tcdrain to terminal with {terminal_id} - could not find its file descriptor"); - Ok(()) - }, + fn tcdrain(&self, terminal_id: u32) -> Result<()> { + let err_context = || format!("failed to tcdrain to TTY ID {}", terminal_id); + + match self + .terminal_id_to_raw_fd + .lock() + .to_anyhow() + .with_context(err_context)? + .get(&terminal_id) + { + Some(Some(fd)) => termios::tcdrain(*fd).with_context(err_context), + _ => Err(anyhow!("could not find raw file descriptor")).with_context(err_context), } } fn box_clone(&self) -> Box { Box::new((*self).clone()) } - fn kill(&self, pid: Pid) -> Result<(), nix::Error> { + fn kill(&self, pid: Pid) -> Result<()> { let _ = kill(pid, Some(Signal::SIGHUP)); Ok(()) } - fn force_kill(&self, pid: Pid) -> Result<(), nix::Error> { + fn force_kill(&self, pid: Pid) -> Result<()> { let _ = kill(pid, Some(Signal::SIGKILL)); Ok(()) } - fn send_to_client( - &self, - client_id: ClientId, - msg: ServerToClientMsg, - ) -> Result<(), &'static str> { - if let Some(sender) = self.client_senders.lock().unwrap().get_mut(&client_id) { - sender.send(msg) + fn send_to_client(&self, client_id: ClientId, msg: ServerToClientMsg) -> Result<()> { + let err_context = || format!("failed to send message to client {client_id}"); + + if let Some(sender) = self + .client_senders + .lock() + .to_anyhow() + .with_context(err_context)? + .get_mut(&client_id) + { + sender.send(msg).with_context(err_context) } else { Ok(()) } } + fn new_client( &mut self, client_id: ClientId, stream: LocalSocketStream, - ) -> IpcReceiverWithContext { + ) -> Result> { let receiver = IpcReceiverWithContext::new(stream); let sender = receiver.get_sender(); self.client_senders .lock() - .unwrap() + .to_anyhow() + .with_context(|| format!("failed to create new client {client_id}"))? .insert(client_id, sender); - receiver + Ok(receiver) } - fn remove_client(&mut self, client_id: ClientId) { - let mut client_senders = self.client_senders.lock().unwrap(); + + fn remove_client(&mut self, client_id: ClientId) -> Result<()> { + let mut client_senders = self + .client_senders + .lock() + .to_anyhow() + .with_context(|| format!("failed to remove client {client_id}"))?; if client_senders.contains_key(&client_id) { client_senders.remove(&client_id); } + Ok(()) } + fn load_palette(&self) -> Palette { default_palette() } + fn get_cwd(&self, pid: Pid) -> Option { let mut system_info = System::new(); // Update by minimizing information. @@ -599,45 +633,52 @@ impl ServerOsApi for ServerOsInputOutput { } None } - fn write_to_file(&mut self, buf: String, name: Option) { + + fn write_to_file(&mut self, buf: String, name: Option) -> Result<()> { + let err_context = || "failed to write to file".to_string(); + let mut f: File = match name { - Some(x) => File::create(x).unwrap(), - None => tempfile().unwrap(), + Some(x) => File::create(x).with_context(err_context)?, + None => tempfile().with_context(err_context)?, }; - if let Err(e) = write!(f, "{}", buf) { - log::error!("could not write to file: {}", e); - } + write!(f, "{}", buf).with_context(err_context) } + fn re_run_command_in_terminal( &self, terminal_id: u32, run_command: RunCommand, quit_cb: Box, RunCommand) + Send>, // u32 is the exit status - ) -> Result<(RawFd, RawFd), SpawnTerminalError> { - let orig_termios = self.orig_termios.lock().unwrap(); + ) -> Result<(RawFd, RawFd)> { let default_editor = None; // no need for a default editor when running an explicit command - match spawn_terminal( - TerminalAction::RunCommand(run_command), - orig_termios.clone(), - quit_cb, - default_editor, - terminal_id, - ) { - Ok((pid_primary, pid_secondary)) => { + self.orig_termios + .lock() + .to_anyhow() + .and_then(|orig_termios| { + spawn_terminal( + TerminalAction::RunCommand(run_command), + orig_termios.clone(), + quit_cb, + default_editor, + terminal_id, + ) + }) + .and_then(|(pid_primary, pid_secondary)| { self.terminal_id_to_raw_fd .lock() - .unwrap() + .to_anyhow()? .insert(terminal_id, Some(pid_primary)); Ok((pid_primary, pid_secondary)) - }, - Err(e) => Err(e), - } + }) + .with_context(|| format!("failed to rerun command in terminal id {}", terminal_id)) } - fn clear_terminal_id(&self, terminal_id: u32) { + fn clear_terminal_id(&self, terminal_id: u32) -> Result<()> { self.terminal_id_to_raw_fd .lock() - .unwrap() + .to_anyhow() + .with_context(|| format!("failed to clear terminal ID {}", terminal_id))? .remove(&terminal_id); + Ok(()) } } diff --git a/zellij-server/src/panes/floating_panes/mod.rs b/zellij-server/src/panes/floating_panes/mod.rs index d17be7557..48d4d700f 100644 --- a/zellij-server/src/panes/floating_panes/mod.rs +++ b/zellij-server/src/panes/floating_panes/mod.rs @@ -31,7 +31,9 @@ macro_rules! resize_pty { *pid, $pane.get_content_columns() as u16, $pane.get_content_rows() as u16, - ); + ) + } else { + Ok(()) } }; } @@ -233,7 +235,7 @@ impl FloatingPanes { } else { pane.set_content_offset(Offset::default()); } - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } } pub fn render(&mut self, output: &mut Output) -> Result<()> { @@ -313,7 +315,7 @@ impl FloatingPanes { } pub fn resize_pty_all_panes(&mut self, os_api: &mut Box) { for pane in self.panes.values_mut() { - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } } pub fn resize_active_pane_left( @@ -333,7 +335,7 @@ impl FloatingPanes { ); floating_pane_grid.resize_pane_left(active_floating_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } self.set_force_render(); return true; @@ -357,7 +359,7 @@ impl FloatingPanes { ); floating_pane_grid.resize_pane_right(active_floating_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } self.set_force_render(); return true; @@ -381,7 +383,7 @@ impl FloatingPanes { ); floating_pane_grid.resize_pane_down(active_floating_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } self.set_force_render(); return true; @@ -405,7 +407,7 @@ impl FloatingPanes { ); floating_pane_grid.resize_pane_up(active_floating_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } self.set_force_render(); return true; @@ -429,7 +431,7 @@ impl FloatingPanes { ); floating_pane_grid.resize_increase(active_floating_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } self.set_force_render(); return true; @@ -453,7 +455,7 @@ impl FloatingPanes { ); floating_pane_grid.resize_decrease(active_floating_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, os_api); + resize_pty!(pane, os_api).unwrap(); } self.set_force_render(); return true; diff --git a/zellij-server/src/panes/tiled_panes/mod.rs b/zellij-server/src/panes/tiled_panes/mod.rs index 7e7b7a1f9..86b21efde 100644 --- a/zellij-server/src/panes/tiled_panes/mod.rs +++ b/zellij-server/src/panes/tiled_panes/mod.rs @@ -29,7 +29,9 @@ macro_rules! resize_pty { *pid, $pane.get_content_columns() as u16, $pane.get_content_rows() as u16, - ); + ) + } else { + Ok(()) } }; } @@ -263,7 +265,7 @@ impl TiledPanes { pane.set_content_offset(Offset::shift(pane_rows_offset, pane_columns_offset)); } - resize_pty!(pane, self.os_api); + resize_pty!(pane, self.os_api).unwrap(); } } pub fn can_split_pane_horizontally(&mut self, client_id: ClientId) -> bool { @@ -507,7 +509,7 @@ impl TiledPanes { ); pane_grid.resize_pane_left(&active_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, self.os_api); + resize_pty!(pane, self.os_api).unwrap(); } } } @@ -521,7 +523,7 @@ impl TiledPanes { ); pane_grid.resize_pane_right(&active_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, self.os_api); + resize_pty!(pane, self.os_api).unwrap(); } } } @@ -535,7 +537,7 @@ impl TiledPanes { ); pane_grid.resize_pane_up(&active_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, self.os_api); + resize_pty!(pane, self.os_api).unwrap(); } } } @@ -549,7 +551,7 @@ impl TiledPanes { ); pane_grid.resize_pane_down(&active_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, self.os_api); + resize_pty!(pane, self.os_api).unwrap(); } } } @@ -563,7 +565,7 @@ impl TiledPanes { ); pane_grid.resize_increase(&active_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, self.os_api); + resize_pty!(pane, self.os_api).unwrap(); } } } @@ -577,7 +579,7 @@ impl TiledPanes { ); pane_grid.resize_decrease(&active_pane_id); for pane in self.panes.values_mut() { - resize_pty!(pane, self.os_api); + resize_pty!(pane, self.os_api).unwrap(); } } } @@ -808,7 +810,7 @@ impl TiledPanes { if let Some(geom) = prev_geom_override { new_position.set_geom_override(geom); } - resize_pty!(new_position, self.os_api); + resize_pty!(new_position, self.os_api).unwrap(); new_position.set_should_render(true); let current_position = self.panes.get_mut(&active_pane_id).unwrap(); @@ -816,7 +818,7 @@ impl TiledPanes { if let Some(geom) = next_geom_override { current_position.set_geom_override(geom); } - resize_pty!(current_position, self.os_api); + resize_pty!(current_position, self.os_api).unwrap(); current_position.set_should_render(true); } pub fn move_active_pane_down(&mut self, client_id: ClientId) { @@ -841,7 +843,7 @@ impl TiledPanes { if let Some(geom) = prev_geom_override { new_position.set_geom_override(geom); } - resize_pty!(new_position, self.os_api); + resize_pty!(new_position, self.os_api).unwrap(); new_position.set_should_render(true); let current_position = self.panes.get_mut(active_pane_id).unwrap(); @@ -849,7 +851,7 @@ impl TiledPanes { if let Some(geom) = next_geom_override { current_position.set_geom_override(geom); } - resize_pty!(current_position, self.os_api); + resize_pty!(current_position, self.os_api).unwrap(); current_position.set_should_render(true); } } @@ -876,7 +878,7 @@ impl TiledPanes { if let Some(geom) = prev_geom_override { new_position.set_geom_override(geom); } - resize_pty!(new_position, self.os_api); + resize_pty!(new_position, self.os_api).unwrap(); new_position.set_should_render(true); let current_position = self.panes.get_mut(active_pane_id).unwrap(); @@ -884,7 +886,7 @@ impl TiledPanes { if let Some(geom) = next_geom_override { current_position.set_geom_override(geom); } - resize_pty!(current_position, self.os_api); + resize_pty!(current_position, self.os_api).unwrap(); current_position.set_should_render(true); } } @@ -911,7 +913,7 @@ impl TiledPanes { if let Some(geom) = prev_geom_override { new_position.set_geom_override(geom); } - resize_pty!(new_position, self.os_api); + resize_pty!(new_position, self.os_api).unwrap(); new_position.set_should_render(true); let current_position = self.panes.get_mut(active_pane_id).unwrap(); @@ -919,7 +921,7 @@ impl TiledPanes { if let Some(geom) = next_geom_override { current_position.set_geom_override(geom); } - resize_pty!(current_position, self.os_api); + resize_pty!(current_position, self.os_api).unwrap(); current_position.set_should_render(true); } } @@ -946,7 +948,7 @@ impl TiledPanes { if let Some(geom) = prev_geom_override { new_position.set_geom_override(geom); } - resize_pty!(new_position, self.os_api); + resize_pty!(new_position, self.os_api).unwrap(); new_position.set_should_render(true); let current_position = self.panes.get_mut(active_pane_id).unwrap(); @@ -954,7 +956,7 @@ impl TiledPanes { if let Some(geom) = next_geom_override { current_position.set_geom_override(geom); } - resize_pty!(current_position, self.os_api); + resize_pty!(current_position, self.os_api).unwrap(); current_position.set_should_render(true); } } diff --git a/zellij-server/src/pty.rs b/zellij-server/src/pty.rs index 0ca0b5860..da8fd20dc 100644 --- a/zellij-server/src/pty.rs +++ b/zellij-server/src/pty.rs @@ -1,4 +1,3 @@ -use crate::os_input_output::SpawnTerminalError; use crate::terminal_bytes::TerminalBytes; use crate::{ panes::PaneId, @@ -107,7 +106,10 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box) -> Result<()> { ), _ => (false, None, name), }; - match pty.spawn_terminal(terminal_action, client_or_tab_index) { + match pty + .spawn_terminal(terminal_action, client_or_tab_index) + .with_context(err_context) + { Ok((pid, starts_held)) => { let hold_for_command = if starts_held { run_command } else { None }; pty.bus @@ -121,35 +123,35 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box) -> Result<()> { )) .with_context(err_context)?; }, - Err(SpawnTerminalError::CommandNotFound(pid)) => { - if hold_on_close { - let hold_for_command = None; // we do not hold an "error" pane - pty.bus - .senders - .send_to_screen(ScreenInstruction::NewPane( - PaneId::Terminal(pid), - pane_title, - should_float, - hold_for_command, - client_or_tab_index, - )) - .with_context(err_context)?; - if let Some(run_command) = run_command { - send_command_not_found_to_screen( - pty.bus.senders.clone(), - pid, - run_command.clone(), - ) - .with_context(err_context)?; + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + if hold_on_close { + let hold_for_command = None; // we do not hold an "error" pane + pty.bus + .senders + .send_to_screen(ScreenInstruction::NewPane( + PaneId::Terminal(*terminal_id), + pane_title, + should_float, + hold_for_command, + client_or_tab_index, + )) + .with_context(err_context)?; + if let Some(run_command) = run_command { + send_command_not_found_to_screen( + pty.bus.senders.clone(), + *terminal_id, + run_command.clone(), + ) + .with_context(err_context)?; + } + } else { + log::error!("Failed to spawn terminal: command not found"); + pty.close_pane(PaneId::Terminal(*terminal_id)) + .with_context(err_context)?; } - } else { - log::error!("Failed to spawn terminal: command not found"); - pty.close_pane(PaneId::Terminal(pid)) - .with_context(err_context)?; - } - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + }, + _ => Err::<(), _>(err).non_fatal(), }, } }, @@ -181,7 +183,10 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box) -> Result<()> { ), _ => (false, None, name), }; - match pty.spawn_terminal(terminal_action, ClientOrTabIndex::ClientId(client_id)) { + match pty + .spawn_terminal(terminal_action, ClientOrTabIndex::ClientId(client_id)) + .with_context(err_context) + { Ok((pid, starts_held)) => { let hold_for_command = if starts_held { run_command } else { None }; pty.bus @@ -194,45 +199,45 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box) -> Result<()> { )) .with_context(err_context)?; }, - Err(SpawnTerminalError::CommandNotFound(pid)) => { - if hold_on_close { - let hold_for_command = None; // error panes are never held - pty.bus - .senders - .send_to_screen(ScreenInstruction::VerticalSplit( - PaneId::Terminal(pid), - pane_title, - hold_for_command, - client_id, - )) - .with_context(err_context)?; - if let Some(run_command) = run_command { + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + let hold_for_command = None; // we do not hold an "error" pane + if hold_on_close { pty.bus .senders - .send_to_screen(ScreenInstruction::PtyBytes( - pid, - format!( - "Command not found: {}", - run_command.command.display() - ) - .as_bytes() - .to_vec(), - )) - .with_context(err_context)?; - pty.bus - .senders - .send_to_screen(ScreenInstruction::HoldPane( - PaneId::Terminal(pid), - Some(2), // exit status - run_command, - None, + .send_to_screen(ScreenInstruction::VerticalSplit( + PaneId::Terminal(*terminal_id), + pane_title, + hold_for_command, + client_id, )) .with_context(err_context)?; + if let Some(run_command) = run_command { + pty.bus + .senders + .send_to_screen(ScreenInstruction::PtyBytes( + *terminal_id, + format!( + "Command not found: {}", + run_command.command.display() + ) + .as_bytes() + .to_vec(), + )) + .with_context(err_context)?; + pty.bus + .senders + .send_to_screen(ScreenInstruction::HoldPane( + PaneId::Terminal(*terminal_id), + Some(2), // exit status + run_command, + None, + )) + .with_context(err_context)?; + } } - } - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + }, + _ => Err::<(), _>(err).non_fatal(), }, } }, @@ -245,7 +250,10 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box) -> Result<()> { ), _ => (false, None, name), }; - match pty.spawn_terminal(terminal_action, ClientOrTabIndex::ClientId(client_id)) { + match pty + .spawn_terminal(terminal_action, ClientOrTabIndex::ClientId(client_id)) + .with_context(err_context) + { Ok((pid, starts_held)) => { let hold_for_command = if starts_held { run_command } else { None }; pty.bus @@ -258,45 +266,45 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box) -> Result<()> { )) .with_context(err_context)?; }, - Err(SpawnTerminalError::CommandNotFound(pid)) => { - if hold_on_close { - let hold_for_command = None; // error panes are never held - pty.bus - .senders - .send_to_screen(ScreenInstruction::HorizontalSplit( - PaneId::Terminal(pid), - pane_title, - hold_for_command, - client_id, - )) - .with_context(err_context)?; - if let Some(run_command) = run_command { + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + if hold_on_close { + let hold_for_command = None; // we do not hold an "error" pane pty.bus .senders - .send_to_screen(ScreenInstruction::PtyBytes( - pid, - format!( - "Command not found: {}", - run_command.command.display() - ) - .as_bytes() - .to_vec(), - )) - .with_context(err_context)?; - pty.bus - .senders - .send_to_screen(ScreenInstruction::HoldPane( - PaneId::Terminal(pid), - Some(2), // exit status - run_command, - None, + .send_to_screen(ScreenInstruction::HorizontalSplit( + PaneId::Terminal(*terminal_id), + pane_title, + hold_for_command, + client_id, )) .with_context(err_context)?; + if let Some(run_command) = run_command { + pty.bus + .senders + .send_to_screen(ScreenInstruction::PtyBytes( + *terminal_id, + format!( + "Command not found: {}", + run_command.command.display() + ) + .as_bytes() + .to_vec(), + )) + .with_context(err_context)?; + pty.bus + .senders + .send_to_screen(ScreenInstruction::HoldPane( + PaneId::Terminal(*terminal_id), + Some(2), // exit status + run_command, + None, + )) + .with_context(err_context)?; + } } - } - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + }, + _ => Err::<(), _>(err).non_fatal(), }, } }, @@ -347,32 +355,38 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box) -> Result<()> { .with_context(err_context)?; }, PtyInstruction::ReRunCommandInPane(pane_id, run_command) => { - match pty.rerun_command_in_pane(pane_id, run_command.clone()) { + match pty + .rerun_command_in_pane(pane_id, run_command.clone()) + .with_context(err_context) + { Ok(..) => {}, - Err(SpawnTerminalError::CommandNotFound(pid)) => { - if run_command.hold_on_close { - pty.bus - .senders - .send_to_screen(ScreenInstruction::PtyBytes( - pid, - format!("Command not found: {}", run_command.command.display()) + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + if run_command.hold_on_close { + pty.bus + .senders + .send_to_screen(ScreenInstruction::PtyBytes( + *terminal_id, + format!( + "Command not found: {}", + run_command.command.display() + ) .as_bytes() .to_vec(), - )) - .with_context(err_context)?; - pty.bus - .senders - .send_to_screen(ScreenInstruction::HoldPane( - PaneId::Terminal(pid), - Some(2), // exit status - run_command, - None, - )) - .with_context(err_context)?; - } - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + )) + .with_context(err_context)?; + pty.bus + .senders + .send_to_screen(ScreenInstruction::HoldPane( + PaneId::Terminal(*terminal_id), + Some(2), // exit status + run_command, + None, + )) + .with_context(err_context)?; + } + }, + _ => Err::<(), _>(err).non_fatal(), }, } }, @@ -429,8 +443,10 @@ impl Pty { &mut self, terminal_action: Option, client_or_tab_index: ClientOrTabIndex, - ) -> Result<(u32, bool), SpawnTerminalError> { + ) -> Result<(u32, bool)> { // bool is starts_held + let err_context = || format!("failed to spawn terminal for {:?}", client_or_tab_index); + // returns the terminal id let terminal_action = match client_or_tab_index { ClientOrTabIndex::ClientId(client_id) => { @@ -476,8 +492,11 @@ impl Pty { .bus .os_input .as_mut() - .ok_or_else(|| SpawnTerminalError::GenericSpawnError("os input is none"))? - .spawn_terminal(terminal_action, quit_cb, self.default_editor.clone())?; + .context("no OS I/O interface found") + .and_then(|os_input| { + os_input.spawn_terminal(terminal_action, quit_cb, self.default_editor.clone()) + }) + .with_context(err_context)?; let terminal_bytes = task::spawn({ let err_context = |terminal_id: u32| format!("failed to run async task for terminal {terminal_id}"); @@ -511,18 +530,14 @@ impl Pty { client_id: ClientId, ) -> Result<()> { let err_context = || format!("failed to spawn terminals for layout for client {client_id}"); + let mut default_shell = default_shell.unwrap_or_else(|| self.get_default_terminal(None)); self.fill_cwd(&mut default_shell, client_id); let extracted_run_instructions = layout.extract_run_instructions(); - let mut new_pane_pids: Vec<( - u32, - bool, - Option, - Result, - )> = vec![]; // (terminal_id, - // starts_held, - // run_command, - // file_descriptor) + let mut new_pane_pids: Vec<(u32, bool, Option, Result)> = vec![]; // (terminal_id, + // starts_held, + // run_command, + // file_descriptor) for run_instruction in extracted_run_instructions { let quit_cb = Box::new({ let senders = self.bus.senders.clone(); @@ -553,7 +568,14 @@ impl Pty { let cmd = TerminalAction::RunCommand(command.clone()); if starts_held { // we don't actually open a terminal in this case, just wait for the user to run it - match self.bus.os_input.as_mut().unwrap().reserve_terminal_id() { + match self + .bus + .os_input + .as_mut() + .context("no OS I/O interface found") + .with_context(err_context)? + .reserve_terminal_id() + { Ok(terminal_id) => { new_pane_pids.push(( terminal_id, @@ -572,8 +594,10 @@ impl Pty { .bus .os_input .as_mut() + .context("no OS I/O interface found") .with_context(err_context)? .spawn_terminal(cmd, quit_cb, self.default_editor.clone()) + .with_context(err_context) { Ok((terminal_id, pid_primary, child_fd)) => { self.id_to_child_pid.insert(terminal_id, child_fd); @@ -584,17 +608,18 @@ impl Pty { Ok(pid_primary), )); }, - Err(SpawnTerminalError::CommandNotFound(terminal_id)) => { - let starts_held = false; // we do not hold error panes - new_pane_pids.push(( - terminal_id, - starts_held, - Some(command.clone()), - Err(SpawnTerminalError::CommandNotFound(terminal_id)), - )); - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + new_pane_pids.push(( + *terminal_id, + starts_held, + Some(command.clone()), + Err(err), + )); + }, + _ => { + Err::<(), _>(err).non_fatal(); + }, }, } } @@ -606,23 +631,22 @@ impl Pty { .bus .os_input .as_mut() + .context("no OS I/O interface found") .with_context(err_context)? .spawn_terminal(shell, quit_cb, self.default_editor.clone()) + .with_context(err_context) { Ok((terminal_id, pid_primary, child_fd)) => { self.id_to_child_pid.insert(terminal_id, child_fd); new_pane_pids.push((terminal_id, starts_held, None, Ok(pid_primary))); }, - Err(SpawnTerminalError::CommandNotFound(terminal_id)) => { - new_pane_pids.push(( - terminal_id, - starts_held, - None, - Err(SpawnTerminalError::CommandNotFound(terminal_id)), - )); - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + new_pane_pids.push((*terminal_id, starts_held, None, Err(err))); + }, + _ => { + Err::<(), _>(err).non_fatal(); + }, }, } }, @@ -632,27 +656,26 @@ impl Pty { .bus .os_input .as_mut() + .context("no OS I/O interface found") .with_context(err_context)? .spawn_terminal( TerminalAction::OpenFile(path_to_file, line_number), quit_cb, self.default_editor.clone(), - ) { + ) + .with_context(err_context) + { Ok((terminal_id, pid_primary, child_fd)) => { self.id_to_child_pid.insert(terminal_id, child_fd); new_pane_pids.push((terminal_id, starts_held, None, Ok(pid_primary))); }, - Err(SpawnTerminalError::CommandNotFound(terminal_id)) => { - let starts_held = false; // we do not hold error panes - new_pane_pids.push(( - terminal_id, - starts_held, - None, - Err(SpawnTerminalError::CommandNotFound(terminal_id)), - )); - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + new_pane_pids.push((*terminal_id, starts_held, None, Err(err))); + }, + _ => { + Err::<(), _>(err).non_fatal(); + }, }, } }, @@ -662,23 +685,22 @@ impl Pty { .bus .os_input .as_mut() + .context("no OS I/O interface found") .with_context(err_context)? .spawn_terminal(default_shell.clone(), quit_cb, self.default_editor.clone()) + .with_context(err_context) { Ok((terminal_id, pid_primary, child_fd)) => { self.id_to_child_pid.insert(terminal_id, child_fd); new_pane_pids.push((terminal_id, starts_held, None, Ok(pid_primary))); }, - Err(SpawnTerminalError::CommandNotFound(terminal_id)) => { - new_pane_pids.push(( - terminal_id, - starts_held, - None, - Err(SpawnTerminalError::CommandNotFound(terminal_id)), - )); - }, - Err(e) => { - log::error!("Failed to spawn terminal: {}", e); + Err(err) => match err.downcast_ref::() { + Some(ZellijError::CommandNotFound { terminal_id, .. }) => { + new_pane_pids.push((*terminal_id, starts_held, None, Err(err))); + }, + _ => { + Err::<(), _>(err).non_fatal(); + }, }, } }, @@ -781,8 +803,9 @@ impl Pty { self.bus .os_input .as_ref() - .with_context(err_context)? - .clear_terminal_id(id); + .context("no OS I/O interface found") + .and_then(|os_input| os_input.clear_terminal_id(id)) + .with_context(err_context)?; }, PaneId::Plugin(pid) => drop( self.bus @@ -808,7 +831,9 @@ impl Pty { &mut self, pane_id: PaneId, run_command: RunCommand, - ) -> Result<(), SpawnTerminalError> { + ) -> Result<()> { + let err_context = || format!("failed to rerun command in pane {:?}", pane_id); + match pane_id { PaneId::Terminal(id) => { let _ = self.task_handles.remove(&id); // if all is well, this shouldn't be here @@ -835,8 +860,11 @@ impl Pty { .bus .os_input .as_mut() - .ok_or_else(|| SpawnTerminalError::GenericSpawnError("os input is none"))? - .re_run_command_in_terminal(id, run_command, quit_cb)?; + .context("no OS I/O interface found") + .and_then(|os_input| { + os_input.re_run_command_in_terminal(id, run_command, quit_cb) + }) + .with_context(err_context)?; let terminal_bytes = task::spawn({ let err_context = |pane_id| format!("failed to run async task for pane {pane_id:?}"); @@ -862,9 +890,7 @@ impl Pty { self.id_to_child_pid.insert(id, child_fd); Ok(()) }, - _ => Err(SpawnTerminalError::GenericSpawnError( - "Cannot respawn plugin panes", - )), + _ => Err(anyhow!("cannot respawn plugin panes")).with_context(err_context), } } } diff --git a/zellij-server/src/screen.rs b/zellij-server/src/screen.rs index 42d0c093c..5d2b9e84f 100644 --- a/zellij-server/src/screen.rs +++ b/zellij-server/src/screen.rs @@ -1507,7 +1507,8 @@ pub(crate) fn screen_thread_main( Some(file.to_string()), client_id, full - ) + ), + ? ); screen.render()?; screen.unblock_input()?; diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs index df12b2df9..16d954ed6 100644 --- a/zellij-server/src/tab/mod.rs +++ b/zellij-server/src/tab/mod.rs @@ -56,7 +56,9 @@ macro_rules! resize_pty { *pid, $pane.get_content_columns() as u16, $pane.get_content_rows() as u16, - ); + ) + } else { + Ok(()) } }; } @@ -729,6 +731,9 @@ impl Tab { self.connected_clients.borrow().is_empty() } pub fn toggle_pane_embed_or_floating(&mut self, client_id: ClientId) -> Result<()> { + let err_context = + || format!("failed to toggle embedded/floating pane for client {client_id}"); + if self.tiled_panes.fullscreen_is_active() { self.tiled_panes.unset_fullscreen(); } @@ -739,7 +744,8 @@ impl Tab { .close_pane(focused_floating_pane_id, true) .with_context(|| format!( "failed to find floating pane (ID: {focused_floating_pane_id:?}) to embed for client {client_id}", - ))?; + )) + .with_context(err_context)?; self.tiled_panes .insert_pane(focused_floating_pane_id, floating_pane_to_embed); self.should_clear_display_before_rendering = true; @@ -756,7 +762,7 @@ impl Tab { } if let Some(mut embedded_pane_to_float) = self.close_pane(focused_pane_id, true) { embedded_pane_to_float.set_geom(new_pane_geom); - resize_pty!(embedded_pane_to_float, self.os_api); + resize_pty!(embedded_pane_to_float, self.os_api).with_context(err_context)?; embedded_pane_to_float.set_active_at(Instant::now()); self.floating_panes .add_pane(focused_pane_id, embedded_pane_to_float); @@ -810,13 +816,15 @@ impl Tab { should_float: Option, client_id: Option, ) -> Result<()> { + let err_context = || format!("failed to create new pane with id {pid:?}"); + match should_float { Some(true) => self.floating_panes.toggle_show_panes(true), Some(false) => self.floating_panes.toggle_show_panes(false), None => {}, }; self.close_down_to_max_terminals() - .with_context(|| format!("failed to create new pane with id {pid:?}"))?; + .with_context(err_context)?; if self.floating_panes.panes_are_visible() { if let Some(new_pane_geom) = self.floating_panes.find_room_for_new_pane() { let next_terminal_position = self.get_next_terminal_position(); @@ -835,7 +843,7 @@ impl Tab { initial_pane_title, ); new_pane.set_content_offset(Offset::frame(1)); // floating panes always have a frame - resize_pty!(new_pane, self.os_api); + resize_pty!(new_pane, self.os_api).with_context(err_context)?; self.floating_panes.add_pane(pid, Box::new(new_pane)); self.floating_panes.focus_pane_for_all_clients(pid); } @@ -874,6 +882,8 @@ impl Tab { // this method creates a new pane from pid and replaces it with the active pane // the active pane is then suppressed (hidden and not rendered) until the current // created pane is closed, in which case it will be replaced back by it + let err_context = || format!("failed to suppress active pane for client {client_id}"); + match pid { PaneId::Terminal(pid) => { let next_terminal_position = self.get_next_terminal_position(); // TODO: this is not accurate in this case @@ -904,12 +914,12 @@ impl Tab { Some(replaced_pane) => { self.suppressed_panes .insert(PaneId::Terminal(pid), replaced_pane); - // this will be the newly replaced pane we just created - let current_active_pane = - self.get_active_pane(client_id).with_context(|| { - format!("failed to suppress active pane for client {client_id}") - })?; - resize_pty!(current_active_pane, self.os_api); + self.get_active_pane(client_id) + .with_context(|| format!("no active pane found for client {client_id}")) + .and_then(|current_active_pane| { + resize_pty!(current_active_pane, self.os_api) + }) + .with_context(err_context)?; }, None => { log::error!("Could not find editor pane to replace - is no pane focused?") @@ -1093,6 +1103,7 @@ impl Tab { } fn process_pty_bytes(&mut self, pid: u32, bytes: VteBytes) -> Result<()> { let err_context = || format!("failed to process pty bytes from pid {pid}"); + if let Some(terminal_output) = self .tiled_panes .get_pane_mut(PaneId::Terminal(pid)) @@ -1104,7 +1115,7 @@ impl Tab { }) { if self.pids_waiting_resize.remove(&pid) { - resize_pty!(terminal_output, self.os_api); + resize_pty!(terminal_output, self.os_api).with_context(err_context)?; } terminal_output.handle_pty_bytes(bytes); let messages_to_pty = terminal_output.drain_messages_to_pty(); @@ -1789,7 +1800,7 @@ impl Tab { // the pane there we replaced. Now, we need to update its pty about its new size. // We couldn't do that before, and we can't use the original moved item now - so we // need to refetch it - resize_pty!(suppressed_pane, self.os_api); + resize_pty!(suppressed_pane, self.os_api).unwrap(); } replaced_pane }) @@ -1821,20 +1832,29 @@ impl Tab { file: Option, client_id: ClientId, full: bool, - ) { + ) -> Result<()> { + let err_context = + || format!("failed to dump active terminal screen for client {client_id}"); + if let Some(active_pane) = self.get_active_pane_or_floating_pane_mut(client_id) { let dump = active_pane.dump_screen(client_id, full); - self.os_api.write_to_file(dump, file); + self.os_api + .write_to_file(dump, file) + .with_context(err_context)?; } + Ok(()) } pub fn edit_scrollback(&mut self, client_id: ClientId) -> Result<()> { + let err_context = || format!("failed to edit scrollback for client {client_id}"); + let mut file = temp_dir(); file.push(format!("{}.dump", Uuid::new_v4())); self.dump_active_terminal_screen( Some(String::from(file.to_string_lossy())), client_id, true, - ); + ) + .with_context(err_context)?; let line_number = self .get_active_pane(client_id) .and_then(|a_t| a_t.get_line_number()); @@ -1844,7 +1864,7 @@ impl Tab { line_number, client_id, )) - .with_context(|| format!("failed to edit scrollback for client {client_id}")) + .with_context(err_context) } pub fn scroll_active_terminal_up(&mut self, client_id: ClientId) { if let Some(active_pane) = self.get_active_pane_or_floating_pane_mut(client_id) { diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs index 3f4290fa6..fd4dcbede 100644 --- a/zellij-server/src/tab/unit/tab_integration_tests.rs +++ b/zellij-server/src/tab/unit/tab_integration_tests.rs @@ -4,7 +4,7 @@ use crate::screen::CopyOptions; use crate::Arc; use crate::Mutex; use crate::{ - os_input_output::{AsyncReader, Pid, ServerOsApi, SpawnTerminalError}, + os_input_output::{AsyncReader, Pid, ServerOsApi}, panes::PaneId, thread_bus::ThreadSenders, ClientId, @@ -12,7 +12,7 @@ use crate::{ use std::path::PathBuf; use zellij_utils::channels::Receiver; use zellij_utils::envs::set_session_name; -use zellij_utils::errors::ErrorContext; +use zellij_utils::errors::{prelude::*, ErrorContext}; use zellij_utils::input::layout::{Layout, PaneLayout}; use zellij_utils::ipc::IpcReceiverWithContext; use zellij_utils::pane_size::{Size, SizeInPixels}; @@ -26,8 +26,6 @@ use std::collections::{HashMap, HashSet}; use std::os::unix::io::RawFd; use std::rc::Rc; -use zellij_utils::nix; - use zellij_utils::{ data::{InputMode, ModeInfo, Palette, Style}, input::command::{RunCommand, TerminalAction}, @@ -41,53 +39,50 @@ struct FakeInputOutput { } impl ServerOsApi for FakeInputOutput { - fn set_terminal_size_using_terminal_id(&self, _id: u32, _cols: u16, _rows: u16) { + fn set_terminal_size_using_terminal_id(&self, _id: u32, _cols: u16, _rows: u16) -> Result<()> { // noop + Ok(()) } fn spawn_terminal( &self, _file_to_open: TerminalAction, _quit_db: Box, RunCommand) + Send>, _default_editor: Option, - ) -> Result<(u32, RawFd, RawFd), SpawnTerminalError> { + ) -> Result<(u32, RawFd, RawFd)> { unimplemented!() } - fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result { + fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result { unimplemented!() } fn async_file_reader(&self, _fd: RawFd) -> Box { unimplemented!() } - fn write_to_tty_stdin(&self, _id: u32, _buf: &[u8]) -> Result { + fn write_to_tty_stdin(&self, _id: u32, _buf: &[u8]) -> Result { unimplemented!() } - fn tcdrain(&self, _id: u32) -> Result<(), nix::Error> { + fn tcdrain(&self, _id: u32) -> Result<()> { unimplemented!() } - fn kill(&self, _pid: Pid) -> Result<(), nix::Error> { + fn kill(&self, _pid: Pid) -> Result<()> { unimplemented!() } - fn force_kill(&self, _pid: Pid) -> Result<(), nix::Error> { + fn force_kill(&self, _pid: Pid) -> Result<()> { unimplemented!() } fn box_clone(&self) -> Box { Box::new((*self).clone()) } - fn send_to_client( - &self, - _client_id: ClientId, - _msg: ServerToClientMsg, - ) -> Result<(), &'static str> { + fn send_to_client(&self, _client_id: ClientId, _msg: ServerToClientMsg) -> Result<()> { unimplemented!() } fn new_client( &mut self, _client_id: ClientId, _stream: LocalSocketStream, - ) -> IpcReceiverWithContext { + ) -> Result> { unimplemented!() } - fn remove_client(&mut self, _client_id: ClientId) { + fn remove_client(&mut self, _client_id: ClientId) -> Result<()> { unimplemented!() } fn load_palette(&self) -> Palette { @@ -96,22 +91,23 @@ impl ServerOsApi for FakeInputOutput { fn get_cwd(&self, _pid: Pid) -> Option { unimplemented!() } - fn write_to_file(&mut self, buf: String, name: Option) { + fn write_to_file(&mut self, buf: String, name: Option) -> Result<()> { let f: String = match name { Some(x) => x, None => "tmp-name".to_owned(), }; - self.file_dumps.lock().unwrap().insert(f, buf); + self.file_dumps.lock().to_anyhow()?.insert(f, buf); + Ok(()) } fn re_run_command_in_terminal( &self, _terminal_id: u32, _run_command: RunCommand, _quit_cb: Box, RunCommand) + Send>, // u32 is the exit status - ) -> Result<(RawFd, RawFd), SpawnTerminalError> { + ) -> Result<(RawFd, RawFd)> { unimplemented!() } - fn clear_terminal_id(&self, _terminal_id: u32) { + fn clear_terminal_id(&self, _terminal_id: u32) -> Result<()> { unimplemented!() } } @@ -501,7 +497,8 @@ fn dump_screen() { tab.handle_pty_bytes(2, Vec::from("scratch".as_bytes())) .unwrap(); let file = "/tmp/log.sh"; - tab.dump_active_terminal_screen(Some(file.to_string()), client_id, false); + tab.dump_active_terminal_screen(Some(file.to_string()), client_id, false) + .unwrap(); assert_eq!( map.lock().unwrap().get(file).unwrap(), "scratch", diff --git a/zellij-server/src/tab/unit/tab_tests.rs b/zellij-server/src/tab/unit/tab_tests.rs index 2539b7a93..8a0d59ed1 100644 --- a/zellij-server/src/tab/unit/tab_tests.rs +++ b/zellij-server/src/tab/unit/tab_tests.rs @@ -2,12 +2,13 @@ use super::Tab; use crate::panes::sixel::SixelImageStore; use crate::screen::CopyOptions; use crate::{ - os_input_output::{AsyncReader, Pid, ServerOsApi, SpawnTerminalError}, + os_input_output::{AsyncReader, Pid, ServerOsApi}, panes::PaneId, thread_bus::ThreadSenders, ClientId, }; use std::path::PathBuf; +use zellij_utils::errors::prelude::*; use zellij_utils::input::layout::PaneLayout; use zellij_utils::ipc::IpcReceiverWithContext; use zellij_utils::pane_size::{Size, SizeInPixels}; @@ -17,8 +18,6 @@ use std::collections::{HashMap, HashSet}; use std::os::unix::io::RawFd; use std::rc::Rc; -use zellij_utils::nix; - use zellij_utils::{ data::{ModeInfo, Palette, Style}, input::command::{RunCommand, TerminalAction}, @@ -30,53 +29,50 @@ use zellij_utils::{ struct FakeInputOutput {} impl ServerOsApi for FakeInputOutput { - fn set_terminal_size_using_terminal_id(&self, _id: u32, _cols: u16, _rows: u16) { + fn set_terminal_size_using_terminal_id(&self, _id: u32, _cols: u16, _rows: u16) -> Result<()> { // noop + Ok(()) } fn spawn_terminal( &self, _file_to_open: TerminalAction, _quit_cb: Box, RunCommand) + Send>, _default_editor: Option, - ) -> Result<(u32, RawFd, RawFd), SpawnTerminalError> { + ) -> Result<(u32, RawFd, RawFd)> { unimplemented!() } - fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result { + fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result { unimplemented!() } fn async_file_reader(&self, _fd: RawFd) -> Box { unimplemented!() } - fn write_to_tty_stdin(&self, _id: u32, _buf: &[u8]) -> Result { + fn write_to_tty_stdin(&self, _id: u32, _buf: &[u8]) -> Result { unimplemented!() } - fn tcdrain(&self, _id: u32) -> Result<(), nix::Error> { + fn tcdrain(&self, _id: u32) -> Result<()> { unimplemented!() } - fn kill(&self, _pid: Pid) -> Result<(), nix::Error> { + fn kill(&self, _pid: Pid) -> Result<()> { unimplemented!() } - fn force_kill(&self, _pid: Pid) -> Result<(), nix::Error> { + fn force_kill(&self, _pid: Pid) -> Result<()> { unimplemented!() } fn box_clone(&self) -> Box { Box::new((*self).clone()) } - fn send_to_client( - &self, - _client_id: ClientId, - _msg: ServerToClientMsg, - ) -> Result<(), &'static str> { + fn send_to_client(&self, _client_id: ClientId, _msg: ServerToClientMsg) -> Result<()> { unimplemented!() } fn new_client( &mut self, _client_id: ClientId, _stream: LocalSocketStream, - ) -> IpcReceiverWithContext { + ) -> Result> { unimplemented!() } - fn remove_client(&mut self, _client_id: ClientId) { + fn remove_client(&mut self, _client_id: ClientId) -> Result<()> { unimplemented!() } fn load_palette(&self) -> Palette { @@ -86,7 +82,7 @@ impl ServerOsApi for FakeInputOutput { unimplemented!() } - fn write_to_file(&mut self, _buf: String, _name: Option) { + fn write_to_file(&mut self, _buf: String, _name: Option) -> Result<()> { unimplemented!() } fn re_run_command_in_terminal( @@ -94,10 +90,10 @@ impl ServerOsApi for FakeInputOutput { _terminal_id: u32, _run_command: RunCommand, _quit_cb: Box, RunCommand) + Send>, // u32 is the exit status - ) -> Result<(RawFd, RawFd), SpawnTerminalError> { + ) -> Result<(RawFd, RawFd)> { unimplemented!() } - fn clear_terminal_id(&self, _terminal_id: u32) { + fn clear_terminal_id(&self, _terminal_id: u32) -> Result<()> { unimplemented!() } } diff --git a/zellij-server/src/unit/screen_tests.rs b/zellij-server/src/unit/screen_tests.rs index 8fd1a6217..f55b0a1c9 100644 --- a/zellij-server/src/unit/screen_tests.rs +++ b/zellij-server/src/unit/screen_tests.rs @@ -2,7 +2,7 @@ use super::{screen_thread_main, CopyOptions, Screen, ScreenInstruction}; use crate::panes::PaneId; use crate::{ channels::SenderWithContext, - os_input_output::{AsyncReader, Pid, ServerOsApi, SpawnTerminalError}, + os_input_output::{AsyncReader, Pid, ServerOsApi}, route::route_action, thread_bus::Bus, ClientId, ServerInstruction, SessionMetaData, ThreadSenders, @@ -10,7 +10,7 @@ use crate::{ use insta::assert_snapshot; use std::path::PathBuf; use zellij_utils::cli::CliAction; -use zellij_utils::errors::ErrorContext; +use zellij_utils::errors::{prelude::*, ErrorContext}; use zellij_utils::input::actions::{Action, Direction, ResizeDirection}; use zellij_utils::input::command::{RunCommand, TerminalAction}; use zellij_utils::input::layout::{PaneLayout, SplitDirection}; @@ -25,7 +25,7 @@ use std::sync::{Arc, Mutex}; use crate::{pty::PtyInstruction, wasm_vm::PluginInstruction}; use zellij_utils::ipc::PixelDimensions; -use zellij_utils::nix; + use zellij_utils::{ channels::{self, ChannelWithContext, Receiver}, data::{InputMode, ModeInfo, Palette, PluginCapabilities}, @@ -127,43 +127,45 @@ struct FakeInputOutput { } impl ServerOsApi for FakeInputOutput { - fn set_terminal_size_using_terminal_id(&self, _terminal_id: u32, _cols: u16, _rows: u16) { + fn set_terminal_size_using_terminal_id( + &self, + _terminal_id: u32, + _cols: u16, + _rows: u16, + ) -> Result<()> { // noop + Ok(()) } fn spawn_terminal( &self, _file_to_open: TerminalAction, _quit_db: Box, RunCommand) + Send>, _default_editor: Option, - ) -> Result<(u32, RawFd, RawFd), SpawnTerminalError> { + ) -> Result<(u32, RawFd, RawFd)> { unimplemented!() } - fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result { + fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result { unimplemented!() } fn async_file_reader(&self, _fd: RawFd) -> Box { unimplemented!() } - fn write_to_tty_stdin(&self, _id: u32, _buf: &[u8]) -> Result { + fn write_to_tty_stdin(&self, _id: u32, _buf: &[u8]) -> Result { unimplemented!() } - fn tcdrain(&self, _id: u32) -> Result<(), nix::Error> { + fn tcdrain(&self, _id: u32) -> Result<()> { unimplemented!() } - fn kill(&self, _pid: Pid) -> Result<(), nix::Error> { + fn kill(&self, _pid: Pid) -> Result<()> { unimplemented!() } - fn force_kill(&self, _pid: Pid) -> Result<(), nix::Error> { + fn force_kill(&self, _pid: Pid) -> Result<()> { unimplemented!() } fn box_clone(&self) -> Box { Box::new((*self).clone()) } - fn send_to_client( - &self, - client_id: ClientId, - msg: ServerToClientMsg, - ) -> Result<(), &'static str> { + fn send_to_client(&self, client_id: ClientId, msg: ServerToClientMsg) -> Result<()> { self.server_to_client_messages .lock() .unwrap() @@ -176,10 +178,10 @@ impl ServerOsApi for FakeInputOutput { &mut self, _client_id: ClientId, _stream: LocalSocketStream, - ) -> IpcReceiverWithContext { + ) -> Result> { unimplemented!() } - fn remove_client(&mut self, _client_id: ClientId) { + fn remove_client(&mut self, _client_id: ClientId) -> Result<()> { unimplemented!() } fn load_palette(&self) -> Palette { @@ -188,23 +190,24 @@ impl ServerOsApi for FakeInputOutput { fn get_cwd(&self, _pid: Pid) -> Option { unimplemented!() } - fn write_to_file(&mut self, contents: String, filename: Option) { + fn write_to_file(&mut self, contents: String, filename: Option) -> Result<()> { if let Some(filename) = filename { self.fake_filesystem .lock() .unwrap() .insert(filename, contents); } + Ok(()) } fn re_run_command_in_terminal( &self, _terminal_id: u32, _run_command: RunCommand, _quit_cb: Box, RunCommand) + Send>, // u32 is the exit status - ) -> Result<(RawFd, RawFd), SpawnTerminalError> { + ) -> Result<(RawFd, RawFd)> { unimplemented!() } - fn clear_terminal_id(&self, _terminal_id: u32) { + fn clear_terminal_id(&self, _terminal_id: u32) -> Result<()> { unimplemented!() } } diff --git a/zellij-utils/src/errors.rs b/zellij-utils/src/errors.rs index 7ec02b9a5..1594912b6 100644 --- a/zellij-utils/src/errors.rs +++ b/zellij-utils/src/errors.rs @@ -24,6 +24,7 @@ pub mod prelude { pub use super::LoggableError; #[cfg(not(target_family = "wasm"))] pub use super::ToAnyhow; + pub use super::ZellijError; pub use anyhow::anyhow; pub use anyhow::bail; pub use anyhow::Context; @@ -376,6 +377,25 @@ pub enum PtyWriteContext { Exit, } +use thiserror::Error; +#[derive(Debug, Error)] +pub enum ZellijError { + #[error("could not find command '{command}' for terminal {terminal_id}")] + CommandNotFound { terminal_id: u32, command: String }, + + #[error("could not determine default editor")] + NoEditorFound, + + #[error("failed to allocate another terminal id")] + NoMoreTerminalIds, + + #[error("failed to start PTY")] + FailedToStartPty, + + #[error("an error occured")] + GenericError { source: anyhow::Error }, +} + #[cfg(not(target_family = "wasm"))] pub use not_wasm::*; diff --git a/zellij-utils/src/ipc.rs b/zellij-utils/src/ipc.rs index 930087886..b264f48e5 100644 --- a/zellij-utils/src/ipc.rs +++ b/zellij-utils/src/ipc.rs @@ -2,7 +2,7 @@ use crate::{ cli::CliArgs, data::{ClientId, InputMode, Style}, - errors::{get_current_ctx, ErrorContext}, + errors::{get_current_ctx, prelude::*, ErrorContext}, input::keybinds::Keybinds, input::{actions::Action, layout::Layout, options::Options, plugins::PluginsConfig}, pane_size::{Size, SizeInPixels}, @@ -154,10 +154,10 @@ impl IpcSenderWithContext { } /// Sends an event, along with the current [`ErrorContext`], on this [`IpcSenderWithContext`]'s socket. - pub fn send(&mut self, msg: T) -> Result<(), &'static str> { + pub fn send(&mut self, msg: T) -> Result<()> { let err_ctx = get_current_ctx(); if rmp_serde::encode::write(&mut self.sender, &(msg, err_ctx)).is_err() { - Err("Failed to send message to client") + Err(anyhow!("failed to send message to client")) } else { // TODO: unwrapping here can cause issues when the server disconnects which we don't mind // do we need to handle errors here in other cases?