From cfdcd983e075a8668c854445797390a8e69c85e6 Mon Sep 17 00:00:00 2001 From: Kristofers Solo Date: Wed, 11 Feb 2026 17:47:09 +0200 Subject: [PATCH] feat(runner): add detailed TOML config validation with miette errors - Use miette's NamedSource and SourceSpan for precise error locations - Implement field-level validation (mode, payload, iters, concurrency) - Show exact line and field causing validation errors in CLI output --- runner/src/config.rs | 193 +++++++++++++++++++++++++++++++++++++++++-- runner/src/error.rs | 61 +++++++++++++- 2 files changed, 248 insertions(+), 6 deletions(-) diff --git a/runner/src/config.rs b/runner/src/config.rs index 9c8d0f8..b906f83 100644 --- a/runner/src/config.rs +++ b/runner/src/config.rs @@ -1,7 +1,11 @@ -use crate::{args::Args, error}; +use crate::{ + args::Args, + error::{self, ConfigError}, +}; use common::{self, KeyExchangeMode}; +use miette::{NamedSource, SourceSpan}; use serde::Deserialize; -use std::{fs::read_to_string, net::SocketAddr, path::PathBuf}; +use std::{fs::read_to_string, net::SocketAddr, path::Path}; #[derive(Debug, Clone, Deserialize)] pub struct BenchmarkConfig { @@ -22,9 +26,28 @@ pub struct Config { /// /// # Errors /// Returns an error if the file cannot be read or parsed. -pub fn load_from_file(path: &PathBuf) -> error::Result { - let content = read_to_string(path).map_err(common::Error::Io)?; - let config = toml::from_str::(&content).map_err(common::Error::Toml)?; +pub fn load_from_file(path: &Path) -> error::Result { + let content = read_to_string(path).map_err(|source| ConfigError::ReadError { + source, + path: path.to_owned(), + })?; + + let src = NamedSource::new(path.display().to_string(), content.clone()); + + let config = toml::from_str::(&content).map_err(|source| { + let span = source + .span() + .map(|s| SourceSpan::new(s.start.into(), s.end - s.start)); + + ConfigError::TomlParseError { + src: src.clone(), + span, + source, + } + })?; + + validate_config(&config, &content, path)?; + Ok(config) } @@ -57,3 +80,163 @@ impl Config { .unwrap_or(KeyExchangeMode::X25519) } } + +/// Validate the configuration after parsing. +fn validate_config(config: &Config, content: &str, path: &Path) -> error::Result<()> { + if config.benchmarks.is_empty() { + return Err(ConfigError::EmptyBenchmarks { + src: NamedSource::new(path.display().to_string(), content.to_string()), + } + .into()); + } + + for (idx, benchmark) in config.benchmarks.iter().enumerate() { + validate_benchmark(benchmark, idx, content, path)?; + } + + Ok(()) +} + +/// Validate a single benchmark configuration. +fn validate_benchmark( + benchmark: &BenchmarkConfig, + idx: usize, + content: &str, + path: &Path, +) -> error::Result<()> { + let src = NamedSource::new(path.display().to_string(), content.to_string()); + + // Validate mode + if benchmark.mode.parse::().is_err() { + return Err(ConfigError::ValidationError { + src, + span: find_field_span(content, idx, "mode"), + field: "mode".into(), + idx, + message: format!( + "Invalid key exchange mode '{}'. Valid values: 'x25519', 'x25519mlkem768'", + benchmark.mode + ), + } + .into()); + } + + if benchmark.payload == 0 { + return Err(ConfigError::ValidationError { + src, + span: find_field_span(content, idx, "payload"), + field: "payload".into(), + idx, + message: "Must be greater than 0".into(), + } + .into()); + } + + if benchmark.iters == 0 { + return Err(ConfigError::ValidationError { + src, + span: find_field_span(content, idx, "iters"), + field: "iters".into(), + idx, + message: "Must be greater than 0".into(), + } + .into()); + } + + if benchmark.concurrency == 0 { + return Err(ConfigError::ValidationError { + src, + span: find_field_span(content, idx, "concurrency"), + field: "concurrency".into(), + idx, + message: "Must be greater than 0".into(), + } + .into()); + } + + Ok(()) +} + +/// Find the span of a field in the TOML content for better error reporting. +fn find_field_span(content: &str, benchmark_idx: usize, field_name: &str) -> Option { + let benchmark_section = find_benchmakr_section(content, benchmark_idx)?; + find_field_in_section(content, benchmark_section, field_name) +} + +fn find_benchmakr_section(content: &str, target_idx: usize) -> Option<(usize, usize)> { + let mut current_idx = 0; + let mut section_start = None; + let mut byte_offset = 0; + + for line in content.lines() { + let line_len = line.len() + 1; // +1 for newline + + if line.trim_start().starts_with("[[benchmarks]]") { + if current_idx == target_idx { + section_start = Some(byte_offset); + } else if section_start.is_some() { + return Some((section_start?, byte_offset)); + } + current_idx += 1; + } + byte_offset += line_len; + } + section_start.map(|start| (start, content.len())) +} + +fn find_field_in_section( + content: &str, + (start, end): (usize, usize), + field_name: &str, +) -> Option { + let section = &content[start..end]; + let mut byte_offset = start; + + for line in section.lines() { + let line_len = line.len() + 1; // +1 for newline + if line.trim_start().starts_with('[') && byte_offset != start { + break; + } + + if let Some(span) = extract_field_value_span(line, field_name, byte_offset) { + return Some(span); + } + + byte_offset += line_len; + } + + None +} + +fn extract_field_value_span( + line: &str, + field_name: &str, + line_offset: usize, +) -> Option { + let eq_idx = line.find('=')?; + let field = line[..eq_idx].trim(); + + if field != field_name { + return None; + } + + let after_eq = &line[eq_idx + 1..]; + let value_start_offset = after_eq.len() - after_eq.trim_start().len(); + let value_str = after_eq.trim_start(); + + let value_start = line_offset + eq_idx + 1 + value_start_offset; + let value_len = calculate_value_len(value_str); + + Some(SourceSpan::new(value_start.into(), value_len)) +} + +fn calculate_value_len(value_str: &str) -> usize { + if let Some(stripped) = value_str.strip_prefix('"') { + return stripped.find('"').map_or(value_str.len(), |i| i + 2); + } + + value_str.find('#').map_or_else( + || value_str.trim_end().len(), + |i| value_str[..i].trim_end().len(), + ) +} diff --git a/runner/src/error.rs b/runner/src/error.rs index 72ce337..ab405eb 100644 --- a/runner/src/error.rs +++ b/runner/src/error.rs @@ -1,6 +1,8 @@ +#![allow(unused_assignments)] //! Error types for the benchmark runner. -use miette::Diagnostic; +use miette::{Diagnostic, NamedSource, SourceSpan}; +use std::path::PathBuf; use thiserror::Error; /// Result type using the runner's custom error type. @@ -13,6 +15,10 @@ pub enum Error { #[diagnostic(code(runner::common_error))] Common(#[from] common::Error), + #[error(transparent)] + #[diagnostic(transparent)] + Config(#[from] Box), + /// Network connection failure. #[error("Network error: {0}")] #[diagnostic(code(runner::network_error))] @@ -26,3 +32,56 @@ impl Error { Self::Network(error.into()) } } + +#[derive(Debug, Error, Diagnostic)] +pub enum ConfigError { + #[error("Failed to read config file: {path}")] + #[diagnostic( + code(config::read_error), + help("Make sure the file exists and you have read permissions") + )] + ReadError { + #[source] + source: std::io::Error, + path: PathBuf, + }, + + #[error("Invalid TOML syntax in config file")] + #[diagnostic(code(config::toml_parse_error))] + TomlParseError { + #[source_code] + src: NamedSource, + #[label("here")] + span: Option, + #[source] + source: toml::de::Error, + }, + + #[error("Invalid value for field '{field}' in benchmarks[{idx}]")] + #[diagnostic(code(config::validation_error))] + ValidationError { + #[source_code] + src: NamedSource, + #[label("{message}")] + span: Option, + field: String, + idx: usize, + message: String, + }, + + #[error("Configuration must contain at least one [[benchmarks]] entry")] + #[diagnostic( + code(config::empty_benchmarks), + help("Add at least one [[benchmarks]] section to your config file") + )] + EmptyBenchmarks { + #[source_code] + src: NamedSource, + }, +} + +impl From for Error { + fn from(err: ConfigError) -> Self { + Self::Config(Box::new(err)) + } +}