diff options
author | har7an <99636919+har7an@users.noreply.github.com> | 2022-10-23 13:14:24 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-23 13:14:24 +0000 |
commit | 75801bdb0e559c73d3f5ea99f1ce1429a425cd1b (patch) | |
tree | abcc1aa1f941f038ef8d328d6ffa79abfc2ff1a9 /zellij-server | |
parent | 788bcd6151431222325800e0a46c58384fe0bd22 (diff) |
plugins: Improve error handling on plugin version mismatch (#1838)
* server/tab: Don't panic in `Pane::render`
and do not crash the application on failure to receive a render update
from plugins any longer. Instead, will print a simple string with a hint
to check the application logs, where a more thorough error indication
can be found.
* utils/errors: re-export `anyhow::Error`
to create ad-hoc errors with custom error types, without having to wrap
them into a `context()` before to turn the into anyhow errors.
* plugins: Check plugin version on startup
and terminate execution with a descriptive error message in case the
plugin version is incompatible with the version of zellij being run.
* server/wasm_vm: Add plugin path in version error
so the user knows which plugin to look at in case they're using custom
plugins.
* server/wasm_vm: Check plugin version for equality
Previously we would accept cases where the plugin version was newer than
the zellij version, which doesn't make a lot of sense.
* server/wasm_vm: Prettier error handling
in call to `wasmer::Function::call` in case a plugin version mismatch
can occur.
* tile: Install custom panic handler
that will print the panic message to a plugins stdout and then call a
panic handler on the host that turns it into a real application-level
panic.
* tile: Catch errors in event deserialization
and turn them into proper panics. These errors are symptomatic of an
uncaught plugin version mismatch, for example when developing from main
and compiling zellij/the plugins from source. Normal users should never
get to see this error.
* utils/errors: Improve output in `to_stdout`
for anyhow errors. The default anyhow error formatting of `{:?}` is
already very good, and we just made it worse by trying to invent our own
formatting.
* tile: Reword plugin mismatch error message
* zellij: Apply rustfmt
* changelog: Add PR #1838
Improve error handling on plugin version mismatch.
* server/wasm_vm: Rephrase error in passive voice
Diffstat (limited to 'zellij-server')
-rw-r--r-- | zellij-server/Cargo.toml | 1 | ||||
-rw-r--r-- | zellij-server/src/panes/plugin_pane.rs | 27 | ||||
-rw-r--r-- | zellij-server/src/panes/terminal_pane.rs | 11 | ||||
-rw-r--r-- | zellij-server/src/tab/mod.rs | 2 | ||||
-rw-r--r-- | zellij-server/src/ui/pane_contents_and_ui.rs | 5 | ||||
-rw-r--r-- | zellij-server/src/wasm_vm.rs | 114 |
6 files changed, 131 insertions, 29 deletions
diff --git a/zellij-server/Cargo.toml b/zellij-server/Cargo.toml index 5b8035a89..bf6d0cd25 100644 --- a/zellij-server/Cargo.toml +++ b/zellij-server/Cargo.toml @@ -31,6 +31,7 @@ sixel-tokenizer = "0.1.0" sixel-image = "0.1.0" arrayvec = "0.7.2" uuid = { version = "0.8.2", features = ["serde", "v4"] } +semver = "0.11.0" [dev-dependencies] insta = "1.6.0" diff --git a/zellij-server/src/panes/plugin_pane.rs b/zellij-server/src/panes/plugin_pane.rs index e872d614a..5709d7fee 100644 --- a/zellij-server/src/panes/plugin_pane.rs +++ b/zellij-server/src/panes/plugin_pane.rs @@ -15,6 +15,7 @@ use zellij_utils::shared::ansi_len; use zellij_utils::{ channels::SenderWithContext, data::{Event, InputMode, Mouse, PaletteColor}, + errors::prelude::*, pane_size::{Dimension, PaneGeom}, shared::make_terminal_title, }; @@ -137,12 +138,16 @@ impl Pane for PluginPane { fn render( &mut self, client_id: Option<ClientId>, - ) -> Option<(Vec<CharacterChunk>, Option<String>, Vec<SixelImageChunk>)> { + ) -> Result<Option<(Vec<CharacterChunk>, Option<String>, Vec<SixelImageChunk>)>> { // this is a bit of a hack but works in a pinch - client_id?; - let client_id = client_id.unwrap(); + let client_id = match client_id { + Some(id) => id, + None => return Ok(None), + }; // if self.should_render { if true { + let err_context = || format!("failed to render plugin panes for client {client_id}"); + // while checking should_render rather than rendering each pane every time // is more performant, it causes some problems when the pane to the left should be // rendered and has wide characters (eg. Chinese characters or emoji) @@ -158,12 +163,16 @@ impl Pane for PluginPane { self.get_content_rows(), self.get_content_columns(), )) - .unwrap(); + .to_anyhow() + .with_context(err_context)?; self.should_render = false; + // This is where we receive the text to render from the plugins. let contents = buf_rx .recv() - .expect("Failed to receive reply from plugin. Please check the logs"); + .with_context(err_context) + .to_log() + .unwrap_or("No output from plugin received. See logs".to_string()); for (index, line) in contents.lines().enumerate() { let actual_len = ansi_len(line); let line_to_print = if actual_len > self.get_content_columns() { @@ -185,7 +194,7 @@ impl Pane for PluginPane { self.get_content_x() + 1, line_to_print, ) - .unwrap(); // goto row/col and reset styles + .with_context(err_context)?; // goto row/col and reset styles let line_len = line_to_print.len(); if line_len < self.get_content_columns() { // pad line @@ -206,15 +215,15 @@ impl Pane for PluginPane { y + line_index + 1, x + 1 ) - .unwrap(); // goto row/col and reset styles + .with_context(err_context)?; // goto row/col and reset styles for _col_index in 0..self.get_content_columns() { vte_output.push(' '); } } } - Some((vec![], Some(vte_output), vec![])) // TODO: PluginPanes should have their own grid so that we can return the non-serialized TerminalCharacters and have them participate in the render buffer + Ok(Some((vec![], Some(vte_output), vec![]))) // TODO: PluginPanes should have their own grid so that we can return the non-serialized TerminalCharacters and have them participate in the render buffer } else { - None + Ok(None) } } fn render_frame( diff --git a/zellij-server/src/panes/terminal_pane.rs b/zellij-server/src/panes/terminal_pane.rs index 766cfaa2b..8d0651ad2 100644 --- a/zellij-server/src/panes/terminal_pane.rs +++ b/zellij-server/src/panes/terminal_pane.rs @@ -17,6 +17,7 @@ use zellij_utils::input::command::RunCommand; use zellij_utils::pane_size::Offset; use zellij_utils::{ data::{InputMode, Palette, PaletteColor, Style}, + errors::prelude::*, pane_size::SizeInPixels, pane_size::{Dimension, PaneGeom}, position::Position, @@ -272,7 +273,7 @@ impl Pane for TerminalPane { fn render( &mut self, _client_id: Option<ClientId>, - ) -> Option<(Vec<CharacterChunk>, Option<String>, Vec<SixelImageChunk>)> { + ) -> Result<Option<(Vec<CharacterChunk>, Option<String>, Vec<SixelImageChunk>)>> { if self.should_render() { let mut raw_vte_output = String::new(); let content_x = self.get_content_x(); @@ -332,9 +333,13 @@ impl Pane for TerminalPane { self.grid.ring_bell = false; } self.set_should_render(false); - Some((character_chunks, Some(raw_vte_output), sixel_image_chunks)) + Ok(Some(( + character_chunks, + Some(raw_vte_output), + sixel_image_chunks, + ))) } else { - None + Ok(None) } } fn render_frame( diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs index da2fcda60..c63cac89c 100644 --- a/zellij-server/src/tab/mod.rs +++ b/zellij-server/src/tab/mod.rs @@ -142,7 +142,7 @@ pub trait Pane { fn render( &mut self, client_id: Option<ClientId>, - ) -> Option<(Vec<CharacterChunk>, Option<String>, Vec<SixelImageChunk>)>; // TODO: better + ) -> Result<Option<(Vec<CharacterChunk>, Option<String>, Vec<SixelImageChunk>)>>; // TODO: better fn render_frame( &mut self, client_id: ClientId, diff --git a/zellij-server/src/ui/pane_contents_and_ui.rs b/zellij-server/src/ui/pane_contents_and_ui.rs index 779a69094..4db9feb4b 100644 --- a/zellij-server/src/ui/pane_contents_and_ui.rs +++ b/zellij-server/src/ui/pane_contents_and_ui.rs @@ -45,7 +45,8 @@ impl<'a> PaneContentsAndUi<'a> { &mut self, clients: impl Iterator<Item = ClientId>, ) { - if let Some((character_chunks, raw_vte_output, sixel_image_chunks)) = self.pane.render(None) + if let Some((character_chunks, raw_vte_output, sixel_image_chunks)) = + self.pane.render(None).unwrap() { let clients: Vec<ClientId> = clients.collect(); self.output.add_character_chunks_to_multiple_clients( @@ -73,7 +74,7 @@ impl<'a> PaneContentsAndUi<'a> { } pub fn render_pane_contents_for_client(&mut self, client_id: ClientId) { if let Some((character_chunks, raw_vte_output, sixel_image_chunks)) = - self.pane.render(Some(client_id)) + self.pane.render(Some(client_id)).unwrap() { self.output .add_character_chunks_to_client(client_id, character_chunks, self.z_index); diff --git a/zellij-server/src/wasm_vm.rs b/zellij-server/src/wasm_vm.rs index 6a013a277..4fea6adb0 100644 --- a/zellij-server/src/wasm_vm.rs +++ b/zellij-server/src/wasm_vm.rs @@ -1,9 +1,10 @@ use highway::{HighwayHash, PortableHash}; use log::{debug, info, warn}; +use semver::Version; use serde::{de::DeserializeOwned, Serialize}; use std::{ collections::{HashMap, HashSet}, - fs, + fmt, fs, path::{Path, PathBuf}, process, str::FromStr, @@ -39,12 +40,40 @@ use zellij_utils::{ serde, }; -// String to add as error context when we fail to call the `load` function on a plugin. -// The usual cause of an error in this call is that the plugin versions don't match the zellij -// version. -const PLUGINS_OUT_OF_DATE: &str = - "If you're seeing this error the most likely cause is that your plugin versions -don't match your current zellij version. +/// Custom error for plugin version mismatch. +/// +/// This is thrown when, during starting a plugin, it is detected that the plugin version doesn't +/// match the zellij version. This is treated as a fatal error and leads to instantaneous +/// termination. +#[derive(Debug)] +pub struct VersionMismatchError { + zellij_version: String, + plugin_version: String, + plugin_path: PathBuf, +} + +impl std::error::Error for VersionMismatchError {} + +impl VersionMismatchError { + pub fn new(zellij_version: &str, plugin_version: &str, plugin_path: &PathBuf) -> Self { + VersionMismatchError { + zellij_version: zellij_version.to_owned(), + plugin_version: plugin_version.to_owned(), + plugin_path: plugin_path.to_owned(), + } + } +} + +impl fmt::Display for VersionMismatchError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "If you're seeing this error the plugin versions don't match the current +zellij version. Detected versions: + +- Plugin version: {} +- Zellij version: {} +- Offending plugin: {} If you're a user: Please contact the distributor of your zellij version and report this error @@ -57,7 +86,13 @@ If you're a developer: A possible fix for this error is to remove all contents of the 'PLUGIN DIR' folder from the output of the `zellij setup --check` command. -"; +", + self.plugin_version, + self.zellij_version, + self.plugin_path.display() + ) + } +} #[derive(Clone, Debug)] pub(crate) enum PluginInstruction { @@ -161,9 +196,9 @@ pub(crate) fn wasm_thread_main( PluginInstruction::Update(pid, cid, event) => { let err_context = || { if *DEBUG_MODE.get().unwrap_or(&true) { - format!("failed to update plugin with event: {event:#?}") + format!("failed to update plugin state with event: {event:#?}") } else { - "failed to update plugin".to_string() + "failed to update plugin state".to_string() } }; @@ -187,10 +222,19 @@ pub(crate) fn wasm_thread_main( .get_function("update") .with_context(err_context)?; wasi_write_object(&plugin_env.wasi_env, &event); - update - .call(&[]) - .with_context(err_context) - .context(PLUGINS_OUT_OF_DATE)?; + update.call(&[]).or_else::<anyError, _>(|e| { + match e.downcast::<serde_json::Error>() { + Ok(_) => panic!( + "{}", + anyError::new(VersionMismatchError::new( + VERSION, + "Unavailable", + &plugin_env.plugin.path + )) + ), + Err(e) => Err(e).with_context(err_context), + } + })?; } } drop(bus.senders.send_to_screen(ScreenInstruction::Render)); @@ -386,6 +430,37 @@ fn start_plugin( let zellij = zellij_exports(store, &plugin_env); let instance = Instance::new(&module, &zellij.chain_back(wasi)).with_context(err_context)?; + // Check plugin version + let plugin_version_func = match instance.exports.get_function("plugin_version") { + Ok(val) => val, + Err(_) => panic!( + "{}", + anyError::new(VersionMismatchError::new( + VERSION, + "Unavailable", + &plugin_env.plugin.path + )) + ), + }; + plugin_version_func.call(&[]).with_context(err_context)?; + let plugin_version_str = wasi_read_string(&plugin_env.wasi_env); + let plugin_version = Version::parse(&plugin_version_str) + .context("failed to parse plugin version") + .with_context(err_context)?; + let zellij_version = Version::parse(VERSION) + .context("failed to parse zellij version") + .with_context(err_context)?; + if plugin_version != zellij_version { + panic!( + "{}", + anyError::new(VersionMismatchError::new( + VERSION, + &plugin_version_str, + &plugin_env.plugin.path + )) + ); + } + Ok((instance, plugin_env)) } @@ -425,6 +500,7 @@ pub(crate) fn zellij_exports(store: &Store, plugin_env: &PluginEnv) -> ImportObj host_switch_tab_to, host_set_timeout, host_exec_cmd, + host_report_panic, } } @@ -546,6 +622,16 @@ fn host_exec_cmd(plugin_env: &PluginEnv) { .unwrap(); } +// Custom panic handler for plugins. +// +// This is called when a panic occurs in a plugin. Since most panics will likely originate in the +// code trying to deserialize an `Event` upon a plugin state update, we read some panic message, +// formatted as string from the plugin. +fn host_report_panic(plugin_env: &PluginEnv) { + let msg = wasi_read_string(&plugin_env.wasi_env); + panic!("{}", msg); +} + // Helper Functions --------------------------------------------------------------------------------------------------- pub fn wasi_read_string(wasi_env: &WasiEnv) -> String { |