WIP: transfer feedback envelope ownership

Author: jpnurmi

include/sentry.h

@@ -698,6 +698,15 @@ SENTRY_API int sentry_envelope_write_to_file(
 SENTRY_API int sentry_envelope_write_to_file_n(
     const sentry_envelope_t *envelope, const char *path, size_t path_len);
 
+/**
+ * Reads an envelope from a file.
+ *
+ * `path` is assumed to be in platform-specific filesystem path encoding.
+ */
+SENTRY_API sentry_envelope_t *sentry_envelope_read_from_file(const char *path);
+SENTRY_API sentry_envelope_t *sentry_envelope_read_from_file_n(
+    const char *path, size_t path_len);
+
 /**
  * Submits an envelope, first checking for consent.
  */

src/backends/sentry_backend_breakpad.cpp

@@ -143,7 +143,6 @@ 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(
@@ -177,27 +176,30 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
                 sentry__attachment_free(screenshot);
             }
 
-            // capture the envelope with the disk transport
-            sentry_transport_t *disk_transport
-                = sentry_new_disk_transport(options->run);
-            sentry__capture_envelope(disk_transport, envelope);
-            sentry__transport_dump_queue(disk_transport, options->run);
-            sentry_transport_free(disk_transport);
+            if (options->feedback_handler_path) {
+                sentry__launch_feedback_handler(envelope);
+            } else {
+                // capture the envelope with the disk transport
+                sentry_transport_t *disk_transport
+                    = sentry_new_disk_transport(options->run);
+                sentry__capture_envelope(disk_transport, envelope);
+                sentry__transport_dump_queue(disk_transport, options->run);
+                sentry_transport_free(disk_transport);
+            }
 
             // now that the envelope was written, we can remove the temporary
             // minidump file
             sentry__path_remove(dump_path);
             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 launch the feedback handler
-        sentry__launch_feedback_handler(event);
-        sentry_value_decref(event);
+        // and restore the old transport
     }
     SENTRY_INFO("crash has been captured");
 

src/backends/sentry_backend_inproc.c

@@ -589,7 +589,6 @@ 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
@@ -609,21 +608,23 @@ handle_ucontext(const sentry_ucontext_t *uctx)
                 sentry__attachment_free(screenshot);
             }
 
-            // capture the envelope with the disk transport
-            sentry_transport_t *disk_transport
-                = sentry_new_disk_transport(options->run);
-            sentry__capture_envelope(disk_transport, envelope);
-            sentry__transport_dump_queue(disk_transport, options->run);
-            sentry_transport_free(disk_transport);
+            if (options->feedback_handler_path) {
+                sentry__launch_feedback_handler(envelope);
+            } else {
+                // capture the envelope with the disk transport
+                sentry_transport_t *disk_transport
+                    = sentry_new_disk_transport(options->run);
+                sentry__capture_envelope(disk_transport, envelope);
+                sentry__transport_dump_queue(disk_transport, options->run);
+                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/sentry_core.c

@@ -22,6 +22,7 @@
 #include "sentry_transport.h"
 #include "sentry_uuid.h"
 #include "sentry_value.h"
+#include "transports/sentry_disk_transport.h"
 
 #ifdef SENTRY_INTEGRATION_QT
 #    include "integrations/sentry_integration_qt.h"
@@ -1446,24 +1447,33 @@ sentry_capture_feedback(sentry_value_t user_feedback)
 }
 
 void
-sentry__launch_feedback_handler(sentry_value_t event)
+sentry__launch_feedback_handler(sentry_envelope_t *envelope)
 {
-    sentry_uuid_t event_id = sentry_uuid_nil();
-    sentry__ensure_event_id(event, &event_id);
+    // 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;
+        // capture the envelope with the disk transport
+        sentry_transport_t *disk_transport
+            = sentry_new_feedback_transport(options->run);
+        sentry__capture_envelope(disk_transport, envelope);
+        sentry__transport_dump_queue(disk_transport, options->run);
+        sentry_transport_free(disk_transport);
+
+        sentry_uuid_t event_id = sentry__envelope_get_event_id(envelope);
+        char *envelope_filename
+            = sentry__uuid_as_filename(&event_id, ".envelope");
+
+        sentry_path_t *feedback_path = sentry__path_join_str(
+            options->run->feedback_path, envelope_filename);
+        if (feedback_path) {
+            sentry__process_spawn(
+                options->feedback_handler_path, feedback_path->path, NULL);
         }
-
-        sentry__process_spawn(
-            options->feedback_handler_path, feedback_path->path, NULL);
         sentry__path_free(feedback_path);
     }
 }

src/sentry_core.h

@@ -122,7 +122,7 @@ 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);
+void sentry__launch_feedback_handler(sentry_envelope_t *envelope);
 
 #define SENTRY_WITH_OPTIONS(Options)                                           \
     for (const sentry_options_t *Options = sentry__options_getref(); Options;  \

src/sentry_database.c

@@ -126,44 +126,33 @@ sentry__run_write_envelope(
 
 sentry_path_t *
 sentry__run_write_feedback(
-    const sentry_run_t *run, const sentry_uuid_t *event_id)
+    const sentry_run_t *run, const sentry_envelope_t *envelope)
 {
     if (sentry__path_create_dir_all(run->feedback_path) != 0) {
         SENTRY_ERRORF(
             "mkdir failed: \"%" SENTRY_PATH_PRI "\"", run->feedback_path->path);
         return NULL;
     }
 
-    char *filename = sentry__uuid_as_filename(event_id, ".envelope");
-    sentry_path_t *source_path = sentry__path_join_str(run->run_path, filename);
-
-    size_t buf_len = 0;
-    char *buf = sentry__path_read_to_buffer(source_path, &buf_len);
-    if (!buf || buf_len == 0) {
-        SENTRY_ERRORF("failed to read envelope: \"%" SENTRY_PATH_PRI "\"",
-            source_path->path);
-        sentry_free(filename);
-        sentry__path_free(source_path);
-        sentry_free(buf);
+    sentry_uuid_t event_id = sentry__envelope_get_event_id(envelope);
+    char *envelope_filename = sentry__uuid_as_filename(&event_id, ".envelope");
+
+    sentry_path_t *output_path
+        = sentry__path_join_str(run->feedback_path, envelope_filename);
+    sentry_free(envelope_filename);
+    if (!output_path) {
         return NULL;
     }
 
-    sentry_path_t *target_path
-        = sentry__path_join_str(run->feedback_path, filename);
-    int rv = sentry__path_write_buffer(target_path, buf, buf_len);
-
-    sentry_free(filename);
-    sentry__path_free(source_path);
-    sentry_free(buf);
+    int rv = sentry_envelope_write_to_path(envelope, output_path);
+    sentry__path_free(output_path);
 
     if (rv) {
-        SENTRY_ERRORF("failed to write feedback: \"%" SENTRY_PATH_PRI "\"",
-            target_path->path);
-        sentry__path_free(target_path);
-        return NULL;
+        SENTRY_WARN("writing envelope to file failed");
     }
 
-    return target_path;
+    // the `write_to_path` returns > 0 on failure, but we would like a real bool
+    return !rv ? output_path : NULL;
 }
 
 bool

src/sentry_database.h

@@ -43,12 +43,12 @@ bool sentry__run_write_envelope(
     const sentry_run_t *run, const sentry_envelope_t *envelope);
 
 /**
- * This will copy the specified event's envelope from:
- * `<database>/<uuid>.run/<event-uuid>.envelope` to:
- * `<database>/feedback/<event-uuid>.envelope`.
+ * This will serialize and write the given envelope to disk into a file named
+ * like so:
+ * `<database>/feedback/<event-uuid>.envelope`
  */
 sentry_path_t *sentry__run_write_feedback(
-    const sentry_run_t *run, const sentry_uuid_t *event_id);
+    const sentry_run_t *run, const sentry_envelope_t *envelope);
 
 /**
  * This will serialize and write the given session to disk into a file named:

src/sentry_envelope.c

@@ -189,8 +189,21 @@ sentry_uuid_t
 sentry__envelope_get_event_id(const sentry_envelope_t *envelope)
 {
     if (envelope->is_raw) {
-        return sentry_uuid_nil();
-    }
+        const char *payload = envelope->contents.raw.payload;
+        size_t payload_len = envelope->contents.raw.payload_len;
+        if (!payload || payload_len == 0) {
+            return sentry_uuid_nil();
+        }
+
+        const char *newline = memchr(payload, '\n', payload_len);
+        size_t headers_len
+            = newline ? (size_t)(newline - payload) : payload_len;
+        sentry_value_t headers = sentry__value_from_json(payload, headers_len);
+        sentry_uuid_t event_id = sentry_uuid_from_string(sentry_value_as_string(
+            sentry_value_get_by_key(headers, "event_id")));
+        sentry_value_decref(headers);
+        return event_id;
+    };
     return sentry_uuid_from_string(sentry_value_as_string(
         sentry_value_get_by_key(envelope->contents.items.headers, "event_id")));
 }
@@ -726,6 +739,24 @@ sentry_envelope_write_to_file(
     return sentry_envelope_write_to_file_n(envelope, path, strlen(path));
 }
 
+sentry_envelope_t *
+sentry_envelope_read_from_file(const char *path)
+{
+    sentry_path_t *path_obj = sentry__path_from_str(path);
+    sentry_envelope_t *envelope = sentry__envelope_from_path(path_obj);
+    sentry__path_free(path_obj);
+    return envelope;
+}
+
+sentry_envelope_t *
+sentry_envelope_read_from_file_n(const char *path, size_t path_len)
+{
+    sentry_path_t *path_obj = sentry__path_from_str_n(path, path_len);
+    sentry_envelope_t *envelope = sentry__envelope_from_path(path_obj);
+    sentry__path_free(path_obj);
+    return envelope;
+}
+
 #ifdef SENTRY_UNITTEST
 size_t
 sentry__envelope_get_item_count(const sentry_envelope_t *envelope)

src/transports/sentry_disk_transport.c

@@ -26,3 +26,24 @@ sentry_new_disk_transport(const sentry_run_t *run)
     sentry_transport_set_state(transport, (void *)run);
     return transport;
 }
+
+static void
+send_feedback_disk_transport(sentry_envelope_t *envelope, void *state)
+{
+    const sentry_run_t *run = state;
+
+    sentry__run_write_feedback(run, envelope);
+    sentry_envelope_free(envelope);
+}
+
+sentry_transport_t *
+sentry_new_feedback_transport(const sentry_run_t *run)
+{
+    sentry_transport_t *transport
+        = sentry_transport_new(send_feedback_disk_transport);
+    if (!transport) {
+        return NULL;
+    }
+    sentry_transport_set_state(transport, (void *)run);
+    return transport;
+}

src/transports/sentry_disk_transport.h

@@ -11,4 +11,11 @@
  */
 sentry_transport_t *sentry_new_disk_transport(const sentry_run_t *run);
 
+/**
+ * This creates a new transport that serializes envelopes to disk in the given
+ * `feedback` directory.
+ * See `sentry__run_write_feedback`.
+ */
+sentry_transport_t *sentry_new_feedback_transport(const sentry_run_t *run);
+
 #endif