From 99e2bef8c68bd166cf89e90c8ffe8c02272ab4d3 Mon Sep 17 00:00:00 2001 From: har7an <99636919+har7an@users.noreply.github.com> Date: Fri, 9 Sep 2022 13:21:03 +0000 Subject: Feature: Better error handling/reporting (#1670) * utils: re-export "anyhow" unconditionally even for wasm targets. * utils/errors: Share wasm-compatible code and move everything that can't run in wasm into a separate submodule. * utils: Share "errors" module unconditionally The module is now structured such that all code incompatible with wasm targets lives in its own submodule that isn't included when compiling for wasm targets. * utils/errors: Add "Help wanted" doc section that informs the reader about the endeavour to improve error handling throughout the zellij code base. * plugins: Handle errors returned by `zellij_tile` now that the panic calls have been removed. * utils/errors: Extend `anyhow::Result` with traits that allow for easy/concise logging of `anyhow::Result` types and panicking the application when they are fatal or non-fatal. * utils/errors: Fix doctest * utils/errors: Add prelude that applications can import to conveniently access the error handling functionality part of the module. Re-exports some parts and macros from anyhow and the `LoggableError` and `FatalError` traits. * server/screen: Adopt error handling and make all fallible functions from the public API return a `Result`. Append error contexts in all functions that can come across error types to allow tracing where an error originates and what lead there. * server/lib: Catch errors from `screen` and make them `fatal`. This will log the errors first, before unwrapping on the error type and panicking the application. * server/unit/screen: Fix unit tests and unwrap on the `Result` types introduced from the new error handling. * utils/errors: Track error source in calls to `fatal`, so we keep track of the location where the panic really originates. Otherwise, any call to `fatal` will show the code in `errors` as source, which of course isn't true. Also change the error formatting and do not call `to_log` for fatal errors anymore, because the panic is already logged and contains much more information. * utils/errors: Update `FatalError` docs * plugins: Undo accidental modifications * utils/errors: Improve module docs explain some error handling facilities and the motivation behind using them. * server/screen: Remove `Result` from Infallible functions that are part of the public API. --- zellij-server/src/lib.rs | 5 +- zellij-server/src/screen.rs | 396 +++++++++++++++++------------ zellij-server/src/unit/screen_tests.rs | 66 ++--- zellij-utils/src/errors.rs | 447 ++++++++++++++++++++++++--------- zellij-utils/src/lib.rs | 9 +- 5 files changed, 602 insertions(+), 321 deletions(-) diff --git a/zellij-server/src/lib.rs b/zellij-server/src/lib.rs index 400026b15..b3f17ad33 100644 --- a/zellij-server/src/lib.rs +++ b/zellij-server/src/lib.rs @@ -40,7 +40,7 @@ use zellij_utils::{ cli::CliArgs, consts::{DEFAULT_SCROLL_BUFFER_SIZE, SCROLL_BUFFER_SIZE}, data::{Event, PluginCapabilities}, - errors::{ContextType, ErrorInstruction, ServerContext}, + errors::{ContextType, ErrorInstruction, FatalError, ServerContext}, input::{ command::{RunCommand, TerminalAction}, get_mode_info, @@ -691,7 +691,8 @@ fn init_session( max_panes, client_attributes_clone, config_options, - ); + ) + .fatal(); } }) .unwrap(); diff --git a/zellij-server/src/screen.rs b/zellij-server/src/screen.rs index e3323b41e..b302db4be 100644 --- a/zellij-server/src/screen.rs +++ b/zellij-server/src/screen.rs @@ -6,6 +6,7 @@ use std::os::unix::io::RawFd; use std::rc::Rc; use std::str; +use zellij_utils::errors::prelude::*; use zellij_utils::input::options::Clipboard; use zellij_utils::pane_size::{Size, SizeInPixels}; use zellij_utils::{input::command::TerminalAction, input::layout::Layout, position::Position}; @@ -419,13 +420,20 @@ impl Screen { }, } } + /// A helper function to switch to a new tab at specified position. - fn switch_active_tab(&mut self, new_tab_pos: usize, client_id: ClientId) { + fn switch_active_tab(&mut self, new_tab_pos: usize, client_id: ClientId) -> Result<()> { + let err_context = || { + format!( + "Failed to switch to active tab at position {new_tab_pos} for client id: {client_id:?}" + ) + }; + if let Some(new_tab) = self.tabs.values().find(|t| t.position == new_tab_pos) { if let Some(current_tab) = self.get_active_tab(client_id) { // If new active tab is same as the current one, do nothing. if current_tab.position == new_tab_pos { - return; + return Ok(()); } let current_tab_index = current_tab.index; @@ -454,27 +462,29 @@ impl Screen { log::error!("Tab index: {:?} not found", current_tab_index); } - self.update_tabs(); - self.render(); + self.update_tabs().with_context(err_context)?; + return self.render().with_context(err_context); } else { - log::error!("Active tab not found for client_id: {:?}", client_id); + log::error!("Active tab not found for client id: {client_id:?}"); } } + Ok(()) } /// Sets this [`Screen`]'s active [`Tab`] to the next tab. - pub fn switch_tab_next(&mut self, client_id: ClientId) { + pub fn switch_tab_next(&mut self, client_id: ClientId) -> Result<()> { if let Some(active_tab) = self.get_active_tab(client_id) { let active_tab_pos = active_tab.position; let new_tab_pos = (active_tab_pos + 1) % self.tabs.len(); - self.switch_active_tab(new_tab_pos, client_id); + self.switch_active_tab(new_tab_pos, client_id) } else { - log::error!("Active tab not found for client_id: {:?}", client_id); + log::error!("Active tab not found for client id: {client_id:?}"); + Ok(()) } } /// Sets this [`Screen`]'s active [`Tab`] to the previous tab. - pub fn switch_tab_prev(&mut self, client_id: ClientId) { + pub fn switch_tab_prev(&mut self, client_id: ClientId) -> Result<()> { if let Some(active_tab) = self.get_active_tab(client_id) { let active_tab_pos = active_tab.position; let new_tab_pos = if active_tab_pos == 0 { @@ -483,18 +493,20 @@ impl Screen { active_tab_pos - 1 }; - self.switch_active_tab(new_tab_pos, client_id); + self.switch_active_tab(new_tab_pos, client_id) } else { - log::error!("Active tab not found for client_id: {:?}", client_id); + log::error!("Active tab not found for client id: {client_id:?}"); + Ok(()) } } - pub fn go_to_tab(&mut self, tab_index: usize, client_id: ClientId) { - self.switch_active_tab(tab_index - 1, client_id); + pub fn go_to_tab(&mut self, tab_index: usize, client_id: ClientId) -> Result<()> { + self.switch_active_tab(tab_index - 1, client_id) } - fn close_tab_at_index(&mut self, tab_index: usize) { - let mut tab_to_close = self.tabs.remove(&tab_index).unwrap(); + fn close_tab_at_index(&mut self, tab_index: usize) -> Result<()> { + let err_context = || format!("failed to close tab at index {tab_index:?}"); + let mut tab_to_close = self.tabs.remove(&tab_index).with_context(err_context)?; let pane_ids = tab_to_close.get_all_pane_ids(); // below we don't check the result of sending the CloseTab instruction to the pty thread // because this might be happening when the app is closing, at which point the pty thread @@ -502,13 +514,13 @@ impl Screen { self.bus .senders .send_to_pty(PtyInstruction::CloseTab(pane_ids)) - .unwrap(); + .with_context(err_context)?; if self.tabs.is_empty() { self.active_tab_indices.clear(); self.bus .senders .send_to_server(ServerInstruction::Render(None)) - .unwrap(); + .with_context(err_context) } else { let client_mode_infos_in_closed_tab = tab_to_close.drain_connected_clients(None); self.move_clients_from_closed_tab(client_mode_infos_in_closed_tab); @@ -523,25 +535,33 @@ impl Screen { t.position -= 1; } } - self.update_tabs(); - self.render(); + self.update_tabs().with_context(err_context)?; + self.render().with_context(err_context) } } // Closes the client_id's focused tab - pub fn close_tab(&mut self, client_id: ClientId) { - let active_tab_index = *self.active_tab_indices.get(&client_id).unwrap(); - self.close_tab_at_index(active_tab_index); + pub fn close_tab(&mut self, client_id: ClientId) -> Result<()> { + let err_context = || format!("failed to close tab for client {client_id:?}"); + + let active_tab_index = *self + .active_tab_indices + .get(&client_id) + .with_context(err_context)?; + self.close_tab_at_index(active_tab_index) + .with_context(err_context) } - pub fn resize_to_screen(&mut self, new_screen_size: Size) { + pub fn resize_to_screen(&mut self, new_screen_size: Size) -> Result<()> { self.size = new_screen_size; for tab in self.tabs.values_mut() { tab.resize_whole_tab(new_screen_size); tab.set_force_render(); } - self.render(); + self.render() + .context("failed to resize to screen size: {new_screen_size:#?}") } + pub fn update_pixel_dimensions(&mut self, pixel_dimensions: PixelDimensions) { self.pixel_dimensions.merge(pixel_dimensions); if let Some(character_cell_size) = self.pixel_dimensions.character_cell_size { @@ -556,6 +576,7 @@ impl Screen { *self.character_cell_size.borrow_mut() = Some(character_cell_size); } } + pub fn update_terminal_background_color(&mut self, background_color_instruction: String) { if let Some(AnsiCode::RgbCode((r, g, b))) = xparse_color(background_color_instruction.as_bytes()) @@ -564,6 +585,7 @@ impl Screen { self.terminal_emulator_colors.borrow_mut().bg = bg_palette_color; } } + pub fn update_terminal_foreground_color(&mut self, foreground_color_instruction: String) { if let Some(AnsiCode::RgbCode((r, g, b))) = xparse_color(foreground_color_instruction.as_bytes()) @@ -572,6 +594,7 @@ impl Screen { self.terminal_emulator_colors.borrow_mut().fg = fg_palette_color; } } + pub fn update_terminal_color_registers(&mut self, color_registers: Vec<(usize, String)>) { let mut terminal_emulator_color_codes = self.terminal_emulator_color_codes.borrow_mut(); for (color_register, color_sequence) in color_registers { @@ -580,7 +603,9 @@ impl Screen { } /// Renders this [`Screen`], which amounts to rendering its active [`Tab`]. - pub fn render(&mut self) { + pub fn render(&mut self) -> Result<()> { + let err_context = || "failed to render screen".to_string(); + let mut output = Output::new( self.sixel_image_store.clone(), self.character_cell_size.clone(), @@ -597,13 +622,14 @@ impl Screen { } } for tab_index in tabs_to_close { - self.close_tab_at_index(tab_index); + self.close_tab_at_index(tab_index) + .with_context(err_context)?; } let serialized_output = output.serialize(); self.bus .senders .send_to_server(ServerInstruction::Render(Some(serialized_output))) - .unwrap(); + .with_context(err_context) } /// Returns a mutable reference to this [`Screen`]'s tabs. @@ -621,11 +647,20 @@ impl Screen { /// Returns an immutable reference to this [`Screen`]'s previous active [`Tab`]. /// Consumes the last entry in tab history. - pub fn get_previous_tab(&mut self, client_id: ClientId) -> Option<&Tab> { - match self.tab_history.get_mut(&client_id).unwrap().pop() { - Some(tab) => self.tabs.get(&tab), - None => None, - } + pub fn get_previous_tab(&mut self, client_id: ClientId) -> Result> { + Ok( + match self + .tab_history + .get_mut(&client_id) + .with_context(|| { + format!("failed to retrieve tab history for client {client_id:?}") + })? + .pop() + { + Some(tab) => self.tabs.get(&tab), + None => None, + }, + ) } /// Returns a mutable reference to this [`Screen`]'s active [`Tab`]. @@ -648,7 +683,14 @@ impl Screen { /// Creates a new [`Tab`] in this [`Screen`], applying the specified [`Layout`] /// and switching to it. - pub fn new_tab(&mut self, layout: Layout, new_pids: Vec, client_id: ClientId) { + pub fn new_tab( + &mut self, + layout: Layout, + new_pids: Vec, + client_id: ClientId, + ) -> Result<()> { + let err_context = || format!("failed to create new tab for client {client_id:?}",); + let tab_index = self.get_new_tab_index(); let position = self.tabs.len(); let mut tab = Tab::new( @@ -658,7 +700,11 @@ impl Screen { self.size, self.character_cell_size.clone(), self.sixel_image_store.clone(), - self.bus.os_input.as_ref().unwrap().clone(), + self.bus + .os_input + .as_ref() + .with_context(|| format!("failed to create new tab for client {client_id:?}"))? + .clone(), self.bus.senders.clone(), self.max_panes, self.style, @@ -699,14 +745,14 @@ impl Screen { self.tabs.insert(tab_index, tab); if !self.active_tab_indices.contains_key(&client_id) { // this means this is a new client and we need to add it to our state properly - self.add_client(client_id); + self.add_client(client_id).with_context(err_context)?; } - self.update_tabs(); + self.update_tabs().with_context(err_context)?; - self.render(); + self.render().with_context(err_context) } - pub fn add_client(&mut self, client_id: ClientId) { + pub fn add_client(&mut self, client_id: ClientId) -> Result<()> { let mut tab_history = vec![]; if let Some((_first_client, first_tab_history)) = self.tab_history.iter().next() { tab_history = first_tab_history.clone(); @@ -729,10 +775,12 @@ impl Screen { self.tab_history.insert(client_id, tab_history); self.tabs .get_mut(&tab_index) - .unwrap_or_else(|| panic!("Failed to attach client to tab with index {tab_index}")) + .with_context(|| format!("Failed to attach client to tab with index {tab_index}"))? .add_client(client_id, None); + Ok(()) } - pub fn remove_client(&mut self, client_id: ClientId) { + + pub fn remove_client(&mut self, client_id: ClientId) -> Result<()> { self.tabs.iter_mut().for_each(|(_, tab)| { tab.remove_client(client_id); if tab.has_no_connected_clients() { @@ -746,10 +794,11 @@ impl Screen { self.tab_history.remove(&client_id); } self.connected_clients.borrow_mut().remove(&client_id); - self.update_tabs(); + self.update_tabs() + .with_context(|| format!("failed to remote client {client_id:?}")) } - pub fn update_tabs(&self) { + pub fn update_tabs(&self) -> Result<()> { for (client_id, active_tab_index) in self.active_tab_indices.iter() { let mut tab_data = vec![]; for tab in self.tabs.values() { @@ -783,11 +832,17 @@ impl Screen { Some(*client_id), Event::TabUpdate(tab_data), )) - .unwrap(); + .or_else(|err| { + let (_, error_context) = err.0; + Err(anyhow!("failed to send data to plugins")) + .context(error_context) + .context("failed to update tabs") + })?; } + Ok(()) } - pub fn update_active_tab_name(&mut self, buf: Vec, client_id: ClientId) { + pub fn update_active_tab_name(&mut self, buf: Vec, client_id: ClientId) -> Result<()> { let s = str::from_utf8(&buf).unwrap(); if let Some(active_tab) = self.get_active_tab_mut(client_id) { match s { @@ -805,21 +860,28 @@ impl Screen { } }, } - self.update_tabs(); + self.update_tabs() + .context("failed to update active tabs name for client id: {client_id:?}") } else { - log::error!("Active tab not found for client id: {:?}", client_id); + log::error!("Active tab not found for client id: {client_id:?}"); + Ok(()) } } - pub fn undo_active_rename_tab(&mut self, client_id: ClientId) { + + pub fn undo_active_rename_tab(&mut self, client_id: ClientId) -> Result<()> { if let Some(active_tab) = self.get_active_tab_mut(client_id) { if active_tab.name != active_tab.prev_name { active_tab.name = active_tab.prev_name.clone(); - self.update_tabs(); + self.update_tabs() + .context("failed to undo renaming of active tab")?; } + Ok(()) } else { - log::error!("Active tab not found for client id: {:?}", client_id); + log::error!("Active tab not found for client id: {client_id:?}"); + Ok(()) } } + pub fn change_mode(&mut self, mode_info: ModeInfo, client_id: ClientId) { let previous_mode = self .mode_info @@ -866,33 +928,41 @@ impl Screen { tab.mark_active_pane_for_rerender(client_id); } } - pub fn move_focus_left_or_previous_tab(&mut self, client_id: ClientId) { + + pub fn move_focus_left_or_previous_tab(&mut self, client_id: ClientId) -> Result<()> { if let Some(active_tab) = self.get_active_tab_mut(client_id) { if !active_tab.move_focus_left(client_id) { - self.switch_tab_prev(client_id); + self.switch_tab_prev(client_id) + .context("failed to move focus left")?; } } else { log::error!("Active tab not found for client id: {:?}", client_id); } + Ok(()) } - pub fn move_focus_right_or_next_tab(&mut self, client_id: ClientId) { + pub fn move_focus_right_or_next_tab(&mut self, client_id: ClientId) -> Result<()> { if let Some(active_tab) = self.get_active_tab_mut(client_id) { if !active_tab.move_focus_right(client_id) { - self.switch_tab_next(client_id); + self.switch_tab_next(client_id) + .context("failed to move focus right")?; } } else { log::error!("Active tab not found for client id: {:?}", client_id); } + Ok(()) } - pub fn toggle_tab(&mut self, client_id: ClientId) { - let tab = self.get_previous_tab(client_id); + pub fn toggle_tab(&mut self, client_id: ClientId) -> Result<()> { + let tab = self + .get_previous_tab(client_id) + .context("failed to toggle tabs")?; if let Some(t) = tab { let position = t.position; - self.go_to_tab(position + 1, client_id); + self.go_to_tab(position + 1, client_id) + .context("failed to toggle tabs")?; }; - self.update_tabs(); - self.render(); + self.update_tabs().context("failed to toggle tabs")?; + self.render() } fn unblock_input(&self) { @@ -911,7 +981,7 @@ pub(crate) fn screen_thread_main( max_panes: Option, client_attributes: ClientAttributes, config_options: Box, -) { +) -> Result<()> { // let mut scrollbacks: HashMap = HashMap::new(); let capabilities = config_options.simplified_ui; let draw_pane_frames = config_options.pane_frames.unwrap_or(true); @@ -942,7 +1012,8 @@ pub(crate) fn screen_thread_main( let (event, mut err_ctx) = screen .bus .recv() - .expect("failed to receive event on channel"); + .context("failed to receive event on channel") + .fatal(); err_ctx.add_call(ContextType::Screen((&event).into())); match event { @@ -956,7 +1027,7 @@ pub(crate) fn screen_thread_main( } }, ScreenInstruction::Render => { - screen.render(); + screen.render()?; }, ScreenInstruction::NewPane(pid, client_or_tab_index) => { match client_or_tab_index { @@ -973,45 +1044,45 @@ pub(crate) fn screen_thread_main( }, }; screen.unblock_input(); - screen.update_tabs(); + screen.update_tabs()?; - screen.render(); + screen.render()?; }, ScreenInstruction::OpenInPlaceEditor(pid, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .suppress_active_pane(pid, client_id)); screen.unblock_input(); - screen.update_tabs(); + screen.update_tabs()?; - screen.render(); + screen.render()?; }, ScreenInstruction::TogglePaneEmbedOrFloating(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .toggle_pane_embed_or_floating(client_id)); screen.unblock_input(); - screen.update_tabs(); // update tabs so that the ui indication will be send to the plugins - screen.render(); + screen.update_tabs()?; // update tabs so that the ui indication will be send to the plugins + screen.render()?; }, ScreenInstruction::ToggleFloatingPanes(client_id, default_shell) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .toggle_floating_panes(client_id, default_shell)); screen.unblock_input(); - screen.update_tabs(); // update tabs so that the ui indication will be send to the plugins - screen.render(); + screen.update_tabs()?; // update tabs so that the ui indication will be send to the plugins + screen.render()?; }, ScreenInstruction::HorizontalSplit(pid, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .horizontal_split(pid, client_id)); screen.unblock_input(); - screen.update_tabs(); - screen.render(); + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::VerticalSplit(pid, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .vertical_split(pid, client_id)); screen.unblock_input(); - screen.update_tabs(); - screen.render(); + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::WriteCharacter(bytes, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| { @@ -1024,166 +1095,166 @@ pub(crate) fn screen_thread_main( ScreenInstruction::ResizeLeft(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .resize_left(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ResizeRight(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .resize_right(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ResizeDown(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .resize_down(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ResizeUp(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab.resize_up(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ResizeIncrease(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .resize_increase(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ResizeDecrease(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .resize_decrease(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::SwitchFocus(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .focus_next_pane(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::FocusNextPane(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .focus_next_pane(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::FocusPreviousPane(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .focus_previous_pane(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MoveFocusLeft(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_focus_left(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MoveFocusLeftOrPreviousTab(client_id) => { - screen.move_focus_left_or_previous_tab(client_id); + screen.move_focus_left_or_previous_tab(client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; }, ScreenInstruction::MoveFocusDown(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_focus_down(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MoveFocusRight(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_focus_right(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MoveFocusRightOrNextTab(client_id) => { - screen.move_focus_right_or_next_tab(client_id); + screen.move_focus_right_or_next_tab(client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; }, ScreenInstruction::MoveFocusUp(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_focus_up(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::DumpScreen(file, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .dump_active_terminal_screen(Some(file.to_string()), client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::EditScrollback(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .edit_scrollback(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ScrollUp(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .scroll_active_terminal_up(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MovePane(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_active_pane(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MovePaneDown(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_active_pane_down(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MovePaneUp(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_active_pane_up(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MovePaneRight(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_active_pane_right(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MovePaneLeft(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .move_active_pane_left(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ScrollUpAt(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_scrollwheel_up(&point, 3, client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ScrollDown(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .scroll_active_terminal_down(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ScrollDownAt(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_scrollwheel_down(&point, 3, client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ScrollToBottom(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .scroll_active_terminal_to_bottom(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::PageScrollUp(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .scroll_active_terminal_up_page(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::PageScrollDown(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .scroll_active_terminal_down_page(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::HalfPageScrollUp(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .scroll_active_terminal_up_half_page(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::HalfPageScrollDown(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .scroll_active_terminal_down_half_page(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ClearScroll(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .clear_active_terminal_scroll(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::CloseFocusedPane(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .close_focused_pane(client_id)); - screen.update_tabs(); // update_tabs eventually calls render through the plugin thread + screen.update_tabs()?; // update_tabs eventually calls render through the plugin thread }, ScreenInstruction::SetSelectable(id, selectable, tab_index) => { screen.get_indexed_tab_mut(tab_index).map_or_else( @@ -1197,7 +1268,7 @@ pub(crate) fn screen_thread_main( |tab| tab.set_pane_selectable(id, selectable), ); - screen.render(); + screen.render()?; }, ScreenInstruction::ClosePane(id, client_id) => { match client_id { @@ -1213,71 +1284,71 @@ pub(crate) fn screen_thread_main( } }, } - screen.update_tabs(); + screen.update_tabs()?; }, ScreenInstruction::UpdatePaneName(c, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .update_active_pane_name(c, client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::UndoRenamePane(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .undo_active_rename_pane(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::ToggleActiveTerminalFullscreen(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .toggle_active_pane_fullscreen(client_id)); - screen.update_tabs(); - screen.render(); + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::TogglePaneFrames => { screen.draw_pane_frames = !screen.draw_pane_frames; for tab in screen.tabs.values_mut() { tab.set_pane_frames(screen.draw_pane_frames); } - screen.render(); + screen.render()?; }, ScreenInstruction::SwitchTabNext(client_id) => { - screen.switch_tab_next(client_id); + screen.switch_tab_next(client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; }, ScreenInstruction::SwitchTabPrev(client_id) => { - screen.switch_tab_prev(client_id); + screen.switch_tab_prev(client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; }, ScreenInstruction::CloseTab(client_id) => { - screen.close_tab(client_id); + screen.close_tab(client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; }, ScreenInstruction::NewTab(layout, new_pane_pids, client_id) => { - screen.new_tab(layout, new_pane_pids, client_id); + screen.new_tab(layout, new_pane_pids, client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; }, ScreenInstruction::GoToTab(tab_index, client_id) => { if let Some(client_id) = client_id.or_else(|| screen.active_tab_indices.keys().next().copied()) { - screen.go_to_tab(tab_index as usize, client_id); + screen.go_to_tab(tab_index as usize, client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; } }, ScreenInstruction::UpdateTabName(c, client_id) => { - screen.update_active_tab_name(c, client_id); - screen.render(); + screen.update_active_tab_name(c, client_id)?; + screen.render()?; }, ScreenInstruction::UndoRenameTab(client_id) => { - screen.undo_active_rename_tab(client_id); - screen.render(); + screen.undo_active_rename_tab(client_id)?; + screen.render()?; }, ScreenInstruction::TerminalResize(new_size) => { - screen.resize_to_screen(new_size); - screen.render(); + screen.resize_to_screen(new_size)?; + screen.render()?; }, ScreenInstruction::TerminalPixelDimensions(pixel_dimensions) => { screen.update_pixel_dimensions(pixel_dimensions); @@ -1293,86 +1364,86 @@ pub(crate) fn screen_thread_main( }, ScreenInstruction::ChangeMode(mode_info, client_id) => { screen.change_mode(mode_info, client_id); - screen.render(); + screen.render()?; }, ScreenInstruction::ToggleActiveSyncTab(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .toggle_sync_panes_is_active()); - screen.update_tabs(); - screen.render(); + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::LeftClick(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_left_click(&point, client_id)); - screen.update_tabs(); - screen.render(); + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::RightClick(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_right_click(&point, client_id)); - screen.update_tabs(); - screen.render(); + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::MiddleClick(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_middle_click(&point, client_id)); - screen.update_tabs(); - screen.render(); + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::LeftMouseRelease(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_left_mouse_release(&point, client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::RightMouseRelease(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_right_mouse_release(&point, client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MiddleMouseRelease(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .handle_middle_mouse_release(&point, client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::MouseHoldLeft(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| { tab.handle_mouse_hold_left(&point, client_id); }); - screen.render(); + screen.render()?; }, ScreenInstruction::MouseHoldRight(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| { tab.handle_mouse_hold_right(&point, client_id); }); - screen.render(); + screen.render()?; }, ScreenInstruction::MouseHoldMiddle(point, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| { tab.handle_mouse_hold_middle(&point, client_id); }); - screen.render(); + screen.render()?; }, ScreenInstruction::Copy(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .copy_selection(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::Exit => { break; }, ScreenInstruction::ToggleTab(client_id) => { - screen.toggle_tab(client_id); + screen.toggle_tab(client_id)?; screen.unblock_input(); - screen.render(); + screen.render()?; }, ScreenInstruction::AddClient(client_id) => { - screen.add_client(client_id); - screen.update_tabs(); - screen.render(); + screen.add_client(client_id)?; + screen.update_tabs()?; + screen.render()?; }, ScreenInstruction::RemoveClient(client_id) => { - screen.remove_client(client_id); - screen.render(); + screen.remove_client(client_id)?; + screen.render()?; }, ScreenInstruction::AddOverlay(overlay, _client_id) => { screen.get_active_overlays_mut().pop(); @@ -1381,7 +1452,7 @@ pub(crate) fn screen_thread_main( }, ScreenInstruction::RemoveOverlay(_client_id) => { screen.get_active_overlays_mut().pop(); - screen.render(); + screen.render()?; screen.unblock_input(); }, ScreenInstruction::ConfirmPrompt(_client_id) => { @@ -1394,40 +1465,41 @@ pub(crate) fn screen_thread_main( }, ScreenInstruction::DenyPrompt(_client_id) => { screen.get_active_overlays_mut().pop(); - screen.render(); + screen.render()?; screen.unblock_input(); }, ScreenInstruction::UpdateSearch(c, client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .update_search_term(c, client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::SearchDown(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .search_down(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::SearchUp(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab.search_up(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::SearchToggleCaseSensitivity(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .toggle_search_case_sensitivity(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::SearchToggleWrap(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .toggle_search_wrap(client_id)); - screen.render(); + screen.render()?; }, ScreenInstruction::SearchToggleWholeWord(client_id) => { active_tab!(screen, client_id, |tab: &mut Tab| tab .toggle_search_whole_words(client_id)); - screen.render(); + screen.render()?; }, } } + Ok(()) } #[cfg(test)] diff --git a/zellij-server/src/unit/screen_tests.rs b/zellij-server/src/unit/screen_tests.rs index 53cb62d1c..28aec39cb 100644 --- a/zellij-server/src/unit/screen_tests.rs +++ b/zellij-server/src/unit/screen_tests.rs @@ -114,11 +114,13 @@ fn create_new_screen(size: Size) -> Screen { fn new_tab(screen: &mut Screen, pid: i32) { let client_id = 1; - screen.new_tab( - LayoutTemplate::default().try_into().unwrap(), - vec![pid], - client_id, - ); + screen + .new_tab( + LayoutTemplate::default().try_into().unwrap(), + vec![pid], + client_id, + ) + .expect("TEST"); } #[test] @@ -150,7 +152,7 @@ pub fn switch_to_prev_tab() { new_tab(&mut screen, 1); new_tab(&mut screen, 2); - screen.switch_tab_prev(1); + screen.switch_tab_prev(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, @@ -169,8 +171,8 @@ pub fn switch_to_next_tab() { new_tab(&mut screen, 1); new_tab(&mut screen, 2); - screen.switch_tab_prev(1); - screen.switch_tab_next(1); + screen.switch_tab_prev(1).expect("TEST"); + screen.switch_tab_next(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, @@ -189,7 +191,7 @@ pub fn close_tab() { new_tab(&mut screen, 1); new_tab(&mut screen, 2); - screen.close_tab(1); + screen.close_tab(1).expect("TEST"); assert_eq!(screen.tabs.len(), 1, "Only one tab left"); assert_eq!( @@ -210,8 +212,8 @@ pub fn close_the_middle_tab() { new_tab(&mut screen, 1); new_tab(&mut screen, 2); new_tab(&mut screen, 3); - screen.switch_tab_prev(1); - screen.close_tab(1); + screen.switch_tab_prev(1).expect("TEST"); + screen.close_tab(1).expect("TEST"); assert_eq!(screen.tabs.len(), 2, "Two tabs left"); assert_eq!( @@ -232,8 +234,8 @@ fn move_focus_left_at_left_screen_edge_changes_tab() { new_tab(&mut screen, 1); new_tab(&mut screen, 2); new_tab(&mut screen, 3); - screen.switch_tab_prev(1); - screen.move_focus_left_or_previous_tab(1); + screen.switch_tab_prev(1).expect("TEST"); + screen.move_focus_left_or_previous_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, @@ -253,8 +255,8 @@ fn move_focus_right_at_right_screen_edge_changes_tab() { new_tab(&mut screen, 1); new_tab(&mut screen, 2); new_tab(&mut screen, 3); - screen.switch_tab_prev(1); - screen.move_focus_right_or_next_tab(1); + screen.switch_tab_prev(1).expect("TEST"); + screen.move_focus_right_or_next_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, @@ -273,17 +275,17 @@ pub fn toggle_to_previous_tab_simple() { new_tab(&mut screen, 1); new_tab(&mut screen, 2); - screen.go_to_tab(1, 1); - screen.go_to_tab(2, 1); + screen.go_to_tab(1, 1).expect("TEST"); + screen.go_to_tab(2, 1).expect("TEST"); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, 0, "Active tab toggler to previous tab" ); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, 1, @@ -309,7 +311,7 @@ pub fn toggle_to_previous_tab_create_tabs_only() { "Tab history is invalid" ); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, 1, @@ -321,7 +323,7 @@ pub fn toggle_to_previous_tab_create_tabs_only() { "Tab history is invalid" ); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, 2, @@ -333,7 +335,7 @@ pub fn toggle_to_previous_tab_create_tabs_only() { "Tab history is invalid" ); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, 1, @@ -365,7 +367,7 @@ pub fn toggle_to_previous_tab_delete() { "Active tab toggler to previous tab" ); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.tab_history.get(&1).unwrap(), &[0, 1, 3], @@ -377,7 +379,7 @@ pub fn toggle_to_previous_tab_delete() { "Active tab toggler to previous tab" ); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.tab_history.get(&1).unwrap(), &[0, 1, 2], @@ -389,7 +391,7 @@ pub fn toggle_to_previous_tab_delete() { "Active tab toggler to previous tab" ); - screen.switch_tab_prev(1); + screen.switch_tab_prev(1).expect("TEST"); assert_eq!( screen.tab_history.get(&1).unwrap(), &[0, 1, 3], @@ -400,7 +402,7 @@ pub fn toggle_to_previous_tab_delete() { 2, "Active tab toggler to previous tab" ); - screen.switch_tab_prev(1); + screen.switch_tab_prev(1).expect("TEST"); assert_eq!( screen.tab_history.get(&1).unwrap(), &[0, 3, 2], @@ -412,7 +414,7 @@ pub fn toggle_to_previous_tab_delete() { "Active tab toggler to previous tab" ); - screen.close_tab(1); + screen.close_tab(1).expect("TEST"); assert_eq!( screen.tab_history.get(&1).unwrap(), &[0, 3], @@ -424,7 +426,7 @@ pub fn toggle_to_previous_tab_delete() { "Active tab toggler to previous tab" ); - screen.toggle_tab(1); + screen.toggle_tab(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, 2, @@ -453,7 +455,7 @@ fn switch_to_tab_with_fullscreen() { } new_tab(&mut screen, 2); - screen.switch_tab_prev(1); + screen.switch_tab_prev(1).expect("TEST"); assert_eq!( screen.get_active_tab(1).unwrap().position, @@ -566,7 +568,7 @@ fn attach_after_first_tab_closed() { } new_tab(&mut screen, 2); - screen.close_tab_at_index(0); - screen.remove_client(1); - screen.add_client(1); + screen.close_tab_at_index(0).expect("TEST"); + screen.remove_client(1).expect("TEST"); + screen.add_client(1).expect("TEST"); } diff --git a/zellij-utils/src/errors.rs b/zellij-utils/src/errors.rs index 07dfb89e0..a46427ed6 100644 --- a/zellij-utils/src/errors.rs +++ b/zellij-utils/src/errors.rs @@ -1,19 +1,106 @@ //! Error context system based on a thread-local representation of the call stack, itself based on //! the instructions that are sent between threads. - -use crate::channels::{SenderWithContext, ASYNCOPENCALLS, OPENCALLS}; +//! +//! # Help wanted +//! +//! As of writing this, zellij relies on `unwrap()` to catch errors (terminate execution) in many +//! functions, rather than returning a [`Result`] to propagate these errors further up. While we +//! don't consider `unwrap` to be a bad thing in general, it hides the underlying error and leaves +//! the user only with a stack trace to go on. Worse than this, it will crash the application. This +//! is particularly bad when the user is using long-running sessions to perform tasks. +//! +//! Hence, we would like to eliminate `unwrap()` statements from the code where possible, and apply +//! better error handling instead. This way, functions higher up in the call stack can react to +//! errors from underlying functions and either try to recover, or give some meaningful error +//! messages if recovery isn't possible. +//! +//! Since the zellij codebase is pretty big and growing rapidly, this endeavour will continue to be +//! pursued over time, as zellij develops. The idea is that modules or single files are converted +//! bit by bit, preferrably in small PRs that each target a specific module or file. **If you are +//! looking to contribute to zellij, this may be an ideal start for you!** This way you get to know +//! the codebase and get an idea which modules are used at which other places in the code. +//! +//! If you have an interest in this, don't hesitate to get in touch with us. +//! +//! +//! # Error handling facilities +//! +//! ## Displaying panic messages +//! +//! Panics are generally handled via the [`Panic`] error type and the +//! [`handle_panic`][`handle_panic`] panic handler function. The fancy formatting is performed by +//! the [`miette`] crate. +//! +//! +//! ## Propagating errors +//! +//! We use the [`anyhow`] crate to propagate errors up the call stack. At the moment, zellij +//! doesn't have custom error types, so we wrap whatever errors the underlying libraries give us, +//! if any. [`anyhow`] serves the purpose of providing [`context`][`context`] about where (i.e. +//! under which circumstances) an error happened. +//! +//! A critical requirement for propagating errors is that all functions involved must return the +//! [`Result`] type. This allows convenient error handling with the `?` operator. +//! +//! At some point you will likely stop propagating errors and decide what to do with the error. +//! Generally you can: +//! +//! 1. Try to recover from the error, or +//! 2. Report the error to the user and either +//! 1. Terminate program execution (See [`fatal`][`fatal`]), or +//! 2. Continue program execution (See [`non_fatal`][`non_fatal`]) +//! +//! +//! ## Handling errors +//! +//! Ideally, when the program encounters an error it will try to recover as best as it can. This +//! can mean falling back to some sane default if a specific value (e.g. an environment variable) +//! cannot be found. Note that this isn't always applicable. If in doubt, don't hesitate to ask. +//! +//! Recovery usually isn't an option if an operation has changed the internal state (i.e. the value +//! or content of specific variables) of objects in the code. In this case, if an error is +//! encountered, it is best to declare the program state corrupted and terminate the whole +//! application. This can be done by [`unwrap`]ing on the [`Result`] type. Always try to propagate +//! the error as best as you can and attach meaningful context before [`unwrap`]ing. This gives the +//! user an idea what went wrong and can also help developers in quickly identifying which parts of +//! the code to debug if necessary. +//! +//! When you encounter such a fatal error and cannot propagate it further up (e.g. because the +//! current function cannot be changed to return a [`Result`], or because it is the "root" function +//! of a program thread), use the [`fatal`][`fatal`] function to panic the application. It will +//! attach some small context to the error and finally [`unwrap`] it. Using this function over the +//! regular [`unwrap`] has the added benefit that other developers seeing this in the code know +//! that someone has previously spent some thought about error handling at this location. +//! +//! If you encounter a non-fatal error, use the [`non_fatal`][`non_fatal`] function to handle +//! it. Instead of [`panic`]ing the application, the error is written to the application log and +//! execution continues. Please use this sparingly, as an error usually calls for actions to be +//! taken rather than ignoring it. +//! +//! +//! [`handle_panic`]: not_wasm::handle_panic +//! [`context`]: anyhow::Context +//! [`fatal`]: FatalError::fatal +//! [`non_fatal`]: FatalError::non_fatal + +use anyhow::Context; use colored::*; use log::error; use serde::{Deserialize, Serialize}; use std::fmt::{Display, Error, Formatter}; -use std::panic::PanicInfo; -use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme, Report}; +use miette::Diagnostic; use thiserror::Error as ThisError; -/// The maximum amount of calls an [`ErrorContext`] will keep track -/// of in its stack representation. This is a per-thread maximum. -const MAX_THREAD_CALL_STACK: usize = 6; +/// Re-exports of common error-handling code. +pub mod prelude { + pub use super::FatalError; + pub use super::LoggableError; + pub use anyhow::anyhow; + pub use anyhow::bail; + pub use anyhow::Context; + pub use anyhow::Result; +} pub trait ErrorInstruction { fn error(err: String) -> Self; @@ -44,136 +131,106 @@ Also, if you want to see the backtrace, you can set the `RUST_BACKTRACE` environ } } -fn fmt_report(diag: Report) -> String { - let mut out = String::new(); - GraphicalReportHandler::new_themed(GraphicalTheme::unicode()) - .render_report(&mut out, diag.as_ref()) - .unwrap(); - out -} - -/// Custom panic handler/hook. Prints the [`ErrorContext`]. -pub fn handle_panic(info: &PanicInfo<'_>, sender: &SenderWithContext) -where - T: ErrorInstruction + Clone, -{ - use std::{process, thread}; - let thread = thread::current(); - let thread = thread.name().unwrap_or("unnamed"); - - let msg = match info.payload().downcast_ref::<&'static str>() { - Some(s) => Some(*s), - None => info.payload().downcast_ref::().map(|s| &**s), - } - .unwrap_or("An unexpected error occurred!"); - - let err_ctx = OPENCALLS.with(|ctx| *ctx.borrow()); - - let mut report: Report = Panic(format!("\u{1b}[0;31m{}\u{1b}[0;0m", msg)).into(); - - let mut location_string = String::new(); - if let Some(location) = info.location() { - location_string = format!( - "At {}:{}:{}", - location.file(), - location.line(), - location.column() - ); - report = report.wrap_err(location_string.clone()); +/// Helper trait to easily log error types. +/// +/// The `print_error` function takes a closure which takes a `&str` and fares with it as necessary +/// to log the error to some usable location. For convenience, logging to stdout, stderr and +/// `log::error!` is already implemented. +/// +/// Note that the trait functions pass the error through unmodified, so they can be chained with +/// the usual handling of [`std::result::Result`] types. +pub trait LoggableError: Sized { + /// Gives a formatted error message derived from `self` to the closure `fun` for + /// printing/logging as appropriate. + /// + /// # Examples + /// + /// ```should_panic + /// use anyhow; + /// use zellij_utils::errors::LoggableError; + /// + /// let my_err: anyhow::Result<&str> = Err(anyhow::anyhow!("Test error")); + /// my_err + /// .print_error(|msg| println!("{msg}")) + /// .unwrap(); + /// ``` + fn print_error(self, fun: F) -> Self; + + /// Convenienve function, calls `print_error` with the closure `|msg| log::error!("{}", msg)`. + fn to_log(self) -> Self { + self.print_error(|msg| log::error!("{}", msg)) } - if !err_ctx.is_empty() { - report = report.wrap_err(format!("{}", err_ctx)); + /// Convenienve function, calls `print_error` with the closure `|msg| eprintln!("{}", msg)`. + fn to_stderr(self) -> Self { + self.print_error(|msg| eprintln!("{}", msg)) } - report = report.wrap_err(format!( - "Thread '\u{1b}[0;31m{}\u{1b}[0;0m' panicked.", - thread - )); - - error!( - "{}", - format!( - "Panic occured: - thread: {} - location: {} - message: {}", - thread, location_string, msg - ) - ); - - if thread == "main" { - // here we only show the first line because the backtrace is not readable otherwise - // a better solution would be to escape raw mode before we do this, but it's not trivial - // to get os_input here - println!("\u{1b}[2J{}", fmt_report(report)); - process::exit(1); - } else { - let _ = sender.send(T::error(fmt_report(report))); + /// Convenienve function, calls `print_error` with the closure `|msg| println!("{}", msg)`. + fn to_stdout(self) -> Self { + self.print_error(|msg| println!("{}", msg)) } } -pub fn get_current_ctx() -> ErrorContext { - ASYNCOPENCALLS - .try_with(|ctx| *ctx.borrow()) - .unwrap_or_else(|_| OPENCALLS.with(|ctx| *ctx.borrow())) -} - -/// A representation of the call stack. -#[derive(Clone, Copy, Serialize, Deserialize, Debug)] -pub struct ErrorContext { - calls: [ContextType; MAX_THREAD_CALL_STACK], -} - -impl ErrorContext { - /// Returns a new, blank [`ErrorContext`] containing only [`Empty`](ContextType::Empty) - /// calls. - pub fn new() -> Self { - Self { - calls: [ContextType::Empty; MAX_THREAD_CALL_STACK], - } - } - - /// Returns `true` if the calls has all [`Empty`](ContextType::Empty) calls. - pub fn is_empty(&self) -> bool { - self.calls.iter().all(|c| c == &ContextType::Empty) - } - - /// Adds a call to this [`ErrorContext`]'s call stack representation. - pub fn add_call(&mut self, call: ContextType) { - for ctx in &mut self.calls { - if let ContextType::Empty = ctx { - *ctx = call; - break; +impl LoggableError for anyhow::Result { + fn print_error(self, fun: F) -> Self { + if let Err(ref err) = self { + let mut msg = format!("ERROR: {}", err); + for cause in err.chain().skip(1) { + msg = format!("{msg}\nbecause: {cause}"); } + fun(&msg); } - self.update_thread_ctx() + self } +} - /// Updates the thread local [`ErrorContext`]. - pub fn update_thread_ctx(&self) { - ASYNCOPENCALLS - .try_with(|ctx| *ctx.borrow_mut() = *self) - .unwrap_or_else(|_| OPENCALLS.with(|ctx| *ctx.borrow_mut() = *self)); - } +/// Special trait to mark fatal/non-fatal errors. +/// +/// This works in tandem with `LoggableError` above and is meant to make reading code easier with +/// regard to whether an error is fatal or not (i.e. can be ignored, or at least doesn't make the +/// application crash). +/// +/// This essentially degrades any `std::result::Result<(), _>` to a simple `()`. +pub trait FatalError { + /// Mark results as being non-fatal. + /// + /// If the result is an `Err` variant, this will [print the error to the log][`to_log`]. + /// Discards the result type afterwards. + /// + /// [`to_log`]: LoggableError::to_log + fn non_fatal(self); + + /// Mark results as being fatal. + /// + /// If the result is an `Err` variant, this will unwrap the error and panic the application. + /// If the result is an `Ok` variant, the inner value is unwrapped and returned instead. + /// + /// # Panics + /// + /// If the given result is an `Err` variant. + #[track_caller] + fn fatal(self) -> T; } -impl Default for ErrorContext { - fn default() -> Self { - Self::new() +/// Helper function to silence `#[warn(unused_must_use)]` cargo warnings. Used exclusively in +/// `FatalError::non_fatal`! +fn discard_result(_arg: anyhow::Result) {} + +impl FatalError for anyhow::Result { + fn non_fatal(self) { + if self.is_err() { + discard_result(self.context("a non-fatal error occured").to_log()); + } } -} -impl Display for ErrorCont