From e45a3e58267925b710a5612b4d01b175c2962bcf Mon Sep 17 00:00:00 2001 From: har7an <99636919+har7an@users.noreply.github.com> Date: Wed, 2 Nov 2022 05:29:50 +0000 Subject: [PATCH] errors: Don't unwrap in `server::os_input_output` (#1895) * server/os_io: Redefine `ServerOsApi` result types to use `anyhow::Result` instead. This mostly makes the need of custom `SpawnTerminalError` obsolete (tbd in subsequent commits) and unifies error handling across the application. * utils/errors: Implement new `ZellijError` type to replace any previously defined, isolated custom error types throughout the application. Currently implements all error variants found in `SpawnTerminalError`. In the long term, this will allow zellij to fall back to a single error type for all application-specific errors, instead of having different error types per module. * server/unit/screen: Impl new `ServerOsApi` with updated `Result`-types. * server/tab/unit: Impl new `ServerOsApi` with updated `Result`-types. * server/os_io: Impl new `ServerOsApi` with updated `Result`-types. * utils/ipc: Return `anyhow::Error` in `send` rather than a `&'static str`, which isn't compatible with `anyhow::Context`. * server/tab: Handle `Result` in `resize_pty!` which is returned due to the changed return types in `ServerOsApi`. * server/tab: Handle new `Result`s originating in the change to the `ServerOsApi` trait definition. * server/screen: Handle new `Result`s originating in the change to the `ServerOsApi` trait definition. * server/panes/tiled: Handle new `Result`s originating in the change to the `ServerOsApi` trait definition. * server/panes/floating: Handle new `Result`s originating in the change to the `ServerOsApi` trait definition. * server/lib: Unwrap on new `Result`s originating in the change to the `ServerOsApi` trait definition. The functions here don't return a `Result` yet, this is better left to a follow-up PR. * server: Remove `SpawnTerminalError` and make use of the new `ZellijError` instead. Make use of `anyhow`s downcast capabilities to restore the underlying original errors where necessary, as was done previously. This gives us the flexibility to attach context information to all errors while still allowing us to handle specific errors in greater detail. * server/pty: Fix vars broken in rebase * server/os_io: Remove last `SpawnTerminalError` * changelog: Add PR #1895 --- CHANGELOG.md | 1 + zellij-server/src/lib.rs | 4 +- zellij-server/src/os_input_output.rs | 327 +++++++------- zellij-server/src/panes/floating_panes/mod.rs | 20 +- zellij-server/src/panes/tiled_panes/mod.rs | 38 +- zellij-server/src/pty.rs | 400 ++++++++++-------- zellij-server/src/screen.rs | 3 +- zellij-server/src/tab/mod.rs | 54 ++- .../src/tab/unit/tab_integration_tests.rs | 43 +- zellij-server/src/tab/unit/tab_tests.rs | 36 +- zellij-server/src/unit/screen_tests.rs | 43 +- zellij-utils/src/errors.rs | 20 + zellij-utils/src/ipc.rs | 6 +- 13 files changed, 552 insertions(+), 443 deletions(-) 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?