chore(runtime): warn on cleanup failures in shell and web_fetch_to_file#6255
Conversation
Replace silent `let _ =` ignores with `tracing::warn` logs when: - `shell_exec` fails to kill the process tree on interrupt or timeout. - `web_fetch_to_file` fails to remove the temporary partial-download file after a write or rename error. This makes orphaned processes / leaked temp files observable without changing the public error returned to callers.
houko
left a comment
There was a problem hiding this comment.
LGTM. Reviewed: replaces three silent let _ = cleanup ignores with tracing::warn structured logs (orphaned process trees, leaked partial-download temp files become observable); control flow is preserved exactly and it follows the repo logging conventions. Real CI — Quality/clippy, all 4 Ubuntu test shards, Unit, Build, Security, Live Integration Smoke — is green.
The red detect-secrets scan check is a stale-base artifact: this branch predates #6262, which retired detect-secrets in favor of gitleaks, so the legacy baseline gate fails here. It is not a real secret (the change is logging-only). Merging onto current main — which uses gitleaks — is clean.
houko
left a comment
There was a problem hiding this comment.
LGTM — reviewed clean (structured-log cleanup, control flow preserved); real Rust CI green; branch updated to current main.
Summary
Replaces two silent
let _ =ignores withtracing::warnlogs so cleanup failures are observable:tool_runner/shell.rs: whenshell_execis interrupted or times out, it now logs a warning ifkill_process_treefails. Previously the failure was swallowed, making it hard to notice orphaned processes.web_fetch_to_file.rs: when the atomic write or rename fails, it now logs a warning if removing the temporary partial-download file fails. Previously the temp file leak was silent.Verification
cargo fmt --allcargo clippy -p librefang-runtime --all-targets -- -D warningscargo test -p librefang-runtime web_fetch_to_file(4 passed)cargo test -p librefang-runtime shell(108 passed)