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

1 files changed, 184 insertions, 143 deletions

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<i32> {
+fn handle_command_exit(mut child: Child) -> Result<Option<i32>> {
+    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<i32> {
                 // 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<i32> {
         } 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<dyn Fn(PaneId, Option<i32>, 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<dyn Fn(PaneId, Option<i32>, 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<dyn Fn(PaneId, Option<i32>, RunCommand) + Send>, // u32 is the exit_status
     default_editor: Option<PathBuf>,
     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<Mutex<termios::Termios>>,
@@ -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<dyn Fn(PaneId, Option<i32>, RunCommand) + Send>, // u32 is the exit status
         default_editor: Option<PathBuf>,
-    ) -> Result<(u32, RawFd, RawFd), SpawnTerminalError>;
+    ) -> Result<(u32, RawFd, RawFd)>;
     // reserves a terminal id without actually opening a terminal
-    fn reserve_terminal_id(&self) -> Result<u32, SpawnTerminalError> {
+    fn reserve_terminal_id(&self) -> Result<u32> {
         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<usize, nix::Error>;
+    fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result<usize>;
     /// Creates an `AsyncReader` that can be used to read from `fd` in an async context
     fn async_file_reader(&self, fd: RawFd) -> Box<dyn AsyncReader>;
     /// 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<usize, nix::Error>;
+    fn write_to_tty_stdin(&self, terminal_id: u32, buf: &[u8]) -> Result<usize>;
     /// 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<dyn ServerOsApi>;
-    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<ClientToServerMsg>;
-    fn remove_client(&mut self, client_id: ClientId);
+    ) -> Result<IpcReceiverWithContext<ClientToServerMsg>>;
+    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<PathBuf>;
     /// Writes the given buffer to a string
-    fn write_to_file(&mut self, buf: String, file: Option<String>);
+    fn write_to_file(&mut self, buf: String, file: Option<String>) -> Result<()>;
 
     fn re_run_command_in_terminal(
         &self,
         terminal_id: u32,
         run_command: RunCommand,
         quit_cb: Box<dyn Fn(PaneId, Option<i32>, 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<dyn Fn(PaneId, Option<i32>, RunCommand) + Send>, // u32 is the exit status
         default_editor: Option<PathBuf>,
-    ) -> 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<u32> = 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<u32, SpawnTerminalError> {
+    fn reserve_terminal_id(&self) -> Result<u32> {
+        let err_context = || "failed to reserve a terminal ID".to_string();
+
         let mut terminal_id = None;
         {
             let current_ids: HashSet<u32> = 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<usize, nix::Error> {
-        unistd::read(fd, buf)
+    fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result<usize> {
+        unistd::read(fd, buf).with_context(|| format!("failed to read stdout of raw FD {}", fd))
     }
     fn async_file_reader(&self, fd: RawFd) -> Box<dyn AsyncReader> {
         Box::new(RawFdAsyncReader::new(fd))
     }
-    fn write_to_tty_stdin(&self, terminal_id: u32, buf: &[u8]) -> Result<usize, nix::Error> {
-        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<usize> {
+        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<dyn ServerOsApi> {
         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<ClientToServerMsg> {
+    ) -> Result<IpcReceiverWithContext<ClientToServerMsg>> {
         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<PathBuf> {
         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<String>) {
+
+    fn write_to_file(&mut self, buf: String, name: Option<String>) -> 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<dyn Fn(PaneId, Option<i32>, 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(())
     }
 }