Skip to content

Commit 809ae18

Browse files
committed
Improve package manager error reporting and add retry logic
- Add syntax-highlighted error output with line number highlighting - New error_formatter module with TTY detection and ANSI color support - Highlight problematic YAML/JSON lines with context - Show resource path, source name, and formatted source type - Fix Helm template path extraction - Parse paths and YAML content together to maintain 1:1 mapping - Ensure resource paths appear in all error messages - Prevent index mismatch between paths and documents - Add retry logic for network operations - 3 retry attempts with exponential backoff for Helm repository fetches - 3 retry attempts for Helm chart downloads - Detailed error messages showing all attempted URLs and failures - Add --no-color flag to disable colored output - Affects both error messages and log output - Useful for piping to files and CI/CD systems - Improve CLI verbosity flags - Support -v for debug and -vv for trace logging - Add --log-level flag for custom log filters - Exclude hyper/reqwest from verbose logging by default - Remove verbose debug info from error messages - Keep error output clean and focused on actionable information Fixes #45
1 parent bdd0cd6 commit 809ae18

9 files changed

Lines changed: 648 additions & 110 deletions

File tree

operators/mows-package-manager/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,5 @@ jsonptr = "0.7.1"
7777
toml = "0.8.20"
7878
serde_variant = { workspace = true }
7979
schemars = { workspace = true, features = ["default"] }
80+
syntect = "5.2.0"
81+
atty = "0.2"

operators/mows-package-manager/src/bin/cli.rs

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
use anyhow::Context;
22
use clap::{Parser, Subcommand};
33

4-
use mows_common_rust::observability::init_minimal_observability;
4+
use mows_common_rust::observability::init_minimal_observability_with_color;
55
use mows_package_manager::{
66
dev::get_fake_cluster_config,
7+
error_formatter::set_no_color,
78
rendered_document::{CrdHandling, RenderedDocument},
89
repository::Repository,
910
};
@@ -13,9 +14,15 @@ use mows_package_manager::{
1314
struct Cli {
1415
#[clap(subcommand)]
1516
command: Commands,
16-
/// Log debug information
17-
#[arg(short, long, global = true)]
18-
verbose: Option<String>,
17+
/// Set log level or increase verbosity (-v for debug, -vv for trace, or --verbose=level)
18+
#[arg(short, long, global = true, action = clap::ArgAction::Count)]
19+
verbose: u8,
20+
/// Custom log level filter
21+
#[arg(long = "log-level", global = true)]
22+
log_level: Option<String>,
23+
/// Disable colored output
24+
#[arg(long = "no-color", global = true)]
25+
no_color: bool,
1926
}
2027

2128
#[derive(Subcommand, Debug)]
@@ -45,18 +52,26 @@ enum Commands {
4552
async fn main() -> anyhow::Result<()> {
4653
let cli = Cli::parse();
4754

48-
let log_level = match cli.verbose.as_deref() {
49-
Some(value) => {
50-
if value.is_empty() {
51-
"info"
52-
} else {
53-
value
54-
}
55+
// Build log filter based on verbosity level or custom log level
56+
// -v = debug for mows_package_manager and mows_common_rust
57+
// -vv = trace for mows_package_manager and mows_common_rust
58+
// --log-level=level = use custom log level string
59+
// Always keep hyper and reqwest at warn level
60+
let log_filter = if let Some(custom_level) = &cli.log_level {
61+
format!("{},hyper=warn,reqwest=warn", custom_level)
62+
} else {
63+
match cli.verbose {
64+
0 => "info,hyper=warn,reqwest=warn".to_string(),
65+
1 => "mows_package_manager=debug,mows_common_rust=debug,info,hyper=warn,reqwest=warn".to_string(),
66+
_ => "mows_package_manager=trace,mows_common_rust=trace,info,hyper=warn,reqwest=warn".to_string(),
5567
}
56-
None => "info",
5768
};
5869

59-
init_minimal_observability(&log_level).await?;
70+
// Set the global no-color flag
71+
set_no_color(cli.no_color);
72+
73+
// Initialize observability with color settings
74+
init_minimal_observability_with_color(&log_filter, !cli.no_color).await?;
6075

6176
match cli.command {
6277
Commands::Install { url, name } => {
Lines changed: 290 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,290 @@
1+
use std::sync::atomic::{AtomicBool, Ordering};
2+
use syntect::easy::HighlightLines;
3+
use syntect::highlighting::{Style, ThemeSet};
4+
use syntect::parsing::SyntaxSet;
5+
use syntect::util::{as_24_bit_terminal_escaped, LinesWithEndings};
6+
7+
/// Global flag to disable colors
8+
static NO_COLOR: AtomicBool = AtomicBool::new(false);
9+
10+
/// Set whether colors should be disabled globally
11+
pub fn set_no_color(no_color: bool) {
12+
NO_COLOR.store(no_color, Ordering::Relaxed);
13+
}
14+
15+
/// Check if we should use colors (are we in a terminal?)
16+
pub fn should_use_colors() -> bool {
17+
// If --no-color flag is set, always return false
18+
if NO_COLOR.load(Ordering::Relaxed) {
19+
return false;
20+
}
21+
// Check if stderr is a TTY (that's where errors typically go)
22+
atty::is(atty::Stream::Stderr)
23+
}
24+
25+
/// Conditionally apply color codes
26+
pub fn color_text(text: &str, ansi_code: &str) -> String {
27+
if should_use_colors() {
28+
format!("{}{}\x1b[0m", ansi_code, text)
29+
} else {
30+
text.to_string()
31+
}
32+
}
33+
34+
/// Get colored prefix for error messages
35+
pub fn error_prefix() -> String {
36+
if should_use_colors() {
37+
"\x1b[1;31m✗\x1b[0m".to_string()
38+
} else {
39+
"ERROR:".to_string()
40+
}
41+
}
42+
43+
/// Get colored label
44+
pub fn label(text: &str) -> String {
45+
if text.is_empty() {
46+
return String::new();
47+
}
48+
if should_use_colors() {
49+
format!("\x1b[1m{}:\x1b[0m", text)
50+
} else {
51+
format!("{}:", text)
52+
}
53+
}
54+
55+
/// Get hint prefix
56+
pub fn hint_prefix() -> String {
57+
if should_use_colors() {
58+
"\x1b[1;33m💡 Hint:\x1b[0m".to_string()
59+
} else {
60+
"HINT:".to_string()
61+
}
62+
}
63+
64+
/// Format JSON with optional syntax highlighting
65+
pub fn format_json_highlighted(json_str: &str, show_line_numbers: bool) -> String {
66+
if !should_use_colors() {
67+
return format_plain_with_line_numbers(json_str, show_line_numbers);
68+
}
69+
70+
let ps = SyntaxSet::load_defaults_newlines();
71+
let ts = ThemeSet::load_defaults();
72+
73+
let syntax = ps
74+
.find_syntax_by_extension("json")
75+
.unwrap_or_else(|| ps.find_syntax_plain_text());
76+
77+
let mut h = HighlightLines::new(syntax, &ts.themes["base16-ocean.dark"]);
78+
79+
let mut output = String::new();
80+
81+
for (line_num, line) in LinesWithEndings::from(json_str).enumerate() {
82+
let ranges: Vec<(Style, &str)> = h.highlight_line(line, &ps).unwrap_or_default();
83+
if show_line_numbers {
84+
output.push_str(&format!("{:4} │ ", line_num + 1));
85+
}
86+
let escaped = as_24_bit_terminal_escaped(&ranges[..], false);
87+
output.push_str(&escaped);
88+
}
89+
90+
output
91+
}
92+
93+
/// Format text with line numbers (no colors)
94+
fn format_plain_with_line_numbers(text: &str, show_line_numbers: bool) -> String {
95+
if !show_line_numbers {
96+
return text.to_string();
97+
}
98+
99+
text.lines()
100+
.enumerate()
101+
.map(|(i, line)| format!(" {:4} │ {}", i + 1, line))
102+
.collect::<Vec<_>>()
103+
.join("\n")
104+
}
105+
106+
/// Format YAML string with optional syntax highlighting and line numbers
107+
pub fn format_yaml_highlighted(yaml_str: &str, show_line_numbers: bool) -> String {
108+
if !should_use_colors() {
109+
return format_plain_with_line_numbers(yaml_str, show_line_numbers);
110+
}
111+
112+
let ps = SyntaxSet::load_defaults_newlines();
113+
let ts = ThemeSet::load_defaults();
114+
115+
let syntax = ps
116+
.find_syntax_by_extension("yaml")
117+
.unwrap_or_else(|| ps.find_syntax_plain_text());
118+
119+
let mut h = HighlightLines::new(syntax, &ts.themes["base16-ocean.dark"]);
120+
121+
let mut output = String::new();
122+
123+
for (line_num, line) in LinesWithEndings::from(yaml_str).enumerate() {
124+
let ranges: Vec<(Style, &str)> = h.highlight_line(line, &ps).unwrap_or_default();
125+
if show_line_numbers {
126+
output.push_str(&format!(" {:4} │ ", line_num + 1));
127+
}
128+
let escaped = as_24_bit_terminal_escaped(&ranges[..], false);
129+
output.push_str(&escaped);
130+
}
131+
132+
output
133+
}
134+
135+
/// Format YAML with specific error line highlighted
136+
pub fn format_yaml_with_error_line(
137+
yaml_str: &str,
138+
error_line: usize,
139+
error_message: &str,
140+
) -> String {
141+
let use_colors = should_use_colors();
142+
let lines: Vec<&str> = yaml_str.lines().collect();
143+
144+
// Show context: 3 lines before and after the error
145+
let start = error_line.saturating_sub(4).max(0);
146+
let end = (error_line + 3).min(lines.len());
147+
148+
if !use_colors {
149+
// Plain text version
150+
let mut output = String::new();
151+
for line_num in start..end {
152+
let line = lines.get(line_num).unwrap_or(&"");
153+
let is_error_line = line_num == error_line - 1;
154+
155+
if is_error_line {
156+
output.push_str(&format!(
157+
" {:4} │ {} ← {}\n",
158+
line_num + 1,
159+
line,
160+
error_message
161+
));
162+
} else {
163+
output.push_str(&format!(" {:4} │ {}\n", line_num + 1, line));
164+
}
165+
}
166+
return output;
167+
}
168+
169+
// Colored version
170+
let ps = SyntaxSet::load_defaults_newlines();
171+
let ts = ThemeSet::load_defaults();
172+
173+
let syntax = ps
174+
.find_syntax_by_extension("yaml")
175+
.unwrap_or_else(|| ps.find_syntax_plain_text());
176+
177+
let mut h = HighlightLines::new(syntax, &ts.themes["base16-ocean.dark"]);
178+
179+
let mut output = String::new();
180+
181+
for line_num in start..end {
182+
let line = lines.get(line_num).unwrap_or(&"");
183+
let line_with_newline = format!("{}\n", line);
184+
let ranges: Vec<(Style, &str)> = h
185+
.highlight_line(&line_with_newline, &ps)
186+
.unwrap_or_default();
187+
188+
let is_error_line = line_num == error_line - 1;
189+
let line_num_str = format!("{:4}", line_num + 1);
190+
191+
// Print line number and bar without background
192+
output.push_str(&format!(" {} │ ", line_num_str));
193+
194+
if is_error_line {
195+
// Apply dim red background highlight only to the code content (dark red)
196+
output.push_str("\x1b[48;2;60;20;20m"); // RGB: very dark red background
197+
}
198+
199+
let escaped = as_24_bit_terminal_escaped(&ranges[..], false);
200+
// Remove the newline from the escaped string
201+
let escaped_trimmed = escaped.trim_end();
202+
output.push_str(escaped_trimmed);
203+
204+
// Add padding spaces with background if it's an error line (to extend background to edge)
205+
if is_error_line {
206+
output.push_str(" "); // Extra spaces with background before resetting
207+
output.push_str("\x1b[0m"); // Reset
208+
// Add error message to the right with some padding
209+
output.push_str(&format!("\x1b[1;31m ← {}\x1b[0m", error_message));
210+
}
211+
212+
output.push('\n');
213+
}
214+
215+
output
216+
}
217+
218+
/// Extract line number from serde error message
219+
pub fn extract_line_number_from_error(error: &str) -> Option<usize> {
220+
// Try to extract line number from common error patterns
221+
// Example: "invalid type: map, expected a string at line 5 column 10"
222+
let patterns = [
223+
regex::Regex::new(r"at line (\d+)").ok()?,
224+
regex::Regex::new(r"line (\d+)").ok()?,
225+
regex::Regex::new(r":(\d+):").ok()?,
226+
];
227+
228+
for pattern in &patterns {
229+
if let Some(captures) = pattern.captures(error) {
230+
if let Some(line_match) = captures.get(1) {
231+
if let Ok(line_num) = line_match.as_str().parse::<usize>() {
232+
return Some(line_num);
233+
}
234+
}
235+
}
236+
}
237+
238+
None
239+
}
240+
241+
/// Try to find the problematic field in YAML and return its line number
242+
pub fn find_field_in_yaml(yaml_str: &str, field_hint: &str) -> Option<usize> {
243+
// Common problematic patterns in Kubernetes YAML
244+
let problematic_patterns = [
245+
"annotations:", // Often misplaced
246+
"labels:",
247+
"metadata:",
248+
];
249+
250+
for (line_num, line) in yaml_str.lines().enumerate() {
251+
let trimmed = line.trim_start();
252+
253+
// Check if this line might be the problem
254+
for pattern in &problematic_patterns {
255+
if trimmed.starts_with(pattern) {
256+
// If annotations is under labels (common mistake), flag it
257+
if pattern == &"annotations:" && field_hint.contains("map") {
258+
// Look at previous lines to see if we're nested under labels
259+
if line_num > 0 {
260+
let prev_lines: Vec<&str> = yaml_str.lines().take(line_num).collect();
261+
for prev_line in prev_lines.iter().rev().take(5) {
262+
if prev_line.trim().starts_with("labels:") {
263+
return Some(line_num + 1);
264+
}
265+
}
266+
}
267+
}
268+
}
269+
}
270+
}
271+
272+
None
273+
}
274+
275+
#[cfg(test)]
276+
mod tests {
277+
use super::*;
278+
279+
#[test]
280+
fn test_extract_line_number() {
281+
assert_eq!(
282+
extract_line_number_from_error("error at line 42 column 10"),
283+
Some(42)
284+
);
285+
assert_eq!(
286+
extract_line_number_from_error("foo.yaml:15: invalid syntax"),
287+
Some(15)
288+
);
289+
}
290+
}

operators/mows-package-manager/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
pub mod config;
22
pub mod dev;
3+
pub mod error_formatter;
34
pub mod errors;
45
pub mod rendered_document;
56

0 commit comments

Comments
 (0)