Merge branch 'problem-report-automatically-include-frontend-logs'

3 files changed, 89 insertions, 24 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b5c2b0abdf..b8666cbe48 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,6 +34,7 @@ Line wrap the file at 100 chars.                                              Th
 - Add translations for the current location displayed on the main screen in the GUI.
 - Allow a subset of NDP (Router solicitation, router advertisement and redirects) in the firewall.
 - Allow setting proxy mode from UI.
+- Automatically include frontend logs in problem report when ran from CLI.
 
 #### Linux
 - Add standard window decorations to the application window.
diff --git a/gui/src/main/index.ts b/gui/src/main/index.ts
index b4c97cbb2c..15a137dd10 100644
--- a/gui/src/main/index.ts
+++ b/gui/src/main/index.ts
@@ -70,9 +70,6 @@ class ApplicationMain {
   private daemonRpc = new DaemonRpc();
   private reconnectBackoff = new ReconnectionBackoff();
   private connectedToDaemon = false;
-
-  private logFilePaths: string[] = [];
-  private oldLogFilePaths: string[] = [];
   private quitStage = AppQuitStage.unready;
 
   private accountHistory: AccountToken[] = [];
@@ -188,17 +185,15 @@ class ApplicationMain {
   private initLogging() {
     const logDirectory = getLogsDirectory();
     const mainLogFile = getMainLogFile();
-    const rendererLogFile = getRendererLogFile();
-    const logFiles = [mainLogFile, rendererLogFile];
+    const logFiles = [mainLogFile, getRendererLogFile()];
 
     if (process.env.NODE_ENV !== 'development') {
       // Ensure log directory exists
       mkdirp.sync(logDirectory);
 
-      this.logFilePaths = logFiles;
-      this.oldLogFilePaths = logFiles
-        .map((logFile) => backupLogFile(logFile))
-        .filter((oldLogFile): oldLogFile is string => typeof oldLogFile === 'string');
+      for (const logFile of logFiles) {
+        backupLogFile(logFile);
+      }
     }
 
     setupLogging(mainLogFile);
@@ -886,10 +881,8 @@ class ApplicationMain {
       const executable = resolveBin('problem-report');
       const args = ['collect', '--output', reportPath];
       if (toRedact.length > 0) {
-        args.push('--redact', ...toRedact, '--');
+        args.push('--redact', ...toRedact);
       }
-      args.push(...this.logFilePaths);
-      args.push(...this.oldLogFilePaths);
 
       execFile(executable, args, { windowsHide: true }, (error, stdout, stderr) => {
         if (error) {
diff --git a/mullvad-problem-report/src/main.rs b/mullvad-problem-report/src/main.rs
index 8d47ebbb98..868aadd2ef 100644
--- a/mullvad-problem-report/src/main.rs
+++ b/mullvad-problem-report/src/main.rs
@@ -15,6 +15,7 @@ use std::{
     borrow::Cow,
     cmp::min,
     collections::{HashMap, HashSet},
+    env,
     ffi::OsStr,
     fs::{self, File},
     io::{self, BufWriter, Read, Seek, SeekFrom, Write},
@@ -87,10 +88,7 @@ pub enum LogError {
     #[error(display = "Unable to get log directory")]
     GetLogDir(#[error(source)] mullvad_paths::Error),
 
-    #[error(
-        display = "Failed to list the files in the mullvad-daemon log directory: {}",
-        path
-    )]
+    #[error(display = "Failed to list the files in the log directory: {}", path)]
     ListLogDir {
         path: String,
         #[error(cause)]
@@ -99,6 +97,14 @@ pub enum LogError {
 
     #[error(display = "Error reading the contents of log file: {}", path)]
     ReadLogError { path: String },
+
+    #[cfg(any(target_os = "linux", target_os = "macos"))]
+    #[error(display = "No home directory for current user")]
+    NoHomeDir,
+
+    #[cfg(target_os = "windows")]
+    #[error(display = "Missing %LOCALAPPDATA% environment variable")]
+    NoLocalAppDataDir,
 }
 
 fn main() {
@@ -191,7 +197,19 @@ fn run() -> Result<(), Error> {
             .map(|os_values| os_values.map(Path::new).collect())
             .unwrap_or_else(Vec::new);
         let output_path = Path::new(collect_matches.value_of_os("output").unwrap());
-        collect_report(&extra_logs, output_path, redact_custom_strings)
+        collect_report(&extra_logs, output_path, redact_custom_strings)?;
+
+        let expanded_output_path = output_path
+            .canonicalize()
+            .unwrap_or_else(|_| output_path.to_owned());
+        println!(
+            "Problem report written to {}",
+            expanded_output_path.display()
+        );
+        println!("");
+        println!("Send the problem report to support via the send subcommand. See:");
+        println!(" $ {} send --help", env::args().next().unwrap());
+        Ok(())
     } else if let Some(send_matches) = matches.subcommand_matches("send") {
         let report_path = Path::new(send_matches.value_of_os("report").unwrap());
         let user_email = send_matches.value_of("email").unwrap_or("");
@@ -209,10 +227,13 @@ fn collect_report(
 ) -> Result<(), Error> {
     let mut problem_report = ProblemReport::new(redact_custom_strings);
 
-    match logs_from_log_directory() {
-        Ok(logs) => {
+    let daemon_logs = mullvad_paths::get_log_dir()
+        .map_err(LogError::GetLogDir)
+        .and_then(list_logs);
+    match daemon_logs {
+        Ok(daemon_logs) => {
             let mut other_logs = Vec::new();
-            for log in logs {
+            for log in daemon_logs {
                 match log {
                     Ok(path) => {
                         if is_tunnel_log(&path) {
@@ -228,8 +249,24 @@ fn collect_report(
                 problem_report.add_log(&other_log);
             }
         }
-        Err(error) => problem_report.add_error("Failed to list logs in log directory", &error),
+        Err(error) => {
+            problem_report.add_error("Failed to list logs in daemon log directory", &error)
+        }
     };
+    match frontend_log_dir().map(|dir| dir.and_then(list_logs)) {
+        Some(Ok(frontend_logs)) => {
+            for log in frontend_logs {
+                match log {
+                    Ok(path) => problem_report.add_log(&path),
+                    Err(error) => problem_report.add_error("Unable to get log path", &error),
+                }
+            }
+        }
+        Some(Err(error)) => {
+            problem_report.add_error("Failed to list logs in frontend log directory", &error)
+        }
+        None => {}
+    }
 
     problem_report.add_logs(extra_logs);
 
@@ -239,9 +276,10 @@ fn collect_report(
     })
 }
 
-fn logs_from_log_directory() -> Result<impl Iterator<Item = Result<PathBuf, LogError>>, LogError> {
-    let log_dir = mullvad_paths::get_log_dir().map_err(LogError::GetLogDir)?;
-
+/// Returns an iterator over all files in the given directory that has the `.log` extension.
+fn list_logs(
+    log_dir: PathBuf,
+) -> Result<impl Iterator<Item = Result<PathBuf, LogError>>, LogError> {
     fs::read_dir(&log_dir)
         .map_err(|source| LogError::ListLogDir {
             path: log_dir.display().to_string(),
@@ -268,6 +306,38 @@ fn logs_from_log_directory() -> Result<impl Iterator<Item = Result<PathBuf, LogE
         })
 }
 
+/// Returns the directory where the Mullvad GUI frontend stores its logs.
+/// If the current platform has a separate directory for frontend logs.
+fn frontend_log_dir() -> Option<Result<PathBuf, LogError>> {
+    #[cfg(target_os = "linux")]
+    {
+        Some(
+            dirs::home_dir()
+                .ok_or(LogError::NoHomeDir)
+                .map(|home_dir| home_dir.join(".config/Mullvad VPN/logs")),
+        )
+    }
+    #[cfg(target_os = "macos")]
+    {
+        Some(
+            dirs::home_dir()
+                .ok_or(LogError::NoHomeDir)
+                .map(|home_dir| home_dir.join("Library/Logs/Mullvad VPN")),
+        )
+    }
+    #[cfg(target_os = "windows")]
+    {
+        Some(match std::env::var_os("LOCALAPPDATA") {
+            Some(dir) => Ok(Path::new(&dir).join("Mullvad VPN/logs")),
+            None => Err(LogError::NoLocalAppDataDir),
+        })
+    }
+    #[cfg(target_os = "android")]
+    {
+        None
+    }
+}
+
 fn is_tunnel_log(path: &Path) -> bool {
     match path.file_name() {
         Some(file_name) => file_name.to_string_lossy().contains("openvpn"),
@@ -362,6 +432,7 @@ impl ProblemReport {
                 },
             ));
             self.logs.push((redacted_path, content));
+            println!("Adding {}", expanded_path.display());
         }
     }