Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugins: Improve error handling on plugin version mismatch #1838

Merged
merged 13 commits into from
Oct 23, 2022
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions zellij-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
27 changes: 18 additions & 9 deletions zellij-server/src/panes/plugin_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand All @@ -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
Expand All @@ -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(
Expand Down
11 changes: 8 additions & 3 deletions zellij-server/src/panes/terminal_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion zellij-server/src/tab/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions zellij-server/src/ui/pane_contents_and_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
114 changes: 100 additions & 14 deletions zellij-server/src/wasm_vm.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
}
};

Expand All @@ -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));
Expand Down Expand Up @@ -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))
}

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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 {
Expand Down
31 changes: 28 additions & 3 deletions zellij-tile/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -18,17 +30,25 @@ 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();
});
}

#[no_mangle]
pub fn update() {
let object = $crate::shim::object_from_stdin()
.context($crate::PLUGIN_MISMATCH)
.to_stdout()
.unwrap();

STATE.with(|state| {
state
.borrow_mut()
.update($crate::shim::object_from_stdin().unwrap());
state.borrow_mut().update(object);
});
}

Expand All @@ -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);
}
};
}
2 changes: 2 additions & 0 deletions zellij-tile/src/prelude.rs
Original file line number Diff line number Diff line change
@@ -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;
Loading