Skip to content

Commit

Permalink
errors: handle missing plugin caches (#2093)
Browse files Browse the repository at this point in the history 
* server/plugins: Always recreate plugin folders

in case they aren't existent and don't mark errors to do so as
non-fatal. The latter masks the underlying cause when e.g. the `.cache`
folder is, for some reason, not writable by zellij (See #2092), whereas
the former fixes problems arising from the user having purged their
.cache/zellij folder entirely.

* utils/errors: Rewrite panic message

* changelog: Add PR #2093
  • Loading branch information
@har7an
har7an authored Jan 19, 2023
1 parent 670b9c2 commit b274fc5
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 58 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
* 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)
* fix: support UTF-8 character in tab name and pane name (https://github.com/zellij-org/zellij/pull/2102)
* fix: handle missing/inaccessible cache directory (https://github.com/zellij-org/zellij/pull/2093)

## [0.34.4] - 2022-12-13

Expand Down
39 changes: 18 additions & 21 deletions zellij-server/src/plugins/wasm_bridge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,15 @@ impl WasmBridge {
let plugin_own_data_dir = ZELLIJ_CACHE_DIR.join(Url::from(&plugin.location).to_string());
let cache_hit = self.plugin_cache.contains_key(&plugin.path);

// Create filesystem entries mounted into WASM.
// We create them here to get expressive error messages in case they fail.
fs::create_dir_all(&plugin_own_data_dir)
.with_context(|| format!("failed to create datadir in {plugin_own_data_dir:?}"))
.with_context(err_context)?;
fs::create_dir_all(ZELLIJ_TMP_DIR.as_path())
.with_context(|| format!("failed to create tmpdir at {:?}", &ZELLIJ_TMP_DIR.as_path()))
.with_context(err_context)?;

// We remove the entry here and repopulate it at the very bottom, if everything went well.
// We must do that because a `get` will only give us a borrow of the Module. This suffices for
// the purpose of setting everything up, but we cannot return a &Module from the "None" match
Expand Down Expand Up @@ -277,19 +286,6 @@ impl WasmBridge {
.with_context(err_context)
.fatal();

fs::create_dir_all(&plugin_own_data_dir)
.with_context(|| format!("failed to create datadir in {plugin_own_data_dir:?}"))
.with_context(err_context)
.non_fatal();

// ensure tmp dir exists, in case it somehow was deleted (e.g systemd-tmpfiles)
fs::create_dir_all(ZELLIJ_TMP_DIR.as_path())
.with_context(|| {
format!("failed to create tmpdir at {:?}", &ZELLIJ_TMP_DIR.as_path())
})
.with_context(err_context)
.non_fatal();

let hash: String = PortableHash::default()
.hash256(&wasm_bytes)
.iter()
Expand All @@ -310,16 +306,17 @@ impl WasmBridge {
Err(e) => {
let inner_context = || format!("failed to recover from {e:?}");

let m = Module::new(&self.store, &wasm_bytes)
.with_context(inner_context)
.with_context(err_context)?;
fs::create_dir_all(ZELLIJ_CACHE_DIR.to_owned())
.map_err(anyError::new)
.and_then(|_| {
Module::new(&self.store, &wasm_bytes).map_err(anyError::new)
})
.and_then(|m| {
m.serialize_to_file(&cached_path).map_err(anyError::new)?;
Ok(m)
})
.with_context(inner_context)
.with_context(err_context)?;
m.serialize_to_file(&cached_path)
.with_context(inner_context)
.with_context(err_context)?;
m
.with_context(err_context)?
},
}
}
Expand Down
76 changes: 39 additions & 37 deletions zellij-utils/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ use serde::{Deserialize, Serialize};
use std::fmt::{Display, Error, Formatter};
use std::path::PathBuf;

use miette::Diagnostic;
use thiserror::Error as ThisError;

/// Re-exports of common error-handling code.
pub mod prelude {
pub use super::FatalError;
Expand All @@ -37,39 +34,6 @@ pub trait ErrorInstruction {
fn error(err: String) -> Self;
}

#[derive(Debug, ThisError, Diagnostic)]
#[error("{0}{}", self.show_backtrace())]
#[diagnostic(help("{}", self.show_help()))]
struct Panic(String);

impl Panic {
// We already capture a backtrace with `anyhow` using the `backtrace` crate in the background.
// The advantage is that this is the backtrace of the real errors source (i.e. where we first
// encountered the error and turned it into an `anyhow::Error`), whereas the backtrace recorded
// here is the backtrace leading to the call to any `panic`ing function. Since now we propagate
// errors up before `unwrap`ing them (e.g. in `zellij_server::screen::screen_thread_main`), the
// former is what we really want to diagnose.
// We still keep the second one around just in case the first backtrace isn't meaningful or
// non-existent in the first place (Which really shouldn't happen, but you never know).
fn show_backtrace(&self) -> String {
if let Ok(var) = std::env::var("RUST_BACKTRACE") {
if !var.is_empty() && var != "0" {
return format!("\n\nPanic backtrace:\n{:?}", backtrace::Backtrace::new());
}
}
"".into()
}

fn show_help(&self) -> String {
r#"If you are seeing this message, it means that something went wrong.
Please report this error to the github issue.
(https://github.com/zellij-org/zellij/issues)
Also, if you want to see the backtrace, you can set the `RUST_BACKTRACE` environment variable to `1`.
"#.into()
}
}

/// Helper trait to easily log error types.
///
/// The `print_error` function takes a closure which takes a `&str` and fares with it as necessary
Expand Down Expand Up @@ -514,13 +478,51 @@ pub use not_wasm::*;
mod not_wasm {
use super::*;
use crate::channels::{SenderWithContext, ASYNCOPENCALLS, OPENCALLS};
use miette::{GraphicalReportHandler, GraphicalTheme, Report};
use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme, Report};
use std::panic::PanicInfo;
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;

#[derive(Debug, ThisError, Diagnostic)]
#[error("{0}{}", self.show_backtrace())]
#[diagnostic(help("{}", self.show_help()))]
struct Panic(String);

impl Panic {
// We already capture a backtrace with `anyhow` using the `backtrace` crate in the background.
// The advantage is that this is the backtrace of the real errors source (i.e. where we first
// encountered the error and turned it into an `anyhow::Error`), whereas the backtrace recorded
// here is the backtrace leading to the call to any `panic`ing function. Since now we propagate
// errors up before `unwrap`ing them (e.g. in `zellij_server::screen::screen_thread_main`), the
// former is what we really want to diagnose.
// We still keep the second one around just in case the first backtrace isn't meaningful or
// non-existent in the first place (Which really shouldn't happen, but you never know).
fn show_backtrace(&self) -> String {
if let Ok(var) = std::env::var("RUST_BACKTRACE") {
if !var.is_empty() && var != "0" {
return format!("\n\nPanic backtrace:\n{:?}", backtrace::Backtrace::new());
}
}
"".into()
}

fn show_help(&self) -> String {
format!(
"If you are seeing this message, it means that something went wrong.
-> To get additional information, check the log at: {}
-> To see a backtrace next time, reproduce the error with: RUST_BACKTRACE=1 zellij [...]
-> To help us fix this, please open an issue: https://github.com/zellij-org/zellij/issues
",
crate::consts::ZELLIJ_TMP_LOG_FILE.display().to_string()
)
}
}

/// Custom panic handler/hook. Prints the [`ErrorContext`].
pub fn handle_panic<T>(info: &PanicInfo<'_>, sender: &SenderWithContext<T>)
where
Expand Down

0 comments on commit b274fc5

Please sign in to comment.