Skip to content

Commit ee51fea

Browse files
inureyesinureyes
authored
Merge pull request #35 from lablup/refactor/issue-32-remove-unwrap-calls
refactor: remove dangerous unwrap() calls and improve error handling
2 parents a279da8 + 23f2890 commit ee51fea

5 files changed

Lines changed: 329 additions & 78 deletions

File tree

‎src/config.rs‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,10 @@ impl Config {
530530
if cluster.interactive.is_none() {
531531
cluster.interactive = Some(InteractiveConfig::default());
532532
}
533-
cluster.interactive.as_mut().unwrap()
533+
cluster
534+
.interactive
535+
.as_mut()
536+
.expect("interactive config should exist after initialization")
534537
} else {
535538
// Update global config
536539
&mut self.interactive
@@ -667,10 +670,23 @@ fn expand_env_vars(input: &str) -> String {
667670
}
668671

669672
if i > start + 1 {
670-
let var_name = std::str::from_utf8(&bytes[start + 1..i]).unwrap();
673+
let var_name = match std::str::from_utf8(&bytes[start + 1..i]) {
674+
Ok(name) => name,
675+
Err(_) => {
676+
// Invalid UTF-8 in environment variable name, skip
677+
new_result.push('$');
678+
continue;
679+
}
680+
};
671681
let replacement = std::env::var(var_name).unwrap_or_else(|_| {
672682
tracing::debug!("Environment variable {} not found", var_name);
673-
String::from_utf8(bytes[start..i].to_vec()).unwrap()
683+
match String::from_utf8(bytes[start..i].to_vec()) {
684+
Ok(original) => original,
685+
Err(_) => {
686+
// Invalid UTF-8, use placeholder
687+
format!("$INVALID_UTF8_{start}")
688+
}
689+
}
674690
});
675691
new_result.push_str(&replacement);
676692
} else {

‎src/executor.rs‎

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ impl ParallelExecutor {
133133

134134
let style = ProgressStyle::default_bar()
135135
.template("{prefix:.bold} {spinner:.cyan} {msg}")
136-
.unwrap()
136+
.map_err(|e| anyhow::anyhow!("Failed to create progress bar template: {}", e))?
137137
.tick_chars("⣾⣽⣻⢿⡿⣟⣯⣷ ");
138138

139139
let tasks: Vec<_> = self
@@ -166,7 +166,20 @@ impl ParallelExecutor {
166166
pb.enable_steady_tick(std::time::Duration::from_millis(PROGRESS_BAR_TICK_RATE_MS));
167167

168168
tokio::spawn(async move {
169-
let _permit = semaphore.acquire().await.unwrap();
169+
let _permit = match semaphore.acquire().await {
170+
Ok(permit) => permit,
171+
Err(e) => {
172+
pb.finish_with_message(format!(
173+
"{} {}",
174+
"●".red(),
175+
"Semaphore closed".red()
176+
));
177+
return ExecutionResult {
178+
node,
179+
result: Err(anyhow::anyhow!("Semaphore acquisition failed: {}", e)),
180+
};
181+
}
182+
};
170183

171184
pb.set_message(format!("{}", "Executing...".blue()));
172185

@@ -240,7 +253,9 @@ impl ParallelExecutor {
240253

241254
let style = ProgressStyle::default_bar()
242255
.template("{prefix:.bold} {spinner:.cyan} {msg}")
243-
.unwrap()
256+
.map_err(|e| {
257+
anyhow::anyhow!("Failed to create progress bar template for upload: {}", e)
258+
})?
244259
.tick_chars("⣾⣽⣻⢿⡿⣟⣯⣷ ");
245260

246261
let tasks: Vec<_> = self
@@ -272,7 +287,20 @@ impl ParallelExecutor {
272287
pb.enable_steady_tick(std::time::Duration::from_millis(PROGRESS_BAR_TICK_RATE_MS));
273288

274289
tokio::spawn(async move {
275-
let _permit = semaphore.acquire().await.unwrap();
290+
let _permit = match semaphore.acquire().await {
291+
Ok(permit) => permit,
292+
Err(e) => {
293+
pb.finish_with_message(format!(
294+
"{} {}",
295+
"●".red(),
296+
"Semaphore closed".red()
297+
));
298+
return UploadResult {
299+
node,
300+
result: Err(anyhow::anyhow!("Semaphore acquisition failed: {}", e)),
301+
};
302+
}
303+
};
276304

277305
pb.set_message(format!("{}", "Uploading (SFTP)...".blue()));
278306

@@ -337,7 +365,9 @@ impl ParallelExecutor {
337365

338366
let style = ProgressStyle::default_bar()
339367
.template("{prefix:.bold} {spinner:.cyan} {msg}")
340-
.unwrap()
368+
.map_err(|e| {
369+
anyhow::anyhow!("Failed to create progress bar template for download: {}", e)
370+
})?
341371
.tick_chars("⣾⣽⣻⢿⡿⣟⣯⣷ ");
342372

343373
let tasks: Vec<_> = self
@@ -369,7 +399,20 @@ impl ParallelExecutor {
369399
pb.enable_steady_tick(std::time::Duration::from_millis(PROGRESS_BAR_TICK_RATE_MS));
370400

371401
tokio::spawn(async move {
372-
let _permit = semaphore.acquire().await.unwrap();
402+
let _permit = match semaphore.acquire().await {
403+
Ok(permit) => permit,
404+
Err(e) => {
405+
pb.finish_with_message(format!(
406+
"{} {}",
407+
"●".red(),
408+
"Semaphore closed".red()
409+
));
410+
return DownloadResult {
411+
node,
412+
result: Err(anyhow::anyhow!("Semaphore acquisition failed: {}", e)),
413+
};
414+
}
415+
};
373416

374417
pb.set_message(format!("{}", "Downloading (SFTP)...".blue()));
375418

@@ -439,7 +482,12 @@ impl ParallelExecutor {
439482

440483
let style = ProgressStyle::default_bar()
441484
.template("{prefix:.bold} {spinner:.cyan} {msg}")
442-
.unwrap()
485+
.map_err(|e| {
486+
anyhow::anyhow!(
487+
"Failed to create progress bar template for multi-download: {}",
488+
e
489+
)
490+
})?
443491
.tick_chars("⣾⣽⣻⢿⡿⣟⣯⣷ ");
444492

445493
let mut all_results = Vec::new();
@@ -471,7 +519,23 @@ impl ParallelExecutor {
471519
));
472520

473521
tokio::spawn(async move {
474-
let _permit = semaphore.acquire().await.unwrap();
522+
let _permit = match semaphore.acquire().await {
523+
Ok(permit) => permit,
524+
Err(e) => {
525+
pb.finish_with_message(format!(
526+
"{} {}",
527+
"●".red(),
528+
"Semaphore closed".red()
529+
));
530+
return DownloadResult {
531+
node,
532+
result: Err(anyhow::anyhow!(
533+
"Semaphore acquisition failed: {}",
534+
e
535+
)),
536+
};
537+
}
538+
};
475539

476540
// Generate unique filename for each node and file
477541
let filename = if let Some(file_name) = Path::new(&remote_path).file_name()

‎src/main.rs‎

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -683,20 +683,34 @@ async fn handle_cache_stats(detailed: bool, clear: bool, maintain: bool) {
683683
use owo_colors::OwoColorize;
684684

685685
if clear {
686-
GLOBAL_CACHE.clear();
686+
if let Err(e) = GLOBAL_CACHE.clear() {
687+
eprintln!("Failed to clear cache: {e}");
688+
return;
689+
}
687690
println!("{}", "Cache cleared".green());
688691
}
689692

690693
if maintain {
691-
let removed = GLOBAL_CACHE.maintain().await;
692-
println!(
693-
"{}: Removed {} expired/stale entries",
694-
"Cache maintenance".yellow(),
695-
removed
696-
);
694+
match GLOBAL_CACHE.maintain().await {
695+
Ok(removed) => println!(
696+
"{}: Removed {} expired/stale entries",
697+
"Cache maintenance".yellow(),
698+
removed
699+
),
700+
Err(e) => {
701+
eprintln!("Failed to maintain cache: {e}");
702+
return;
703+
}
704+
}
697705
}
698706

699-
let stats = GLOBAL_CACHE.stats();
707+
let stats = match GLOBAL_CACHE.stats() {
708+
Ok(stats) => stats,
709+
Err(e) => {
710+
eprintln!("Failed to get cache stats: {e}");
711+
return;
712+
}
713+
};
700714
let config = GLOBAL_CACHE.config();
701715

702716
println!("\n{}", "SSH Configuration Cache Statistics".cyan().bold());
@@ -755,10 +769,15 @@ async fn handle_cache_stats(detailed: bool, clear: bool, maintain: bool) {
755769

756770
if detailed && stats.current_entries > 0 {
757771
println!("\n{}", "Detailed Entry Information:".bright_blue());
758-
let debug_info = GLOBAL_CACHE.debug_info();
759-
760-
for (path, info) in debug_info {
761-
println!(" {}: {}", path.display().to_string().cyan(), info);
772+
match GLOBAL_CACHE.debug_info() {
773+
Ok(debug_info) => {
774+
for (path, info) in debug_info {
775+
println!(" {}: {}", path.display().to_string().cyan(), info);
776+
}
777+
}
778+
Err(e) => {
779+
eprintln!("Failed to get debug info: {e}");
780+
}
762781
}
763782
}
764783

0 commit comments

Comments
 (0)