summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorhar7an <99636919+har7an@users.noreply.github.com>2023-01-14 05:14:17 +0000
committerGitHub <noreply@github.com>2023-01-14 05:14:17 +0000
commit04b294aabb1ac31ba385f6152c6f4c709bf85097 (patch)
tree8226d753fd8b4d4f5ca631cd5b83401df6b00abc
parent1e02754e1993be4fa982a96b60c82abdd4676e58 (diff)
Errors: less unwrap in server (#2069)
* server/pty: Remove last `unwrap` * server/route: Remove calls to `unwrap` * server/screen: Remove calls to `unwrap` * WIP: server/plugins: Remove calls to unwrap * server/route: Apply rustfmt * server/plugins: Remove last `unwrap`s * server/screen: update tabs before rendering which was previously accidentally changed. * server/tab: Remove calls to `unwrap` * server/plugins: Add context to plugin panic reporter * changelog: Add PR #2069
-rw-r--r--CHANGELOG.md1
-rw-r--r--zellij-server/src/plugins/wasm_bridge.rs239
-rw-r--r--zellij-server/src/pty.rs8
-rw-r--r--zellij-server/src/route.rs45
-rw-r--r--zellij-server/src/screen.rs92
-rw-r--r--zellij-server/src/tab/layout_applier.rs22
-rw-r--r--zellij-server/src/tab/mod.rs110
-rw-r--r--zellij-server/src/tab/unit/tab_integration_tests.rs34
-rw-r--r--zellij-server/src/tab/unit/tab_tests.rs362
9 files changed, 563 insertions, 350 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d12a27d41..b0b39a6b7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
* fix: copy_on_select = false sticky selection (https://github.com/zellij-org/zellij/pull/2086)
* fix: do not drop wide chars when resizing to width of 1 column (https://github.com/zellij-org/zellij/pull/2082)
* fix: disallow path-like names for sessions (https://github.com/zellij-org/zellij/pull/2082)
+* errors: Remove more `unwrwap`s from server code (https://github.com/zellij-org/zellij/pull/2069)
## [0.34.4] - 2022-12-13
diff --git a/zellij-server/src/plugins/wasm_bridge.rs b/zellij-server/src/plugins/wasm_bridge.rs
index e3a4c5828..abb405ba1 100644
--- a/zellij-server/src/plugins/wasm_bridge.rs
+++ b/zellij-server/src/plugins/wasm_bridge.rs
@@ -122,6 +122,17 @@ pub struct PluginEnv {
plugin_own_data_dir: PathBuf,
}
+impl PluginEnv {
+ // Get the name (path) of the containing plugin
+ pub fn name(&self) -> String {
+ format!(
+ "{} (ID {})",
+ self.plugin.path.display().to_string(),
+ self.plugin_id
+ )
+ }
+}
+
type PluginMap = HashMap<(u32, ClientId), (Instance, PluginEnv, (usize, usize))>; // u32 =>
// plugin_id,
// (usize, usize)
@@ -407,18 +418,21 @@ impl WasmBridge {
*current_columns = new_columns;
// TODO: consolidate with above render function
- let render = instance
+ let rendered_bytes = instance
.exports
.get_function("render")
+ .map_err(anyError::new)
+ .and_then(|render| {
+ render
+ .call(&[
+ Value::I32(*current_rows as i32),
+ Value::I32(*current_columns as i32),
+ ])
+ .map_err(anyError::new)
+ })
+ .and_then(|_| wasi_read_string(&plugin_env.wasi_env))
.with_context(err_context)?;
- render
- .call(&[
- Value::I32(*current_rows as i32),
- Value::I32(*current_columns as i32),
- ])
- .with_context(err_context)?;
- let rendered_bytes = wasi_read_string(&plugin_env.wasi_env);
plugin_bytes.push((*plugin_id, *client_id, rendered_bytes.as_bytes().to_vec()));
}
}
@@ -462,7 +476,7 @@ impl WasmBridge {
.exports
.get_function("update")
.with_context(err_context)?;
- wasi_write_object(&plugin_env.wasi_env, &event);
+ wasi_write_object(&plugin_env.wasi_env, &event).with_context(err_context)?;
let update_return = update.call(&[]).or_else::<anyError, _>(|e| {
match e.downcast::<serde_json::Error>() {
Ok(_) => panic!(
@@ -483,14 +497,17 @@ impl WasmBridge {
};
if *rows > 0 && *columns > 0 && should_render {
- let render = instance
+ let rendered_bytes = instance
.exports
.get_function("render")
+ .map_err(anyError::new)
+ .and_then(|render| {
+ render
+ .call(&[Value::I32(*rows as i32), Value::I32(*columns as i32)])
+ .map_err(anyError::new)
+ })
+ .and_then(|_| wasi_read_string(&plugin_env.wasi_env))
.with_context(err_context)?;
- render
- .call(&[Value::I32(*rows as i32), Value::I32(*columns as i32)])
- .with_context(err_context)?;
- let rendered_bytes = wasi_read_string(&plugin_env.wasi_env);
plugin_bytes.push((
plugin_id,
client_id,
@@ -531,10 +548,12 @@ fn assert_plugin_version(instance: &Instance, plugin_env: &PluginEnv) -> Result<
)))
},
};
- 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")
+
+ let plugin_version = plugin_version_func
+ .call(&[])
+ .map_err(anyError::new)
+ .and_then(|_| wasi_read_string(&plugin_env.wasi_env))
+ .and_then(|string| Version::parse(&string).context("failed to parse plugin version"))
.with_context(err_context)?;
let zellij_version = Version::parse(VERSION)
.context("failed to parse zellij version")
@@ -542,7 +561,7 @@ fn assert_plugin_version(instance: &Instance, plugin_env: &PluginEnv) -> Result<
if plugin_version != zellij_version {
return Err(anyError::new(VersionMismatchError::new(
VERSION,
- &plugin_version_str,
+ &plugin_version.to_string(),
&plugin_env.plugin.path,
plugin_env.plugin.is_builtin(),
)));
@@ -590,15 +609,27 @@ pub(crate) fn zellij_exports(store: &Store, plugin_env: &PluginEnv) -> ImportObj
}
fn host_subscribe(plugin_env: &PluginEnv) {
- let mut subscriptions = plugin_env.subscriptions.lock().unwrap();
- let new: HashSet<EventType> = wasi_read_object(&plugin_env.wasi_env);
- subscriptions.extend(new);
+ wasi_read_object::<HashSet<EventType>>(&plugin_env.wasi_env)
+ .and_then(|new| {
+ plugin_env.subscriptions.lock().to_anyhow()?.extend(new);
+ Ok(())
+ })
+ .with_context(|| format!("failed to subscribe for plugin {}", plugin_env.name()))
+ .fatal();
}
fn host_unsubscribe(plugin_env: &PluginEnv) {
- let mut subscriptions = plugin_env.subscriptions.lock().unwrap();
- let old: HashSet<EventType> = wasi_read_object(&plugin_env.wasi_env);
- subscriptions.retain(|k| !old.contains(k));
+ wasi_read_object::<HashSet<EventType>>(&plugin_env.wasi_env)
+ .and_then(|old| {
+ plugin_env
+ .subscriptions
+ .lock()
+ .to_anyhow()?
+ .retain(|k| !old.contains(k));
+ Ok(())
+ })
+ .with_context(|| format!("failed to unsubscribe for plugin {}", plugin_env.name()))
+ .fatal();
}
fn host_set_selectable(plugin_env: &PluginEnv, selectable: i32) {
@@ -612,7 +643,14 @@ fn host_set_selectable(plugin_env: &PluginEnv, selectable: i32) {
selectable,
tab_index,
))
- .unwrap()
+ .with_context(|| {
+ format!(
+ "failed to set plugin {} selectable from plugin {}",
+ selectable,
+ plugin_env.name()
+ )
+ })
+ .non_fatal();
},
_ => {
debug!(
@@ -628,24 +666,46 @@ fn host_get_plugin_ids(plugin_env: &PluginEnv) {
plugin_id: plugin_env.plugin_id,
zellij_pid: process::id(),
};
- wasi_write_object(&plugin_env.wasi_env, &ids);
+ wasi_write_object(&plugin_env.wasi_env, &ids)
+ .with_context(|| {
+ format!(
+ "failed to query plugin IDs from host for plugin {}",
+ plugin_env.name()
+ )
+ })
+ .non_fatal();
}
fn host_get_zellij_version(plugin_env: &PluginEnv) {
- wasi_write_object(&plugin_env.wasi_env, VERSION);
+ wasi_write_object(&plugin_env.wasi_env, VERSION)
+ .with_context(|| {
+ format!(
+ "failed to request zellij version from host for plugin {}",
+ plugin_env.name()
+ )
+ })
+ .non_fatal();
}
fn host_open_file(plugin_env: &PluginEnv) {
- let path: PathBuf = wasi_read_object(&plugin_env.wasi_env);
- plugin_env
- .senders
- .send_to_pty(PtyInstruction::SpawnTerminal(
- Some(TerminalAction::OpenFile(path, None)),
- None,
- None,
- ClientOrTabIndex::TabIndex(plugin_env.tab_index),
- ))
- .unwrap();
+ wasi_read_object::<PathBuf>(&plugin_env.wasi_env)
+ .and_then(|path| {
+ plugin_env
+ .senders
+ .send_to_pty(PtyInstruction::SpawnTerminal(
+ Some(TerminalAction::OpenFile(path, None)),
+ None,
+ None,
+ ClientOrTabIndex::TabIndex(plugin_env.tab_index),
+ ))
+ })
+ .with_context(|| {
+ format!(
+ "failed to open file on host from plugin {}",
+ plugin_env.name()
+ )
+ })
+ .non_fatal();
}
fn host_switch_tab_to(plugin_env: &PluginEnv, tab_idx: u32) {
@@ -655,7 +715,13 @@ fn host_switch_tab_to(plugin_env: &PluginEnv, tab_idx: u32) {
tab_idx,
Some(plugin_env.client_id),
))
- .unwrap();
+ .with_context(|| {
+ format!(
+ "failed to switch host to tab {tab_idx} from plugin {}",
+ plugin_env.name()
+ )
+ })
+ .non_fatal();
}
fn host_set_timeout(plugin_env: &PluginEnv, secs: f64) {
@@ -671,6 +737,7 @@ fn host_set_timeout(plugin_env: &PluginEnv, secs: f64) {
let send_plugin_instructions = plugin_env.senders.to_plugin.clone();
let update_target = Some(plugin_env.plugin_id);
let client_id = plugin_env.client_id;
+ let plugin_name = plugin_env.name();
thread::spawn(move || {
let start_time = Instant::now();
thread::sleep(Duration::from_secs_f64(secs));
@@ -679,18 +746,37 @@ fn host_set_timeout(plugin_env: &PluginEnv, secs: f64) {
let elapsed_time = Instant::now().duration_since(start_time).as_secs_f64();
send_plugin_instructions
- .unwrap()
- .send(PluginInstruction::Update(vec![(
- update_target,
- Some(client_id),
- Event::Timer(elapsed_time),
- )]))
- .unwrap();
+ .ok_or(anyhow!("found no sender to send plugin instruction to"))
+ .and_then(|sender| {
+ sender
+ .send(PluginInstruction::Update(vec![(
+ update_target,
+ Some(client_id),
+ Event::Timer(elapsed_time),
+ )]))
+ .to_anyhow()
+ })
+ .with_context(|| {
+ format!(
+ "failed to set host timeout of {secs} s for plugin {}",
+ plugin_name
+ )
+ })
+ .non_fatal();
});
}
fn host_exec_cmd(plugin_env: &PluginEnv) {
- let mut cmdline: Vec<String> = wasi_read_object(&plugin_env.wasi_env);
+ let err_context = || {
+ format!(
+ "failed to execute command on host for plugin '{}'",
+ plugin_env.name()
+ )
+ };
+
+ let mut cmdline: Vec<String> = wasi_read_object(&plugin_env.wasi_env)
+ .with_context(err_context)
+ .fatal();
let command = cmdline.remove(0);
// Bail out if we're forbidden to run command
@@ -704,7 +790,8 @@ fn host_exec_cmd(plugin_env: &PluginEnv) {
process::Command::new(command)
.args(cmdline)
.spawn()
- .unwrap();
+ .with_context(err_context)
+ .non_fatal();
}
// Custom panic handler for plugins.
@@ -713,32 +800,58 @@ fn host_exec_cmd(plugin_env: &PluginEnv) {
// 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);
+ let msg = wasi_read_string(&plugin_env.wasi_env)
+ .with_context(|| format!("failed to report panic for plugin '{}'", plugin_env.name()))
+ .fatal();
panic!("{}", msg);
}
// Helper Functions ---------------------------------------------------------------------------------------------------
-pub fn wasi_read_string(wasi_env: &WasiEnv) -> String {
- let mut state = wasi_env.state();
- let wasi_file = state.fs.stdout_mut().unwrap().as_mut().unwrap();
+pub fn wasi_read_string(wasi_env: &WasiEnv) -> Result<String> {
+ let err_context = || format!("failed to read string from WASI env '{wasi_env:?}'");
+
let mut buf = String::new();
- wasi_file.read_to_string(&mut buf).unwrap();
+ wasi_env
+ .state()
+ .fs
+ .stdout_mut()
+ .map_err(anyError::new)
+ .and_then(|stdout| {
+ stdout
+ .as_mut()
+ .ok_or(anyhow!("failed to get mutable reference to stdout"))
+ })
+ .and_then(|wasi_file| wasi_file.read_to_string(&mut buf).map_err(anyError::new))
+ .with_context(err_context)?;
// https://stackoverflow.com/questions/66450942/in-rust-is-there-a-way-to-make-literal-newlines-in-r-using-windows-c
- buf.replace("\n", "\n\r")
+ Ok(buf.replace("\n", "\n\r"))
}
-pub fn wasi_write_string(wasi_env: &WasiEnv, buf: &str) {
- let mut state = wasi_env.state();
- let wasi_file = state.fs.stdin_mut().unwrap().as_mut().unwrap();
- writeln!(wasi_file, "{}\r", buf).unwrap();
+pub fn wasi_write_string(wasi_env: &WasiEnv, buf: &str) -> Result<()> {
+ wasi_env
+ .state()
+ .fs
+ .stdin_mut()
+ .map_err(anyError::new)
+ .and_then(|stdin| {
+ stdin
+ .as_mut()
+ .ok_or(anyhow!("failed to get mutable reference to stdin"))
+ })
+ .and_then(|stdin| writeln!(stdin, "{}\r", buf).map_err(anyError::new))
+ .with_context(|| format!("failed to write string to WASI env '{wasi_env:?}'"))
}
-pub fn wasi_write_object(wasi_env: &WasiEnv, object: &(impl Serialize + ?Sized)) {
- wasi_write_string(wasi_env, &serde_json::to_string(&object).unwrap());
+pub fn wasi_write_object(wasi_env: &WasiEnv, object: &(impl Serialize + ?Sized)) -> Result<()> {
+ serde_json::to_string(&object)
+ .map_err(anyError::new)
+ .and_then(|string| wasi_write_string(wasi_env, &string))
+ .with_context(|| format!("failed to serialize object for WASI env '{wasi_env:?}'"))
}
-pub fn wasi_read_object<T: DeserializeOwned>(wasi_env: &WasiEnv) -> T {
- let json = wasi_read_string(wasi_env);
- serde_json::from_str(&json).unwrap()
+pub fn wasi_read_object<T: DeserializeOwned>(wasi_env: &WasiEnv) -> Result<T> {
+ wasi_read_string(wasi_env)
+ .and_then(|string| serde_json::from_str(&string).map_err(anyError::new))
+ .with_context(|| format!("failed to deserialize object from WASI env '{wasi_env:?}'"))
}
diff --git a/zellij-server/src/pty.rs b/zellij-server/src/pty.rs
index 04e3391f3..f1b0735a2 100644
--- a/zellij-server/src/pty.rs
+++ b/zellij-server/src/pty.rs
@@ -513,7 +513,13 @@ impl Pty {
if hold_on_start {
// we don't actually open a terminal in this case, just wait for the user to run it
let starts_held = hold_on_start;
- let terminal_id = self.bus.os_input.as_mut().unwrap().reserve_terminal_id()?;
+ let terminal_id = self
+ .bus
+ .os_input
+ .as_mut()
+ .context("couldn't get mutable reference to OS interface")
+ .and_then(|os_input| os_input.reserve_terminal_id())
+ .with_context(err_context)?;
return Ok((terminal_id, starts_held));
}
diff --git a/zellij-server/src/route.rs b/zellij-server/src/route.rs
index ea5f66932..5b5e437ee 100644
--- a/zellij-server/src/route.rs
+++ b/zellij-server/src/route.rs
@@ -618,14 +618,13 @@ pub(crate) fn route_action(
macro_rules! send_to_screen_or_retry_queue {
($rlocked_sessions:expr, $message:expr, $instruction: expr, $retry_queue:expr) => {{
match $rlocked_sessions.as_ref() {
- Some(session_metadata) => {
- session_metadata.senders.send_to_screen($message).unwrap();
- },
+ Some(session_metadata) => session_metadata.senders.send_to_screen($message),
None => {
log::warn!("Server not ready, trying to place instruction in retry queue...");
if let Some(retry_queue) = $retry_queue.as_mut() {
retry_queue.push($instruction);
}
+ Ok(())
},
}
}};
@@ -645,7 +644,7 @@ pub(crate) fn route_thread_main(
match receiver.recv() {
Some((instruction, err_ctx)) => {
err_ctx.update_thread_ctx();
- let rlocked_sessions = session_data.read().unwrap();
+ let rlocked_sessions = session_data.read().to_anyhow().with_context(err_context)?;
let handle_instruction = |instruction: ClientToServerMsg,
mut retry_queue: Option<&mut Vec<ClientToServerMsg>>|
-> Result<bool> {
@@ -679,18 +678,24 @@ pub(crate) fn route_thread_main(
ClientToServerMsg::TerminalResize(new_size) => {
session_state
.write()
- .unwrap()
+ .to_anyhow()
+ .with_context(err_context)?
.set_client_size(client_id, new_size);
- let min_size = session_state
+ session_state
.read()
- .unwrap()
- .min_client_terminal_size()
- .with_context(err_context)?;
- rlocked_sessions
- .as_ref()
- .unwrap()
- .senders
- .send_to_screen(ScreenInstruction::TerminalResize(min_size))
+ .to_anyhow()
+ .and_then(|state| {
+ state.min_client_terminal_size().ok_or(anyhow!(
+ "failed to determine minimal client terminal size"
+ ))
+ })
+ .and_then(|min_size| {
+ rlocked_sessions
+ .as_ref()
+ .context("couldn't get reference to read-locked session")?
+ .senders
+ .send_to_screen(ScreenInstruction::TerminalResize(min_size))
+ })
.with_context(err_context)?;
},
ClientToServerMsg::TerminalPixelDimensions(pixel_dimensions) => {
@@ -699,7 +704,8 @@ pub(crate) fn route_thread_main(
ScreenInstruction::TerminalPixelDimensions(pixel_dimensions),
instruction,
retry_queue
- );
+ )
+ .with_context(err_context)?;
},
ClientToServerMsg::BackgroundColor(ref background_color_instruction) => {
send_to_screen_or_retry_queue!(
@@ -709,7 +715,8 @@ pub(crate) fn route_thread_main(
),
instruction,
retry_queue
- );
+ )
+ .with_context(err_context)?;
},
ClientToServerMsg::ForegroundColor(ref foreground_color_instruction) => {
send_to_screen_or_retry_queue!(
@@ -719,7 +726,8 @@ pub(crate) fn route_thread_main(
),
instruction,
retry_queue
- );
+ )
+ .with_context(err_context)?;
},
ClientToServerMsg::ColorRegisters(ref color_registers) => {
send_to_screen_or_retry_queue!(
@@ -727,7 +735,8 @@ pub(crate) fn route_thread_main(
ScreenInstruction::TerminalColorRegisters(color_registers.clone()),
instruction,
retry_queue
- );
+ )
+ .with_context(err_context)?;
},
ClientToServerMsg::NewClient(
client_attributes,
diff --git a/zellij-server/src/screen.rs b/zellij-server/src/screen.rs
index 2c5e44b55..84fbb7688 100644
--- a/zellij-server/src/screen.rs
+++ b/zellij-server/src/screen.rs
@@ -738,13 +738,15 @@ impl Screen {
}
pub fn resize_to_screen(&mut self, new_screen_size: Size) -> Result<()> {
+ let err_context = || format!("failed to resize to screen size: {new_screen_size:#?}");
+
self.size = new_screen_size;
for tab in self.tabs.values_mut() {
- tab.resize_whole_tab(new_screen_size);
+ tab.resize_whole_tab(new_screen_size)
+ .with_context(err_context)?;
tab.set_force_render();
}
- self.render()
- .with_context(|| format!("failed to resize to screen size: {new_screen_size:#?}"))
+ self.render().with_context(err_context)
}
pub fn update_pixel_dimensions(&mut self, pixel_dimensions: PixelDimensions) {
@@ -970,29 +972,35 @@ impl Screen {
};
// apply the layout to the new tab
- let tab = self.tabs.get_mut(&tab_index).unwrap(); // TODO: no unwrap
- tab.apply_layout(
- layout,
- floating_panes_layout,
- new_terminal_ids,
- new_floating_terminal_ids,
- new_plugin_ids,
- client_id,
- )
- .with_context(err_context)?;
- tab.update_input_modes().with_context(err_context)?;
- tab.visible(true).with_context(err_context)?;
- if let Some(drained_clients) = drained_clients {
- tab.add_multiple_clients(drained_clients)
- .with_context(err_context)?;
- }
+ self.tabs
+ .get_mut(&tab_index)
+ .context("couldn't find tab with index {tab_index}")
+ .and_then(|tab| {
+ tab.apply_layout(
+ layout,
+ floating_panes_layout,
+ new_terminal_ids,
+ new_floating_terminal_ids,
+ new_plugin_ids,
+ client_id,
+ )?;
+ tab.update_input_modes()?;
+ tab.visible(true)?;
+ if let Some(drained_clients) = drained_clients {
+ tab.add_multiple_clients(drained_clients)?;
+ }
+ Ok(())
+ })
+ .with_context(err_context)?;
+
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).with_context(err_context)?;
}
- self.update_tabs().with_context(err_context)?;
- self.render().with_context(err_context)
+ self.update_tabs()
+ .and_then(|_| self.render())
+ .with_context(err_context)
}
pub fn add_client(&mut self, client_id: ClientId) -> Result<()> {
@@ -1243,10 +1251,17 @@ impl Screen {
if let Some(client_id) = client_id {
match self.get_active_tab_mut(client_id) {
Ok(active_tab) => {
- if !active_tab.move_focus_left(client_id) {
- self.switch_tab_prev(client_id)
- .context("failed to move focus left")?;
- }
+ active_tab
+ .move_focus_left(client_id)
+ .and_then(|success| {
+ if !success {
+ self.switch_tab_prev(client_id)
+ .context("failed to move focus to previous tab")
+ } else {
+ Ok(())
+ }
+ })
+ .with_context(err_context)?;
},
Err(err) => Err::<(), _>(err).with_context(err_context).non_fatal(),
};
@@ -1270,10 +1285,17 @@ impl Screen {
if let Some(client_id) = client_id {
match self.get_active_tab_mut(client_id) {
Ok(active_tab) => {
- if !active_tab.move_focus_right(client_id) {
- self.switch_tab_next(client_id)
- .context("failed to move focus right")?;
- }
+ active_tab
+ .move_focus_right(client_id)
+ .and_then(|success| {
+ if !success {
+ self.switch_tab_next(client_id)
+ .context("failed to move focus to next tab")