From aa9a81578441bf18dc74a1401cf17bcd30eeed91 Mon Sep 17 00:00:00 2001
From: Reactor Scram <ReactorScram@users.noreply.github.com>
Date: Mon, 12 Aug 2024 14:27:57 -0500
Subject: [PATCH] fix(gui-client): don't delete the log file we're currently
 writing to (#6226)

Closes #4764

```[tasklist]
- [x] Update changelog draft
- [x] Manual test Linux
```
---
 .../src-tauri/src/client/logging.rs           | 102 ++++++++++++++++--
 website/src/components/Changelog/GUI.tsx      |   4 +-
 2 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/rust/gui-client/src-tauri/src/client/logging.rs b/rust/gui-client/src-tauri/src/client/logging.rs
index 7c9060616..fc1dfc005 100644
--- a/rust/gui-client/src-tauri/src/client/logging.rs
+++ b/rust/gui-client/src-tauri/src/client/logging.rs
@@ -5,6 +5,7 @@ use anyhow::{bail, Context, Result};
 use firezone_headless_client::known_dirs;
 use serde::Serialize;
 use std::{
+    ffi::OsStr,
     fs,
     io::{self, ErrorKind::NotFound},
     path::{Path, PathBuf},
@@ -122,16 +123,15 @@ pub(crate) async fn clear_logs_inner() -> Result<()> {
                 return Err(error.into());
             }
         };
+        let mut paths = vec![];
         while let Some(entry) = dir.next_entry().await? {
-            let path = entry.path();
-            if let Err(error) = tokio::fs::remove_file(&path).await {
-                tracing::error!(
-                    ?error,
-                    path = path.display().to_string(),
-                    "Error while removing log file"
-                );
-                // We'll return the most recent error, it loses some information but it's better than nothing.
-                result = Err(error);
+            paths.push(entry.path());
+        }
+
+        let to_delete = choose_logs_to_delete(&paths);
+        for path in &to_delete {
+            if let Err(e) = tokio::fs::remove_file(path).await {
+                result = Err(e);
             }
         }
     }
@@ -139,6 +139,50 @@ pub(crate) async fn clear_logs_inner() -> Result<()> {
     Ok(result?)
 }
 
+fn choose_logs_to_delete(paths: &[PathBuf]) -> Vec<&Path> {
+    let mut most_recent_stem = None;
+    for path in paths {
+        if path.extension() != Some(OsStr::new("log")) {
+            continue;
+        }
+        let Some(stem) = path.file_stem() else {
+            continue;
+        };
+        match most_recent_stem {
+            None => most_recent_stem = Some(stem),
+            Some(most_recent) if stem > most_recent => most_recent_stem = Some(stem),
+            Some(_) => {}
+        }
+    }
+    let Some(most_recent_stem) = most_recent_stem else {
+        tracing::warn!(
+            "Nothing to delete, should be impossible since both processes always write logs"
+        );
+        return vec![];
+    };
+    let Some(most_recent_stem) = most_recent_stem.to_str() else {
+        tracing::warn!("Most recent log file does not have a UTF-8 path");
+        return vec![];
+    };
+
+    paths
+        .iter()
+        .filter(|path| {
+            let Some(stem) = path.file_stem() else {
+                return false;
+            };
+            let Some(stem) = stem.to_str() else {
+                return false;
+            };
+            if !stem.starts_with("connlib.") {
+                return false;
+            }
+            stem < most_recent_stem
+        })
+        .map(|x| x.as_path())
+        .collect()
+}
+
 /// Pops up the "Save File" dialog
 fn show_export_dialog(ctlr_tx: CtlrTx) -> Result<()> {
     let now = chrono::Local::now();
@@ -267,3 +311,43 @@ fn log_paths() -> Result<Vec<LogPath>> {
         },
     ])
 }
+
+#[cfg(test)]
+mod tests {
+    use std::path::Path;
+
+    #[test]
+    fn clear_logs_logic() {
+        // These are out of order just to make sure it works anyway
+        let paths: Vec<_> = [
+            "connlib.2024-08-05-19-41-46.jsonl",
+            "connlib.2024-08-05-19-41-46.log",
+            "connlib.2024-08-07-14-17-56.jsonl",
+            "connlib.2024-08-07-14-17-56.log",
+            "connlib.2024-08-06-14-21-13.jsonl",
+            "connlib.2024-08-06-14-21-13.log",
+            "connlib.2024-08-06-14-51-19.jsonl",
+            "connlib.2024-08-06-14-51-19.log",
+            "crash.2024-07-22-21-16-20.dmp",
+            "last_crash.dmp",
+        ]
+        .into_iter()
+        .map(|x| Path::new("/bogus").join(x))
+        .collect();
+        let to_delete = super::choose_logs_to_delete(&paths);
+        assert_eq!(
+            to_delete,
+            [
+                "/bogus/connlib.2024-08-05-19-41-46.jsonl",
+                "/bogus/connlib.2024-08-05-19-41-46.log",
+                "/bogus/connlib.2024-08-06-14-21-13.jsonl",
+                "/bogus/connlib.2024-08-06-14-21-13.log",
+                "/bogus/connlib.2024-08-06-14-51-19.jsonl",
+                "/bogus/connlib.2024-08-06-14-51-19.log",
+            ]
+            .into_iter()
+            .map(Path::new)
+            .collect::<Vec<_>>()
+        );
+    }
+}
diff --git a/website/src/components/Changelog/GUI.tsx b/website/src/components/Changelog/GUI.tsx
index 171dcdf30..3d5652014 100644
--- a/website/src/components/Changelog/GUI.tsx
+++ b/website/src/components/Changelog/GUI.tsx
@@ -15,7 +15,9 @@ export default function GUI({ title }: { title: string }) {
       {/*
       <Entry version="1.2.0" date={new Date("Invalid date")}>
         <ul className="list-disc space-y-2 pl-4 mb-4">
-          TODO
+          <ChangeItem pull="6226">
+            Fixes a bug where clearing the log files would delete the current files, preventing logs from being written.
+          </ChangeItem>
         </ul>
       </Entry>
       */}