Deactivate the signal handler & activate Sentry on_crash callback (#308)

* Deactivate the signal handler & activate Sentry on_crash callback * Fix tests * Address Luc's comments * Test of writeEvent added * Address Luc's comments * Fix Windows test * Fix Windows test --------- Co-authored-by: Herve Eruam <[email protected]>
Infomaniak · Sep 20, 2024 · 976f60f · 976f60f
1 parent c99d61c
commit 976f60f
Show file tree

Hide file tree

Showing 10 changed files with 157 additions and 51 deletions.
diff --git a/src/gui/mainclient.cpp b/src/gui/mainclient.cpp
@@ -65,16 +65,13 @@ void signalHandler(int signum) {
 }
 
 int main(int argc, char **argv) {
-#if defined(__APPLE__) || defined(_WIN32)
-    // No sig handler on Linux as it interferes with Sentry
-    KDC::CommonUtility::handleSignals(signalHandler);
-#endif
+    // KDC::CommonUtility::handleSignals(signalHandler); // !!! The signal handler interferes with Sentry !!!
 
     std::cout << "kDrive client starting" << std::endl;
 
     // Working dir;
     KDC::CommonUtility::_workingDirPath = KDC::SyncPath(argv[0]).parent_path();
-    KDC::SentryHandler::init(KDC::SentryHandler::SentryProject::Client);
+    KDC::SentryHandler::init(KDC::AppType::Client);
     KDC::SentryHandler::instance()->setGlobalConfidentialityLevel(KDC::SentryConfidentialityLevel::Authenticated);
 
 #ifdef Q_OS_LINUX

diff --git a/src/libcommon/log/sentry/sentryhandler.cpp b/src/libcommon/log/sentry/sentryhandler.cpp
@@ -22,12 +22,19 @@
 #include "utility/utility.h"
 
 #include <iostream>
+#include <fstream>
+#include <sstream>
+
 #include <asserts.h>
 
 namespace KDC {
 
 std::shared_ptr<SentryHandler> SentryHandler::_instance = nullptr;
 
+KDC::AppType SentryHandler::_appType = KDC::AppType::None;
+bool SentryHandler::_debugCrashCallback = false;
+bool SentryHandler::_debugBeforeSendCallback = false;
+
 /*
  *  sentry_value_t reader implementation - begin
  *  Used for debbuging
@@ -69,53 +76,53 @@ static thing_t *valueAsThing(sentry_value_t value)
     return (thing_t *)(size_t)value._bits;
 }
 
-static void readObject(const sentry_value_t event, int level = 0);
-static void readList(const sentry_value_t event, int level = 0);
+static void readObject(const sentry_value_t event, std::stringstream &ss, int level = 0);
+static void readList(const sentry_value_t event, std::stringstream &ss, int level = 0);
 
-static void printValue(const sentry_value_t value, int level) {
+static void printValue(std::stringstream &ss, const sentry_value_t value, int level) {
     switch (sentry_value_get_type(value)) {
         case SENTRY_VALUE_TYPE_NULL:
-            std::cout << "NULL" << std::endl;
+            ss << "NULL" << std::endl;
             break;
         case SENTRY_VALUE_TYPE_BOOL:
-            std::cout << (sentry_value_is_true(value) ? "true" : "false") << std::endl;
+            ss << (sentry_value_is_true(value) ? "true" : "false") << std::endl;
             break;
         case SENTRY_VALUE_TYPE_INT32:
-            std::cout << sentry_value_as_int32(value) << std::endl;
+            ss << sentry_value_as_int32(value) << std::endl;
             break;
         case SENTRY_VALUE_TYPE_DOUBLE:
-            std::cout << sentry_value_as_double(value) << std::endl;
+            ss << sentry_value_as_double(value) << std::endl;
             break;
         case SENTRY_VALUE_TYPE_STRING:
-            std::cout << sentry_value_as_string(value) << std::endl;
+            ss << sentry_value_as_string(value) << std::endl;
             break;
         case SENTRY_VALUE_TYPE_LIST: {
-            std::cout << "LIST" << std::endl;
-            readList(value, level + 1);
+            ss << "LIST" << std::endl;
+            readList(value, ss, level + 1);
             break;
         }
         case SENTRY_VALUE_TYPE_OBJECT: {
-            std::cout << "OBJECT" << std::endl;
-            readObject(value, level + 1);
+            ss << "OBJECT" << std::endl;
+            readObject(value, ss, level + 1);
             break;
         }
     }
 }
 
-static void readList(const sentry_value_t event, int level) {
+static void readList(const sentry_value_t event, std::stringstream &ss, int level) {
     if (sentry_value_get_type(event) != SENTRY_VALUE_TYPE_LIST) {
         return;
     }
 
     const list_t *l = (list_t *)valueAsThing(event)->payload._ptr;
     const std::string indent(level, ' ');
     for (size_t i = 0; i < l->len; i++) {
-        std::cout << indent.c_str() << "val=";
-        printValue(l->items[i], level);
+        ss << indent.c_str() << "val=";
+        printValue(ss, l->items[i], level);
     }
 }
 
-static void readObject(const sentry_value_t event, int level) {
+static void readObject(const sentry_value_t event, std::stringstream &ss, int level) {
     if (sentry_value_get_type(event) != SENTRY_VALUE_TYPE_OBJECT) {
         return;
     }
@@ -129,8 +136,8 @@ static void readObject(const sentry_value_t event, int level) {
     const std::string indent(level, ' ');
     for (size_t i = 0; i < obj->len; i++) {
         char *key = obj->pairs[i].k;
-        std::cout << indent.c_str() << "key=" << key << " val=";
-        printValue(obj->pairs[i].v, level);
+        ss << indent.c_str() << "key=" << key << " val=";
+        printValue(ss, obj->pairs[i].v, level);
     }
 }
 
@@ -142,13 +149,17 @@ static sentry_value_t crashCallback(const sentry_ucontext_t *uctx, sentry_value_
     (void)uctx;
     (void)closure;
 
-    readObject(event);
+    std::cerr << "Sentry detected a crash in the app " << SentryHandler::appType() << std::endl;
 
-    // signum is unknown, all crashes will be considered as kills
-    KDC::SignalType signalType = KDC::fromInt<KDC::SignalType>(0);
-    std::cerr << "Server stopped with signal " << signalType << std::endl;
+    // As `signum` is unknown, a crash is considered as a kill.
+    const int signum{0};
+    KDC::CommonUtility::writeSignalFile(SentryHandler::appType(), KDC::fromInt<KDC::SignalType>(signum));
 
-    KDC::CommonUtility::writeSignalFile(KDC::AppType::Server, signalType);
+    if (SentryHandler::debugCrashCallback()) {
+        std::stringstream ss;
+        readObject(event, ss);
+        SentryHandler::writeEvent(ss.str(), true);
+    }
 
     return event;
 }
@@ -157,7 +168,13 @@ sentry_value_t beforeSendCallback(sentry_value_t event, void *hint, void *closur
     (void)hint;
     (void)closure;
 
-    readObject(event);
+    std::cout << "Sentry will send an event for the app " << SentryHandler::appType() << std::endl;
+
+    if (SentryHandler::debugBeforeSendCallback()) {
+        std::stringstream ss;
+        readObject(event, ss);
+        SentryHandler::writeEvent(ss.str(), false);
+    }
 
     return event;
 }
@@ -172,28 +189,43 @@ std::shared_ptr<SentryHandler> SentryHandler::instance() {
     return _instance;
 }
 
-void SentryHandler::init(SentryProject project, int breadCrumbsSize) {
+void SentryHandler::init(KDC::AppType appType, int breadCrumbsSize) {
     if (_instance) {
         assert(false && "SentryHandler already initialized");
         return;
     }
 
     _instance = std::shared_ptr<SentryHandler>(new SentryHandler());
 
-    if (project == SentryProject::Deactivated) {
+    if (appType == KDC::AppType::None) {
         _instance->_isSentryActivated = false;
         return;
     }
 
+    _appType = appType;
+
+    // For debugging: if the following environment variable is set, the crash event will be printed into a debug file
+    bool isSet = false;
+    if (CommonUtility::envVarValue("KDRIVE_DEBUG_SENTRY_CRASH_CB", isSet); isSet) {
+        _debugCrashCallback = true;
+    }
+
+    // For debbuging: if the following environment variable is set, the send events will be printed into a debug file
+    // If this variable is set, the previous one is inoperative
+    isSet = false;
+    if (CommonUtility::envVarValue("KDRIVE_DEBUG_SENTRY_BEFORE_SEND_CB", isSet); isSet) {
+        _debugBeforeSendCallback = true;
+    }
+
     // Sentry init
     sentry_options_t *options = sentry_options_new();
-    sentry_options_set_dsn(options, ((project == SentryProject::Server) ? SENTRY_SERVER_DSN : SENTRY_CLIENT_DSN));
+    sentry_options_set_dsn(options, ((appType == KDC::AppType::Server) ? SENTRY_SERVER_DSN : SENTRY_CLIENT_DSN));
 #if defined(Q_OS_WIN) || defined(Q_OS_MAC)
     const SyncPath appWorkingPath = CommonUtility::getAppWorkingDir() / SENTRY_CRASHPAD_HANDLER_NAME;
 #endif
 
     SyncPath appSupportPath = CommonUtility::getAppSupportDir();
-    appSupportPath /= (project == SentryProject::Server) ? SENTRY_SERVER_DB_PATH : SENTRY_CLIENT_DB_PATH;
+    appSupportPath /= (_appType == AppType::Server) ? SENTRY_SERVER_DB_PATH : SENTRY_CLIENT_DB_PATH;
 #if defined(Q_OS_WIN)
     sentry_options_set_handler_pathw(options, appWorkingPath.c_str());
     sentry_options_set_database_pathw(options, appSupportPath.c_str());
@@ -205,15 +237,13 @@ void SentryHandler::init(SentryProject project, int breadCrumbsSize) {
     sentry_options_set_debug(options, false);
     sentry_options_set_max_breadcrumbs(options, breadCrumbsSize);
 
-    bool isSet = false;
-#if defined(Q_OS_LINUX)
-    if (CommonUtility::envVarValue("KDRIVE_DEBUG_SENTRY_CRASH_CB", isSet); isSet) {
-        sentry_options_set_on_crash(options, crashCallback, NULL);
-    }
-    if (CommonUtility::envVarValue("KDRIVE_DEBUG_SENTRY_BEFORE_SEND_CB", isSet); isSet) {
+    // !!! Not Supported in Crashpad on macOS & Limitations in Crashpad on Windows for Fast-fail Crashes !!!
+    // See https://docs.sentry.io/platforms/native/configuration/filtering/
+    if (_debugBeforeSendCallback) {
         sentry_options_set_before_send(options, beforeSendCallback, nullptr);
+    } else {
+        sentry_options_set_on_crash(options, crashCallback, nullptr);
     }
-#endif
 
     // Set the environment
     isSet = false;
@@ -374,6 +404,20 @@ void SentryHandler::updateEffectiveSentryUser(const SentryUser &user) {
     sentry_set_user(userValue);
 }
 
+void SentryHandler::writeEvent(const std::string &eventStr, bool crash) noexcept {
+    using namespace KDC::event_dump_files;
+    auto eventFilePath =
+            std::filesystem::temp_directory_path() / (SentryHandler::appType() == AppType::Server
+                                                              ? (crash ? serverCrashEventFileName : serverSendEventFileName)
+                                                              : (crash ? clientCrashEventFileName : clientSendEventFileName));
+
+    std::ofstream eventFile(eventFilePath, std::ios::app);
+    if (eventFile) {
+        eventFile << eventStr << std::endl;
+        eventFile.close();
+    }
+}
+
 SentryHandler::~SentryHandler() {
     if (this == _instance.get()) {
         _instance.reset();

diff --git a/src/libcommon/log/sentry/sentryhandler.h b/src/libcommon/log/sentry/sentryhandler.h
@@ -54,11 +54,9 @@ inline std::string toString(SentryLevel level) {
 
 class SentryHandler {
     public:
-        enum class SentryProject { Server, Client, Deactivated }; // Only used for initialization, don't need to be in types.h
-
         virtual ~SentryHandler();
         static std::shared_ptr<SentryHandler> instance();
-        static void init(SentryProject project, int breadCrumbsSize = 100);
+        static void init(KDC::AppType appType, int breadCrumbsSize = 100);
         void setAuthenticatedUser(const SentryUser &user);
         void setGlobalConfidentialityLevel(SentryConfidentialityLevel level);
         /*   If the same event has been captured more than 10 times in the last 10 minutes, it will be flagged as a rate limited
@@ -74,6 +72,12 @@ class SentryHandler {
          */
         void captureMessage(SentryLevel level, const std::string &title, std::string message,
                             const SentryUser &user = SentryUser());
+        inline static AppType appType() { return _appType; }
+        inline static bool debugCrashCallback() { return _debugCrashCallback; }
+        inline static bool debugBeforeSendCallback() { return _debugBeforeSendCallback; }
+
+        // Print an event description into a file (for debugging)
+        static void writeEvent(const std::string &eventStr, bool crash) noexcept;
 
     protected:
         SentryHandler() = default;
@@ -95,7 +99,7 @@ class SentryHandler {
 
                 SentryEvent(const std::string &title, const std::string &message, SentryLevel level,
                             SentryConfidentialityLevel userType, const SentryUser &user);
-                std::string getStr() const { return title + message + static_cast<char>(level) + userId; };
+                std::string getStr() const { return title + message + static_cast<char>(level) + userId; }
                 std::string title;
                 std::string message;
                 SentryLevel level;
@@ -146,5 +150,9 @@ class SentryHandler {
         unsigned int _sentryMaxCaptureCountBeforeRateLimit = 10; // Number of capture before rate limiting an event
         int _sentryMinUploadIntervaOnRateLimit = 60; // Min. interval between two uploads of a rate limited event (seconds)
         bool _isSentryActivated = false;
+
+        static KDC::AppType _appType;
+        static bool _debugCrashCallback;
+        static bool _debugBeforeSendCallback;
 };
 } // namespace KDC
diff --git a/src/libcommon/utility/types.cpp b/src/libcommon/utility/types.cpp
@@ -793,6 +793,8 @@ std::string toString(const SentryConfidentialityLevel e) {
 
 std::string toString(const AppType e) {
     switch (e) {
+        case AppType::None:
+            return "None";
         case AppType::Server:
             return "Server";
         case AppType::Client:

diff --git a/src/libcommon/utility/types.h b/src/libcommon/utility/types.h
@@ -91,6 +91,7 @@ using OStringStream = std::ostringstream;
 #define Str2Path(s) std::filesystem::path(KDC::Utility::s2ws(s))
 #endif
 
+namespace event_dump_files {
 static constexpr std::string_view serverCrashFileName(
         "kdrive.crash"); // Name of the file generated by the crash handler when a crash of the server occurs
 static constexpr std::string_view serverKillFileName(
@@ -100,7 +101,17 @@ static constexpr std::string_view clientCrashFileName(
 static constexpr std::string_view clientKillFileName(
         "kdrive_client.kill"); // Name of the file generated by the crash handler when a kill of the client occurs
 
-enum class AppType { Server, Client };
+static constexpr std::string_view serverCrashEventFileName(
+        "kdrive.crash.event"); // Name of the debug file generated by Sentry on_crash callback of the server
+static constexpr std::string_view serverSendEventFileName(
+        "kdrive.send.event"); // Name of the debug file generated by Sentry before_send callback of the server
+static constexpr std::string_view clientCrashEventFileName(
+        "kdrive_client.crash.event"); // Name of the debug file generated by Sentry on_crash callback of the client
+static constexpr std::string_view clientSendEventFileName(
+        "kdrive_client.send.event"); // Name of the debug file generated by Sentry before_send callback of the client
+} // namespace event_dump_files
+
+enum class AppType { None, Server, Client };
 std::string toString(AppType e);
 
 enum class SignalCategory { Kill, Crash };

diff --git a/src/libcommon/utility/utility.cpp b/src/libcommon/utility/utility.cpp
@@ -713,6 +713,7 @@ void CommonUtility::extractIntFromStrVersion(const std::string &version, std::ve
 }
 
 SyncPath CommonUtility::signalFilePath(AppType appType, SignalCategory signalCategory) {
+    using namespace KDC::event_dump_files;
     auto sigFilePath =
             std::filesystem::temp_directory_path() /
             (appType == AppType::Server ? (signalCategory == SignalCategory::Crash ? serverCrashFileName : serverKillFileName)

diff --git a/src/server/mainserver.cpp b/src/server/mainserver.cpp
@@ -64,16 +64,13 @@ void signalHandler(int signum) {
 }
 
 int main(int argc, char **argv) {
-#if defined(__APPLE__) || defined(_WIN32)
-    // No sig handler on Linux as it interferes with Sentry
-    KDC::CommonUtility::handleSignals(signalHandler);
-#endif
+    // KDC::CommonUtility::handleSignals(signalHandler); // !!! The signal handler interferes with Sentry !!!
 
     std::cout << "kDrive server starting" << std::endl;
 
     // Working dir;
     KDC::CommonUtility::_workingDirPath = KDC::SyncPath(argv[0]).parent_path();
-    KDC::SentryHandler::init(KDC::SentryHandler::SentryProject::Server);
+    KDC::SentryHandler::init(KDC::AppType::Server);
     KDC::SentryHandler::instance()->setGlobalConfidentialityLevel(KDC::SentryConfidentialityLevel::Authenticated); 
 
     Q_INIT_RESOURCE(client);