From d50cbf4ca19de4a323ec458c7c6772baad0c5eef Mon Sep 17 00:00:00 2001 From: trivernis Date: Wed, 19 Oct 2022 22:27:32 +0200 Subject: [PATCH] Improve error reporting --- Cargo.lock | 1 + Cargo.toml | 1 + configs/crystal/config.schema.json | 32 +++++++++++- configs/crystal/distro.toml | 18 ++++--- configs/crystal/setup-users/up.nu | 1 + src/args.rs | 4 +- src/distro/distro_config.rs | 8 +-- src/distro/os_config/base_config.rs | 6 +-- src/distro/os_config/loader.rs | 41 ++++++++-------- src/distro/os_config/mod.rs | 6 +-- src/error.rs | 75 ++++++++++++++++++++++------- src/lib.rs | 1 + src/main.rs | 11 +++-- src/task/base_task.rs | 2 + src/task/custom_task.rs | 15 +++++- src/task/exec_builder.rs | 22 +++++---- src/task/task_executor.rs | 3 ++ 17 files changed, 173 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 60d3778..4514d4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3523,6 +3523,7 @@ dependencies = [ "serde_json", "thiserror", "tokio", + "toml", "tracing", "tracing-subscriber", "valico", diff --git a/Cargo.toml b/Cargo.toml index ac2e527..4b02839 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ serde = { version = "1.0.145", features = ["derive"] } serde_json = "1.0.86" thiserror = "1.0.37" tokio = { version = "1.21.2", features = ["rt", "io-std", "io-util", "process", "time", "macros", "tracing", "fs"] } +toml = "0.5.9" tracing = "0.1.37" tracing-subscriber = "0.3.16" valico = "3.6.1" diff --git a/configs/crystal/config.schema.json b/configs/crystal/config.schema.json index 49605e6..c1e5275 100644 --- a/configs/crystal/config.schema.json +++ b/configs/crystal/config.schema.json @@ -1,6 +1,7 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://getcryst.al/config.schema.json", + "type": "object", "properties": { "enable_flatpak": { "type": "boolean" @@ -10,6 +11,35 @@ }, "enable_zramd": { "type": "boolean" + }, + "unakite": { + "type": "object", + "properties": { + "root": { + "type": "string" + }, + "old_root": { + "type": "string" + }, + "efidir": { + "type": "string" + }, + "bootdev": { + "type": "string" + } + }, + "required": [ + "root", + "old_root", + "efidir", + "bootdev" + ] } - } + }, + "required": [ + "enable_flatpak", + "enable_timeshift", + "enable_zramd", + "unakite" + ] } \ No newline at end of file diff --git a/configs/crystal/distro.toml b/configs/crystal/distro.toml index b083e63..251c84d 100644 --- a/configs/crystal/distro.toml +++ b/configs/crystal/distro.toml @@ -7,11 +7,17 @@ schema = "config.schema.json" [tasks] -[tasks.enable_flatpak] -config_field = "enable_flatpak" +[tasks.install-flatpak] +config_key = "enable_flatpak" +skip_on_false = true -[tasks.enable_timeshift] -config_field = "enable_timeshift" +[tasks.install-timeshift] +config_key = "enable_timeshift" +skip_on_false = true -[tasks.enable_zramd] -config_field = "enable_zramd" \ No newline at end of file +[tasks.install-zramd] +config_key = "enable_zramd" +skip_on_false = true + +[tasks.configure-unakite] +config_key = "unakite" diff --git a/configs/crystal/setup-users/up.nu b/configs/crystal/setup-users/up.nu index 8b5f6fe..b8b65f8 100644 --- a/configs/crystal/setup-users/up.nu +++ b/configs/crystal/setup-users/up.nu @@ -1,4 +1,5 @@ # Applies all system changes of `setup-users` def main [cfg] { echo "Executing up task `setup-users` with config" $cfg + echo $TRM_CONFIG } diff --git a/src/args.rs b/src/args.rs index 26e73e7..ca1f2b2 100644 --- a/src/args.rs +++ b/src/args.rs @@ -30,7 +30,7 @@ pub enum Command { /// *For testing purposes only* /// Generates the JSON for an empty config file #[command()] - CreateEmptyConfig(GenerateEmptyConfigArgs), + CreateEmptyConfig(CreateEmptyConfigArgs), } #[derive(Debug, Clone, Parser)] @@ -48,7 +48,7 @@ pub struct GenerateScriptsArgs { } #[derive(Debug, Clone, Parser)] -pub struct GenerateEmptyConfigArgs { +pub struct CreateEmptyConfigArgs { /// The path to the empty configuration file #[arg(default_value = "config.json")] pub path: PathBuf, diff --git a/src/distro/distro_config.rs b/src/distro/distro_config.rs index d16d5aa..7cca61a 100644 --- a/src/distro/distro_config.rs +++ b/src/distro/distro_config.rs @@ -3,7 +3,7 @@ use std::{collections::HashMap, path::PathBuf}; use serde::Deserialize; use tokio::fs; -use crate::{error::AppResult, utils::CFG_PATH}; +use crate::{error::DistroConfigError, utils::CFG_PATH}; /// The config file of a distro that defines /// how that distro should be installed @@ -43,14 +43,16 @@ pub struct TaskConfig { /// If the task should be skipped if the /// config value of that task is null + #[serde(default)] pub skip_on_false: bool, } impl DistroConfig { - pub async fn load() -> AppResult { + #[tracing::instrument(level = "trace", skip_all)] + pub async fn load() -> Result { let path = CFG_PATH.join("distro.toml"); let contents = fs::read_to_string(path).await?; - let cfg = serde_json::from_str::(&contents)?; + let cfg = toml::from_str::(&contents)?; Ok(cfg) } diff --git a/src/distro/os_config/base_config.rs b/src/distro/os_config/base_config.rs index 4190a8d..457ccbb 100644 --- a/src/distro/os_config/base_config.rs +++ b/src/distro/os_config/base_config.rs @@ -46,7 +46,7 @@ impl BaseConfig { additional: Vec::new(), }, desktop: DesktopConfig::KdePlasma, - users: UsersConfig { users: Vec::new() }, + users: UsersConfig(Vec::new()), root_user: RootUserConfig { password: String::new(), }, @@ -151,9 +151,7 @@ pub struct RootUserConfig { } #[derive(Clone, Debug, Deserialize, RustyValue)] -pub struct UsersConfig { - pub users: Vec, -} +pub struct UsersConfig(Vec); #[derive(Clone, Debug, Deserialize, RustyValue)] pub struct User { diff --git a/src/distro/os_config/loader.rs b/src/distro/os_config/loader.rs index 96d50f6..8c3274a 100644 --- a/src/distro/os_config/loader.rs +++ b/src/distro/os_config/loader.rs @@ -5,12 +5,13 @@ use valico::json_schema::Scope; use crate::{ distro::distro_config::DistroConfig, - error::{AppError, AppResult}, + error::{AppResult, OSConfigError, SchemaError}, utils::CFG_PATH, }; use super::OSConfig; +#[derive(Debug)] pub struct OSConfigLoader<'a> { distro_cfg: &'a DistroConfig, cfg_path: PathBuf, @@ -24,6 +25,7 @@ impl<'a> OSConfigLoader<'a> { } } + #[tracing::instrument(level = "trace", skip_all)] pub async fn load(&self) -> AppResult { let schema = self.load_extension_schema().await?; let os_config = OSConfig::load(&self.cfg_path).await?; @@ -32,13 +34,14 @@ impl<'a> OSConfigLoader<'a> { Ok(os_config) } - async fn load_extension_schema(&self) -> AppResult { + #[tracing::instrument(level = "trace", skip_all)] + async fn load_extension_schema(&self) -> Result { let schema_path = self .distro_cfg .config .schema .as_ref() - .map(PathBuf::from) + .map(|p| CFG_PATH.join(p)) .unwrap_or_else(|| CFG_PATH.join("config.schema.json")); let contents = fs::read_to_string(schema_path).await?; let schema = serde_json::from_str(&contents)?; @@ -46,33 +49,29 @@ impl<'a> OSConfigLoader<'a> { Ok(schema) } + #[tracing::instrument(level = "trace", skip_all)] fn validate_config(schema: serde_json::Value, config: &OSConfig) -> AppResult<()> { let mut scope = Scope::new(); - let schema = scope.compile_and_return(schema, true)?; - let mut errors = Vec::new(); + let schema = scope + .compile_and_return(schema, true) + .map_err(SchemaError::ParseSchema)?; + let ext_value = serde_json::Value::Object(config.extended.clone().into_iter().collect()); - for (key, value) in config.extended.iter() { - let result = schema.validate_in(value, key); + let result = schema.validate(&ext_value); - for error in result.errors { - tracing::error!( - "ConfigError: {} ({}) at {}", - error.get_title(), - error.get_code(), - error.get_path(), - ); - errors.push(error); - } - } - if errors.is_empty() { + if result.is_valid() { + tracing::debug!("Config is valid"); Ok(()) } else { - let msg = errors + let msg = result + .errors .into_iter() - .map(|e| format!("{} ({}) at {}", e.get_title(), e.get_code(), e.get_path())) + .map(|e| format!("{} > {}", e.get_path(), e.get_title())) .collect::>() .join("\n"); - Err(AppError::InvalidConfig(msg)) + tracing::error!("Config is invalid"); + + Err(OSConfigError::Validation(msg).into()) } } } diff --git a/src/distro/os_config/mod.rs b/src/distro/os_config/mod.rs index 2608f67..d2fbf9d 100644 --- a/src/distro/os_config/mod.rs +++ b/src/distro/os_config/mod.rs @@ -13,7 +13,7 @@ use embed_nu::{ use serde::Deserialize; use tokio::fs; -use crate::error::{AppError, AppResult}; +use crate::error::{AppResult, OSConfigError}; /// Represents the full configuration of the OS including extensions defined /// by the distro @@ -40,7 +40,7 @@ impl OSConfig { fields .remove(key.as_ref()) .map(|v| v.into_value()) - .ok_or_else(|| AppError::MissingConfigKey(key.as_ref().to_owned())) + .ok_or_else(|| OSConfigError::MissingConfigKey(key.as_ref().to_owned()).into()) } } @@ -114,7 +114,7 @@ fn json_to_rusty_value(val: serde_json::Value) -> embed_nu::rusty_value::Value { } impl OSConfig { - pub(crate) async fn load(path: &Path) -> AppResult { + pub(crate) async fn load(path: &Path) -> Result { let contents = fs::read_to_string(path).await?; let cfg = serde_json::from_str::(&contents)?; diff --git a/src/error.rs b/src/error.rs index acf6012..c1279df 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,36 +6,75 @@ pub type AppResult = std::result::Result; #[derive(Error, Debug)] pub enum AppError { - #[error("Could not find the script file")] + #[error("Missing config")] + MissingConfig, + + #[error("IO Error: {0}")] + Io(#[from] io::Error), + + #[error("JSON deserialization error {0}")] + JSON(#[from] serde_json::Error), + + #[error(transparent)] + Script(#[from] ScriptError), + + #[error(transparent)] + DistroConfig(#[from] DistroConfigError), + + #[error(transparent)] + Schema(#[from] SchemaError), + + #[error(transparent)] + OSConfig(#[from] OSConfigError), +} + +#[derive(Error, Debug)] +pub enum ScriptError { + #[error("IO Error when trying to read script file: {0}")] + Io(#[from] io::Error), + + #[error("Could not find the script file at {0}")] ScriptNotFound(PathBuf), - #[error("Nu error {0}")] + #[error("Nu error when executing script: {0}")] NuError(#[from] embed_nu::Error), - #[error("Could not find the main mehod in the script file {0}")] + #[error("Could not find the main method in the script file: {0}")] MissingMain(PathBuf), +} - #[error("Failed to execute script")] - FailedToExecuteScript, +#[derive(Error, Debug)] +pub enum DistroConfigError { + #[error("IO Error when trying to read distro config: {0}")] + Io(#[from] io::Error), - #[error("Missing config")] - MissingConfig, + #[error("Encountered invalid Toml when parsing distro config: {0}")] + InvalidToml(#[from] toml::de::Error), +} - #[error("Missing config key {0}")] - MissingConfigKey(String), +#[derive(Error, Debug)] +pub enum SchemaError { + #[error("IO Error when trying to read json-schema file: {0}")] + Io(#[from] io::Error), - #[error("The os config is invalid: {0}")] - InvalidConfig(String), + #[error("Encountered invalid JSON when parsing json-schema: {0}")] + InvalidJson(#[from] serde_json::Error), - #[error("IO Error: {0}")] + #[error("Failed to parse the json-schema: {0}")] + ParseSchema(#[from] valico::json_schema::SchemaError), +} + +#[derive(Error, Debug)] +pub enum OSConfigError { + #[error("IO Error when trying to read OSConfig file: {0}")] Io(#[from] io::Error), - #[error("JSON deserialization error {0}")] - JSON(#[from] serde_json::Error), + #[error("Encountered invalid JSON when parsing OSConfig: {0}")] + InvalidJson(#[from] serde_json::Error), - #[error("The task has been skipped")] - Skipped, + #[error("The os config is invalid:\n{0}")] + Validation(String), - #[error("Failed to process config schema: {0}")] - ConfigSchema(#[from] valico::json_schema::SchemaError), + #[error("Missing config key {0}")] + MissingConfigKey(String), } diff --git a/src/lib.rs b/src/lib.rs index 0285a26..95fe6cd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ pub mod distro; pub mod task; /// Creates a new executor with the given os config for the current distro +#[tracing::instrument(level = "trace")] pub async fn create_executor(os_cfg_path: PathBuf) -> AppResult { let distro_config = DistroConfig::load().await?; let os_config = OSConfigLoader::new(os_cfg_path, &distro_config) diff --git a/src/main.rs b/src/main.rs index 813b3cf..68730dd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,4 @@ -use args::{Args, Command, GenerateEmptyConfigArgs, GenerateScriptsArgs, InstallFromConfigArgs}; +use args::{Args, Command, CreateEmptyConfigArgs, GenerateScriptsArgs, InstallFromConfigArgs}; use clap::Parser; use rusty_value::into_json::{EnumRepr, IntoJson, IntoJsonOptions}; use tokio::fs; @@ -7,7 +7,7 @@ use tourmaline::{distro::OSConfig, error::AppResult, generate_script_files}; mod args; #[tokio::main(flavor = "current_thread")] -async fn main() { +async fn main() -> color_eyre::Result<()> { color_eyre::install().unwrap(); dotenv::dotenv().unwrap(); let args = Args::parse(); @@ -16,8 +16,9 @@ async fn main() { Command::InstallFromConfig(args) => install_from_config(args).await, Command::GenerateScripts(args) => generate_scripts(args).await, Command::CreateEmptyConfig(args) => generate_empty_config(args).await, - } - .unwrap(); + }?; + + Ok(()) } /// Installs the distro from a given configuration file @@ -34,7 +35,7 @@ async fn generate_scripts(args: GenerateScriptsArgs) -> AppResult<()> { generate_script_files(args.path).await } -async fn generate_empty_config(args: GenerateEmptyConfigArgs) -> AppResult<()> { +async fn generate_empty_config(args: CreateEmptyConfigArgs) -> AppResult<()> { let config = OSConfig::empty().into_json_with_options(&IntoJsonOptions { enum_repr: EnumRepr::Untagged, }); diff --git a/src/task/base_task.rs b/src/task/base_task.rs index 35741ef..a73707a 100644 --- a/src/task/base_task.rs +++ b/src/task/base_task.rs @@ -69,6 +69,7 @@ impl BaseTask { } impl TaskTrait for BaseTask { + #[tracing::instrument(level = "trace", skip_all)] fn up(&self, config: &OSConfig) -> AppResult> { let key_data = self.key_data(); let script = PathBuf::from(key_data.task_name).join("up.nu"); @@ -86,6 +87,7 @@ impl TaskTrait for BaseTask { })) } + #[tracing::instrument(level = "trace", skip_all)] fn down(&self, config: &OSConfig) -> AppResult> { let key_data = self.key_data(); let script = PathBuf::from(key_data.task_name).join("down.nu"); diff --git a/src/task/custom_task.rs b/src/task/custom_task.rs index e090984..52d1ea9 100644 --- a/src/task/custom_task.rs +++ b/src/task/custom_task.rs @@ -25,10 +25,11 @@ impl CustomTask { } impl TaskTrait for CustomTask { + #[tracing::instrument(level = "trace", skip_all)] fn up(&self, config: &OSConfig) -> AppResult> { let task_config = config.get_nu_value(&self.config_key)?; - if self.skip_on_false && task_config.is_nothing() { + if self.skip_on_false && config_is_falsy(&task_config) { Ok(None) } else { Ok(Some(ExecBuilder { @@ -39,10 +40,11 @@ impl TaskTrait for CustomTask { } } + #[tracing::instrument(level = "trace", skip_all)] fn down(&self, config: &OSConfig) -> AppResult> { let task_config = config.get_nu_value(&self.config_key)?; - if self.skip_on_false && task_config.is_nothing() { + if self.skip_on_false && config_is_falsy(&task_config) { Ok(None) } else { Ok(Some(ExecBuilder { @@ -53,3 +55,12 @@ impl TaskTrait for CustomTask { } } } + +fn config_is_falsy(config: &embed_nu::Value) -> bool { + if config.is_nothing() { + return true; + } else if let Ok(b) = config.as_bool() { + return !b; + } + return false; +} diff --git a/src/task/exec_builder.rs b/src/task/exec_builder.rs index 179c663..b20a19b 100644 --- a/src/task/exec_builder.rs +++ b/src/task/exec_builder.rs @@ -3,10 +3,7 @@ use std::path::PathBuf; use embed_nu::{Argument, CommandGroupConfig, Context, ValueIntoExpression}; use tokio::fs; -use crate::{ - distro::OSConfig, - error::{AppError, AppResult}, -}; +use crate::{distro::OSConfig, error::ScriptError, utils::CFG_PATH}; pub struct ExecBuilder { pub script: PathBuf, @@ -15,13 +12,15 @@ pub struct ExecBuilder { } impl ExecBuilder { - pub async fn exec(self) -> AppResult<()> { + #[tracing::instrument(level = "trace", skip_all)] + pub async fn exec(self) -> Result<(), ScriptError> { let script_contents = self.get_script_contents().await?; let mut ctx = Context::builder() .with_command_groups(CommandGroupConfig::default().all_groups(true))? .add_var("TRM_CONFIG", self.os_config)? .add_script(script_contents)? .build()?; + if ctx.has_fn("main") { let pipeline = ctx.call_fn( "main", @@ -31,13 +30,18 @@ impl ExecBuilder { Ok(()) } else { - Err(AppError::MissingMain(self.script)) + Err(ScriptError::MissingMain(self.script)) } } - async fn get_script_contents(&self) -> AppResult { - let contents = fs::read_to_string(&self.script).await?; + #[tracing::instrument(level = "trace", skip_all)] + async fn get_script_contents(&self) -> Result { + let path = CFG_PATH.join(&self.script); - Ok(contents) + if path.exists() { + fs::read_to_string(path).await.map_err(ScriptError::from) + } else { + Err(ScriptError::ScriptNotFound(path)) + } } } diff --git a/src/task/task_executor.rs b/src/task/task_executor.rs index 965668e..f07f636 100644 --- a/src/task/task_executor.rs +++ b/src/task/task_executor.rs @@ -22,6 +22,7 @@ impl TaskExecutor { } /// Adds all base tasks to the executor + #[tracing::instrument(level = "trace", skip_all)] pub fn with_base_tasks(&mut self) -> &mut Self { let mut base_tasks = (*ALL_BASE_TASKS) .iter() @@ -34,6 +35,7 @@ impl TaskExecutor { } /// Adds all custom tasks to the executor + #[tracing::instrument(level = "trace", skip_all)] pub fn with_custom_tasks(&mut self) -> &mut Self { let mut custom_tasks = self .distro_config @@ -54,6 +56,7 @@ impl TaskExecutor { } /// Executes all tasks + #[tracing::instrument(level = "trace", skip_all)] pub async fn execute(&mut self) -> AppResult<()> { for task in &self.tasks { if let Some(up_task) = task.up(&self.os_config)? {