Radish alpha
h
heartwood
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
Patches › a9a32d1b
radicle: improve config errors
Merged fintohaps opened 10 months ago

Improve the granularity of config errors mentioning the path location where the configuration is expected to be loading from or saving to.

Activity Changes +188 -54
5 files changed +188 -54 6686f86c → 6f34124d
modified crates/radicle-cli/examples/rad-config.md
@@ -107,7 +107,7 @@ $ rad config unset web.name


``` (fail)

$ rad config get web.name
- 
✗ Error: configuration error: web.name does not exist
+ 
✗ Error: web.name does not exist in configuration found at "[..]/.radicle/config.json"

```



Values along the path will be created if necessary.
@@ -131,19 +131,19 @@ Values that are required for a valid config can't be deleted.


``` (fail)

$ rad config unset node.alias
- 
✗ Error: configuration JSON error: missing field `alias`
+ 
✗ Error: writing configuration to "[..]/.radicle/config.json" failed: validation failure due to missing field `alias`

```



Values for changes are being validated.



``` (fail)

$ rad config set web.pinned.repositories 5
- 
✗ Error: configuration JSON error: invalid type: integer `5`, expected a sequence
+ 
✗ Error: writing configuration to "[..]/.radicle/config.json" failed: validation failure due to invalid type: integer `5`, expected a sequence

```



The type of the operation is validated.



``` (fail)

$ rad config push node.alias eve
- 
✗ Error: the element at the path 'node.alias' is not a JSON array
+ 
✗ Error: failed to modify configuration found at "[..]/.radicle/config.json" due to the element at the path 'node.alias' is not a JSON array

```
modified crates/radicle-cli/src/commands/config.rs
@@ -5,7 +5,7 @@ use std::str::FromStr;


use anyhow::anyhow;

use radicle::node::Alias;
- 
use radicle::profile::{Config, ConfigError, ConfigPath, RawConfig};
+ 
use radicle::profile::{config, Config, ConfigPath, RawConfig};



use crate::terminal as term;

use crate::terminal::args::{Args, Error, Help};
@@ -149,33 +149,25 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
        Operation::Get(key) => {

            let mut temp_config = RawConfig::from_file(&path)?;

            let key: ConfigPath = key.into();
- 
            let value = temp_config
- 
                .get_mut(&key)
- 
                .ok_or_else(|| ConfigError::Custom(format!("{key} does not exist")))?;
+ 
            let value = temp_config.get_mut(&key).ok_or_else(|| {
+ 
                anyhow::anyhow!("{key} does not exist in configuration found at {path:?}")
+ 
            })?;

            print_value(value)?;

        }

        Operation::Set(key, value) => {
- 
            let mut temp_config = RawConfig::from_file(&path)?;
- 
            let value = temp_config.set(&key.into(), value.into())?;
- 
            temp_config.write(&path)?;
+ 
            let value = modify(path, |tmp| tmp.set(&key.into(), value.into()))?;

            print_value(&value)?;

        }

        Operation::Push(key, value) => {
- 
            let mut temp_config = RawConfig::from_file(&path)?;
- 
            let value = temp_config.push(&key.into(), value.into())?;
- 
            temp_config.write(&path)?;
+ 
            let value = modify(path, |tmp| tmp.push(&key.into(), value.into()))?;

            print_value(&value)?;

        }

        Operation::Remove(key, value) => {
- 
            let mut temp_config = RawConfig::from_file(&path)?;
- 
            let value = temp_config.remove(&key.into(), value.into())?;
- 
            temp_config.write(&path)?;
+ 
            let value = modify(path, |tmp| tmp.remove(&key.into(), value.into()))?;

            print_value(&value)?;

        }

        Operation::Unset(key) => {
- 
            let mut temp_config = RawConfig::from_file(&path)?;
- 
            let value = temp_config.unset(&key.into())?;
- 
            temp_config.write(&path)?;
+ 
            let value = modify(path, |tmp| tmp.unset(&key.into()))?;

            print_value(&value)?;

        }

        Operation::Init => {
@@ -204,6 +196,20 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
    Ok(())

}
+ 
fn modify<P, M>(path: P, modification: M) -> anyhow::Result<serde_json::Value>
+ 
where
+ 
    P: AsRef<Path>,
+ 
    M: FnOnce(&mut RawConfig) -> Result<serde_json::Value, config::ModifyError>,
+ 
{
+ 
    let path = path.as_ref();
+ 
    let mut temp_config = RawConfig::from_file(path)?;
+ 
    let value = modification(&mut temp_config).map_err(|err| {
+ 
        anyhow::anyhow!("failed to modify configuration found at {path:?} due to {err}")
+ 
    })?;
+ 
    temp_config.write(path)?;
+ 
    Ok(value)
+ 
}
+ 


/// Print a JSON Value.

fn print_value(value: &serde_json::Value) -> anyhow::Result<()> {

    match value {
modified crates/radicle-cli/src/terminal.rs
@@ -137,7 +137,7 @@ impl Context for DefaultContext {
                hint: "To setup your radicle profile, run `rad auth`.",

            }

            .into()),
- 
            Err(radicle::profile::Error::Config(e)) => Err(e.into()),
+ 
            Err(radicle::profile::Error::LoadConfig(e)) => Err(e.into()),

            Err(e) => Err(anyhow::anyhow!("Could not load radicle profile: {e}")),

        }

    }
modified crates/radicle/src/profile.rs
@@ -12,7 +12,7 @@
//!



pub mod config;
- 
pub use config::{Config, ConfigError, ConfigPath, RawConfig};
+ 
pub use config::{Config, ConfigPath, RawConfig, WriteError};



use std::collections::{BTreeMap, BTreeSet};

use std::path::{Path, PathBuf};
@@ -163,7 +163,9 @@ pub enum Error {
    #[error(transparent)]

    Io(#[from] io::Error),

    #[error(transparent)]
- 
    Config(#[from] ConfigError),
+ 
    InitConfig(#[from] config::InitError),
+ 
    #[error(transparent)]
+ 
    LoadConfig(#[from] config::LoadError),

    #[error(transparent)]

    Node(#[from] node::Error),

    #[error(transparent)]
modified crates/radicle/src/profile/config.rs
@@ -1,5 +1,5 @@
use std::io::Write;
- 
use std::path::Path;
+ 
use std::path::{Path, PathBuf};

use std::{fmt, fs, io};



use serde::Serialize as _;
@@ -13,13 +13,117 @@ use crate::node::Alias;
use crate::{cli, node, web};



#[derive(Debug, Error)]
- 
pub enum ConfigError {
- 
    #[error("configuration I/O error: {0}")]
- 
    Io(#[from] io::Error),
- 
    #[error("configuration JSON error: {0}")]
- 
    Json(#[from] json::Error),
- 
    #[error("configuration error: {0}")]
- 
    Custom(String),
+ 
#[error("writing configuration to {path:?} failed: {kind}")]
+ 
pub struct WriteError {
+ 
    path: PathBuf,
+ 
    kind: WriteErrorKind,
+ 
}
+
+ 
impl WriteError {
+ 
    fn open_file<P>(path: P, err: io::Error) -> Self
+ 
    where
+ 
        P: AsRef<Path>,
+ 
    {
+ 
        Self {
+ 
            path: path.as_ref().to_path_buf(),
+ 
            kind: WriteErrorKind::OpenFile { err },
+ 
        }
+ 
    }
+
+ 
    fn to_json<P>(path: P, err: serde_json::Error) -> Self
+ 
    where
+ 
        P: AsRef<Path>,
+ 
    {
+ 
        Self {
+ 
            path: path.as_ref().to_path_buf(),
+ 
            kind: WriteErrorKind::ToJson { err },
+ 
        }
+ 
    }
+
+ 
    fn write_file<P>(path: P, err: io::Error) -> Self
+ 
    where
+ 
        P: AsRef<Path>,
+ 
    {
+ 
        Self {
+ 
            path: path.as_ref().to_path_buf(),
+ 
            kind: WriteErrorKind::WriteFile { err },
+ 
        }
+ 
    }
+
+ 
    fn validate<P>(path: P, err: serde_json::Error) -> Self
+ 
    where
+ 
        P: AsRef<Path>,
+ 
    {
+ 
        Self {
+ 
            path: path.as_ref().to_path_buf(),
+ 
            kind: WriteErrorKind::Validate { err },
+ 
        }
+ 
    }
+
+ 
    fn serialize_json<P>(path: P, err: serde_json::Error) -> Self
+ 
    where
+ 
        P: AsRef<Path>,
+ 
    {
+ 
        Self {
+ 
            path: path.as_ref().to_path_buf(),
+ 
            kind: WriteErrorKind::SerializeJson { err },
+ 
        }
+ 
    }
+ 
}
+
+ 
#[derive(Debug, Error)]
+ 
pub enum WriteErrorKind {
+ 
    #[error("could not open due to {err}")]
+ 
    OpenFile {
+ 
        #[source]
+ 
        err: io::Error,
+ 
    },
+ 
    #[error("could not convert to JSON due to {err}")]
+ 
    ToJson {
+ 
        #[source]
+ 
        err: json::Error,
+ 
    },
+ 
    #[error("could not write to file due to {err}")]
+ 
    WriteFile {
+ 
        #[source]
+ 
        err: io::Error,
+ 
    },
+ 
    #[error("validation failure due to {err}")]
+ 
    Validate {
+ 
        #[source]
+ 
        err: serde_json::Error,
+ 
    },
+ 
    #[error("could not serialize due to {err}")]
+ 
    SerializeJson {
+ 
        #[source]
+ 
        err: serde_json::Error,
+ 
    },
+ 
}
+
+ 
#[derive(Debug, Error)]
+ 
pub enum InitError {
+ 
    #[error("failed to initialize configuration file {path:?}: {err}")]
+ 
    Write {
+ 
        path: PathBuf,
+ 
        #[source]
+ 
        err: WriteError,
+ 
    },
+ 
}
+
+ 
#[derive(Debug, Error)]
+ 
pub enum LoadError {
+ 
    #[error("failed to open configuration file {path:?}: {err}, perhaps you need to initialise one `rad config init --alias <alias>`")]
+ 
    File {
+ 
        path: PathBuf,
+ 
        #[source]
+ 
        err: io::Error,
+ 
    },
+ 
    #[error("failed to parse JSON of configuration file {path:?}: {err}")]
+ 
    Json {
+ 
        path: PathBuf,
+ 
        #[source]
+ 
        err: serde_json::Error,
+ 
    },

}



/// Local radicle configuration.
@@ -59,15 +163,25 @@ impl Config {
    }



    /// Initialize a new configuration. Fails if the path already exists.
- 
    pub fn init(alias: Alias, path: &Path) -> Result<Self, ConfigError> {
+ 
    pub fn init(alias: Alias, path: &Path) -> Result<Self, InitError> {

        let cfg = Config::new(alias);
- 
        cfg.write(path)?;
+ 
        cfg.write(path).map_err(|err| InitError::Write {
+ 
            path: path.to_path_buf(),
+ 
            err,
+ 
        })?;

        Ok(cfg)

    }



    /// Load a configuration from the given path.
- 
    pub fn load(path: &Path) -> Result<Self, ConfigError> {
- 
        let mut cfg: Self = json::from_reader(fs::File::open(path)?)?;
+ 
    pub fn load(path: &Path) -> Result<Self, LoadError> {
+ 
        let file = fs::File::open(path).map_err(|err| LoadError::File {
+ 
            path: path.to_path_buf(),
+ 
            err,
+ 
        })?;
+ 
        let mut cfg: Self = json::from_reader(file).map_err(|err| LoadError::Json {
+ 
            path: path.to_path_buf(),
+ 
            err,
+ 
        })?;



        // Handle deprecated policy configuration.

        // Nb. This will override "seedingPolicy" if set! This code should be removed after 1.0.
@@ -87,15 +201,16 @@ impl Config {
    }



    /// Write configuration to disk.
- 
    pub fn write(&self, path: &Path) -> Result<(), ConfigError> {
- 
        let value = json::to_value(self)?;
+ 
    pub fn write(&self, path: &Path) -> Result<(), WriteError> {
+ 
        let value = json::to_value(self).map_err(|err| WriteError::to_json(path, err))?;

        let tmp = RawConfig(value);

        let file = fs::OpenOptions::new()

            .create_new(true)

            .write(true)
- 
            .open(path)?;
+ 
            .open(path)
+ 
            .map_err(|err| WriteError::open_file(path, err))?;
- 
        tmp.write_file(file)
+ 
        tmp.write_file(path, file)

    }



    /// Get the user alias.
@@ -124,9 +239,9 @@ pub enum ModifyError {


impl RawConfig {

    /// Creates a temporary configuration, by reading a configuration file from disk.
- 
    pub fn from_file(path: &Path) -> Result<Self, ConfigError> {
- 
        let file = fs::File::open(path)?;
- 
        let config = json::from_reader(file)?;
+ 
    pub fn from_file(path: &Path) -> Result<Self, WriteError> {
+ 
        let file = fs::File::open(path).map_err(|err| WriteError::open_file(path, err))?;
+ 
        let config = json::from_reader(file).map_err(|err| WriteError::to_json(path, err))?;

        Ok(RawConfig(config))

    }
@@ -219,26 +334,37 @@ impl RawConfig {


    /// Writes the configuration, including extra values, to disk. Errors if the config is not

    /// valid.
- 
    pub fn write(&self, path: &Path) -> Result<(), ConfigError> {
- 
        let _valid_config: Config = self.clone().try_into()?;
+ 
    pub fn write(&self, path: &Path) -> Result<(), WriteError> {
+ 
        let _valid_config: Config = self
+ 
            .clone()
+ 
            .try_into()
+ 
            .map_err(|err| WriteError::validate(path, err))?;

        let file = fs::OpenOptions::new()

            .create(true)

            .write(true)

            .truncate(true)
- 
            .open(path)?;
+ 
            .open(path)
+ 
            .map_err(|err| WriteError::open_file(path, err))?;
- 
        self.write_file(file)
+ 
        self.write_file(path, file)

    }



    /// Write to an open file.
- 
    fn write_file(&self, mut file: fs::File) -> Result<(), ConfigError> {
- 
        let _valid_config: Config = self.clone().try_into()?;
+ 
    fn write_file(&self, path: &Path, mut file: fs::File) -> Result<(), WriteError> {
+ 
        let _valid_config: Config = self
+ 
            .clone()
+ 
            .try_into()
+ 
            .map_err(|err| WriteError::validate(path, err))?;

        let formatter = json::ser::PrettyFormatter::with_indent(b"  ");

        let mut serializer = json::Serializer::with_formatter(&file, formatter);
- 
        self.0.serialize(&mut serializer)?;
- 
        file.write_all(b"\n")?;
- 
        file.sync_all()?;
+ 
        self.0
+ 
            .serialize(&mut serializer)
+ 
            .map_err(|err| WriteError::serialize_json(path, err))?;
+ 
        file.write_all(b"\n")
+ 
            .map_err(|err| WriteError::write_file(path, err))?;
+ 
        file.sync_all()
+ 
            .map_err(|err| WriteError::write_file(path, err))?;



        Ok(())

    }
@@ -268,11 +394,11 @@ impl RawConfig {
    }

}
- 
impl TryInto<Config> for RawConfig {
+ 
impl TryFrom<RawConfig> for Config {

    type Error = json::Error;
- 
    fn try_into(self) -> Result<Config, Self::Error> {
- 
        json::from_value(self.0)
+ 
    fn try_from(raw: RawConfig) -> Result<Config, Self::Error> {
+ 
        json::from_value(raw.0)

    }

}