Bug 1620998 - Add logging to the crash helper r=afranchuk

Differential Revision: https://phabricator.services.mozilla.com/D234401

Cargo.lock

@@ -1132,12 +1132,15 @@ dependencies = [
 name = "crash_helper_server"
 version = "0.1.0"
 dependencies = [
+ "android_logger",
  "anyhow",
  "cc",
  "cfg-if",
  "crash_helper_common",
  "dirs",
+ "env_logger",
  "linked-hash-map",
+ "log",
  "mozannotation_server",
  "mozbuild",
  "mozilla-central-workspace-hack",
@@ -1822,6 +1825,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "85cdab6a89accf66733ad5a1693a4dcced6aeff64602b634530dd73c1f3ee9f0"
 dependencies = [
  "log",
+ "regex",
  "termcolor",
 ]
 

toolkit/crashreporter/crash_helper_server/Cargo.toml

@@ -9,6 +9,7 @@ anyhow = "1"
 cfg-if = "1"
 crash_helper_common = { path = "../crash_helper_common" }
 dirs = "4"
+log = "0.4"
 mozannotation_server = { path = "../mozannotation_server" }
 mozbuild = "0.1"
 mozilla-central-workspace-hack = { version = "0.1", features = [
@@ -20,6 +21,12 @@ once_cell = "1"
 thiserror = "2"
 uuid = { version = "1.0", features = ["v4"] }
 
+# Use android_logger on Android, env_logger everywhere else
+[target.'cfg(not(target_os = "android"))'.dependencies]
+env_logger = { version = "0.10", default-features = false }
+[target.'cfg(target_os = "android")'.dependencies]
+android_logger = "0.12"
+
 [target."cfg(any(target_os = \"android\", target_os = \"linux\"))".dependencies]
 nix = { version = "0.29", features = ["poll", "socket", "uio"] }
 rust_minidump_writer_linux = { path = "../rust_minidump_writer_linux" }

toolkit/crashreporter/crash_helper_server/src/ipc_server.rs

@@ -50,9 +50,13 @@ impl IPCServer {
                         .connectors
                         .get_mut(index)
                         .expect("Invalid connector index");
-                    let _res = Self::handle_message(connector, &header, generator);
-                    // TODO: Errors at this level are always survivable, but we
-                    // should probably log them.
+                    let res = Self::handle_message(connector, &header, generator);
+                    if let Err(error) = res {
+                        log::error!(
+                            "Error {error} while handling a message of {:?} kind",
+                            header.kind
+                        );
+                    }
                 }
                 IPCEvent::Disconnect(index) => {
                     let connector = self

toolkit/crashreporter/crash_helper_server/src/lib.rs

@@ -8,6 +8,7 @@ extern crate rust_minidump_writer_linux;
 mod breakpad_crash_generator;
 mod crash_generation;
 mod ipc_server;
+mod logging;
 mod phc;
 
 use crash_helper_common::{BreakpadData, BreakpadRawData, IPCConnector, IPCListener, Pid};
@@ -39,6 +40,8 @@ pub unsafe extern "C" fn crash_generator_logic_desktop(
     listener: *const c_char,
     pipe: *const c_char,
 ) -> i32 {
+    logging::init();
+
     let breakpad_data = BreakpadData::new(breakpad_data);
     let minidump_path = unsafe { CStr::from_ptr(minidump_path) }
         .to_owned()
@@ -46,10 +49,24 @@ pub unsafe extern "C" fn crash_generator_logic_desktop(
         .unwrap();
     let minidump_path = OsString::from(minidump_path);
     let listener = unsafe { CStr::from_ptr(listener) };
-    let listener = IPCListener::deserialize(listener, client_pid).unwrap();
+    let listener = IPCListener::deserialize(listener, client_pid)
+        .map_err(|error| {
+            log::error!("Could not parse the crash generator's listener (error: {error})");
+        })
+        .unwrap();
     let pipe = unsafe { CStr::from_ptr(pipe) };
-    let connector = IPCConnector::deserialize(pipe).unwrap();
-    let crash_generator = CrashGenerator::new(client_pid, breakpad_data, minidump_path).unwrap();
+    let connector = IPCConnector::deserialize(pipe)
+        .map_err(|error| {
+            log::error!("Could not parse the crash generator's connector (error: {error})");
+        })
+        .unwrap();
+
+    let crash_generator = CrashGenerator::new(client_pid, breakpad_data, minidump_path)
+        .map_err(|error| {
+            log::error!("Could not create the crash generator (error: {error})");
+            error
+        })
+        .unwrap();
 
     let ipc_server = IPCServer::new(client_pid, listener, connector);
 
@@ -77,18 +94,33 @@ pub unsafe extern "C" fn crash_generator_logic_android(
     listener: RawFd,
     pipe: RawFd,
 ) {
+    logging::init();
+
     let breakpad_data = BreakpadData::new(breakpad_data);
     let minidump_path = unsafe { CStr::from_ptr(minidump_path) }
         .to_owned()
         .into_string()
         .unwrap();
     let minidump_path = OsString::from(minidump_path);
-    let crash_generator = CrashGenerator::new(client_pid, breakpad_data, minidump_path).unwrap();
+    let crash_generator = CrashGenerator::new(client_pid, breakpad_data, minidump_path)
+        .map_err(|error| {
+            log::error!("Could not create the crash generator (error: {error})");
+            error
+        })
+        .unwrap();
 
     let listener = unsafe { OwnedFd::from_raw_fd(listener) };
-    let listener = IPCListener::from_fd(client_pid, listener).unwrap();
+    let listener = IPCListener::from_fd(client_pid, listener)
+        .map_err(|error| {
+            log::error!("Could not use the listener (error: {error})");
+        })
+        .unwrap();
     let pipe = unsafe { OwnedFd::from_raw_fd(pipe) };
-    let connector = IPCConnector::from_fd(pipe).unwrap();
+    let connector = IPCConnector::from_fd(pipe)
+        .map_err(|error| {
+            log::error!("Could not use the pipe (error: {error})");
+        })
+        .unwrap();
     let ipc_server = IPCServer::new(client_pid, listener, connector);
 
     // On Android the main thread is used to respond to the intents so we
@@ -102,7 +134,8 @@ fn main_loop(mut ipc_server: IPCServer, mut crash_generator: CrashGenerator) ->
             Ok(_result @ IPCServerState::ClientDisconnected) => {
                 return 0;
             }
-            Err(_error) => {
+            Err(error) => {
+                log::error!("The crashhelper encountered an error, exiting (error: {error})");
                 return -1;
             }
             _ => {} // Go on

toolkit/crashreporter/crash_helper_server/src/logging.rs

@@ -0,0 +1,15 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+#[cfg(target_os = "android")]
+pub(crate) use android::init;
+
+#[cfg(target_os = "android")]
+pub(crate) mod android;
+
+#[cfg(not(target_os = "android"))]
+pub(crate) use env::init;
+
+#[cfg(not(target_os = "android"))]
+pub(crate) mod env;

toolkit/crashreporter/crash_helper_server/src/logging/android.rs

@@ -0,0 +1,12 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+/// Initialize logging and place the logging file in
+pub(crate) fn init() {
+    android_logger::init_once(
+        android_logger::Config::default()
+            .with_max_level(log::LevelFilter::Trace)
+            .with_tag("GeckoCrashHelper"),
+    );
+}

toolkit/crashreporter/crash_helper_server/src/logging/env.rs

@@ -0,0 +1,58 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+use cfg_if::cfg_if;
+use mozbuild::config;
+use std::{
+    fs::{self, File},
+    io,
+    path::PathBuf,
+};
+
+/// Initialize logging
+pub(crate) fn init() {
+    let user_app_data_dir = guess_user_app_data_dir();
+    let log_target = make_log_target(user_app_data_dir);
+
+    env_logger::builder()
+        .filter_level(log::LevelFilter::Warn)
+        .parse_env(
+            env_logger::Env::new()
+                .filter("CRASH_HELPER_LOG")
+                .write_style("CRASH_HELPER_LOG_STYLE"),
+        )
+        .target(env_logger::fmt::Target::Pipe(log_target))
+        .init();
+}
+
+// The crash helper might be launched before Firefox has a chance to provide
+// the UAppData special directory, so we generate its value autonomously here.
+fn guess_user_app_data_dir() -> Option<PathBuf> {
+    let home_dir = dirs::home_dir()?;
+
+    cfg_if! {
+        if #[cfg(target_os = "linux")] {
+            Some(home_dir.join(".mozilla").join(config::MOZ_APP_NAME))
+        } else if #[cfg(target_os = "macos")] {
+            Some(home_dir.join("Library").join("Application Support").join(config::MOZ_APP_BASENAME))
+        } else if #[cfg(target_os = "windows")] {
+            Some(home_dir.join("AppData").join("Roaming").join(config::MOZ_APP_VENDOR).join(config::MOZ_APP_BASENAME))
+        } else {
+            None
+        }
+    }
+}
+
+fn make_log_target(log_directory: Option<PathBuf>) -> Box<dyn io::Write + Send + 'static> {
+    if let Some(log_directory) = log_directory.map(|path| path.join("Crash Reports")) {
+        if fs::create_dir_all(&log_directory).is_ok() {
+            let log_path = log_directory.join(concat!(env!("CARGO_PKG_NAME"), ".log"));
+            if let Ok(log) = File::create(log_path) {
+                return Box::new(log);
+            }
+        }
+    }
+
+    Box::new(io::stderr())
+}

toolkit/crashreporter/nsExceptionHandler.cpp

@@ -420,13 +420,18 @@ static void CreateFileFromPath(const xpstring& path, nsIFile** file) {
       DependentPathString(path.c_str(), path.size()), file);
 }
 
-[[nodiscard]] static std::optional<xpstring> CreatePathFromFile(nsIFile* file) {
-  AutoPathString path;
+nsresult GetNativePathFromFile(nsIFile* aFile, PathString& aPathString) {
 #ifdef XP_WIN
-  nsresult rv = file->GetPath(path);
+  return aFile->GetPath(aPathString);
 #else
-  nsresult rv = file->GetNativePath(path);
+  return aFile->GetNativePath(aPathString);
 #endif
+}
+
+[[nodiscard]]
+static std::optional<xpstring> CreatePathFromFile(nsIFile* file) {
+  AutoPathString path;
+  nsresult rv = GetNativePathFromFile(file, path);
   if (NS_FAILED(rv)) {
     return {};
   }
@@ -2285,11 +2290,7 @@ nsresult SetupExtraData(nsIFile* aAppDataDirectory,
   memset(lastCrashTimeFilename, 0, sizeof(lastCrashTimeFilename));
 
   PathString filename;
-#if defined(XP_WIN)
-  rv = lastCrashFile->GetPath(filename);
-#else
-  rv = lastCrashFile->GetNativePath(filename);
-#endif
+  rv = GetNativePathFromFile(lastCrashFile, filename);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (filename.Length() < XP_PATH_MAX) {
@@ -2871,12 +2872,9 @@ static void PopulatePendingDir(nsIFile* aUserAppDataDir) {
   pendingDir->Append(u"pending"_ns);
 
   PathString path;
-#ifdef XP_WIN
-  pendingDir->GetPath(path);
-#else
-  pendingDir->GetNativePath(path);
-#endif
-  pendingDirectory = xpstring(path.get());
+  if (NS_SUCCEEDED(GetNativePathFromFile(pendingDir, path))) {
+    pendingDirectory = xpstring(path.get());
+  }
 }
 
 void SetUserAppDataDirectory(nsIFile* aDir) {
@@ -2930,12 +2928,9 @@ void SetMemoryReportFile(nsIFile* aFile) {
   }
 
   PathString path;
-#ifdef XP_WIN
-  aFile->GetPath(path);
-#else
-  aFile->GetNativePath(path);
-#endif
-  memoryReportPath = xpstring(path.get());
+  if (NS_SUCCEEDED(GetNativePathFromFile(aFile, path))) {
+    memoryReportPath = xpstring(path.get());
+  }
 }
 
 nsresult GetDefaultMemoryReportFile(nsIFile** aFile) {
@@ -3128,11 +3123,7 @@ bool WriteExtraFile(const nsAString& id, const AnnotationTable& annotations) {
 
   extra->Append(id + u".extra"_ns);
   PathString path;
-#ifdef XP_WIN
-  NS_ENSURE_SUCCESS(extra->GetPath(path), false);
-#elif defined(XP_UNIX)
-  NS_ENSURE_SUCCESS(extra->GetNativePath(path), false);
-#endif
+  NS_ENSURE_SUCCESS(GetNativePathFromFile(extra, path), false);
 
   PlatformWriter pw(path.get());
   return WriteExtraFile(pw, annotations);