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

CLI: improve error reporting #659

Merged
merged 6 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rust/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 rust/agama-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ async-std = { version ="1.12.0", features = ["attributes"] }
thiserror = "1.0.39"
convert_case = "0.6.0"
console = "0.15.7"
anyhow = "1.0.71"

[[bin]]
name = "agama"
Expand Down
8 changes: 4 additions & 4 deletions rust/agama-cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub enum ConfigAction {
Load(String),
}

pub async fn run(subcommand: ConfigCommands, format: Format) -> Result<(), Box<dyn Error>> {
pub async fn run(subcommand: ConfigCommands, format: Format) -> anyhow::Result<()> {
let store = SettingsStore::new(connection().await?).await?;

match parse_config_command(subcommand) {
Expand All @@ -44,7 +44,7 @@ pub async fn run(subcommand: ConfigCommands, format: Format) -> Result<(), Box<d
for (key, value) in changes {
model.set(&key.to_case(Case::Snake), SettingValue(value))?;
}
store.store(&model).await
Ok(store.store(&model).await?)
}
ConfigAction::Show => {
let model = store.load(None).await?;
Expand All @@ -55,15 +55,15 @@ pub async fn run(subcommand: ConfigCommands, format: Format) -> Result<(), Box<d
let scope = key_to_scope(&key).unwrap();
let mut model = store.load(Some(vec![scope])).await?;
model.add(&key.to_case(Case::Snake), SettingObject::from(values))?;
store.store(&model).await
Ok(store.store(&model).await?)
}
ConfigAction::Load(path) => {
let contents = std::fs::read_to_string(path)?;
let result: InstallSettings = serde_json::from_str(&contents)?;
let scopes = result.defined_scopes();
let mut model = store.load(Some(scopes)).await?;
model.merge(&result);
store.store(&model).await
Ok(store.store(&model).await?)
}
}
}
Expand Down
44 changes: 30 additions & 14 deletions rust/agama-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ use config::run as run_config_cmd;
use printers::Format;
use profile::run as run_profile_cmd;
use progress::InstallerProgress;
use std::error::Error;
use std::thread::sleep;
use std::time::Duration;
use std::{
process::{ExitCode, Termination},
thread::sleep,
time::Duration,
};

#[derive(Parser)]
#[command(name = "agama", version, about, long_about = None)]
Expand All @@ -32,7 +34,7 @@ struct Cli {
pub format: Format,
}

async fn probe() -> Result<(), Box<dyn Error>> {
async fn probe() -> anyhow::Result<()> {
let another_manager = build_manager().await?;
let probe = task::spawn(async move { another_manager.probe().await });
show_progress().await?;
Expand All @@ -45,13 +47,13 @@ async fn probe() -> Result<(), Box<dyn Error>> {
/// Before starting, it makes sure that the manager is idle.
///
/// * `manager`: the manager client.
async fn install(manager: &ManagerClient<'_>, max_attempts: u8) -> Result<(), Box<dyn Error>> {
async fn install(manager: &ManagerClient<'_>, max_attempts: u8) -> anyhow::Result<()> {
if manager.is_busy().await {
println!("Agama's manager is busy. Waiting until it is ready...");
}

if !manager.can_install().await? {
return Err(Box::new(CliError::ValidationError));
return Err(CliError::ValidationError)?;
}

// Display the progress (if needed) and makes sure that the manager is ready
Expand All @@ -72,7 +74,7 @@ async fn install(manager: &ManagerClient<'_>, max_attempts: u8) -> Result<(), Bo
}
if attempts == max_attempts {
eprintln!("Giving up.");
return Err(Box::new(CliError::InstallationError));
return Err(CliError::InstallationError)?;
}
attempts += 1;
sleep(Duration::from_secs(1));
Expand All @@ -94,7 +96,7 @@ async fn show_progress() -> Result<(), ServiceError> {
Ok(())
}

async fn wait_for_services(manager: &ManagerClient<'_>) -> Result<(), Box<dyn Error>> {
async fn wait_for_services(manager: &ManagerClient<'_>) -> Result<(), ServiceError> {
let services = manager.busy_services().await?;
// TODO: having it optional
if !services.is_empty() {
Expand All @@ -104,12 +106,12 @@ async fn wait_for_services(manager: &ManagerClient<'_>) -> Result<(), Box<dyn Er
Ok(())
}

async fn build_manager<'a>() -> Result<ManagerClient<'a>, Box<dyn Error>> {
async fn build_manager<'a>() -> anyhow::Result<ManagerClient<'a>> {
let conn = agama_lib::connection().await?;
Ok(ManagerClient::new(conn).await?)
}

async fn run_command(cli: Cli) -> Result<(), Box<dyn Error>> {
async fn run_command(cli: Cli) -> anyhow::Result<()> {
match cli.command {
Commands::Config(subcommand) => {
let manager = build_manager().await?;
Expand All @@ -130,13 +132,27 @@ async fn run_command(cli: Cli) -> Result<(), Box<dyn Error>> {
}
}

/// Represents the result of execution.
pub enum CliResult {
/// Successful execution.
Ok = 0,
/// Something went wrong.
Error = 1,
}

impl Termination for CliResult {
fn report(self) -> ExitCode {
ExitCode::from(self as u8)
}
}

#[async_std::main]
async fn main() -> Result<(), Box<dyn Error>> {
async fn main() -> CliResult {
let cli = Cli::parse();

if let Err(error) = run_command(cli).await {
eprintln!("{}", error);
return Err(error);
eprintln!("{:?}", error);
return CliResult::Error;
}
Ok(())
CliResult::Ok
}
12 changes: 6 additions & 6 deletions rust/agama-cli/src/printers.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use anyhow;
use serde::Serialize;
use std::error;
use std::fmt::Debug;
use std::io::Write;

Expand All @@ -16,7 +16,7 @@ use std::io::Write;
/// print(user, io::stdout(), Some(Format::Json))
/// .expect("Something went wrong!")
/// ```
pub fn print<T, W>(content: T, writer: W, format: Format) -> Result<(), Box<dyn error::Error>>
pub fn print<T, W>(content: T, writer: W, format: Format) -> anyhow::Result<()>
where
T: serde::Serialize + Debug,
W: Write,
Expand All @@ -38,7 +38,7 @@ pub enum Format {
}

pub trait Printer<T, W> {
fn print(self: Box<Self>) -> Result<(), Box<dyn error::Error>>;
fn print(self: Box<Self>) -> anyhow::Result<()>;
}

pub struct JsonPrinter<T, W> {
Expand All @@ -47,7 +47,7 @@ pub struct JsonPrinter<T, W> {
}

impl<T: Serialize + Debug, W: Write> Printer<T, W> for JsonPrinter<T, W> {
fn print(self: Box<Self>) -> Result<(), Box<dyn error::Error>> {
fn print(self: Box<Self>) -> anyhow::Result<()> {
Ok(serde_json::to_writer(self.writer, &self.content)?)
}
}
Expand All @@ -57,7 +57,7 @@ pub struct TextPrinter<T, W> {
}

impl<T: Serialize + Debug, W: Write> Printer<T, W> for TextPrinter<T, W> {
fn print(mut self: Box<Self>) -> Result<(), Box<dyn error::Error>> {
fn print(mut self: Box<Self>) -> anyhow::Result<()> {
Ok(write!(self.writer, "{:?}", &self.content)?)
}
}
Expand All @@ -68,7 +68,7 @@ pub struct YamlPrinter<T, W> {
}

impl<T: Serialize + Debug, W: Write> Printer<T, W> for YamlPrinter<T, W> {
fn print(self: Box<Self>) -> Result<(), Box<dyn error::Error>> {
fn print(self: Box<Self>) -> anyhow::Result<()> {
Ok(serde_yaml::to_writer(self.writer, &self.content)?)
}
}
21 changes: 13 additions & 8 deletions rust/agama-cli/src/profile.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use agama_lib::error::ProfileError;
use agama_lib::profile::{download, ProfileEvaluator, ProfileValidator, ValidationResult};
use anyhow::Context;
use clap::Subcommand;
use std::path::Path;

Expand All @@ -15,16 +15,18 @@ pub enum ProfileCommands {
Evaluate { path: String },
}

fn validate(path: String) -> Result<(), ProfileError> {
fn validate(path: String) -> anyhow::Result<()> {
let validator = ProfileValidator::default_schema()?;
let path = Path::new(&path);
let result = validator.validate_file(path)?;
let result = validator
.validate_file(path)
.context("Could not validate the profile")?;
match result {
ValidationResult::Valid => {
println!("The profile is valid")
}
ValidationResult::NotValid(errors) => {
println!("The profile is not valid. Please, check the following errors:\n");
eprintln!("The profile is not valid. Please, check the following errors:\n");
for error in errors {
println!("* {error}")
}
Expand All @@ -33,14 +35,17 @@ fn validate(path: String) -> Result<(), ProfileError> {
Ok(())
}

fn evaluate(path: String) -> Result<(), ProfileError> {
fn evaluate(path: String) -> anyhow::Result<()> {
let evaluator = ProfileEvaluator {};
evaluator.evaluate(Path::new(&path))
evaluator
.evaluate(Path::new(&path))
.context(format!("Could not evaluate the profile"))?;
Ok(())
}

pub fn run(subcommand: ProfileCommands) -> Result<(), ProfileError> {
pub fn run(subcommand: ProfileCommands) -> anyhow::Result<()> {
match subcommand {
ProfileCommands::Download { url } => download(&url),
ProfileCommands::Download { url } => Ok(download(&url)?),
ProfileCommands::Validate { path } => validate(path),
ProfileCommands::Evaluate { path } => evaluate(path),
}
Expand Down
8 changes: 4 additions & 4 deletions rust/agama-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ fn expand_set_fn(field_name: &Vec<Ident>) -> TokenStream2 {
}

quote! {
fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), &'static str> {
fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), crate::settings::SettingsError> {
match attr {
#(stringify!(#field_name) => self.#field_name = value.try_into()?,)*
_ => return Err("unknown attribute")
_ => return Err(SettingsError::UnknownAttribute(attr.to_string()))
};
Ok(())
}
Expand Down Expand Up @@ -85,10 +85,10 @@ fn expand_add_fn(field_name: &Vec<Ident>) -> TokenStream2 {
}

quote! {
fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), &'static str> {
fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), crate::settings::SettingsError> {
match attr {
#(stringify!(#field_name) => self.#field_name.push(value.try_into()?),)*
_ => return Err("unknown attribute")
_ => return Err(SettingsError::UnknownCollection(attr.to_string()))
};
Ok(())
}
Expand Down
22 changes: 8 additions & 14 deletions rust/agama-lib/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,24 @@ use zbus;
pub enum ServiceError {
#[error("D-Bus service error: {0}")]
DBus(#[from] zbus::Error),
#[error("Unknown product '{0}'. Available products: '{1:?}'")]
UnknownProduct(String, Vec<String>),
// it's fine to say only "Error" because the original
// specific error will be printed too
#[error("Error: {0}")]
Anyhow(#[from] anyhow::Error),
#[error("Wrong user parameters: '{0:?}'")]
WrongUser(Vec<String>),
}

#[derive(Error, Debug)]
pub enum ProfileError {
#[error("Cannot read the profile '{0}'")]
#[error("Could not read the profile")]
Unreachable(#[from] curl::Error),
#[error("No hardware information available: '{0}'")]
NoHardwareInfo(io::Error),
#[error("Could not evaluate the profile: '{0}'")]
EvaluationError(io::Error),
#[error("Input/output error: '{0}'")]
#[error("Jsonnet evaluation failed:\n{0}")]
EvaluationError(String),
#[error("I/O error: '{0}'")]
InputOutputError(#[from] io::Error),
#[error("The profile is not a valid JSON file")]
FormatError(#[from] serde_json::Error),
}

#[derive(Error, Debug)]
pub enum WrongParameter {
#[error("Unknown product '{0}'. Available products: '{1:?}'")]
UnknownProduct(String, Vec<String>),
#[error("Wrong user parameters: '{0:?}'")]
WrongUser(Vec<String>),
}
10 changes: 5 additions & 5 deletions rust/agama-lib/src/install_settings.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Configuration settings handling
//!
//! This module implements the mechanisms to load and store the installation settings.
use crate::settings::{SettingObject, SettingValue, Settings};
use crate::settings::{SettingObject, SettingValue, Settings, SettingsError};
use crate::{
network::NetworkSettings, software::SoftwareSettings, storage::StorageSettings,
users::UserSettings,
Expand Down Expand Up @@ -93,7 +93,7 @@ impl InstallSettings {
}

impl Settings for InstallSettings {
fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), &'static str> {
fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), SettingsError> {
if let Some((ns, id)) = attr.split_once('.') {
match ns {
"network" => {
Expand All @@ -112,13 +112,13 @@ impl Settings for InstallSettings {
let storage = self.storage.get_or_insert(Default::default());
storage.add(id, value)?
}
_ => return Err("unknown attribute"),
_ => return Err(SettingsError::UnknownCollection(attr.to_string())),
}
}
Ok(())
}

fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), &'static str> {
fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), SettingsError> {
if let Some((ns, id)) = attr.split_once('.') {
match ns {
"network" => {
Expand All @@ -143,7 +143,7 @@ impl Settings for InstallSettings {
let storage = self.storage.get_or_insert(Default::default());
storage.set(id, value)?
}
_ => return Err("unknown attribute"),
_ => return Err(SettingsError::UnknownAttribute(attr.to_string())),
}
}
Ok(())
Expand Down
Loading
Loading