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

CHANGELOG.md

@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * feat: allow defining tab cwd in layouts (https://github.com/zellij-org/zellij/pull/1828)
 * debugging: Remove calls to `unwrap` from plugin WASM VM (https://github.com/zellij-org/zellij/pull/1827)
 * debugging: Improve error handling in `server/route` (https://github.com/zellij-org/zellij/pull/1808)
+* debugging: Detect plugin version mismatches (https://github.com/zellij-org/zellij/pull/1838)
 
 ## [0.31.4] - 2022-09-09
 * Terminal compatibility: improve vttest compliance (https://github.com/zellij-org/zellij/pull/1671)

Cargo.lock

@@ -3405,6 +3405,7 @@ dependencies = [
  "highway",
  "insta",
  "log",
+ "semver",
  "serde_json",
  "sixel-image",
  "sixel-tokenizer",

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"

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 = match client_id {
-        let client_id = client_id.unwrap();
+            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(

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(

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,

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);

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.
+/// Custom error for plugin version mismatch.
-// The usual cause of an error in this call is that the plugin versions don't match the zellij
+///
-// version.
+/// This is thrown when, during starting a plugin, it is detected that the plugin version doesn't
-const PLUGINS_OUT_OF_DATE: &str =
+/// match the zellij version. This is treated as a fatal error and leads to instantaneous
-    "If you're seeing this error the most likely cause is that your plugin versions
+/// termination.
-don't match your current zellij version.
+#[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
+                        update.call(&[]).or_else::<anyError, _>(|e| {
-                            .call(&[])
+                            match e.downcast::<serde_json::Error>() {
-                            .with_context(err_context)
+                                Ok(_) => panic!(
-                            .context(PLUGINS_OUT_OF_DATE)?;
+                                    "{}",
+                                    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 {

zellij-tile/src/lib.rs

@@ -10,6 +10,18 @@ pub trait ZellijPlugin {
     fn render(&mut self, rows: usize, cols: usize) {}
 }
 
+pub const PLUGIN_MISMATCH: &str =
+    "An error occured in a plugin while receiving an Event from zellij. This means
+that the plugins aren't compatible with the current zellij version.
+
+The most likely explanation for this is that you're running either a
+self-compiled zellij or plugin version. Please make sure that, while developing,
+you also rebuild the plugins in order to pick up changes to the plugin code.
+
+Please refer to the documentation for further information:
+    https://github.com/zellij-org/zellij/blob/main/CONTRIBUTING.md#building
+";
+
 #[macro_export]
 macro_rules! register_plugin {
     ($t:ty) => {
@@ -18,6 +30,11 @@ macro_rules! register_plugin {
         }
 
         fn main() {
+            // Register custom panic handler
+            std::panic::set_hook(Box::new(|info| {
+                report_panic(info);
+            }));
+
             STATE.with(|state| {
                 state.borrow_mut().load();
             });
@@ -25,10 +42,13 @@ macro_rules! register_plugin {
 
         #[no_mangle]
         pub fn update() {
+            let object = $crate::shim::object_from_stdin()
+                .context($crate::PLUGIN_MISMATCH)
+                .to_stdout()
+                .unwrap();
+
             STATE.with(|state| {
-                state
+                state.borrow_mut().update(object);
-                    .borrow_mut()
-                    .update($crate::shim::object_from_stdin().unwrap());
             });
         }
 
@@ -38,5 +58,10 @@ macro_rules! register_plugin {
                 state.borrow_mut().render(rows as usize, cols as usize);
             });
         }
+
+        #[no_mangle]
+        pub fn plugin_version() {
+            println!("{}", $crate::prelude::VERSION);
+        }
     };
 }

zellij-tile/src/prelude.rs

@@ -1,4 +1,6 @@
 pub use crate::shim::*;
 pub use crate::*;
+pub use zellij_utils::consts::VERSION;
 pub use zellij_utils::data::*;
+pub use zellij_utils::errors::prelude::*;
 pub use zellij_utils::input::actions;

zellij-tile/src/shim.rs

@@ -1,6 +1,7 @@
 use serde::{de::DeserializeOwned, Serialize};
 use std::{io, path::Path};
 use zellij_utils::data::*;
+use zellij_utils::errors::prelude::*;
 
 // Subscription Handling
 
@@ -50,13 +51,22 @@ pub fn exec_cmd(cmd: &[&str]) {
     unsafe { host_exec_cmd() };
 }
 
+pub fn report_panic(info: &std::panic::PanicInfo) {
+    println!("");
+    println!("A panic occured in a plugin");
+    println!("{:#?}", info);
+    unsafe { host_report_panic() };
+}
+
 // Internal Functions
 
 #[doc(hidden)]
-pub fn object_from_stdin<T: DeserializeOwned>() -> Result<T, serde_json::Error> {
+pub fn object_from_stdin<T: DeserializeOwned>() -> Result<T> {
+    let err_context = || "failed to deserialize object from stdin".to_string();
+
     let mut json = String::new();
-    io::stdin().read_line(&mut json).unwrap();
+    io::stdin().read_line(&mut json).with_context(err_context)?;
-    serde_json::from_str(&json)
+    serde_json::from_str(&json).with_context(err_context)
 }
 
 #[doc(hidden)]
@@ -75,4 +85,5 @@ extern "C" {
     fn host_switch_tab_to(tab_idx: u32);
     fn host_set_timeout(secs: f64);
     fn host_exec_cmd();
+    fn host_report_panic();
 }

zellij-utils/src/errors.rs

@@ -27,6 +27,7 @@ pub mod prelude {
     pub use anyhow::anyhow;
     pub use anyhow::bail;
     pub use anyhow::Context;
+    pub use anyhow::Error as anyError;
     pub use anyhow::Result;
 }
 
@@ -111,11 +112,7 @@ pub trait LoggableError<T>: Sized {
 impl<T> LoggableError<T> for anyhow::Result<T> {
     fn print_error<F: Fn(&str)>(self, fun: F) -> Self {
         if let Err(ref err) = self {
-            let mut msg = format!("ERROR: {}", err);
+            fun(&format!("{:?}", err));
-            for cause in err.chain().skip(1) {
-                msg = format!("{msg}\nbecause: {cause}");
-            }
-            fun(&msg);
         }
         self
     }