Skip to content

Commit 905b6bf

Browse files
fix: log silently swallowed errors instead of discarding them (#915)
* fix: log silently swallowed errors instead of discarding them Replace bare `let _ = fallible_call()` and `.unwrap_or_default()` patterns with explicit error logging via tracing: - warn level: user-visible state loss (config save, reload, GC failures) - debug level: non-actionable info (state files, network checks, lock release) This makes failures observable in debug.log without changing control flow. Closes #914 * fix: revert dead-code match on load_all_instances load_all_instances() always returns Ok (errors are swallowed internally), so the Err(e) branch was unreachable dead code. Revert to the original unwrap_or_default() until the function is fixed to propagate errors. * fix: log gc_stale failure on test-notification path Mirror the change at push.rs:623 so the analogous gc_stale call on the SendOutcome::Gone branch of the test-notification handler also logs failures instead of silently discarding them. Issue #914 listed both sites; this picks up the one missed by the original commit. --------- Co-authored-by: njbrake <[email protected]> Co-authored-by: Nathan Brake (Mozilla.ai) <[email protected]>
1 parent 31c4f45 commit 905b6bf

6 files changed

Lines changed: 49 additions & 14 deletions

File tree

src/server/mod.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,9 @@ pub async fn start_server(config: ServerConfig<'_>) -> anyhow::Result<()> {
495495
// render the right transport label: "tunnel" for Cloudflare,
496496
// "tailscale" for Tailscale Funnel, "local" for local-only.
497497
let mode = format!("{}\n", handle.mode_label());
498-
let _ = std::fs::write(app_dir.join("serve.mode"), mode);
498+
if let Err(e) = std::fs::write(app_dir.join("serve.mode"), mode) {
499+
tracing::debug!("Failed to write serve.mode: {e}");
500+
}
499501
}
500502

501503
// Start health monitor (uses CancellationToken internally)
@@ -554,7 +556,9 @@ pub async fn start_server(config: ServerConfig<'_>) -> anyhow::Result<()> {
554556
contents.push('\n');
555557
}
556558
write_secret_file(&app_dir.join("serve.url"), &contents);
557-
let _ = std::fs::write(app_dir.join("serve.mode"), "local\n");
559+
if let Err(e) = std::fs::write(app_dir.join("serve.mode"), "local\n") {
560+
tracing::debug!("Failed to write serve.mode: {e}");
561+
}
558562
}
559563

560564
None

src/server/push.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,17 @@ impl VapidKeypair {
9595
// Re-check: another process may have generated while we were
9696
// waiting for the lock.
9797
if path.exists() {
98-
let _ = FileExt::unlock(&lock_file);
98+
if let Err(e) = FileExt::unlock(&lock_file) {
99+
tracing::debug!("Failed to release lock file: {e}");
100+
}
99101
return Self::load(path);
100102
}
101103

102104
let kp = Self::generate()?;
103105
kp.persist(path)?;
104-
let _ = FileExt::unlock(&lock_file);
106+
if let Err(e) = FileExt::unlock(&lock_file) {
107+
tracing::debug!("Failed to release lock file: {e}");
108+
}
105109
Ok(kp)
106110
}
107111

@@ -620,7 +624,9 @@ async fn fire_due_pushes(
620624
let outcome =
621625
super::push_send::send_one(&client, push.as_ref(), &sub, &payload_clone).await;
622626
if outcome == super::push_send::SendOutcome::Gone {
623-
let _ = push.store.gc_stale(&sub.endpoint, sub.generation).await;
627+
if let Err(e) = push.store.gc_stale(&sub.endpoint, sub.generation).await {
628+
tracing::warn!("Failed to GC stale push subscription: {e}");
629+
}
624630
}
625631
});
626632
}
@@ -819,10 +825,13 @@ pub async fn test(
819825
// Best-effort GC; the result still reports gone=1 even if GC
820826
// races with a re-subscribe (that's what the generation
821827
// counter in gc_stale prevents).
822-
let _ = push
828+
if let Err(e) = push
823829
.store
824830
.gc_stale(&body.endpoint, subscription.generation)
825-
.await;
831+
.await
832+
{
833+
tracing::warn!("Failed to GC stale push subscription: {e}");
834+
}
826835
}
827836
}
828837
Ok(Json(result))

src/tui/home/input.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,9 @@ impl HomeView {
232232
crate::session::config::load_config().map(|c| c.unwrap_or_default())
233233
{
234234
config.app_state.has_acknowledged_agent_hooks = true;
235-
let _ = crate::session::config::save_config(&config);
235+
if let Err(e) = crate::session::config::save_config(&config) {
236+
tracing::warn!("Failed to save config: {e}");
237+
}
236238
}
237239
// Resume session creation
238240
if let Some(data) = self.pending_hooks_install_data.take() {

src/tui/home/mod.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,9 @@ impl HomeView {
405405
}
406406
if !orphan_ids.is_empty() {
407407
tracing::info!("Cleaned up {} orphaned creating sessions", orphan_ids.len());
408-
let _ = view.save();
408+
if let Err(e) = view.save() {
409+
tracing::warn!("Failed to save view state: {e}");
410+
}
409411
}
410412

411413
// Batch-sync instance IDs and captured session IDs to tmux hidden env
@@ -671,7 +673,9 @@ impl HomeView {
671673
if let Err(e) = self.save() {
672674
tracing::error!("Failed to save after deletion: {}", e);
673675
}
674-
let _ = self.reload();
676+
if let Err(e) = self.reload() {
677+
tracing::warn!("Failed to reload session state: {e}");
678+
}
675679
} else {
676680
let error = result.error;
677681
self.mutate_instance(&result.session_id, |inst| {
@@ -933,7 +937,9 @@ impl HomeView {
933937
self.on_launch_hooks_ran.insert(session_id.clone());
934938
}
935939

936-
let _ = self.reload();
940+
if let Err(e) = self.reload() {
941+
tracing::warn!("Failed to reload session state: {e}");
942+
}
937943
self.new_dialog = None;
938944

939945
Some(session_id)
@@ -1136,7 +1142,9 @@ impl HomeView {
11361142
fn save_list_width(&self) {
11371143
if let Ok(mut config) = load_config().map(|c| c.unwrap_or_default()) {
11381144
config.app_state.home_list_width = Some(self.list_width);
1139-
let _ = save_config(&config);
1145+
if let Err(e) = save_config(&config) {
1146+
tracing::warn!("Failed to save config: {e}");
1147+
}
11401148
}
11411149
}
11421150

src/tui/settings/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,13 @@ impl SettingsView {
128128
.map(repo_config_to_profile)
129129
.unwrap_or_default();
130130

131-
let mut available_profiles = list_profiles().unwrap_or_default();
131+
let mut available_profiles = match list_profiles() {
132+
Ok(p) => p,
133+
Err(e) => {
134+
tracing::debug!("Failed to list profiles: {e}");
135+
Vec::new()
136+
}
137+
};
132138
if !available_profiles.contains(&profile.to_string()) {
133139
available_profiles.push(profile.to_string());
134140
available_profiles.sort();

src/update/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,13 @@ pub async fn check_for_update(current_version: &str, force: bool) -> Result<Upda
110110
.build()?;
111111

112112
// Fetch all releases (includes body/release notes)
113-
let releases = fetch_releases(&client).await.unwrap_or_default();
113+
let releases = match fetch_releases(&client).await {
114+
Ok(r) => r,
115+
Err(e) => {
116+
tracing::debug!("Failed to fetch releases: {e}");
117+
Vec::new()
118+
}
119+
};
114120

115121
let latest_version = releases
116122
.first()

0 commit comments

Comments
 (0)