Skip to content

Commit 08b89be

Browse files
committed
fix(tracker-client): surface JSON parse detail in checker config errors (#1042)
- Added `error.rs` with `ConfigSource` and `AppError` types that decouple delivery mechanism from error presentation - Replaced generic `.context("invalid config format")` with source-aware `AppError::InvalidConfig` - Binary no longer panics on config errors; prints JSON envelope to stderr and exits with code 2 (config) or 1 (runtime) - Follows Tracker CLI I/O Contract: `{"error":{"kind":"...","source":"...","message":"..."}}` - Added 12 unit tests and 9 integration tests; all 44 tests pass
1 parent a0476ea commit 08b89be

8 files changed

Lines changed: 442 additions & 47 deletions

File tree

console/tracker-client/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,6 @@ url = { version = "2", features = [ "serde" ] }
3737

3838
[package.metadata.cargo-machete]
3939
ignored = [ "serde_bytes" ]
40+
41+
[dev-dependencies]
42+
tempfile = "3"

console/tracker-client/src/bin/tracker_checker.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,9 @@ use torrust_tracker_client::console::clients::checker::app;
33

44
#[tokio::main]
55
async fn main() {
6-
app::run().await.expect("Some checks fail");
6+
if let Err(e) = app::run().await {
7+
let (json, exit_code) = e.to_stderr_json_and_exit_code();
8+
eprintln!("{json}");
9+
std::process::exit(exit_code);
10+
}
711
}

console/tracker-client/src/console/clients/checker/app.rs

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,12 @@
5959
use std::path::PathBuf;
6060
use std::sync::Arc;
6161

62-
use anyhow::{Context, Result};
6362
use clap::Parser;
6463
use tracing::level_filters::LevelFilter;
6564

6665
use super::config::Configuration;
6766
use super::console::Console;
67+
use super::error::{AppError, ConfigSource};
6868
use super::service::{CheckResult, Service};
6969
use crate::console::clients::checker::config::parse_from_json;
7070

@@ -82,8 +82,9 @@ struct Args {
8282

8383
/// # Errors
8484
///
85-
/// Will return an error if the configuration was not provided.
86-
pub async fn run() -> Result<Vec<CheckResult>> {
85+
/// Will return an `AppError::InvalidConfig` if the configuration cannot be parsed,
86+
/// or an `AppError::Runtime` if the checks fail to execute.
87+
pub async fn run() -> Result<Vec<CheckResult>, AppError> {
8788
tracing_stdout_init(LevelFilter::INFO);
8889

8990
let args = Args::parse();
@@ -97,24 +98,36 @@ pub async fn run() -> Result<Vec<CheckResult>> {
9798
console: console_printer,
9899
};
99100

100-
service.run_checks().await.context("it should run the check tasks")
101+
service.run_checks().await.map_err(|e| AppError::Runtime(e.to_string()))
101102
}
102103

103104
fn tracing_stdout_init(filter: LevelFilter) {
104105
tracing_subscriber::fmt().with_max_level(filter).init();
105106
tracing::debug!("Logging initialized");
106107
}
107108

108-
fn setup_config(args: Args) -> Result<Configuration> {
109+
fn setup_config(args: Args) -> Result<Configuration, AppError> {
109110
match (args.config_path, args.config_content) {
110111
(Some(config_path), _) => load_config_from_file(&config_path),
111-
(_, Some(config_content)) => parse_from_json(&config_content).context("invalid config format"),
112-
_ => Err(anyhow::anyhow!("no configuration provided")),
112+
(_, Some(config_content)) => parse_from_json(&config_content).map_err(|e| AppError::InvalidConfig {
113+
source: ConfigSource::EnvVar("TORRUST_CHECKER_CONFIG"),
114+
message: e.to_string(),
115+
}),
116+
_ => Err(AppError::InvalidConfig {
117+
source: ConfigSource::EnvVar("TORRUST_CHECKER_CONFIG"),
118+
message: "no configuration provided".to_string(),
119+
}),
113120
}
114121
}
115122

116-
fn load_config_from_file(path: &PathBuf) -> Result<Configuration> {
117-
let file_content = std::fs::read_to_string(path).with_context(|| format!("can't read config file {}", path.display()))?;
123+
fn load_config_from_file(path: &PathBuf) -> Result<Configuration, AppError> {
124+
let file_content = std::fs::read_to_string(path).map_err(|e| AppError::InvalidConfig {
125+
source: ConfigSource::File(path.clone()),
126+
message: format!("can't read config file {}: {e}", path.display()),
127+
})?;
118128

119-
parse_from_json(&file_content).context("invalid config format")
129+
parse_from_json(&file_content).map_err(|e| AppError::InvalidConfig {
130+
source: ConfigSource::File(path.clone()),
131+
message: e.to_string(),
132+
})
120133
}

console/tracker-client/src/console/clients/checker/config.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,4 +279,72 @@ mod tests {
279279
}
280280
}
281281
}
282+
283+
mod parsing_from_json {
284+
use crate::console::clients::checker::config::parse_from_json;
285+
286+
#[test]
287+
fn it_should_succeed_with_valid_json() {
288+
let json = r#"{"udp_trackers":[],"http_trackers":[],"health_checks":[]}"#;
289+
assert!(parse_from_json(json).is_ok());
290+
}
291+
292+
#[test]
293+
fn it_should_fail_with_trailing_comma_and_include_serde_detail_in_error() {
294+
let json = r#"{
295+
"udp_trackers": [],
296+
"http_trackers": [
297+
"http://127.0.0.1:7070",
298+
],
299+
"health_checks": []
300+
}"#;
301+
302+
let err = parse_from_json(json).err().expect("Expected a parse error");
303+
let message = err.to_string();
304+
305+
// The specific serde_json detail must be present, not just "invalid config format"
306+
assert!(
307+
message.contains("trailing comma"),
308+
"Expected 'trailing comma' in error message, got: {message}"
309+
);
310+
}
311+
312+
#[test]
313+
fn it_should_fail_with_missing_field_and_include_serde_detail_in_error() {
314+
// Missing required fields entirely
315+
let json = r#"{"udp_trackers":[]}"#;
316+
317+
let err = parse_from_json(json)
318+
.err()
319+
.expect("Expected a parse error for missing fields");
320+
let message = err.to_string();
321+
322+
assert!(!message.is_empty(), "Expected a non-empty error message, got empty string");
323+
}
324+
325+
#[test]
326+
fn it_should_fail_with_malformed_json_and_include_serde_detail_in_error() {
327+
let json = r#"not json at all"#;
328+
329+
let err = parse_from_json(json)
330+
.err()
331+
.expect("Expected a parse error for malformed JSON");
332+
let message = err.to_string();
333+
334+
assert!(
335+
message.contains("JSON parse error"),
336+
"Expected 'JSON parse error' prefix in error message, got: {message}"
337+
);
338+
}
339+
340+
#[test]
341+
fn it_should_fail_with_invalid_url_and_include_detail_in_error() {
342+
let json = r#"{"udp_trackers":["not a url"],"http_trackers":[],"health_checks":[]}"#;
343+
344+
let err = parse_from_json(json).err().expect("Expected an error for an invalid URL");
345+
let message = err.to_string();
346+
347+
assert!(!message.is_empty(), "Expected a non-empty error message");
348+
}
349+
}
282350
}
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
//! Application-level errors for the tracker checker binary.
2+
//!
3+
//! This module separates two concerns:
4+
//! - **Delivery mechanism**: how the configuration was provided (env var, file path, …)
5+
//! - **Error presentation**: what structured JSON the binary emits on stderr
6+
//!
7+
//! `ConfigSource` captures the delivery mechanism so that error messages can
8+
//! reference it without coupling the parsing layer to delivery specifics.
9+
//!
10+
//! The JSON envelope emitted to stderr follows the Tracker CLI I/O Contract:
11+
//!
12+
//! ```json
13+
//! { "error": { "kind": "...", "source": "...", "message": "..." } }
14+
//! ```
15+
use std::fmt;
16+
use std::path::PathBuf;
17+
18+
/// Where the configuration content was delivered from.
19+
#[derive(Debug, Clone)]
20+
pub enum ConfigSource {
21+
/// Configuration delivered via an environment variable (stores the variable name).
22+
EnvVar(&'static str),
23+
/// Configuration delivered via a file (stores the file path).
24+
File(PathBuf),
25+
}
26+
27+
impl fmt::Display for ConfigSource {
28+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
29+
match self {
30+
ConfigSource::EnvVar(name) => write!(f, "{name}"),
31+
ConfigSource::File(path) => write!(f, "{}", path.display()),
32+
}
33+
}
34+
}
35+
36+
/// Top-level application errors for the tracker checker.
37+
#[derive(Debug)]
38+
pub enum AppError {
39+
/// The provided configuration was invalid (bad JSON, invalid URLs, etc.).
40+
InvalidConfig {
41+
/// How the configuration was delivered (env var or file path).
42+
source: ConfigSource,
43+
/// Human-readable detail from the underlying parse error.
44+
message: String,
45+
},
46+
/// An unexpected runtime failure occurred after configuration was accepted.
47+
Runtime(String),
48+
}
49+
50+
impl AppError {
51+
/// Serializes the error to the contract JSON envelope and returns the
52+
/// appropriate process exit code.
53+
///
54+
/// Exit codes:
55+
/// - `2` — configuration error
56+
/// - `1` — generic runtime failure
57+
#[must_use]
58+
pub fn to_stderr_json_and_exit_code(&self) -> (String, i32) {
59+
match self {
60+
AppError::InvalidConfig { source, message } => {
61+
let json =
62+
format!(r#"{{"error":{{"kind":"invalid_configuration","source":"{source}","message":"{message}"}}}}"#,);
63+
(json, 2)
64+
}
65+
AppError::Runtime(message) => {
66+
let json = format!(r#"{{"error":{{"kind":"runtime_failure","source":"runtime","message":"{message}"}}}}"#);
67+
(json, 1)
68+
}
69+
}
70+
}
71+
}
72+
73+
impl fmt::Display for AppError {
74+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
75+
match self {
76+
AppError::InvalidConfig { source, message } => {
77+
write!(f, "invalid configuration from {source}: {message}")
78+
}
79+
AppError::Runtime(msg) => write!(f, "runtime failure: {msg}"),
80+
}
81+
}
82+
}
83+
84+
#[cfg(test)]
85+
mod tests {
86+
use super::*;
87+
88+
#[test]
89+
fn config_source_env_var_displays_as_variable_name() {
90+
let source = ConfigSource::EnvVar("TORRUST_CHECKER_CONFIG");
91+
assert_eq!(source.to_string(), "TORRUST_CHECKER_CONFIG");
92+
}
93+
94+
#[test]
95+
fn config_source_file_displays_as_path() {
96+
let source = ConfigSource::File(PathBuf::from("/etc/tracker/config.json"));
97+
assert_eq!(source.to_string(), "/etc/tracker/config.json");
98+
}
99+
100+
#[test]
101+
fn invalid_config_error_produces_exit_code_2() {
102+
let error = AppError::InvalidConfig {
103+
source: ConfigSource::EnvVar("TORRUST_CHECKER_CONFIG"),
104+
message: "JSON parse error: trailing comma at line 7 column 5".to_string(),
105+
};
106+
let (_, exit_code) = error.to_stderr_json_and_exit_code();
107+
assert_eq!(exit_code, 2);
108+
}
109+
110+
#[test]
111+
fn runtime_error_produces_exit_code_1() {
112+
let error = AppError::Runtime("failed to bind socket".to_string());
113+
let (_, exit_code) = error.to_stderr_json_and_exit_code();
114+
assert_eq!(exit_code, 1);
115+
}
116+
117+
#[test]
118+
fn invalid_config_error_json_contains_expected_fields() {
119+
let error = AppError::InvalidConfig {
120+
source: ConfigSource::EnvVar("TORRUST_CHECKER_CONFIG"),
121+
message: "JSON parse error: trailing comma at line 7 column 5".to_string(),
122+
};
123+
let (json, _) = error.to_stderr_json_and_exit_code();
124+
assert!(json.contains(r#""kind":"invalid_configuration""#));
125+
assert!(json.contains(r#""source":"TORRUST_CHECKER_CONFIG""#));
126+
assert!(json.contains("trailing comma at line 7 column 5"));
127+
}
128+
129+
#[test]
130+
fn runtime_error_json_contains_expected_fields() {
131+
let error = AppError::Runtime("failed to bind socket".to_string());
132+
let (json, _) = error.to_stderr_json_and_exit_code();
133+
assert!(json.contains(r#""kind":"runtime_failure""#));
134+
assert!(json.contains(r#""source":"runtime""#));
135+
assert!(json.contains("failed to bind socket"));
136+
}
137+
138+
#[test]
139+
fn invalid_config_error_from_file_includes_path_in_json() {
140+
let error = AppError::InvalidConfig {
141+
source: ConfigSource::File(PathBuf::from("/etc/tracker/config.json")),
142+
message: "JSON parse error: trailing comma at line 3 column 1".to_string(),
143+
};
144+
let (json, _) = error.to_stderr_json_and_exit_code();
145+
assert!(json.contains(r#""source":"/etc/tracker/config.json""#));
146+
}
147+
}

console/tracker-client/src/console/clients/checker/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ pub mod app;
22
pub mod checks;
33
pub mod config;
44
pub mod console;
5+
pub mod error;
56
pub mod logger;
67
pub mod printer;
78
pub mod service;

0 commit comments

Comments
 (0)