Enhance configuration errors

[0x009922]

This might be probably solved as part of #4297.

Points:

• Diagnostics with source spans, hints etc
• Improve the way multiple errors are collected
• Possibly, use an in-house error type instead of an umbrella eyre::Report.

[0x009922]

error_stack might be a good option too.

[0x009922]

Alternative design

So far, I have come up with the following design.

Imagine we have a simplified configuration, having the following structure:

#[derive(Deserialize, Debug)]  
pub struct Root {  
    pub chain_id: String,  
    pub torii: Torii,  
    pub kura: Kura,  
    pub telemetry: Telemetry,  
    pub logger: Logger,  
}  
  
#[derive(Deserialize, Debug)]  
pub struct Torii {  
    pub address: WithOrigin<SocketAddr>,  
    pub max_content_len: u64,  
}  
  
#[derive(Deserialize, Debug)]  
pub struct Kura {  
    pub store_dir: WithOrigin<PathBuf>,  
    pub debug_force: bool,  
}  
  
#[derive(Deserialize, Debug)]  
pub struct Telemetry {  
    pub out_file: Option<WithOrigin<PathBuf>>,  
}  
  
#[derive(Deserialize, Debug)]  
pub struct Logger {  
    pub level: LogLevel,  
}  
  
#[derive(Deserialize, Debug, Default)]  
pub enum LogLevel {  
    Debug,  
    #[default]  
    Info,  
    Warning,  
    Error,  
}

The only new thing here so far is WithOrigin<T> struct. It encapsulates the read value with information about where this value comes from, using something like ParameterOrigin:

pub enum ParameterOrigin {  
    File { path: PathBuf, name: String },  
    Env { key: String, name: String },  
    Default { name: String },  
}  
  
pub struct WithOrigin<T> { /* ... */ }

This ParameterOrigin might be used for the following cases:

• Primarily, to be able to produce useful warnings/errors later, in parts of Iroha which are far from configuration parsing. For example, when Torii fails to bind to a specific address (failed to listen to localhost:1337), it will be able to also say

  btw this value comes from torii.address parameter set in iroha.toml file
  ...or API_ADDRESS env var
  ...or we used a default value for this parameter since it wasn't set in config files/ENV. You can use API_ADDRESS env var btw.

• Secondarily: for WithOrigin<PathBuf>, it gives an easy way to resolve the inner PathBuf relative to the originating file, if it was defined in a file.

The second part of my idea is to abandon Partials, work with native format values (e.g. toml::Value), and use a declarative builder approach similar to how clap works. Here is how my todo PoC looks like: (here we compose Root - the user configuration view)

impl Root {  
    pub fn compose(path: impl AsRef<Path>) -> Result<Self, ReadConfigError> {  
        let reader =  
            // reads the file by the path as well as a whole "extends" chain  
            ConfigReader::new(Some(path))?;  
  
        let (chain_id, reader) = reader  
            .parameter::<String, _>(["chain_id"])  
            .env("CHAIN_ID")  
            .value_required()  
            .finish();  
  
        let (torii_address, reader) = reader  
            .parameter::<SocketAddr, _>(["torii", "address"])  
            .env("API_ADDRESS")  
            .value_or_else(|| todo!())  
            .finish_with_origin();  
  
        let (torii_max_content_len, reader) = reader  
            .parameter::<u64, _>(["torii", "max_content_length"])  
            .value_or(1024)  
            .finish();  
  
        // origin needed so that we can resolve the path relative to the origin  
        let (kura_store_dir, reader) = reader  
            .parameter::<PathBuf, _>(["kura", "store_dir"])  
            .env("KURA_STORE_DIR")  
            .value_or_else(|| PathBuf::from("./storage"))  
            .finish_with_origin();  
  
        let (kura_debug_force, reader) = reader  
            .parameter::<bool, _>(["kura", "debug_force"])  
            .value_or(false)  
            .finish();  
  
        let (log_level, reader) = reader  
            .parameter::<LogLevel, _>(["logger", "level"])  
            .env("LOG_LEVEL")  
            .value_or_default()  
            .finish();  
  
        // origin needed so that we can resolve the path relative to the origin  
        let (telemetry_out_file, reader) = reader  
            .parameter::<PathBuf, _>(["telemetry", "dev", "out_file"])  
            .value_optional()  
            .finish_with_origin();  
  
        reader.finish()?;  
  
        // Done! _User_ layer of the config might be successfully composed now  
  
        Ok(Self {  
            chain_id: chain_id.unwrap(),  
            torii: Torii {  
                address: torii_address,  
                max_content_len: torii_max_content_len,  
            },  
            kura: Kura {  
                store_dir: kura_store_dir,  
                debug_force: kura_debug_force,  
            },  
            telemetry: Telemetry {  
                out_file: telemetry_out_file,  
            },  
            logger: Logger { level: log_level },  
        })  
    }  
}

With this design, there are the following things done by ConfigReader under the hood:

• it reads a chain of *.toml files by the path, expanding their root extends field. They are stored as native toml::Tables;
• it tracks all accessed parameters (chain_id, torii.address etc), their ENVs, whether they have default values;
• whenever a parameter is accessed, it runs through all read sources, and logs where a certain parameter is defined, overwritten, fallen back to a default value; see Implement config tracing #4300
• it tracks all required parameters which aren't found anywhere, and emits them all at once on finish()?
• it tracks all extra, unaccessed parameters, and emits them all at once on finish()?
• for each parameter, if a deserialization error occurs, it also stores it for future, always returning an Option<T>. Then, all such errors are emitted all at once on finish()?

Benefits:

• it is possible to get lots of errors in a single run
• builder design is friendly for proc macro
• even without a proc macro, the boilerplate amount is much less than it is now
• builder is declarative, it is easy to observe/patch the config behaviour

Downsides:

• It is heavier than just using serde, I suppose
• Might be difficult to implement optimally (expect to see many allocations internally), but that's not a goal, at least now

What is not included here:

• exact locations in TOML files. Although it possibly might be implemented with serde_spanned and custom top-level TOML structures (maps and arrays, specifically)
• fancy ariadne reports with labels in the exact config files are also omitted

[0x009922]

Closed by #4456