Merge remote-tracking branch 'upstream/master' into jpnurmi/feat/native-offline-caching

Author: jpnurmi

CHANGELOG.md

@@ -9,6 +9,7 @@
 **Fixes**:
 
 - Fix `cache_keep` to only cache envelopes when HTTP send fails, instead of unconditionally on restart. ([#1585](https://github.com/getsentry/sentry-native/pull/1585))
+- Fix external crash reporter to work with the new experimental `native` backend. ([#1589](https://github.com/getsentry/sentry-native/pull/1589))
 
 ## 0.13.3
 

src/backends/native/sentry_crash_daemon.c

@@ -2032,10 +2032,13 @@ write_envelope_with_native_stacktrace(const sentry_options_t *options,
 
     // Serialize event to JSON
     char *event_json = sentry_value_to_json(event);
+    char *event_id = sentry__string_clone(
+        sentry_value_as_string(sentry_value_get_by_key(event, "event_id")));
     sentry_value_decref(event);
 
     if (!event_json) {
         SENTRY_WARN("Failed to serialize native crash event to JSON");
+        sentry_free(event_id);
         return false;
     }
 
@@ -2054,6 +2057,7 @@ write_envelope_with_native_stacktrace(const sentry_options_t *options,
     if (fd < 0) {
         SENTRY_WARN("Failed to open envelope file for writing");
         sentry_free(event_json);
+        sentry_free(event_id);
         return false;
     }
 
@@ -2062,12 +2066,19 @@ write_envelope_with_native_stacktrace(const sentry_options_t *options,
         = options && options->dsn ? sentry_options_get_dsn(options) : NULL;
     char header_buf[SENTRY_CRASH_ENVELOPE_HEADER_SIZE];
     int header_len;
-    if (dsn) {
+    if (dsn && event_id && event_id[0] != '\0') {
+        header_len = snprintf(header_buf, sizeof(header_buf),
+            "{\"dsn\":\"%s\",\"event_id\":\"%s\"}\n", dsn, event_id);
+    } else if (dsn) {
         header_len = snprintf(
             header_buf, sizeof(header_buf), "{\"dsn\":\"%s\"}\n", dsn);
+    } else if (event_id && event_id[0] != '\0') {
+        header_len = snprintf(header_buf, sizeof(header_buf),
+            "{\"event_id\":\"%s\"}\n", event_id);
     } else {
         header_len = snprintf(header_buf, sizeof(header_buf), "{}\n");
     }
+    sentry_free(event_id);
     if (header_len > 0 && header_len < (int)sizeof(header_buf)) {
 #if defined(SENTRY_PLATFORM_UNIX)
         if (write(fd, header_buf, header_len) != header_len) {
@@ -2238,6 +2249,23 @@ write_envelope_with_minidump(const sentry_options_t *options,
     const char *envelope_path, const char *event_msgpack_path,
     const char *minidump_path, sentry_path_t *run_folder)
 {
+    // Read event JSON data
+    size_t event_size = 0;
+    char *event_json = NULL;
+    char *event_id = NULL;
+    sentry_path_t *ev_path = sentry__path_from_str(event_msgpack_path);
+    if (ev_path) {
+        event_json = sentry__path_read_to_buffer(ev_path, &event_size);
+        sentry__path_free(ev_path);
+        if (event_json && event_size > 0) {
+            sentry_value_t event
+                = sentry__value_from_json(event_json, event_size);
+            event_id = sentry__string_clone(sentry_value_as_string(
+                sentry_value_get_by_key(event, "event_id")));
+            sentry_value_decref(event);
+        }
+    }
+
     // Open envelope file for writing
 #if defined(SENTRY_PLATFORM_UNIX)
     int fd = open(envelope_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
@@ -2251,20 +2279,29 @@ write_envelope_with_minidump(const sentry_options_t *options,
 #endif
     if (fd < 0) {
         SENTRY_WARN("Failed to open envelope file for writing");
+        sentry_free(event_json);
+        sentry_free(event_id);
         return false;
     }
 
-    // Write envelope headers (just DSN if available)
+    // Write envelope headers (just DSN and event ID if available)
     const char *dsn
         = options && options->dsn ? sentry_options_get_dsn(options) : NULL;
     char header_buf[SENTRY_CRASH_ENVELOPE_HEADER_SIZE];
     int header_len;
-    if (dsn) {
+    if (dsn && event_id && event_id[0] != '\0') {
+        header_len = snprintf(header_buf, sizeof(header_buf),
+            "{\"dsn\":\"%s\",\"event_id\":\"%s\"}\n", dsn, event_id);
+    } else if (dsn) {
         header_len = snprintf(
             header_buf, sizeof(header_buf), "{\"dsn\":\"%s\"}\n", dsn);
+    } else if (event_id && event_id[0] != '\0') {
+        header_len = snprintf(header_buf, sizeof(header_buf),
+            "{\"event_id\":\"%s\"}\n", event_id);
     } else {
         header_len = snprintf(header_buf, sizeof(header_buf), "{}\n");
     }
+    sentry_free(event_id);
     if (header_len > 0 && header_len < (int)sizeof(header_buf)) {
 #if defined(SENTRY_PLATFORM_UNIX)
         if (write(fd, header_buf, header_len) != header_len) {
@@ -2275,39 +2312,30 @@ write_envelope_with_minidump(const sentry_options_t *options,
 #endif
     }
 
-    // Read event JSON data
-    sentry_path_t *ev_path = sentry__path_from_str(event_msgpack_path);
-    if (ev_path) {
-        size_t event_size = 0;
-        char *event_json = sentry__path_read_to_buffer(ev_path, &event_size);
-        sentry__path_free(ev_path);
-
-        if (event_json && event_size > 0) {
-            // Write event item header
-            char event_header[SENTRY_CRASH_ITEM_HEADER_SIZE];
-            int ev_header_len = snprintf(event_header, sizeof(event_header),
-                "{\"type\":\"event\",\"length\":%zu}\n", event_size);
-            if (ev_header_len > 0
-                && ev_header_len < (int)sizeof(event_header)) {
+    // Write event item
+    if (event_json && event_size > 0) {
+        char event_header[SENTRY_CRASH_ITEM_HEADER_SIZE];
+        int ev_header_len = snprintf(event_header, sizeof(event_header),
+            "{\"type\":\"event\",\"length\":%zu}\n", event_size);
+        if (ev_header_len > 0 && ev_header_len < (int)sizeof(event_header)) {
 #if defined(SENTRY_PLATFORM_UNIX)
-                if (write(fd, event_header, ev_header_len) != ev_header_len) {
-                    SENTRY_WARN("Failed to write event header to envelope");
-                }
-                if (write(fd, event_json, event_size) != (ssize_t)event_size) {
-                    SENTRY_WARN("Failed to write event data to envelope");
-                }
-                if (write(fd, "\n", 1) != 1) {
-                    SENTRY_WARN("Failed to write event newline to envelope");
-                }
+            if (write(fd, event_header, ev_header_len) != ev_header_len) {
+                SENTRY_WARN("Failed to write event header to envelope");
+            }
+            if (write(fd, event_json, event_size) != (ssize_t)event_size) {
+                SENTRY_WARN("Failed to write event data to envelope");
+            }
+            if (write(fd, "\n", 1) != 1) {
+                SENTRY_WARN("Failed to write event newline to envelope");
+            }
 #elif defined(SENTRY_PLATFORM_WINDOWS)
-                _write(fd, event_header, (unsigned int)ev_header_len);
-                _write(fd, event_json, (unsigned int)event_size);
-                _write(fd, "\n", 1);
+            _write(fd, event_header, (unsigned int)ev_header_len);
+            _write(fd, event_json, (unsigned int)event_size);
+            _write(fd, "\n", 1);
 #endif
-            }
-            sentry_free(event_json);
         }
     }
+    sentry_free(event_json);
 
     // Add minidump as attachment
 #if defined(SENTRY_PLATFORM_UNIX)
@@ -2712,14 +2740,17 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc)
 
     SENTRY_DEBUG("Envelope loaded, sending via transport");
 
-    // Send directly via transport
-    if (options && options->transport) {
-        SENTRY_DEBUG("Calling transport send_envelope");
-        sentry__transport_send_envelope(options->transport, envelope);
-        SENTRY_DEBUG("Crash envelope sent to transport (queued)");
-    } else {
-        SENTRY_WARN("No transport available for sending envelope");
-        sentry_envelope_free(envelope);
+    // Send directly via transport, or to external crash reporter
+    if (!sentry__launch_external_crash_reporter(options, envelope)) {
+        // Send directly via transport
+        if (options && options->transport) {
+            SENTRY_DEBUG("Calling transport send_envelope");
+            sentry__transport_send_envelope(options->transport, envelope);
+            SENTRY_DEBUG("Crash envelope sent to transport (queued)");
+        } else {
+            SENTRY_WARN("No transport available for sending envelope");
+            sentry_envelope_free(envelope);
+        }
     }
 
     // Clean up temporary envelope file (keep minidump for

src/backends/sentry_backend_breakpad.cpp

@@ -190,7 +190,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
                 sentry__attachment_free(screenshot);
             }
 
-            if (!sentry__launch_external_crash_reporter(envelope)) {
+            if (!sentry__launch_external_crash_reporter(options, envelope)) {
                 // capture the envelope with the disk transport
                 sentry_transport_t *disk_transport
                     = sentry_new_disk_transport(options->run);

src/backends/sentry_backend_inproc.c

@@ -1190,7 +1190,7 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx,
                 sentry__attachment_free(screenshot);
             }
 
-            if (!sentry__launch_external_crash_reporter(envelope)) {
+            if (!sentry__launch_external_crash_reporter(options, envelope)) {
                 // capture the envelope with the disk transport
                 sentry_transport_t *disk_transport
                     = sentry_new_disk_transport(options->run);

src/backends/sentry_backend_native.c

@@ -542,7 +542,7 @@ native_backend_free(sentry_backend_t *backend)
 
 static void
 native_backend_flush_scope(
-    sentry_backend_t *backend, const sentry_options_t *options)
+    sentry_backend_t *backend, const sentry_options_t *UNUSED(options))
 {
     native_backend_state_t *state = (native_backend_state_t *)backend->data;
     if (!state || !state->event_path) {
@@ -659,16 +659,6 @@ native_backend_flush_scope(
             }
         }
     }
-
-    // Flush external crash report envelope if configured
-    if (options->external_crash_reporter && state->envelope_path) {
-        sentry_envelope_t *envelope = sentry__envelope_new();
-        if (envelope && options->session) {
-            sentry__envelope_add_session(envelope, options->session);
-            sentry__run_write_external(options->run, envelope);
-        }
-        sentry_envelope_free(envelope);
-    }
 }
 
 static void

src/sentry_core.c

@@ -1662,44 +1662,44 @@ sentry_capture_feedback_with_hint(
 }
 
 bool
-sentry__launch_external_crash_reporter(sentry_envelope_t *envelope)
+sentry__launch_external_crash_reporter(
+    const sentry_options_t *options, sentry_envelope_t *envelope)
 {
-    SENTRY_WITH_OPTIONS (options) {
-        if (!options->external_crash_reporter) {
-            return false;
-        }
-
-        sentry_uuid_t event_id = sentry__envelope_get_event_id(envelope);
-        char *envelope_filename
-            = sentry__uuid_as_filename(&event_id, ".envelope");
-        if (!envelope_filename) {
-            return false;
-        }
+    if (!options || !options->run || !options->external_crash_reporter
+        || !options->external_crash_reporter->path
+        || options->external_crash_reporter->path[0] == '\0') {
+        return false;
+    }
 
-        sentry_path_t *report_path = sentry__path_join_str(
-            options->run->external_path, envelope_filename);
-        if (!report_path) {
-            sentry_free(envelope_filename);
-            return false;
-        }
+    sentry_uuid_t event_id = sentry__envelope_get_event_id(envelope);
+    char *envelope_filename = sentry__uuid_as_filename(&event_id, ".envelope");
+    if (!envelope_filename) {
+        return false;
+    }
 
-        // capture the envelope with the disk transport
-        sentry_transport_t *disk_transport
-            = sentry_new_external_disk_transport(options->run);
-        if (!disk_transport) {
-            sentry__path_free(report_path);
-            sentry_free(envelope_filename);
-            return false;
-        }
-        sentry__capture_envelope(disk_transport, envelope);
-        sentry__transport_dump_queue(disk_transport, options->run);
-        sentry_transport_free(disk_transport);
+    sentry_path_t *report_path
+        = sentry__path_join_str(options->run->external_path, envelope_filename);
+    if (!report_path) {
+        sentry_free(envelope_filename);
+        return false;
+    }
 
-        sentry__process_spawn(
-            options->external_crash_reporter, report_path->path, NULL);
+    // capture the envelope with the disk transport
+    sentry_transport_t *disk_transport
+        = sentry_new_external_disk_transport(options->run);
+    if (!disk_transport) {
         sentry__path_free(report_path);
         sentry_free(envelope_filename);
+        return false;
     }
+    sentry__transport_send_envelope(disk_transport, envelope);
+    sentry__transport_dump_queue(disk_transport, options->run);
+    sentry_transport_free(disk_transport);
+
+    sentry__process_spawn(
+        options->external_crash_reporter, report_path->path, NULL);
+    sentry__path_free(report_path);
+    sentry_free(envelope_filename);
     return true;
 }
 

src/sentry_core.h

@@ -128,7 +128,8 @@ void sentry__set_propagation_context(const char *key, sentry_value_t value);
 void sentry__apply_attributes(
     sentry_value_t telemetry, sentry_value_t attributes);
 
-bool sentry__launch_external_crash_reporter(sentry_envelope_t *envelope);
+bool sentry__launch_external_crash_reporter(
+    const sentry_options_t *options, sentry_envelope_t *envelope);
 
 #define SENTRY_WITH_OPTIONS(Options)                                           \
     for (const sentry_options_t *Options = sentry__options_getref(); Options;  \

tests/test_integration_native.py

@@ -12,14 +12,18 @@
 import pytest
 
 from . import (
+    is_feedback_envelope,
     make_dsn,
     run,
     Envelope,
+    split_log_request_cond,
 )
 from .assertions import (
+    assert_breadcrumb,
     assert_meta,
     assert_session,
     wait_for_file,
+    assert_user_feedback,
 )
 from .conditions import has_native, is_kcov, is_asan
 
@@ -473,26 +477,34 @@ def test_native_external_crash_reporter(cmake, httpserver):
     )
 
     env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver))
-    httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK")
+    httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data("OK")
+    httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data("OK")
 
     # Crash and use external reporter
-    run_crash(
-        tmp_path,
-        "sentry_example",
-        ["log", "crash-reporter", "crash"],
-        env=env,
-    )
+    with httpserver.wait(timeout=10) as waiting:
+        run_crash(
+            tmp_path,
+            "sentry_example",
+            ["log", "crash-reporter", "crash"],
+            env=env,
+        )
+    assert waiting.result
 
-    # Give time for external reporter to run
-    time.sleep(2)
+    # Should have sent crash report and feedback via external reporter
+    assert len(httpserver.log) == 2
+    feedback_request, crash_request = split_log_request_cond(
+        httpserver.log, is_feedback_envelope
+    )
+    feedback = feedback_request.get_data()
+    crash = crash_request.get_data()
 
-    # Should have sent crash report via external reporter
-    assert len(httpserver.log) >= 1
+    # Verify it's a minidump crash report and user feedback
+    envelope = Envelope.deserialize(crash)
+    assert_meta(envelope)
+    assert_breadcrumb(envelope)
 
-    # Verify it's a minidump crash report
-    envelope = Envelope.deserialize(httpserver.log[0][0].get_data())
-    event = envelope.get_event()
-    assert event is not None
+    envelope = Envelope.deserialize(feedback)
+    assert_user_feedback(envelope)
 
 
 def test_crash_mode_minidump_only(cmake, httpserver):