From 9869ddaa4bd72f4c6ff261cf932176fd5f70a679 Mon Sep 17 00:00:00 2001 From: fanyang Date: Sun, 21 Jun 2026 21:56:49 +0800 Subject: [PATCH] fix: clarify config parse errors (#2360) * fix: improve config parse diagnostics * fix: polish config error context * test: cover non-ascii config diagnostics --- Cargo.lock | 11 +++ easytier/Cargo.toml | 1 + easytier/src/common/config.rs | 164 +++++++++++++++++++++++++++++++--- easytier/src/core.rs | 15 ++-- 4 files changed, 170 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 59f33927..0ca21153 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -241,6 +241,16 @@ dependencies = [ "password-hash", ] +[[package]] +name = "ariadne" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "36f5e3dca4e09a6f340a61a0e9c7b61e030c69fc27bf29d73218f7e5e3b7638f" +dependencies = [ + "unicode-width 0.1.11", + "yansi", +] + [[package]] name = "arrayvec" version = "0.7.6" @@ -2234,6 +2244,7 @@ dependencies = [ "aes-gcm", "anyhow", "arc-swap", + "ariadne", "async-recursion", "async-ringbuf", "async-stream", diff --git a/easytier/Cargo.toml b/easytier/Cargo.toml index 15d80243..19e560ce 100644 --- a/easytier/Cargo.toml +++ b/easytier/Cargo.toml @@ -134,6 +134,7 @@ prost-wkt-types = "0.7.1" pbjson = "0.9.0" anyhow = "1.0" +ariadne = "0.5" url = { version = "2.5", features = ["serde"] } percent-encoding = "2.3.1" diff --git a/easytier/src/common/config.rs b/easytier/src/common/config.rs index b057122d..d3f2619e 100644 --- a/easytier/src/common/config.rs +++ b/easytier/src/common/config.rs @@ -6,6 +6,7 @@ use std::{ }; use anyhow::Context; +use ariadne::{CharSet, Config as AriadneConfig, IndexType, Label, Report, ReportKind, Source}; use base64::{Engine as _, prelude::BASE64_STANDARD}; use clap::ValueEnum; use clap::builder::PossibleValue; @@ -569,6 +570,35 @@ struct Config { source: Option, } +fn format_toml_parse_error(source_name: &str, config_str: &str, error: &toml::de::Error) -> String { + let message = format!("failed to parse config TOML from {source_name}"); + + let Some(span) = error.span() else { + return format!("{message}\ndetail: {error}"); + }; + + let mut output = Vec::new(); + let report = Report::build(ReportKind::Error, (source_name, span.clone())) + .with_config( + AriadneConfig::default() + .with_color(false) + .with_char_set(CharSet::Ascii) + .with_index_type(IndexType::Byte), + ) + .with_message(&message) + .with_label(Label::new((source_name, span)).with_message(error.message())) + .finish(); + + if report + .write((source_name, Source::from(config_str)), &mut output) + .is_ok() + { + String::from_utf8_lossy(&output).into_owned() + } else { + format!("{message}\ndetail: {error}") + } +} + #[derive(Debug, Clone)] pub struct TomlConfigLoader { config: Arc>, @@ -591,11 +621,35 @@ impl TomlConfigLoader { } pub fn new_from_str(config_str: &str) -> Result { - let mut config = toml::de::from_str::(config_str) - .with_context(|| format!("failed to parse config file: {}", config_str))?; + Self::new_from_str_with_source("inline config", config_str) + } + + pub fn new(config_path: &PathBuf) -> Result { + let config_str = std::fs::read_to_string(config_path) + .with_context(|| format!("failed to read config file: {}", config_path.display()))?; + + let source_name = config_path.display().to_string(); + Self::new_from_str_with_source(&source_name, &config_str) + } + + pub(crate) fn new_from_str_with_source( + source_name: &str, + config_str: &str, + ) -> Result { + let mut config = toml::de::from_str::(config_str).map_err(|err| { + let message = format_toml_parse_error(source_name, config_str, &err); + anyhow::Error::new(err).context(message) + })?; Self::normalize_config_source(&mut config); + Self::new_from_config(config).map_err(|err| { + let message = format!("failed to load config from {source_name}: {err}"); + err.context(message) + }) + } + + fn new_from_config(mut config: Config) -> Result { config.flags_struct = Some( Self::gen_flags(config.flags.clone().unwrap_or_default()) .context("failed to parse flags")?, @@ -631,14 +685,6 @@ impl TomlConfigLoader { Ok(config) } - pub fn new(config_path: &PathBuf) -> Result { - let config_str = std::fs::read_to_string(config_path) - .with_context(|| format!("failed to read config file: {:?}", config_path))?; - let ret = Self::new_from_str(&config_str)?; - - Ok(ret) - } - fn gen_flags( flags_hashmap: serde_json::Map, ) -> serde_json::Result { @@ -1204,13 +1250,13 @@ pub async fn load_config_from_file( .read_to_string(&mut stdin) .await .context("failed to read config from stdin")?; - let config = TomlConfigLoader::new_from_str(&stdin)?; + let config = TomlConfigLoader::new_from_str_with_source("stdin", &stdin)?; return Ok((config, ConfigFileControl::STATIC_CONFIG)); } let config_str = tokio::fs::read_to_string(config_file) .await - .with_context(|| format!("failed to read config file: {:?}", config_file))?; + .with_context(|| format!("failed to read config file: {}", config_file.display()))?; let (expanded_config_str, uses_env_vars) = if disable_env_parsing { (config_str.clone(), false) @@ -1232,8 +1278,8 @@ pub async fn load_config_from_file( ); } - let config = TomlConfigLoader::new_from_str(&expanded_config_str) - .with_context(|| format!("failed to load config file: {:?}", config_file))?; + let source_name = config_file.display().to_string(); + let config = TomlConfigLoader::new_from_str_with_source(&source_name, &expanded_config_str)?; let mut control = ConfigFileControl::from_path(config_file.clone()).await; @@ -1273,6 +1319,96 @@ pub mod tests { use std::path::PathBuf; use tempfile::NamedTempFile; + #[test] + fn invalid_toml_error_includes_location_and_source_line() { + let error = TomlConfigLoader::new_from_str("dhcp = \"yes\"").unwrap_err(); + let display = error.to_string(); + + assert!(display.contains("failed to parse config TOML")); + assert!(display.contains("inline config")); + assert!(display.contains("dhcp = \"yes\"")); + assert!(display.contains("^")); + assert!(display.contains("invalid type: string")); + assert!(!display.contains("")); + assert!( + error + .chain() + .any(|err| err.downcast_ref::().is_some()) + ); + } + + #[test] + fn invalid_file_toml_error_includes_config_source() { + let mut config_file = NamedTempFile::new().unwrap(); + writeln!(config_file, "dhcp = \"yes\"").unwrap(); + + let error = TomlConfigLoader::new(&config_file.path().to_path_buf()).unwrap_err(); + let error = error.to_string(); + + assert!(error.contains(config_file.path().to_string_lossy().as_ref())); + assert!(error.contains("failed to parse config TOML")); + assert!(error.contains("dhcp = \"yes\"")); + assert!(error.contains("^")); + assert!(error.contains("invalid type: string")); + assert!(!error.contains("")); + } + + #[test] + fn invalid_stdin_toml_error_includes_config_source_in_display() { + let error = TomlConfigLoader::new_from_str_with_source("stdin", "dhcp = \"yes\"") + .unwrap_err() + .to_string(); + + assert!(error.contains("stdin")); + assert!(error.contains("failed to parse config TOML")); + assert!(error.contains("dhcp = \"yes\"")); + assert!(error.contains("^")); + assert!(error.contains("invalid type: string")); + assert!(!error.contains("")); + } + + #[test] + fn invalid_toml_error_handles_non_ascii_before_error() { + let error = TomlConfigLoader::new_from_str("hostname = \"节点\"\ndhcp = \"yes\"") + .unwrap_err() + .to_string(); + + assert!(error.contains("dhcp = \"yes\"")); + assert!(error.contains("^")); + assert!(error.contains("invalid type: string")); + } + + #[test] + fn invalid_toml_error_handles_non_ascii_before_error_on_same_line() { + let error = TomlConfigLoader::new_from_str("hostname = \"节点\" dhcp = \"yes\"") + .unwrap_err() + .to_string(); + + assert!(error.contains("failed to parse config TOML")); + assert!(error.contains("inline config:1:")); + assert!(error.contains("hostname = \"节点\" dhcp = \"yes\"")); + assert!(error.contains("expected newline")); + assert!(!error.contains("")); + } + + #[test] + fn invalid_file_flags_error_includes_config_source_in_display() { + let mut config_file = NamedTempFile::new().unwrap(); + writeln!(config_file, "[flags]").unwrap(); + writeln!(config_file, "socket_mark = \"bad\"").unwrap(); + + let error = TomlConfigLoader::new(&config_file.path().to_path_buf()).unwrap_err(); + + let display = error.to_string(); + assert!(display.contains(config_file.path().to_string_lossy().as_ref())); + assert!(display.contains("failed to load config")); + assert!(display.contains("failed to parse flags")); + + // with_context preserves the cause chain so callers can inspect the root reason. + let chain: Vec = error.chain().map(|e| e.to_string()).collect(); + assert!(chain.iter().any(|m| m.contains("failed to parse flags"))); + } + #[test] fn socket_mark_config_file_roundtrip_none_some_and_zero() { // Omitting the flag leaves socket_mark unset (None) -> SO_MARK untouched. diff --git a/easytier/src/core.rs b/easytier/src/core.rs index ed014faa..b2752bc9 100644 --- a/easytier/src/core.rs +++ b/easytier/src/core.rs @@ -1614,7 +1614,7 @@ pub async fn main() -> ExitCode { // Verify configurations if cli.check_config { if let Err(error) = validate_config(&cli).await { - log::error!(?error, "Config validation failed"); + log::error!(%error, "Config validation failed"); return ExitCode::FAILURE; } else { return ExitCode::SUCCESS; @@ -1624,7 +1624,7 @@ pub async fn main() -> ExitCode { let mut ret_code = 0; if let Err(error) = run_main(cli).await { - log::error!(?error); + log::error!(%error); ret_code = 1; } @@ -1644,12 +1644,13 @@ async fn validate_config(cli: &Cli) -> anyhow::Result<()> { for config_file in config_files { if config_file == &PathBuf::from("-") { let mut stdin = String::new(); - _ = tokio::io::stdin().read_to_string(&mut stdin).await?; - TomlConfigLoader::new_from_str(stdin.as_str()) - .with_context(|| "config source: stdin")?; + _ = tokio::io::stdin() + .read_to_string(&mut stdin) + .await + .context("failed to read config from stdin")?; + TomlConfigLoader::new_from_str_with_source("stdin", stdin.as_str())?; } else { - TomlConfigLoader::new(config_file) - .with_context(|| format!("config source: {:?}", config_file))?; + TomlConfigLoader::new(config_file)?; }; }