Repository: zellij-org/zellij (language: rust)

Issue:
Improve startup error handling and reporting for plugins
Moved from #1753:

I'd like to call out an example of how error handling could be improved that is easily reproduced.

I recently messed with my filesystem with specific goals in mind and a side effect that I missed was my user's .cache directory getting owned by root.  Everything I previously used could still function since those .cache artifacts already existed and were still owned by me.  zellij was the first new thing I tried since this mistake.

When starting zellij for the first time it crashed with a pretty verbose file not found and permission denied error but no specifics as to the file.  I then tried the recommended RUST env var to get more info, but the backtrace was unusable as it was all `<unknown>` entries.

The only reason I found /tmp/zellij-1000/zellig-log was because I also tried strace against zellij.  That log is the only place I got usable info to solve my problem as it specifically stated `failed to create datadir in ".../.cache/..."`.  So the quick and easy improvement is to mention the log file in the error messages.  The other improvement would be getting the same information that's in the log file into the error message that's displayed to the user.

_Originally posted by @alaricljs in https://github.com/zellij-org/zellij/issues/1753#issuecomment-1378950836_


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 06d805c95b..e24865d3a4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/zellij-server/src/plugins/wasm_bridge.rs b/zellij-server/src/plugins/wasm_bridge.rs
index abb405ba16..6271295e75 100644
--- a/zellij-server/src/plugins/wasm_bridge.rs
+++ b/zellij-server/src/plugins/wasm_bridge.rs
@@ -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
@@ -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()
@@ -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)?
                         },
                     }
                 }
diff --git a/zellij-utils/src/errors.rs b/zellij-utils/src/errors.rs
index d0e4fad253..7fbed99ed4 100644
--- a/zellij-utils/src/errors.rs
+++ b/zellij-utils/src/errors.rs
@@ -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;
@@ -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
@@ -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


Test patch: