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

fix: log config parsing errors as errors #2739

Merged
merged 2 commits into from
Dec 19, 2024
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 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 crates/pixi_config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ reqwest-middleware = { workspace = true }
serde = { workspace = true }
serde_ignored = { workspace = true }
serde_json = { workspace = true }
thiserror = { workspace = true }
toml_edit = { workspace = true, features = ["serde"] }
tracing = { workspace = true }
url = { workspace = true }
Expand Down
50 changes: 33 additions & 17 deletions crates/pixi_config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,18 @@ impl From<&Config> for rattler_repodata_gateway::ChannelConfig {
}
}

#[derive(thiserror::Error, Debug)]
pub enum ConfigError {
#[error("no file was found at {0}")]
FileNotFound(PathBuf),
#[error("failed to read config from '{0}'")]
ReadError(std::io::Error),
#[error("failed to parse config of {1}: {0}")]
ParseError(miette::Report, PathBuf),
#[error("validation error of {1}: {0}")]
ValidationError(miette::Report, PathBuf),
}

impl Config {
/// Constructs a new config that is optimized to be used in tests.
///
Expand Down Expand Up @@ -745,13 +757,18 @@ impl Config {
/// # Errors
///
/// I/O errors or parsing errors
pub fn from_path(path: &Path) -> miette::Result<Config> {
pub fn from_path(path: &Path) -> Result<Config, ConfigError> {
tracing::debug!("Loading config from {}", path.display());
let s = fs_err::read_to_string(path)
.into_diagnostic()
.wrap_err(format!("failed to read config from '{}'", path.display()))?;
let s = match fs_err::read_to_string(path) {
Ok(content) => content,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
return Err(ConfigError::FileNotFound(path.to_path_buf()))
}
Err(e) => return Err(ConfigError::ReadError(e)),
};

let (mut config, unused_keys) = Config::from_toml(&s)?;
let (mut config, unused_keys) =
Config::from_toml(&s).map_err(|e| ConfigError::ParseError(e, path.to_path_buf()))?;

if !unused_keys.is_empty() {
tracing::warn!(
Expand All @@ -771,7 +788,9 @@ impl Config {
config.loaded_from.push(path.to_path_buf());
tracing::info!("Loaded config from: {}", path.display());

config.validate()?;
config
.validate()
.map_err(|e| ConfigError::ValidationError(e, path.to_path_buf()))?;

Ok(config)
}
Expand All @@ -785,7 +804,7 @@ impl Config {
/// # Errors
///
/// I/O errors or parsing errors
pub fn try_load_system() -> miette::Result<Config> {
pub fn try_load_system() -> Result<Config, ConfigError> {
Self::from_path(&config_path_system())
}

Expand All @@ -796,12 +815,11 @@ impl Config {
/// The loaded system config
pub fn load_system() -> Config {
Self::try_load_system().unwrap_or_else(|e| {
let path = config_path_system();
tracing::debug!(
"Failed to load system config: {} (error: {})",
path.display(),
e
);
match e {
ConfigError::FileNotFound(_) => (), // it's fine that no file is there
e => tracing::error!("{e}"),
}

Self::default()
})
}
Expand Down Expand Up @@ -835,12 +853,10 @@ impl Config {
let mut config = Self::load_system();

for p in config_path_global() {
if !p.is_file() {
continue;
}
match Self::from_path(&p) {
Ok(c) => config = config.merge_config(c),
Err(e) => tracing::warn!(
Err(ConfigError::FileNotFound(_)) => (),
Err(e) => tracing::error!(
"Failed to load global config '{}' with error: {}",
p.display(),
e
Expand Down
Loading