Skip to content

Commit

Permalink
[fix] hyperledger-iroha#3130: Fix the UI/UX of missing configuration …
Browse files Browse the repository at this point in the history 
…parameters

Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
  • Loading branch information
@appetrosyan
appetrosyan committed Mar 22, 2023
1 parent 726f5ea commit 0667010
Show file tree
Hide file tree
Showing 16 changed files with 321 additions and 179 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

150 changes: 75 additions & 75 deletions cli/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
use core::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;

use color_eyre::eyre::{eyre, Result, WrapErr};
use eyre::ContextCompat as _;
use color_eyre::{
eyre::{eyre, Result, WrapErr},
Section,
};
use iroha_actor::{broker::*, prelude::*};
use iroha_config::{
base::proxy::{LoadFromDisk, LoadFromEnv, Override},
Expand All @@ -31,8 +33,9 @@ use iroha_core::{
IrohaNetwork,
};
use iroha_data_model::prelude::*;
use iroha_genesis::{GenesisNetwork, GenesisNetworkTrait, RawGenesisBlock};
use iroha_genesis::GenesisNetwork;
use iroha_p2p::network::NetworkBaseRelayOnlinePeers;
use owo_colors::OwoColorize as _;
use tokio::{
signal,
sync::{broadcast, Notify},
Expand Down Expand Up @@ -154,93 +157,25 @@ impl Handler<iroha_core::NetworkMessage> for FromNetworkBaseRelay {
}

impl Iroha {
/// To make `Iroha` peer work all actors should be started first.
/// After that moment it you can start it with listening to torii events.
///
/// # Side effect
/// - Prints welcome message in the log
///
/// # Errors
/// - Reading genesis from disk
/// - Reading telemetry configs
/// - telemetry setup
/// - Initialization of [`Sumeragi`]
#[allow(clippy::non_ascii_literal)]
pub async fn new(args: &Arguments) -> Result<Self> {
let mut config = args
.config_path
.first_existing_path()
.map_or_else(
|| {
eprintln!(
"Configuration file not found. Using environment variables as fallback."
);
ConfigurationProxy::default()
},
|path| ConfigurationProxy::from_path(&path.as_path()),
)
.override_with(ConfigurationProxy::from_env())
.build()?;

if style::should_disable_color() {
config.disable_panic_terminal_colors = true;
// Remove terminal colors to comply with XDG
// specifications, Rust's conventions as well as remove
// escape codes from logs redirected from STDOUT. If you
// need syntax highlighting, use JSON logging instead.
config.logger.terminal_colors = false;
}

let telemetry = iroha_logger::init(&config.logger)?;
iroha_logger::info!(
git_commit_sha = env!("VERGEN_GIT_SHA"),
"Hyperledgerいろは2にようこそ！(translation) Welcome to Hyperledger Iroha {}!",
env!("CARGO_PKG_VERSION")
);

let genesis = if let Some(genesis_path) = &args.genesis_path {
GenesisNetwork::from_configuration(
args.submit_genesis,
RawGenesisBlock::from_path(
genesis_path
.first_existing_path()
.wrap_err_with(|| {
format!("Genesis block file {genesis_path:?} doesn't exist")
})?
.as_ref(),
)?,
Some(&config.genesis),
&config.sumeragi.transaction_limits,
)
.wrap_err("Failed to initialize genesis.")?
} else {
None
};

Self::with_genesis(genesis, config, Broker::new(), telemetry).await
}

fn prepare_panic_hook(notify_shutdown: Arc<Notify>) {
#[cfg(not(feature = "test-network"))]
use std::panic::set_hook;

// This is a hot-fix for tests
// Hotfix for tests.
//
// # Problem
//
// When running tests in parallel `std::panic::set_hook()` will be set
// the same for all threads. That means, that panic in one test can
// cause another test shutdown, which we don't want.
//
// # Downside
// # Solution drawbacks
//
// A downside of this approach is that this panic hook will not work for
// threads created by Iroha itself (e.g. Sumeragi thread).
//
// # TODO
//
// Remove this when all Rust integrations tests will be converted to a
// separate Python tests.
// # TODO: Remove this when all Rust integrations tests will
// be converted to a separate Python tests.
#[cfg(feature = "test-network")]
use thread_local_panic_hook::set_hook;

Expand Down Expand Up @@ -497,6 +432,71 @@ impl Iroha {
}
}

/// Combine configs from as many sources as are available.
///
/// # Errors
/// If a necessary field isn't specified or specified with an error in
/// the overriding structure, report an error
pub fn combine_configs(args: &Arguments) -> Result<Configuration, color_eyre::Report> {
let config = {
let colors = style::Styling::new();
let config = args
.config_path
.first_existing_path()
.map_or_else(
|| {
eprintln!(
"Configuration file not found at {}.\n{}\n",
args.config_path.style(colors.highlight),
"Falling back to environment variables".style(colors.highlight),
);
ConfigurationProxy::default()
},
|path| ConfigurationProxy::from_path(&path.as_path()),
)
.override_with(ConfigurationProxy::from_env())
.build();

match config {
Ok(mut config) => {
if style::should_disable_color() {
config.disable_panic_terminal_colors = true;
// Remove terminal colors to comply with XDG
// specifications, Rust's conventions as well as remove
// escape codes from logs redirected from STDOUT. If you
// need syntax highlighting, use JSON logging instead.
config.logger.terminal_colors = false;
}
config
}
Err(e) => {
use iroha_config::base::derive::Error::*;

color_eyre::install()?;

return Err(match e {
UnknownField(_) => eyre!(e)
.wrap_err("Failed to combine configurations.")
.suggestion("Double check the spelling of fields"),
MissingField { message, .. } | ProvidedInferredField { message, .. } => {
eyre!(e)
.wrap_err("Failed to combine configurations.")
.suggestion(message)
}
InsaneValue { ref message, .. } => {
let msg = message.clone();
eyre!(e)
.wrap_err("Failed to combine configurations")
.suggestion(msg)
}
_ => eyre!(e).wrap_err("Failed to combine configurations."),
});
}
}
};
Ok(config)
}

fn genesis_account(public_key: iroha_crypto::PublicKey) -> Account {
Account::new(AccountId::genesis(), [public_key]).build()
}
Expand Down
77 changes: 72 additions & 5 deletions cli/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
#![allow(clippy::print_stdout)]
use std::env;

use color_eyre::eyre::WrapErr as _;
use iroha::style::Styling;
use iroha_config::path::Path as ConfigPath;
use iroha_genesis::{GenesisNetwork, GenesisNetworkTrait as _, RawGenesisBlock};
use owo_colors::OwoColorize as _;

const HELP_ARG: [&str; 2] = ["--help", "-h"];
Expand All @@ -27,6 +29,17 @@ const REQUIRED_ENV_VARS: [(&str, &str); 7] = [
];

#[tokio::main]
/// To make `Iroha` peer work all actors should be started first.
/// After that moment it you can start it with listening to torii events.
///
/// # Side effect
/// - Prints welcome message in the log
///
/// # Errors
/// - Reading genesis from disk
/// - Reading telemetry configs
/// - telemetry setup
/// - Initialization of [`Sumeragi`]
async fn main() -> Result<(), color_eyre::Report> {
let styling = Styling::new();
let mut args = iroha::Arguments::default();
Expand All @@ -43,7 +56,12 @@ async fn main() -> Result<(), color_eyre::Report> {
if env::args().any(|a| SUBMIT_ARG.contains(&a.as_str())) {
args.submit_genesis = true;
if let Ok(genesis_path) = env::var("IROHA2_GENESIS_PATH") {
args.genesis_path = Some(ConfigPath::user_provided(&genesis_path)?);
args.genesis_path =
Some(ConfigPath::user_provided(&genesis_path)
.map_err(|e| {color_eyre::install().expect("CRITICAL"); e})
.wrap_err_with(
||
format!("Could not read `{genesis_path}`, which is required, given you requested `--submit-genesis` on the command line."))?);
}
} else {
args.genesis_path = None;
Expand All @@ -52,10 +70,19 @@ async fn main() -> Result<(), color_eyre::Report> {
for arg in env::args().skip(1) {
if !arg.is_empty()
&& !([HELP_ARG, SUBMIT_ARG]
.iter()
.any(|group| group.contains(&arg.as_str())))
.iter()
.any(|group| group.contains(&arg.as_str())))
{
print_help(&styling)?;
// NOTE: because of how `color_eyre` works, we need to
// install the hook before creating any instance of
// `eyre::Report`, otherwise the `eyre` default reporting
// hook is going to be installed automatically.

// This results in a nasty repetition of the
// `color_eyre::install().unwrap()` pattern, which is the
// lesser of two evils
color_eyre::install().expect("CRITICAL");
eyre::bail!(
"Unrecognised command-line flag `{}`",
arg.style(styling.negative)
Expand All @@ -64,7 +91,12 @@ async fn main() -> Result<(), color_eyre::Report> {
}

if let Ok(config_path) = env::var("IROHA2_CONFIG_PATH") {
args.config_path = ConfigPath::user_provided(&config_path)?;
args.config_path = ConfigPath::user_provided(&config_path)
.map_err(|e| {
color_eyre::install().expect("CRITICAL");
e
})
.wrap_err_with(|| format!("Failed to parse `{config_path}` as configuration path"))?;
}
if !args.config_path.exists() {
// Require all the fields defined in default `config.json`
Expand All @@ -84,7 +116,42 @@ async fn main() -> Result<(), color_eyre::Report> {
}
}

<iroha::Iroha>::new(&args).await?.start().await?;
// <iroha::Iroha>::new(&args).await?.start().await?;
let config = iroha::combine_configs(&args)?;
let telemetry = iroha_logger::init(&config.logger)?;
iroha_logger::info!(
git_commit_sha = env!("VERGEN_GIT_SHA"),
"Hyperledgerいろは2にようこそ！(translation) Welcome to Hyperledger Iroha {}!",
env!("CARGO_PKG_VERSION")
);

let genesis = if let Some(genesis_path) = &args.genesis_path {
GenesisNetwork::from_configuration(
args.submit_genesis,
RawGenesisBlock::from_path(
genesis_path
.first_existing_path()
.ok_or({
color_eyre::install().expect("CRITICAL");
color_eyre::eyre::eyre!("Genesis block file {genesis_path:?} doesn't exist")
})?
.as_ref(),
)?,
Some(&config.genesis),
&config.sumeragi.transaction_limits,
)
.wrap_err("Failed to initialize genesis.")?
} else {
None
};

iroha::Iroha::with_genesis(
genesis,
config,
iroha_actor::broker::Broker::new(),
telemetry,
)
.await?;
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions config/base/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ license.workspace = true

[dependencies]
iroha_config_derive = { path = "derive" }
iroha_crypto = { version = "=2.0.0-pre-rc.13", path = "../../crypto" }

serde = { version = "1.0.151", default-features = false, features = ["derive"] }
serde_json = "1.0.91"
Expand Down
8 changes: 7 additions & 1 deletion config/base/derive/src/documented.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,13 @@ fn impl_get_recursive(ast: &StructWithFields) -> proc_macro2::TokenStream {
quote! {
[stringify!(#ident)] => {
serde_json::to_value(&#l_value)
.map_err(|e| ::iroha_config_base::derive::Error::field_error(stringify!(#ident), e))?
.map_err(
|error|
::iroha_config_base::derive::Error::FieldDeserialization {
field: stringify!(#ident),
error
}
)?
}
#inner_thing2
}
Expand Down
13 changes: 7 additions & 6 deletions config/base/derive/src/proxy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub fn impl_load_from_env(ast: &StructWithFields) -> TokenStream {
false
};
let err_ty = quote! { ::iroha_config_base::derive::Error };
let err_variant = quote! { ::iroha_config_base::derive::Error::SerdeError };
let err_variant = quote! { ::iroha_config_base::derive::Error::Json5 };
let inner = if is_string {
quote! { Ok(var) }
} else if as_str_attr {
Expand Down Expand Up @@ -152,8 +152,8 @@ pub fn impl_load_from_disk(ast: &StructWithFields) -> TokenStream {
let proxy_name = &ast.ident;
let disk_trait = quote! { ::iroha_config_base::proxy::LoadFromDisk };
let error_ty = quote! { ::iroha_config_base::derive::Error };
let disk_err_variant = quote! { ::iroha_config_base::derive::Error::DiskError };
let serde_err_variant = quote! { ::iroha_config_base::derive::Error::SerdeError };
let disk_err_variant = quote! { ::iroha_config_base::derive::Error::Disk };
let serde_err_variant = quote! { ::iroha_config_base::derive::Error::Json5 };
let none_proxy = gen_none_fields_proxy(ast);
quote! {
impl #disk_trait for #proxy_name {
Expand Down Expand Up @@ -243,21 +243,22 @@ pub fn impl_build(ast: &StructWithFields) -> TokenStream {
fn gen_none_fields_check(ast: &StructWithFields) -> proc_macro2::TokenStream {
let checked_fields = ast.fields.iter().map(|field| {
let ident = &field.ident;
let err_variant = quote! { ::iroha_config_base::derive::Error::ProxyBuildError };
let missing_field = quote! { ::iroha_config_base::derive::Error::MissingField };
if field.has_inner {
let inner_ty = get_inner_type("Option", &field.ty);
let builder_trait = quote! { ::iroha_config_base::proxy::Builder };
quote! {
#ident: <#inner_ty as #builder_trait>::build(
self.#ident.ok_or(
#err_variant(stringify!(#ident).to_owned())
#missing_field{field: stringify!(#ident), message: ""}
)?
)?
}
} else {
quote! {
#ident: self.#ident.ok_or(
#err_variant(stringify!(#ident).to_owned()))?
#missing_field{field: stringify!(#ident), message: ""}
)?
}
}
});
Expand Down
Loading

0 comments on commit 0667010

Please sign in to comment.