Skip to content

feat: better error message for having no env file #15

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.

Sign up for GitHub

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

Merged
QuantumExplorer merged 4 commits into from
Oct 22, 2024
Merged

feat: better error message for having no env file #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7fd26b9
feat: better error message for having no env file
pauldelucia Oct 22, 2024
d2197c3
feat: better error message
pauldelucia Oct 22, 2024
d88614b
allow app to load with only mainnet configs
pauldelucia Oct 22, 2024
2876f15
better error messages
pauldelucia Oct 22, 2024
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
11 changes: 9 additions & 2 deletions src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,15 @@ impl AppState {

let settings = db.get_settings().expect("expected to get settings");

let mainnet_app_context =
AppContext::new(Network::Dash, db.clone()).expect("expected Dash config for mainnet");
let mainnet_app_context = match AppContext::new(Network::Dash, db.clone()) {
Some(context) => context,
None => {
eprintln!(
"Error: Failed to create the AppContext. Expected Dash config for mainnet."
);
std::process::exit(1);
}
};
let testnet_app_context = AppContext::new(Network::Testnet, db.clone());

let mut identities_screen = IdentitiesScreen::new(&mainnet_app_context);
Expand Down
26 changes: 23 additions & 3 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ impl Config {
}
}

#[derive(Debug, thiserror::Error)]
pub enum ConfigError {
#[error("{0}")]
LoadError(String),
#[error("No valid network configurations found in .env file or environment variables")]
NoValidConfigs,
}

#[derive(Debug, Deserialize, Clone)]
pub struct NetworkConfig {
/// Hostname of the Dash Platform node to connect to
Expand All @@ -49,7 +57,7 @@ pub struct NetworkConfig {

impl Config {
/// Loads the configuration for all networks from environment variables and `.env` file.
pub fn load() -> Self {
pub fn load() -> Result<Self, ConfigError> {
// Load the .env file if available
if let Err(err) = dotenvy::from_path(".env") {
tracing::warn!(
Expand Down Expand Up @@ -83,10 +91,22 @@ impl Config {
}
};

Config {
if mainnet_config.is_none() && testnet_config.is_none() {
return Err(ConfigError::NoValidConfigs);
} else if mainnet_config.is_none() {
return Err(ConfigError::LoadError(
"Failed to load mainnet configuration".into(),
));
} else if testnet_config.is_none() {
tracing::warn!(
"Failed to load testnet configuration, but successfully loaded mainnet config"
);
}

Ok(Config {
mainnet_config,
testnet_config,
}
})
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ pub struct AppContext {

impl AppContext {
pub fn new(network: Network, db: Arc<Database>) -> Option<Arc<Self>> {
let config = Config::load();
let config = match Config::load() {
Ok(config) => config,
Err(e) => {
println!("Failed to load config: {e}");
return None;
}
};
Comment on lines +38 to +44

@coderabbitai coderabbitai Bot Oct 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Inconsistent handling of None when loading Testnet config. Ensure that AppContext::new() failure is handled appropriately.

  • Consider handling None for testnet_app_context similarly to mainnet_app_context.
🔗 Analysis chain

Improved error handling for config loading. Consider enhancing error reporting.

The addition of error handling for Config::load() is a good improvement. It prevents the application from proceeding with an invalid configuration.

Consider using a proper logging mechanism instead of println! for error reporting. This would provide more flexibility in how errors are handled and logged in different environments.

Example:

use log::error;

// ...

Err(e) => {
    error!("Failed to load config: {}", e);
    return None;
}

Let's verify the impact of returning None when config loading fails:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check how `None` is handled when `AppContext::new()` is called

# Search for calls to AppContext::new and their error handling
rg -n "AppContext::new\(" -A 10

Length of output: 1319


let network_config = config.config_for_network(network).clone()?;

Expand Down
5 changes: 5 additions & 0 deletions src/ui/network_chooser_screen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ impl NetworkChooserScreen {
// Display status indicator
ui.colored_label(status_color, if is_working { "Online" } else { "Offline" });

if network == Network::Testnet && self.testnet_app_context.is_none() {
ui.label("(No configs for testnet loaded)");
return AppAction::None;
}

// Display wallet count
let wallet_count = format!(
"{}",
Expand Down