WIP: feat: feedback handler (squashed)

Author: jpnurmi

CMakeLists.txt

@@ -692,6 +692,7 @@ endif()
 
 if(SENTRY_BUILD_TESTS)
 	add_subdirectory(tests/unit)
+	add_subdirectory(tests/fixtures/feedback)
 	add_subdirectory(tests/fixtures/screenshot)
 	if(SENTRY_BUILD_BENCHMARKS)
 		set(BENCHMARK_ENABLE_TESTING OFF)

examples/example.c

@@ -381,6 +381,15 @@ main(int argc, char **argv)
         sentry_options_add_view_hierarchy(options, "./view-hierarchy.json");
     }
 
+    if (has_arg(argc, argv, "install-feedback-handler")) {
+#ifdef SENTRY_PLATFORM_WINDOWS
+        sentry_options_set_feedback_handler_pathw(
+            options, L"sentry_feedback.exe");
+#else
+        sentry_options_set_feedback_handler_path(options, "./sentry_feedback");
+#endif
+    }
+
     sentry_init(options);
 
     if (has_arg(argc, argv, "attachment")) {

external/crashpad

@@ -1316,6 +1316,23 @@ SENTRY_API void sentry_options_set_handler_path(
 SENTRY_API void sentry_options_set_handler_path_n(
     sentry_options_t *opts, const char *path, size_t path_len);
 
+/**
+ * Sets the path to the feedback handler executable.
+ *
+ * The feedback handler is a separate process that gets spawned when a crash
+ * occurs to collect additional user feedback or perform custom actions.
+ * The handler receives the path to the crash event envelope as its first
+ * argument.
+ *
+ * `path` is assumed to be in platform-specific filesystem path encoding.
+ * API Users on windows are encouraged to use
+ * `sentry_options_set_feedback_handler_pathw` instead.
+ */
+SENTRY_API void sentry_options_set_feedback_handler_path(
+    sentry_options_t *opts, const char *path);
+SENTRY_API void sentry_options_set_feedback_handler_path_n(
+    sentry_options_t *opts, const char *path, size_t path_len);
+
 /**
  * Sets the path to the Sentry Database Directory.
  *
@@ -1375,6 +1392,14 @@ SENTRY_API void sentry_options_set_handler_pathw(
 SENTRY_API void sentry_options_set_handler_pathw_n(
     sentry_options_t *opts, const wchar_t *path, size_t path_len);
 
+/**
+ * Wide char version of `sentry_options_set_feedback_handler_path`.
+ */
+SENTRY_API void sentry_options_set_feedback_handler_pathw(
+    sentry_options_t *opts, const wchar_t *path);
+SENTRY_API void sentry_options_set_feedback_handler_pathw_n(
+    sentry_options_t *opts, const wchar_t *path, size_t path_len);
+
 /**
  * Wide char version of `sentry_options_set_database_path`.
  */

include/sentry.h

@@ -143,6 +143,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
         }
 
         if (should_handle) {
+            sentry_value_incref(event);
             sentry_envelope_t *envelope = sentry__prepare_event(
                 options, event, nullptr, !options->on_crash_func, NULL);
             sentry_session_t *session = sentry__end_current_session_with_status(
@@ -189,13 +190,14 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
             sentry__path_free(dump_path);
         } else {
             SENTRY_DEBUG("event was discarded by the `on_crash` hook");
-            sentry_value_decref(event);
         }
 
         // after capturing the crash event, try to dump all the in-flight
         // data of the previous transports
         sentry__transport_dump_queue(options->transport, options->run);
-        // and restore the old transport
+        // and launch the feedback handler
+        sentry__launch_feedback_handler(event);
+        sentry_value_decref(event);
     }
     SENTRY_INFO("crash has been captured");
 

src/backends/sentry_backend_breakpad.cpp

@@ -19,6 +19,7 @@ extern "C" {
 #    include "sentry_unix_pageallocator.h"
 #endif
 #include "sentry_utils.h"
+#include "sentry_uuid.h"
 #include "transports/sentry_disk_transport.h"
 }
 
@@ -114,9 +115,11 @@ typedef struct {
     sentry_path_t *event_path;
     sentry_path_t *breadcrumb1_path;
     sentry_path_t *breadcrumb2_path;
+    sentry_path_t *feedback_path;
     size_t num_breadcrumbs;
     std::atomic<bool> crashed;
     std::atomic<bool> scope_flush;
+    sentry_uuid_t crash_event_id;
 } crashpad_state_t;
 
 /**
@@ -185,7 +188,7 @@ crashpad_register_wer_module(
 #endif
 
 static void
-crashpad_backend_flush_scope_to_event(const sentry_path_t *event_path,
+flush_scope_to_event(const sentry_path_t *event_path,
     const sentry_options_t *options, sentry_value_t crash_event)
 {
     SENTRY_WITH_SCOPE (scope) {
@@ -196,7 +199,6 @@ crashpad_backend_flush_scope_to_event(const sentry_path_t *event_path,
 
     size_t mpack_size;
     char *mpack = sentry_value_to_msgpack(crash_event, &mpack_size);
-    sentry_value_decref(crash_event);
     if (!mpack) {
         return;
     }
@@ -209,6 +211,26 @@ crashpad_backend_flush_scope_to_event(const sentry_path_t *event_path,
     }
 }
 
+static void
+flush_scope_to_feedback(const sentry_path_t *event_path,
+    const sentry_options_t *options, sentry_value_t crash_event)
+{
+    sentry_envelope_t *envelope = sentry__envelope_new();
+    if (!envelope) {
+        return;
+    }
+    sentry__envelope_add_event(envelope, crash_event);
+    if (options->session) {
+        sentry__envelope_add_session(envelope, options->session);
+    }
+
+    if (sentry__path_create_dir_all(options->run->feedback_path) != 0
+        || sentry_envelope_write_to_path(envelope, event_path) != 0) {
+        SENTRY_WARN("flushing scope to feedback failed");
+    }
+    sentry_envelope_free(envelope);
+}
+
 // This function is necessary for macOS since it has no `FirstChanceHandler`.
 // but it is also necessary on Windows if the WER handler is enabled.
 // This means we have to continuously flush the scope on
@@ -234,12 +256,19 @@ crashpad_backend_flush_scope(
     }
 
     sentry_value_t event = sentry_value_new_object();
+    sentry_value_set_by_key(
+        event, "event_id", sentry__value_new_uuid(&data->crash_event_id));
     // Since this will only be uploaded in case of a crash we must make this
     // event fatal.
     sentry_value_set_by_key(
         event, "level", sentry__value_new_level(SENTRY_LEVEL_FATAL));
 
-    crashpad_backend_flush_scope_to_event(data->event_path, options, event);
+    flush_scope_to_event(data->event_path, options, event);
+    if (data->feedback_path) {
+        flush_scope_to_feedback(data->feedback_path, options, event);
+    } else {
+        sentry_value_decref(event);
+    }
     data->scope_flush.store(false, std::memory_order_release);
 #endif
 }
@@ -262,8 +291,12 @@ flush_scope_from_handler(
     }
 
     // now we are the sole flusher and can flush into the crash event
-    crashpad_backend_flush_scope_to_event(
-        state->event_path, options, crash_event);
+    flush_scope_to_event(state->event_path, options, crash_event);
+    if (state->feedback_path) {
+        flush_scope_to_feedback(state->feedback_path, options, crash_event);
+    } else {
+        sentry_value_decref(crash_event);
+    }
 }
 
 #    ifdef SENTRY_PLATFORM_WINDOWS
@@ -281,7 +314,9 @@ sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context)
     bool should_dump = true;
 
     SENTRY_WITH_OPTIONS (options) {
-        sentry_value_t crash_event = sentry_value_new_event();
+        auto state = static_cast<crashpad_state_t *>(options->backend->data);
+        sentry_value_t crash_event
+            = sentry__value_new_event_with_uuid(&state->crash_event_id);
         sentry_value_set_by_key(
             crash_event, "level", sentry__value_new_level(SENTRY_LEVEL_FATAL));
 
@@ -418,6 +453,9 @@ crashpad_backend_startup(
     sentry_path_t *current_run_folder = options->run->run_path;
     auto *data = static_cast<crashpad_state_t *>(backend->data);
 
+    // prepare a predictable event ID for a potential future crash
+    data->crash_event_id = sentry__new_event_id();
+
     base::FilePath database(options->database_path->path);
     base::FilePath handler(absolute_handler_path->path);
 
@@ -455,6 +493,19 @@ crashpad_backend_startup(
         sentry__path_free(screenshot_path);
     }
 
+    base::FilePath feedback_handler;
+    base::FilePath feedback_path;
+    if (options->feedback_handler_path) {
+        char *filename
+            = sentry__uuid_as_filename(&data->crash_event_id, ".envelope");
+        data->feedback_path
+            = sentry__path_join_str(options->run->feedback_path, filename);
+        sentry_free(filename);
+
+        feedback_handler = base::FilePath(options->feedback_handler_path->path);
+        feedback_path = base::FilePath(data->feedback_path->path);
+    }
+
     std::vector<std::string> arguments { "--no-rate-limit" };
 
     // Initialize database first, flushing the consent later on as part of
@@ -482,7 +533,7 @@ crashpad_backend_startup(
         minidump_url ? minidump_url : "", proxy_url, annotations, arguments,
         /* restartable */ true,
         /* asynchronous_start */ false, attachments, screenshot,
-        options->crashpad_wait_for_upload);
+        options->crashpad_wait_for_upload, feedback_handler, feedback_path);
     sentry_free(minidump_url);
 
 #ifdef SENTRY_PLATFORM_WINDOWS
@@ -595,6 +646,7 @@ crashpad_backend_free(sentry_backend_t *backend)
     sentry__path_free(data->event_path);
     sentry__path_free(data->breadcrumb1_path);
     sentry__path_free(data->breadcrumb2_path);
+    sentry__path_free(data->feedback_path);
     sentry_free(data);
 }
 

src/backends/sentry_backend_crashpad.cpp

@@ -589,6 +589,7 @@ handle_ucontext(const sentry_ucontext_t *uctx)
         }
 
         if (should_handle) {
+            sentry_value_incref(event);
             sentry_envelope_t *envelope = sentry__prepare_event(
                 options, event, NULL, !options->on_crash_func, NULL);
             // TODO(tracing): Revisit when investigating transaction flushing
@@ -616,11 +617,13 @@ handle_ucontext(const sentry_ucontext_t *uctx)
             sentry_transport_free(disk_transport);
         } else {
             SENTRY_DEBUG("event was discarded by the `on_crash` hook");
-            sentry_value_decref(event);
         }
 
         // after capturing the crash event, dump all the envelopes to disk
         sentry__transport_dump_queue(options->transport, options->run);
+        // and launch the feedback handler
+        sentry__launch_feedback_handler(event);
+        sentry_value_decref(event);
     }
 
     SENTRY_INFO("crash has been captured");

src/backends/sentry_backend_inproc.c

@@ -11,6 +11,7 @@
 #include "sentry_options.h"
 #include "sentry_os.h"
 #include "sentry_path.h"
+#include "sentry_process.h"
 #include "sentry_random.h"
 #include "sentry_scope.h"
 #include "sentry_screenshot.h"
@@ -19,6 +20,7 @@
 #include "sentry_sync.h"
 #include "sentry_tracing.h"
 #include "sentry_transport.h"
+#include "sentry_uuid.h"
 #include "sentry_value.h"
 
 #ifdef SENTRY_INTEGRATION_QT
@@ -1435,6 +1437,29 @@ sentry_capture_feedback(sentry_value_t user_feedback)
     }
 }
 
+void
+sentry__launch_feedback_handler(sentry_value_t event)
+{
+    sentry_uuid_t event_id = sentry_uuid_nil();
+    sentry__ensure_event_id(event, &event_id);
+
+    SENTRY_WITH_OPTIONS (options) {
+        if (!options->feedback_handler_path) {
+            return;
+        }
+
+        sentry_path_t *feedback_path
+            = sentry__run_write_feedback(options->run, &event_id);
+        if (!feedback_path) {
+            return;
+        }
+
+        sentry__process_spawn(
+            options->feedback_handler_path, feedback_path->path, NULL);
+        sentry__path_free(feedback_path);
+    }
+}
+
 int
 sentry_get_crashed_last_run(void)
 {

src/sentry_core.c

@@ -122,6 +122,8 @@ void sentry__options_unlock(void);
 
 void sentry__set_propagation_context(const char *key, sentry_value_t value);
 
+void sentry__launch_feedback_handler(sentry_value_t event);
+
 #define SENTRY_WITH_OPTIONS(Options)                                           \
     for (const sentry_options_t *Options = sentry__options_getref(); Options;  \
         sentry_options_free((sentry_options_t *)Options), Options = NULL)