fix: clarify config parse errors (#2360)

* fix: improve config parse diagnostics
* fix: polish config error context
* test: cover non-ascii config diagnostics
This commit is contained in:
fanyang
2026-06-21 21:56:49 +08:00
committed by GitHub
parent 5ea6766238
commit 9869ddaa4b
4 changed files with 170 additions and 21 deletions
Generated
+11
View File
@@ -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",
+1
View File
@@ -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"
+150 -14
View File
@@ -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<ConfigSourceConfig>,
}
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<Mutex<Config>>,
@@ -591,11 +621,35 @@ impl TomlConfigLoader {
}
pub fn new_from_str(config_str: &str) -> Result<Self, anyhow::Error> {
let mut config = toml::de::from_str::<Config>(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<Self, anyhow::Error> {
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<Self, anyhow::Error> {
let mut config = toml::de::from_str::<Config>(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<Self, anyhow::Error> {
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<Self, anyhow::Error> {
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<String, serde_json::Value>,
) -> serde_json::Result<Flags> {
@@ -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("<unknown>"));
assert!(
error
.chain()
.any(|err| err.downcast_ref::<toml::de::Error>().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("<unknown>"));
}
#[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("<unknown>"));
}
#[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("<unknown>"));
}
#[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<String> = 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.
+8 -7
View File
@@ -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)?;
};
}