From 6b691ac4d34195504dfa5ba8354764f1f273c892 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 17 Sep 2026 17:59:17 +0200 Subject: [PATCH 01/13] ref: attachment manifest Use a shared MessagePack stream for attachment manifests. The native backend uses the manifest for attachments generally, while both out-of-process crash handlers can also use it for crash-time hint attachments that cannot cross backend IPC. This lets Crashpad consume those attachments without adding JSON support. Stream attachment objects directly to avoid format-specific conversion and buffer the complete manifest for a single file write. Median release benchmarks measured write/read improvements over legacy JSON of 17%/6% for one attachment, 74%/17% for 10, and 86%/24% for 100: 1 attachment 10 attachments 100 attachments MessagePack write 2.3 us 2.4 us 11.8 us JSON write 2.7 us 8.9 us 82.6 us MessagePack read 1.3 us 5.6 us 54.1 us JSON read 1.4 us 6.8 us 71.1 us Keep legacy JSON compatibility private to the native daemon so it can still consume manifests written by older SDK versions. --- external/crashpad | 2 +- src/backends/native/sentry_crash_daemon.c | 238 +++++++++------------- src/backends/sentry_backend_crashpad.cpp | 40 +--- src/backends/sentry_backend_native.c | 84 +------- src/sentry_attachment.c | 137 +++++++++++++ src/sentry_attachment.h | 20 ++ tests/assertions.py | 1 + tests/test_integration_native.py | 7 +- tests/unit/test_attachments.c | 108 ++++++++++ tests/unit/tests.inc | 1 + 10 files changed, 383 insertions(+), 255 deletions(-) diff --git a/external/crashpad b/external/crashpad index 95733c1ee..60be52d4a 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 95733c1ee6fe77fc82ae6bb1bac2c34b9a6d0c17 +Subproject commit 60be52d4aa63db37d68a1bc81aa95182fc878e78 diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index a81e552f3..aab3822d2 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -364,6 +364,103 @@ attachment_is_placeholder(const sentry_options_t *options, const char *path) return is_placeholder; } +static sentry_value_t +read_legacy_manifest(const char *buf, size_t buf_len) +{ + sentry_value_t legacy = sentry__value_from_json(buf, buf_len); + if (sentry_value_get_type(legacy) != SENTRY_VALUE_TYPE_LIST) { + sentry_value_decref(legacy); + return sentry_value_new_null(); + } + + sentry_value_t attachments = sentry_value_new_list(); + size_t len = sentry_value_get_length(legacy); + for (size_t i = 0; i < len; i++) { + sentry_value_t info = sentry_value_get_by_index(legacy, i); + const char *path + = sentry_value_as_string(sentry_value_get_by_key(info, "path")); + const char *filename + = sentry_value_as_string(sentry_value_get_by_key(info, "filename")); + if (sentry__string_empty(path) || sentry__string_empty(filename)) { + continue; + } + sentry_value_t attachment = sentry__attachment_from_file(path); + if (sentry_value_is_null(attachment)) { + continue; + } + sentry_attachment_set_filename(attachment, filename); + sentry_attachment_set_type(attachment, + sentry_value_as_string( + sentry_value_get_by_key(info, "attachment_type"))); + sentry_attachment_set_content_type(attachment, + sentry_value_as_string( + sentry_value_get_by_key(info, "content_type"))); + sentry_value_append(attachments, attachment); + } + sentry_value_decref(legacy); + return attachments; +} + +static sentry_value_t +read_attachment_manifest(const sentry_path_t *run_folder) +{ + sentry_path_t *path + = sentry__path_join_str(run_folder, "__sentry-attachments"); + if (!path) { + return sentry_value_new_null(); + } + sentry_value_t attachments = sentry__read_attachment_manifest(path); + if (!sentry_value_is_null(attachments)) { + sentry__path_free(path); + return attachments; + } + sentry_value_decref(attachments); + + size_t buf_len = 0; + char *buf = sentry__path_read_to_buffer(path, &buf_len); + sentry__path_free(path); + if (!buf) { + return sentry_value_new_null(); + } + const char *start = buf; + const char *end = buf + buf_len; + while (start < end + && (*start == ' ' || *start == '\t' || *start == '\r' + || *start == '\n')) { + start++; + } + const char *trimmed_end = end; + while (trimmed_end > start + && (trimmed_end[-1] == ' ' || trimmed_end[-1] == '\t' + || trimmed_end[-1] == '\r' || trimmed_end[-1] == '\n')) { + trimmed_end--; + } + attachments = start < trimmed_end && *start == '[' && trimmed_end[-1] == ']' + ? read_legacy_manifest(start, (size_t)(trimmed_end - start)) + : sentry_value_new_null(); + sentry_free(buf); + return attachments; +} + +static void +write_attachments_from_manifest( + int fd, const sentry_options_t *options, const sentry_path_t *run_folder) +{ + sentry_value_t attachments = read_attachment_manifest(run_folder); + size_t len = sentry_value_get_length(attachments); + for (size_t i = 0; i < len; i++) { + sentry_value_t attachment = sentry_value_get_by_index(attachments, i); + const char *path = sentry__attachment_get_path(attachment); + if (!attachment_is_placeholder(options, path)) { + write_attachment_to_envelope(fd, path, + sentry__attachment_get_filename(attachment), + sentry__attachment_get_type(attachment), + sentry__attachment_get_content_type(attachment)); + } + } + sentry_value_decref(attachments); +} + // For each large attachment listed in `/__sentry-attachments`, // cache it as an attachment-ref item. Small attachments were already inlined // during envelope writing. @@ -375,23 +472,7 @@ add_attachment_refs(sentry_envelope_t *envelope, || !options->enable_large_attachments || !run_folder) { return; } - sentry_path_t *attach_list_path - = sentry__path_join_str(run_folder, "__sentry-attachments"); - if (!attach_list_path) { - SENTRY_WARN("Failed to resolve attachment manifest path"); - return; - } - size_t attach_json_len = 0; - char *attach_json - = sentry__path_read_to_buffer(attach_list_path, &attach_json_len); - sentry__path_free(attach_list_path); - if (!attach_json) { - return; - } - sentry_value_t list = attach_json_len > 0 - ? sentry__value_from_json(attach_json, attach_json_len) - : sentry_value_new_null(); - sentry_free(attach_json); + sentry_value_t list = read_attachment_manifest(run_folder); if (sentry_value_is_null(list)) { SENTRY_WARN("Failed to parse attachment manifest"); return; @@ -400,34 +481,12 @@ add_attachment_refs(sentry_envelope_t *envelope, bool materialized = false; size_t len = sentry_value_get_length(list); for (size_t i = 0; i < len; i++) { - sentry_value_t info = sentry_value_get_by_index(list, i); - const char *path - = sentry_value_as_string(sentry_value_get_by_key(info, "path")); - const char *filename - = sentry_value_as_string(sentry_value_get_by_key(info, "filename")); - const char *attachment_type = sentry_value_as_string( - sentry_value_get_by_key(info, "attachment_type")); - const char *content_type = sentry_value_as_string( - sentry_value_get_by_key(info, "content_type")); - if (sentry__string_empty(path) || sentry__string_empty(filename)) { - SENTRY_WARN("Skipping malformed attachment manifest entry"); - continue; - } - sentry_value_t attachment = sentry__attachment_from_file(path); - if (sentry_value_is_null(attachment)) { - SENTRY_WARNF("Failed to allocate attachment paths for: %s", path); - continue; - } - sentry_attachment_set_filename(attachment, filename); - sentry_attachment_set_type(attachment, attachment_type); - sentry_attachment_set_content_type(attachment, content_type); + sentry_value_t attachment = sentry_value_get_by_index(list, i); if (!sentry__attachment_is_placeholder(attachment, options)) { - sentry_value_decref(attachment); continue; } if (!materialized && !sentry__envelope_materialize(envelope)) { SENTRY_WARN("Failed to materialize envelope for attachment-refs"); - sentry_value_decref(attachment); break; } materialized = true; @@ -435,7 +494,6 @@ add_attachment_refs(sentry_envelope_t *envelope, envelope, attachment, options->run->cache_path, NULL)) { SENTRY_WARN("failed to cache attachment-ref"); } - sentry_value_decref(attachment); } sentry_value_decref(list); } @@ -3927,54 +3985,7 @@ write_envelope_with_native_stacktrace(const sentry_options_t *options, // Add scope attachments using metadata file if (run_folder) { - sentry_path_t *attach_list_path - = sentry__path_join_str(run_folder, "__sentry-attachments"); - if (attach_list_path) { - size_t attach_json_len = 0; - char *attach_json = sentry__path_read_to_buffer( - attach_list_path, &attach_json_len); - sentry__path_free(attach_list_path); - - if (attach_json && attach_json_len > 0) { - // Parse attachment list JSON - sentry_value_t attach_list - = sentry__value_from_json(attach_json, attach_json_len); - sentry_free(attach_json); - - if (!sentry_value_is_null(attach_list)) { - size_t len = sentry_value_get_length(attach_list); - for (size_t i = 0; i < len; i++) { - sentry_value_t attach_info - = sentry_value_get_by_index(attach_list, i); - sentry_value_t path_val - = sentry_value_get_by_key(attach_info, "path"); - sentry_value_t filename_val - = sentry_value_get_by_key(attach_info, "filename"); - sentry_value_t attachment_type_val - = sentry_value_get_by_key( - attach_info, "attachment_type"); - sentry_value_t content_type_val - = sentry_value_get_by_key( - attach_info, "content_type"); - - const char *path = sentry_value_as_string(path_val); - const char *filename - = sentry_value_as_string(filename_val); - const char *attachment_type - = sentry_value_as_string(attachment_type_val); - const char *content_type - = sentry_value_as_string(content_type_val); - - if (path && filename - && !attachment_is_placeholder(options, path)) { - write_attachment_to_envelope(fd, path, filename, - attachment_type, content_type); - } - } - sentry_value_decref(attach_list); - } - } - } + write_attachments_from_manifest(fd, options, run_folder); } // Add screenshot attachment if captured by the daemon @@ -4204,54 +4215,7 @@ write_envelope_with_minidump(const sentry_options_t *options, // Add scope attachments using metadata file if (run_folder) { - sentry_path_t *attach_list_path - = sentry__path_join_str(run_folder, "__sentry-attachments"); - if (attach_list_path) { - size_t attach_json_len = 0; - char *attach_json = sentry__path_read_to_buffer( - attach_list_path, &attach_json_len); - sentry__path_free(attach_list_path); - - if (attach_json && attach_json_len > 0) { - // Parse attachment list JSON - sentry_value_t attach_list - = sentry__value_from_json(attach_json, attach_json_len); - sentry_free(attach_json); - - if (!sentry_value_is_null(attach_list)) { - size_t len = sentry_value_get_length(attach_list); - for (size_t i = 0; i < len; i++) { - sentry_value_t attach_info - = sentry_value_get_by_index(attach_list, i); - sentry_value_t path_val - = sentry_value_get_by_key(attach_info, "path"); - sentry_value_t filename_val - = sentry_value_get_by_key(attach_info, "filename"); - sentry_value_t attachment_type_val - = sentry_value_get_by_key( - attach_info, "attachment_type"); - sentry_value_t content_type_val - = sentry_value_get_by_key( - attach_info, "content_type"); - - const char *path = sentry_value_as_string(path_val); - const char *filename - = sentry_value_as_string(filename_val); - const char *attachment_type - = sentry_value_as_string(attachment_type_val); - const char *content_type - = sentry_value_as_string(content_type_val); - - if (path && filename - && !attachment_is_placeholder(options, path)) { - write_attachment_to_envelope(fd, path, filename, - attachment_type, content_type); - } - } - sentry_value_decref(attach_list); - } - } - } + write_attachments_from_manifest(fd, options, run_folder); } // Add screenshot attachment if captured by the daemon diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 7133fa2f1..33a3d8a29 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -153,8 +153,6 @@ typedef struct { static void crashpad_backend_add_breadcrumb(sentry_backend_t *backend, sentry_value_t breadcrumb, const sentry_options_t *options); -static sentry_path_t *make_attachment_path( - const sentry_path_t *run_path, sentry_value_t attachment); /** * Correctly destruct C++ members of the crashpad state. @@ -405,7 +403,8 @@ prepare_initial_attachment( { size_t bytes_len = 0; const char *bytes = sentry__attachment_get_bytes(attachment, &bytes_len); - sentry_path_t *path = make_attachment_path(run_path, attachment); + sentry_path_t *path + = sentry__attachment_make_run_path(run_path, attachment); if (!path) { return nullptr; } @@ -883,35 +882,6 @@ process_completed_reports( #if defined(SENTRY_PLATFORM_WINDOWS) || defined(SENTRY_PLATFORM_LINUX) \ || defined(SENTRY_PLATFORM_MACOS) -static sentry_path_t * -make_attachment_path(const sentry_path_t *run_path, sentry_value_t attachment) -{ - if (!sentry__attachment_get_bytes(attachment, nullptr)) { - return sentry__attachment_make_path(attachment); - } - - sentry_uuid_t id = sentry__attachment_get_id(attachment); - const char *filename = sentry__attachment_get_filename(attachment); - if (!run_path || sentry_uuid_is_nil(&id) - || sentry__string_empty(filename)) { - return nullptr; - } - - char uuid[37]; - sentry_uuid_as_string(&id, uuid); - sentry_path_t *dir = sentry__path_join_str(run_path, uuid); - sentry_path_t *path = dir ? sentry__path_join_str(dir, filename) : nullptr; - sentry_path_t *parent = path ? sentry__path_dir(path) : nullptr; - bool valid = parent && sentry__path_eq(parent, dir); - sentry__path_free(parent); - sentry__path_free(dir); - if (!valid) { - sentry__path_free(path); - return nullptr; - } - return path; -} - static void add_attachment(void *state, sentry_value_t attachment) { @@ -922,7 +892,8 @@ add_attachment(void *state, sentry_value_t attachment) size_t bytes_len = 0; const char *bytes = sentry__attachment_get_bytes(attachment, &bytes_len); - sentry_path_t *path = make_attachment_path(data->run_path, attachment); + sentry_path_t *path + = sentry__attachment_make_run_path(data->run_path, attachment); if (!path) { const char *filename = sentry__attachment_get_filename(attachment); SENTRY_WARNF("failed to create path for crashpad attachment \"%s\"", @@ -953,7 +924,8 @@ remove_attachment(void *state, sentry_value_t attachment) if (!data || !data->client) { return; } - sentry_path_t *path = make_attachment_path(data->run_path, attachment); + sentry_path_t *path + = sentry__attachment_make_run_path(data->run_path, attachment); if (!path) { return; } diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index 974c66b0e..e70f5502d 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -245,39 +245,6 @@ native_backend_preload_scope( sentry_value_decref(breadcrumbs); } -/** - * Creates an attachment path, deriving a unique path in the run directory for - * buffer attachments. - */ -static sentry_path_t * -make_attachment_path(const sentry_path_t *run_path, sentry_value_t attachment) -{ - if (!sentry__attachment_get_bytes(attachment, NULL)) { - return sentry__attachment_make_path(attachment); - } - - sentry_uuid_t id = sentry__attachment_get_id(attachment); - const char *filename = sentry__attachment_get_filename(attachment); - if (!run_path || sentry_uuid_is_nil(&id) - || sentry__string_empty(filename)) { - return NULL; - } - - char uuid[37]; - sentry_uuid_as_string(&id, uuid); - sentry_path_t *dir = sentry__path_join_str(run_path, uuid); - sentry_path_t *path = dir ? sentry__path_join_str(dir, filename) : NULL; - sentry_path_t *parent = path ? sentry__path_dir(path) : NULL; - bool valid = parent && sentry__path_eq(parent, dir); - sentry__path_free(parent); - sentry__path_free(dir); - if (!valid) { - sentry__path_free(path); - return NULL; - } - return path; -} - static void add_attachment(void *data, sentry_value_t attachment) { @@ -291,7 +258,8 @@ add_attachment(void *data, sentry_value_t attachment) size_t bytes_len = 0; const char *bytes = sentry__attachment_get_bytes(attachment, &bytes_len); if (bytes) { - sentry_path_t *path = make_attachment_path(state->run_path, attachment); + sentry_path_t *path + = sentry__attachment_make_run_path(state->run_path, attachment); if (!path) { const char *filename = sentry__attachment_get_filename(attachment); SENTRY_WARNF("failed to create path for native backend attachment " @@ -1222,53 +1190,7 @@ native_backend_write_attachments(const sentry_path_t *event_path) sentry_path_t *attach_list_path = sentry__path_join_str(run_path, "__sentry-attachments"); if (attach_list_path) { - sentry_value_t attach_list = sentry_value_new_list(); - size_t len = sentry_value_get_length(attachments); - for (size_t i = 0; i < len; i++) { - sentry_value_t attachment - = sentry_value_get_by_index(attachments, i); - sentry_path_t *path - = make_attachment_path(run_path, attachment); - if (!path) { - continue; - } - // skip missing or partially written attachments - size_t bytes_len = 0; - if (sentry__attachment_get_bytes(attachment, &bytes_len) - && sentry__path_get_size(path) != bytes_len) { - sentry__path_free(path); - continue; - } - sentry_value_t attach_info = sentry_value_new_object(); - sentry_value_set_by_key( - attach_info, "path", sentry_value_new_string(path->path)); - const char *filename - = sentry__attachment_get_filename(attachment); - sentry_value_set_by_key( - attach_info, "filename", sentry_value_new_string(filename)); - const char *type = sentry__attachment_get_type(attachment); - if (!sentry__string_empty(type)) { - sentry_value_set_by_key(attach_info, "attachment_type", - sentry_value_new_string(type)); - } - const char *content_type - = sentry__attachment_get_content_type(attachment); - if (content_type) { - sentry_value_set_by_key(attach_info, "content_type", - sentry_value_new_string(content_type)); - } - sentry_value_append(attach_list, attach_info); - sentry__path_free(path); - } - size_t attach_json_len = 0; - char *attach_json - = sentry__value_to_json(attach_list, &attach_json_len); - sentry_value_decref(attach_list); - if (attach_json) { - sentry__path_write_buffer( - attach_list_path, attach_json, attach_json_len); - sentry_free(attach_json); - } + sentry__write_attachment_manifest(attach_list_path, attachments); sentry__path_free(attach_list_path); } sentry__path_free(run_path); diff --git a/src/sentry_attachment.c b/src/sentry_attachment.c index 11a7791d4..5011844f7 100644 --- a/src/sentry_attachment.c +++ b/src/sentry_attachment.c @@ -1,6 +1,7 @@ #include "sentry_attachment.h" #include "sentry_alloc.h" #include "sentry_logger.h" +#include "sentry_mpack.h" #include "sentry_options.h" #include "sentry_path.h" #include "sentry_string.h" @@ -401,6 +402,36 @@ sentry__attachment_make_path(sentry_value_t attachment) return sentry__path_from_str(sentry__attachment_get_path(attachment)); } +sentry_path_t * +sentry__attachment_make_run_path( + const sentry_path_t *run_path, sentry_value_t attachment) +{ + if (!sentry__attachment_get_bytes(attachment, NULL)) { + return sentry__attachment_make_path(attachment); + } + + sentry_uuid_t id = sentry__attachment_get_id(attachment); + const char *filename = sentry__attachment_get_filename(attachment); + if (!run_path || sentry_uuid_is_nil(&id) + || sentry__string_empty(filename)) { + return NULL; + } + + char uuid[37]; + sentry_uuid_as_string(&id, uuid); + sentry_path_t *dir = sentry__path_join_str(run_path, uuid); + sentry_path_t *path = dir ? sentry__path_join_str(dir, filename) : NULL; + sentry_path_t *parent = path ? sentry__path_dir(path) : NULL; + bool valid = parent && sentry__path_eq(parent, dir); + sentry__path_free(parent); + sentry__path_free(dir); + if (!valid) { + sentry__path_free(path); + return NULL; + } + return path; +} + bool sentry__attachment_is_placeholder( sentry_value_t attachment, const sentry_options_t *options) @@ -662,3 +693,109 @@ sentry__attachments_clone(sentry_value_t attachments) } return clone; } + +static sentry_value_t +read_manifest(const char *buf, size_t buf_len) +{ + if (buf_len == 0) { + return sentry_value_new_list(); + } + + sentry_value_t attachments + = sentry__value_from_msgpack_stream(buf, buf_len); + size_t len = sentry_value_get_length(attachments); + for (size_t i = 0; i < len; i++) { + sentry_value_t attachment = sentry_value_get_by_index(attachments, i); + if (sentry_value_get_type(attachment) != SENTRY_VALUE_TYPE_OBJECT + || sentry__string_empty(sentry__attachment_get_path(attachment)) + || sentry__string_empty( + sentry__attachment_get_filename(attachment))) { + sentry_value_decref(attachments); + return sentry_value_new_null(); + } + } + return attachments; +} + +sentry_value_t +sentry__read_attachment_manifest(const sentry_path_t *path) +{ + if (!path) { + return sentry_value_new_null(); + } + size_t buf_len = 0; + char *buf = sentry__path_read_to_buffer(path, &buf_len); + if (!buf) { + return sentry_value_new_null(); + } + sentry_value_t attachments = read_manifest(buf, buf_len); + sentry_free(buf); + return attachments; +} + +bool +sentry__write_attachment_manifest( + const sentry_path_t *path, sentry_value_t attachments) +{ + if (!path) { + return false; + } + + sentry_path_t *run_path = sentry__path_dir(path); + if (!run_path) { + return false; + } + + const char *keys[] = { ATTACHMENT_ID, ATTACHMENT_FILENAME, ATTACHMENT_TYPE, + ATTACHMENT_CONTENT_TYPE }; + mpack_writer_t writer; + char *buf = NULL; + size_t buf_len = 0; + mpack_writer_init_growable(&writer, &buf, &buf_len); + size_t len = sentry_value_get_length(attachments); + for (size_t i = 0; i < len && mpack_writer_error(&writer) == mpack_ok; + i++) { + sentry_value_t attachment = sentry_value_get_by_index(attachments, i); + sentry_path_t *attachment_path + = sentry__attachment_make_run_path(run_path, attachment); + if (!attachment_path) { + continue; + } + + // skip missing or partially written attachments + size_t bytes_len = 0; + if (sentry__attachment_get_bytes(attachment, &bytes_len) + && sentry__path_get_size(attachment_path) != bytes_len) { + sentry__path_free(attachment_path); + continue; + } + + uint32_t count = 1; + for (size_t j = 0; j < sizeof(keys) / sizeof(keys[0]); j++) { + if (sentry_value_get_type( + sentry_value_get_by_key(attachment, keys[j])) + == SENTRY_VALUE_TYPE_STRING) { + count++; + } + } + mpack_start_map(&writer, count); + mpack_write_cstr(&writer, ATTACHMENT_PATH); + mpack_write_cstr(&writer, attachment_path->path); + sentry__path_free(attachment_path); + for (size_t j = 0; j < sizeof(keys) / sizeof(keys[0]); j++) { + sentry_value_t value = sentry_value_get_by_key(attachment, keys[j]); + if (sentry_value_get_type(value) == SENTRY_VALUE_TYPE_STRING) { + mpack_write_cstr(&writer, keys[j]); + mpack_write_str(&writer, sentry_value_as_string(value), + (uint32_t)sentry_value_get_length(value)); + } + } + mpack_finish_map(&writer); + } + + sentry__path_free(run_path); + bool success = mpack_writer_destroy(&writer) == mpack_ok + && sentry__path_write_buffer(path, buf ? buf : "", buf_len) == 0; + sentry_free(buf); + return success; +} diff --git a/src/sentry_attachment.h b/src/sentry_attachment.h index 1b0668ad0..808c3fe53 100644 --- a/src/sentry_attachment.h +++ b/src/sentry_attachment.h @@ -69,6 +69,13 @@ const char *sentry__attachment_get_path(sentry_value_t attachment); */ sentry_path_t *sentry__attachment_make_path(sentry_value_t attachment); +/** + * Creates an attachment path, deriving a unique path in the run directory for + * buffer attachments. + */ +sentry_path_t *sentry__attachment_make_run_path( + const sentry_path_t *run_path, sentry_value_t attachment); + /** * Returns true if the attachment should be represented as an attachment-ref. */ @@ -142,4 +149,17 @@ sentry_value_t sentry__attachments_find( */ sentry_value_t sentry__attachments_clone(sentry_value_t attachments); +/** + * Reads a list of attachments from a manifest file. + */ +sentry_value_t sentry__read_attachment_manifest(const sentry_path_t *path); + +/** + * Writes attachment metadata to a manifest file in the run directory. + * Byte attachments reference their separately persisted files; missing or + * partially written files are skipped. Does not modify the attachments. + */ +bool sentry__write_attachment_manifest( + const sentry_path_t *path, sentry_value_t attachments); + #endif diff --git a/tests/assertions.py b/tests/assertions.py index d67943c51..904429d99 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -537,6 +537,7 @@ def _load_crashpad_attachments(msg): bytes_bin = None minidump = None for part in msg.walk(): + assert part.get_filename() != "__sentry-attachments" if part.get_filename() is not None: assert part.get("Content-Type") is None diff --git a/tests/test_integration_native.py b/tests/test_integration_native.py index c06352b05..30940ea6e 100644 --- a/tests/test_integration_native.py +++ b/tests/test_integration_native.py @@ -12,6 +12,7 @@ import time import struct +import msgpack import pytest from . import ( @@ -433,8 +434,10 @@ def manifest_is_current(): if not paths: return False try: - manifest = json.loads(paths[0].read_text()) - except (OSError, json.JSONDecodeError): + unpacker = msgpack.Unpacker(raw=False) + unpacker.feed(paths[0].read_bytes()) + manifest = list(unpacker) + except (OSError, msgpack.UnpackException): # rewritten in place, so a read can catch a partial file return False last_manifest = manifest diff --git a/tests/unit/test_attachments.c b/tests/unit/test_attachments.c index 5240f2133..525318304 100644 --- a/tests/unit/test_attachments.c +++ b/tests/unit/test_attachments.c @@ -756,3 +756,111 @@ SENTRY_TEST(attachments_more_than_ten) TEST_CHECK_INT_EQUAL(testdata.called, 1); } + +SENTRY_TEST(attachment_manifest) +{ + sentry_value_t attachment = sentry__attachment_from_file("attachment.bin"); + sentry_attachment_set_filename(attachment, "renamed.bin"); + sentry_value_t attachments = sentry_value_new_list(); + sentry_value_append(attachments, attachment); + + const char bytes[] = { 'a', '\0', 'b' }; + const char *id = "00000000-0000-4000-8000-000000000001"; + sentry_value_t bytes_attachment + = sentry_attachment_from_bytes(bytes, sizeof(bytes), "bytes.bin"); + sentry_value_set_by_key( + bytes_attachment, "id", sentry_value_new_string(id)); + sentry_value_append(attachments, bytes_attachment); + for (size_t i = 0; i < sentry_value_get_length(attachments); i++) { + attachment = sentry_value_get_by_index(attachments, i); + sentry_attachment_set_type(attachment, "event.attachment"); + sentry_attachment_set_content_type( + attachment, "application/octet-stream"); + } + sentry_value_freeze(attachments); + TEST_CHECK(!sentry__write_attachment_manifest(NULL, attachments)); + + sentry_path_t *run_path = sentry__path_from_str( + SENTRY_TEST_PATH_PREFIX ".attachment-manifest-run"); + sentry_path_t *dir = sentry__path_join_str(run_path, id); + sentry_path_t *path = sentry__path_join_str(dir, "bytes.bin"); + sentry_path_t *manifest_path + = sentry__path_join_str(run_path, "__sentry-attachments"); + TEST_ASSERT(sentry__path_create_dir_all(dir) == 0); + sentry__path_remove(path); + + // missing and partially written byte attachments are excluded + for (size_t size = 0; size <= sizeof(bytes); size++) { + if (size) { + TEST_ASSERT(sentry__path_write_buffer(path, bytes, size) == 0); + } + TEST_ASSERT( + sentry__write_attachment_manifest(manifest_path, attachments)); + sentry_value_t parsed = sentry__read_attachment_manifest(manifest_path); + TEST_CHECK(sentry_value_get_type(parsed) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK_INT_EQUAL( + sentry_value_get_length(parsed), size == sizeof(bytes) ? 2 : 1); + for (size_t i = 0; i < sentry_value_get_length(parsed); i++) { + sentry_value_t info = sentry_value_get_by_index(parsed, i); + attachment = sentry_value_get_by_index(attachments, i); + TEST_CHECK_STRING_EQUAL(sentry__attachment_get_path(info), + i == 0 ? "attachment.bin" : path->path); + TEST_CHECK_STRING_EQUAL(sentry__attachment_get_filename(info), + i == 0 ? "renamed.bin" : "bytes.bin"); + TEST_CHECK_STRING_EQUAL( + sentry__attachment_get_type(info), "event.attachment"); + TEST_CHECK_STRING_EQUAL(sentry__attachment_get_content_type(info), + "application/octet-stream"); + TEST_CHECK(sentry__attachment_eq(info, attachment)); + TEST_CHECK(sentry__attachment_get_bytes(info, NULL) == NULL); + } + sentry_value_decref(parsed); + } + + size_t size = 0; + const char *original + = sentry__attachment_get_bytes(bytes_attachment, &size); + TEST_CHECK(size == sizeof(bytes)); + TEST_CHECK(memcmp(original, bytes, sizeof(bytes)) == 0); + TEST_CHECK(sentry__attachment_get_path(bytes_attachment) == NULL); + TEST_CHECK(sentry_value_is_frozen(bytes_attachment)); + char *persisted = sentry__path_read_to_buffer(path, &size); + TEST_ASSERT(persisted != NULL); + TEST_CHECK(size == sizeof(bytes)); + TEST_CHECK(memcmp(persisted, bytes, sizeof(bytes)) == 0); + sentry_free(persisted); + sentry_value_decref(attachments); + + sentry_value_t parsed = sentry__read_attachment_manifest(NULL); + TEST_CHECK(sentry_value_is_null(parsed)); + sentry_value_decref(parsed); + + sentry_path_t *missing_path = sentry__path_from_str( + SENTRY_TEST_PATH_PREFIX ".missing-attachment-manifest"); + sentry__path_remove(missing_path); + parsed = sentry__read_attachment_manifest(missing_path); + TEST_CHECK(sentry_value_is_null(parsed)); + sentry_value_decref(parsed); + sentry__path_free(missing_path); + + TEST_ASSERT( + sentry__path_write_buffer(manifest_path, "incomplete", 10) == 0); + parsed = sentry__read_attachment_manifest(manifest_path); + TEST_CHECK(sentry_value_is_null(parsed)); + sentry_value_decref(parsed); + + sentry_value_t empty = sentry_value_new_list(); + TEST_ASSERT(sentry__write_attachment_manifest(manifest_path, empty)); + parsed = sentry__read_attachment_manifest(manifest_path); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(parsed), 0); + sentry_value_decref(parsed); + sentry_value_decref(empty); + sentry__path_remove(path); + sentry__path_remove(dir); + sentry__path_remove(manifest_path); + sentry__path_remove(run_path); + sentry__path_free(path); + sentry__path_free(dir); + sentry__path_free(manifest_path); + sentry__path_free(run_path); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 623e5bc97..98d78647f 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -10,6 +10,7 @@ XX(assert_sdk_name) XX(assert_sdk_user_agent) XX(assert_sdk_version) XX(attachment_bytes_no_copy) +XX(attachment_manifest) XX(attachment_placeholder) XX(attachment_properties) XX(attachment_rate_limit) From 2b570e2bd60858782c7cad4c02fb55b7b8ac107e Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 09:40:18 +0200 Subject: [PATCH 02/13] exclude ids --- src/sentry_attachment.c | 4 ++-- tests/unit/test_attachments.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sentry_attachment.c b/src/sentry_attachment.c index 5011844f7..f788e5432 100644 --- a/src/sentry_attachment.c +++ b/src/sentry_attachment.c @@ -746,8 +746,8 @@ sentry__write_attachment_manifest( return false; } - const char *keys[] = { ATTACHMENT_ID, ATTACHMENT_FILENAME, ATTACHMENT_TYPE, - ATTACHMENT_CONTENT_TYPE }; + const char *keys[] + = { ATTACHMENT_FILENAME, ATTACHMENT_TYPE, ATTACHMENT_CONTENT_TYPE }; mpack_writer_t writer; char *buf = NULL; size_t buf_len = 0; diff --git a/tests/unit/test_attachments.c b/tests/unit/test_attachments.c index 525318304..16e821f54 100644 --- a/tests/unit/test_attachments.c +++ b/tests/unit/test_attachments.c @@ -811,7 +811,8 @@ SENTRY_TEST(attachment_manifest) sentry__attachment_get_type(info), "event.attachment"); TEST_CHECK_STRING_EQUAL(sentry__attachment_get_content_type(info), "application/octet-stream"); - TEST_CHECK(sentry__attachment_eq(info, attachment)); + sentry_uuid_t info_id = sentry__attachment_get_id(info); + TEST_CHECK(sentry_uuid_is_nil(&info_id)); TEST_CHECK(sentry__attachment_get_bytes(info, NULL) == NULL); } sentry_value_decref(parsed); From 904ecefefacbdcbfa25b4d20a77178ddcf25057f Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 09:45:53 +0200 Subject: [PATCH 03/13] fix absent --- src/backends/native/sentry_crash_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index aab3822d2..563abc700 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -420,7 +420,7 @@ read_attachment_manifest(const sentry_path_t *run_folder) char *buf = sentry__path_read_to_buffer(path, &buf_len); sentry__path_free(path); if (!buf) { - return sentry_value_new_null(); + return sentry_value_new_list(); } const char *start = buf; const char *end = buf + buf_len; From 8b4998bacb80d120620555607c04b646dfad3c21 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 09:54:38 +0200 Subject: [PATCH 04/13] exclude malformed attachment entries --- src/sentry_attachment.c | 16 ++++++++++++---- tests/unit/test_attachments.c | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/sentry_attachment.c b/src/sentry_attachment.c index f788e5432..a02f2d9f6 100644 --- a/src/sentry_attachment.c +++ b/src/sentry_attachment.c @@ -703,16 +703,24 @@ read_manifest(const char *buf, size_t buf_len) sentry_value_t attachments = sentry__value_from_msgpack_stream(buf, buf_len); - size_t len = sentry_value_get_length(attachments); - for (size_t i = 0; i < len; i++) { + size_t i = 0; + while (i < sentry_value_get_length(attachments)) { sentry_value_t attachment = sentry_value_get_by_index(attachments, i); if (sentry_value_get_type(attachment) != SENTRY_VALUE_TYPE_OBJECT || sentry__string_empty(sentry__attachment_get_path(attachment)) || sentry__string_empty( sentry__attachment_get_filename(attachment))) { - sentry_value_decref(attachments); - return sentry_value_new_null(); + if (sentry_value_remove_by_index(attachments, i)) { + sentry_value_decref(attachments); + return sentry_value_new_null(); + } + continue; } + i++; + } + if (sentry_value_get_length(attachments) == 0) { + sentry_value_decref(attachments); + return sentry_value_new_null(); } return attachments; } diff --git a/tests/unit/test_attachments.c b/tests/unit/test_attachments.c index 16e821f54..3ca5f8b84 100644 --- a/tests/unit/test_attachments.c +++ b/tests/unit/test_attachments.c @@ -818,6 +818,33 @@ SENTRY_TEST(attachment_manifest) sentry_value_decref(parsed); } + // malformed manifest entries are excluded + sentry_value_t malformed = sentry_value_new_object(); + size_t valid_len = 0; + size_t malformed_len = 0; + char *valid_buf = sentry_value_to_msgpack( + sentry_value_get_by_index(attachments, 0), &valid_len); + char *malformed_buf = sentry_value_to_msgpack(malformed, &malformed_len); + char *manifest_buf = sentry_malloc(valid_len + malformed_len); + TEST_ASSERT(valid_buf != NULL); + TEST_ASSERT(malformed_buf != NULL); + TEST_ASSERT(manifest_buf != NULL); + memcpy(manifest_buf, malformed_buf, malformed_len); + memcpy(manifest_buf + malformed_len, valid_buf, valid_len); + TEST_ASSERT(sentry__path_write_buffer( + manifest_path, manifest_buf, valid_len + malformed_len) + == 0); + sentry_value_t salvaged = sentry__read_attachment_manifest(manifest_path); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(salvaged), 1); + TEST_CHECK_STRING_EQUAL( + sentry__attachment_get_filename(sentry_value_get_by_index(salvaged, 0)), + "renamed.bin"); + sentry_value_decref(salvaged); + sentry_free(manifest_buf); + sentry_free(malformed_buf); + sentry_free(valid_buf); + sentry_value_decref(malformed); + size_t size = 0; const char *original = sentry__attachment_get_bytes(bytes_attachment, &size); From 59d3c72d350efc257d478b380fe9f6a5ee78fee9 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 10:46:13 +0200 Subject: [PATCH 05/13] revise --- src/backends/native/sentry_crash_daemon.c | 59 ++++++++++++----------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index 563abc700..4794136a8 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -364,10 +364,36 @@ attachment_is_placeholder(const sentry_options_t *options, const char *path) return is_placeholder; } +/** + * Reads a legacy JSON attachment manifest (TODO: remove in 1.0) + */ static sentry_value_t -read_legacy_manifest(const char *buf, size_t buf_len) +read_legacy_manifest(const sentry_path_t *manifest_path) { - sentry_value_t legacy = sentry__value_from_json(buf, buf_len); + size_t buf_len = 0; + char *buf = sentry__path_read_to_buffer(manifest_path, &buf_len); + if (!buf) { + return sentry_value_new_list(); + } + + const char *start = buf; + const char *end = buf + buf_len; + while (start < end + && (*start == ' ' || *start == '\t' || *start == '\r' + || *start == '\n')) { + start++; + } + const char *trimmed_end = end; + while (trimmed_end > start + && (trimmed_end[-1] == ' ' || trimmed_end[-1] == '\t' + || trimmed_end[-1] == '\r' || trimmed_end[-1] == '\n')) { + trimmed_end--; + } + sentry_value_t legacy + = start < trimmed_end && *start == '[' && trimmed_end[-1] == ']' + ? sentry__value_from_json(start, (size_t)(trimmed_end - start)) + : sentry_value_new_null(); + sentry_free(buf); if (sentry_value_get_type(legacy) != SENTRY_VALUE_TYPE_LIST) { sentry_value_decref(legacy); return sentry_value_new_null(); @@ -410,35 +436,10 @@ read_attachment_manifest(const sentry_path_t *run_folder) return sentry_value_new_null(); } sentry_value_t attachments = sentry__read_attachment_manifest(path); - if (!sentry_value_is_null(attachments)) { - sentry__path_free(path); - return attachments; + if (sentry_value_is_null(attachments)) { + attachments = read_legacy_manifest(path); } - sentry_value_decref(attachments); - - size_t buf_len = 0; - char *buf = sentry__path_read_to_buffer(path, &buf_len); sentry__path_free(path); - if (!buf) { - return sentry_value_new_list(); - } - const char *start = buf; - const char *end = buf + buf_len; - while (start < end - && (*start == ' ' || *start == '\t' || *start == '\r' - || *start == '\n')) { - start++; - } - const char *trimmed_end = end; - while (trimmed_end > start - && (trimmed_end[-1] == ' ' || trimmed_end[-1] == '\t' - || trimmed_end[-1] == '\r' || trimmed_end[-1] == '\n')) { - trimmed_end--; - } - attachments = start < trimmed_end && *start == '[' && trimmed_end[-1] == ']' - ? read_legacy_manifest(start, (size_t)(trimmed_end - start)) - : sentry_value_new_null(); - sentry_free(buf); return attachments; } From 82ed4ffec8ec13183c68d18a24a4338e17abd07d Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 13:26:43 +0200 Subject: [PATCH 06/13] fix msvc --- src/sentry_mpack.h | 22 +++++++++++++++++++ src/sentry_value.c | 25 +--------------------- src/session_replay/sentry_session_replay.c | 25 +--------------------- 3 files changed, 24 insertions(+), 48 deletions(-) diff --git a/src/sentry_mpack.h b/src/sentry_mpack.h index 57c36949f..064f232da 100644 --- a/src/sentry_mpack.h +++ b/src/sentry_mpack.h @@ -387,6 +387,28 @@ #define mpack_writer_track_pop sentry__mpack_writer_track_pop #define mpack_writer_track_push sentry__mpack_writer_track_push +#if defined(_MSC_VER) +# pragma warning(push) +# pragma warning(disable : 4127) // conditional expression is constant +# if defined(__clang__) // clang-cl +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wdocumentation" +# pragma clang diagnostic ignored "-Wpre-c11-compat" +# endif +#elif defined(__clang__) +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wstatic-in-inline" +#endif + #include "../vendor/mpack.h" +#if defined(_MSC_VER) +# pragma warning(pop) +# ifdef __clang__ // clang-cl +# pragma clang diagnostic pop +# endif +#elif defined(__clang__) +# pragma clang diagnostic pop +#endif + #endif diff --git a/src/sentry_value.c b/src/sentry_value.c index c5aba36fc..6e09804c2 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -6,33 +6,10 @@ #include #include -#if defined(_MSC_VER) -# pragma warning(push) -# pragma warning(disable : 4127) // conditional expression is constant -# if defined(__clang__) // clang-cl -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wdocumentation" -# pragma clang diagnostic ignored "-Wpre-c11-compat" -# endif -#elif defined(__clang__) -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wstatic-in-inline" -#endif - -#include "sentry_mpack.h" - -#if defined(_MSC_VER) -# pragma warning(pop) -# ifdef __clang__ // clang-cl -# pragma clang diagnostic pop -# endif -#elif defined(__clang__) -# pragma clang diagnostic pop -#endif - #include "sentry_alloc.h" #include "sentry_core.h" #include "sentry_json.h" +#include "sentry_mpack.h" #include "sentry_slice.h" #include "sentry_string.h" #include "sentry_sync.h" diff --git a/src/session_replay/sentry_session_replay.c b/src/session_replay/sentry_session_replay.c index e0705636d..bc7492c00 100644 --- a/src/session_replay/sentry_session_replay.c +++ b/src/session_replay/sentry_session_replay.c @@ -4,35 +4,12 @@ #include "sentry_database.h" #include "sentry_envelope.h" #include "sentry_json.h" +#include "sentry_mpack.h" #include "sentry_string.h" #include "sentry_utils.h" #include "sentry_uuid.h" #include "sentry_value.h" -#if defined(_MSC_VER) -# pragma warning(push) -# pragma warning(disable : 4127) // conditional expression is constant -# if defined(__clang__) // clang-cl -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wdocumentation" -# pragma clang diagnostic ignored "-Wpre-c11-compat" -# endif -#elif defined(__clang__) -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wstatic-in-inline" -#endif - -#include "sentry_mpack.h" - -#if defined(_MSC_VER) -# pragma warning(pop) -# ifdef __clang__ // clang-cl -# pragma clang diagnostic pop -# endif -#elif defined(__clang__) -# pragma clang diagnostic pop -#endif - #include sentry_path_t * From 5f3b00e201d7eb7996a8fe2981bb3f28a333b8ab Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 18:01:24 +0200 Subject: [PATCH 07/13] bump crashpad --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index 60be52d4a..000e3ad7a 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 60be52d4aa63db37d68a1bc81aa95182fc878e78 +Subproject commit 000e3ad7a51c89e49bc00eca9846aa0485cccc96 From b0804e19f9db25fa89889bf61859d4496e180d70 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Tue, 15 Sep 2026 15:45:43 +0200 Subject: [PATCH 08/13] feat!: Add hint support to event capture Accept hints in `sentry_scope_capture_event` and pass them to `before_send`. Keep `sentry_capture_event` unchanged. Merge hint and scope attachments before `before_send` and `before_send_feedback`, then use the resulting hint attachments in the envelope. Add `sentry_hint_remove_attachment` and `sentry_hint_clear_attachments` to filter attachments without modifying scopes. BREAKING CHANGE: `before_send` callbacks now take `sentry_hint_t *` instead of `void *` for the hint parameter. `sentry_scope_capture_event` now requires a hint argument; pass `NULL` when no hint is needed. Scope attachment changes inside either callback no longer affect the current event; modify the hint instead. Close: #2098 --- CHANGELOG.md | 4 + examples/example.c | 6 +- include/sentry.h | 44 ++++-- src/backends/sentry_backend_breakpad.cpp | 4 +- src/backends/sentry_backend_inproc.c | 2 +- src/sentry_app_hang_monitor.c | 2 +- src/sentry_core.c | 129 +++++++-------- src/sentry_core.h | 4 +- src/sentry_hint.c | 18 +++ tests/unit/test_app_hang.c | 4 +- tests/unit/test_attachments.c | 9 +- tests/unit/test_basic.c | 192 ++++++++++++++++++++++- tests/unit/test_scope.c | 33 ++-- tests/unit/test_tracing.c | 2 +- tests/unit/tests.inc | 3 + 15 files changed, 350 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001371ca3..39fc0f4b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Attachment APIs now use `sentry_value_t` and `sentry_uuid_t` instead of `sentry_attachment_t *` handles. Most attachment APIs, function names and arguments, are otherwise unchanged. ([#1974](https://github.com/getsentry/sentry-native/pull/1974)) - `sentry_init()` now consumes `/last_crash` after caching its value, aligning crashed-last-run behavior with other Sentry SDKs. ([#2023](https://github.com/getsentry/sentry-native/pull/2023)) +- Change the `hint` parameter of `before_send` callbacks (`sentry_event_function_t`) from `void *` to `sentry_hint_t *`. Update callbacks registered with `sentry_options_set_before_send` to use the new parameter type. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) +- Add a `sentry_hint_t *hint` argument to `sentry_scope_capture_event`. Pass `NULL` if no hint is needed. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) - Remove `sentry_options_get/set_enable_logs` and `sentry_options_get/set_enable_metrics`. ([#1980](https://github.com/getsentry/sentry-native/pull/1980)) > Structured logs and metrics have been enabled by default since `0.13`, and the options were deprecated and made no-ops in `0.16`. > @@ -16,10 +18,12 @@ - Add `sentry_attachment_from_file/bytes` (and their wide-string variants) for creating attachment values that can be fully configured before they are added. ([#2079](https://github.com/getsentry/sentry-native/pull/2079)) - Add `sentry_add_attachment`, `sentry_scope_add_attachment`, and `sentry_hint_add_attachment` for adding configured attachments to the global scope, a specific scope, or a hint. These functions consume and freeze the attachment value. ([#2079](https://github.com/getsentry/sentry-native/pull/2079), [#1974](https://github.com/getsentry/sentry-native/pull/1974)) - Add `sentry_start_new_trace()` as a clearer name for starting a new trace. ([#2095](https://github.com/getsentry/sentry-native/pull/2095)) +- Add hint support to `sentry_scope_capture_event` to pass event-specific attachments. The hint is also passed to `before_send`, which can modify attachments before the event is sent. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) **Fixes**: - Reduce lock contention for multi-threaded log and metric capture by allowing concurrent reads of scope data, and reusing a single options reference throughout each capture. ([#2091](https://github.com/getsentry/sentry-native/pull/2091), [#2094](https://github.com/getsentry/sentry-native/pull/2094)) +- Add `sentry_hint_remove_attachment` and `sentry_hint_clear_attachments` to filter attachments without modifying scopes. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) **Other changes**: diff --git a/examples/example.c b/examples/example.c index 5a16a1cfa..420d82786 100644 --- a/examples/example.c +++ b/examples/example.c @@ -127,7 +127,7 @@ traces_sampler_callback(const sentry_transaction_context_t *transaction_ctx, } static sentry_value_t -before_send_callback(sentry_value_t event, void *hint, void *user_data) +before_send_callback(sentry_value_t event, sentry_hint_t *hint, void *user_data) { (void)hint; (void)user_data; @@ -142,7 +142,7 @@ before_send_callback(sentry_value_t event, void *hint, void *user_data) static sentry_value_t discarding_before_send_callback( - sentry_value_t event, void *hint, void *user_data) + sentry_value_t event, sentry_hint_t *hint, void *user_data) { (void)hint; (void)user_data; @@ -1247,7 +1247,7 @@ main(int argc, char **argv) sentry_scope_add_attachment(scope, bytes); } - sentry_scope_capture_event(scope, event); + sentry_scope_capture_event(scope, event, NULL); } if (has_arg(argc, argv, "capture-multiple")) { diff --git a/include/sentry.h b/include/sentry.h index 2516a8322..8725f4e85 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1478,6 +1478,13 @@ SENTRY_API void sentry_options_set_send_default_pii( sentry_options_t *opts, int value); #endif +/** + * A hint that can be passed to capture functions to provide additional context, + * such as attachments. + */ +struct sentry_hint_s; +typedef struct sentry_hint_s sentry_hint_t; + /** * Type of the `before_send` callback. * @@ -1486,6 +1493,9 @@ SENTRY_API void sentry_options_set_send_default_pii( * call `sentry_value_decref` on the provided event and return a * `sentry_value_new_null()` instead. * + * The hint is always provided and can be used to modify attachments on the + * event. + * * If you have set an `on_crash` callback (independent of whether it discards or * retains the event), `before_send` will no longer be invoked for crash-events, * which allows you to better distinguish between crashes and all other events @@ -1510,7 +1520,7 @@ SENTRY_API void sentry_options_set_send_default_pii( * though a crash report will be sent. */ typedef sentry_value_t (*sentry_event_function_t)( - sentry_value_t event, void *hint, void *user_data); + sentry_value_t event, sentry_hint_t *hint, void *user_data); /** * Sets the `before_send` callback. @@ -2592,7 +2602,10 @@ SENTRY_API sentry_uuid_t sentry_scope_get_last_event_id( SENTRY_API sentry_uuid_t sentry_capture_event(sentry_value_t event); /** - * Sends a sentry event with a scope. + * Sends a sentry event with a scope and a hint. + * + * This function takes ownership of the event and hint, which will be freed + * automatically. The hint may be NULL. * * If `scope` is a local scope (`sentry_local_scope_new`), this takes ownership * of it and frees it. If `scope` is user-owned (`sentry_scope_new` or @@ -2600,7 +2613,7 @@ SENTRY_API sentry_uuid_t sentry_capture_event(sentry_value_t event); * it yourself with `sentry_scope_free`. */ SENTRY_API sentry_uuid_t sentry_scope_capture_event( - sentry_scope_t *scope, sentry_value_t event); + sentry_scope_t *scope, sentry_value_t event, sentry_hint_t *hint); /** * Deprecated alias for `sentry_scope_capture_event`. Note the reversed argument @@ -4139,16 +4152,7 @@ SENTRY_API sentry_value_t sentry_value_new_feedback_n(const char *message, SENTRY_API void sentry_capture_feedback(sentry_value_t user_feedback); /** - * A hint that can be passed to capture functions to provide additional context, - * such as attachments. - */ -struct sentry_hint_s; -typedef struct sentry_hint_s sentry_hint_t; - -/** - * Creates a new hint to be passed into - * - `sentry_capture_feedback_with_hint` - * - `sentry_scope_capture_feedback` + * Creates a new hint to be passed into capture functions. */ SENTRY_API sentry_hint_t *sentry_hint_new(void); @@ -4202,6 +4206,17 @@ SENTRY_API sentry_uuid_t sentry_hint_attach_bytesw_n(sentry_hint_t *hint, size_t filename_len); #endif +/** + * Removes an attachment from the hint by its ID. Does not modify scopes. + */ +SENTRY_API void sentry_hint_remove_attachment( + sentry_hint_t *hint, sentry_uuid_t attachment_id); + +/** + * Removes all attachments from the hint. Does not modify scopes. + */ +SENTRY_API void sentry_hint_clear_attachments(sentry_hint_t *hint); + /** * Captures a manually created feedback with a hint and sends it to Sentry. * @@ -4237,7 +4252,8 @@ SENTRY_API sentry_uuid_t sentry_scope_capture_feedback( * callback needs to call `sentry_value_decref` on the provided event and * return a `sentry_value_new_null()` instead. * - * The hint is always provided and can be used to add attachments to the event. + * The hint is always provided and can be used to modify attachments on the + * event. * * Feedback events do not go through the `before_send` callback. */ diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index f8a913f9b..58b68b54b 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -183,8 +183,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, } #endif - sentry_envelope_t *envelope = sentry__prepare_event( - options, event, nullptr, !options->on_crash_func, nullptr); + sentry_envelope_t *envelope = sentry__prepare_event(options, event, + nullptr, !options->on_crash_func, nullptr, nullptr); if (envelope) { event_id = sentry__envelope_get_event_id(envelope); } diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 150c29201..a6e274760 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1102,7 +1102,7 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, #endif sentry_envelope_t *envelope = sentry__prepare_event(options, event, - NULL, !options->on_crash_func && !skip_hooks, NULL); + NULL, !options->on_crash_func && !skip_hooks, NULL, NULL); if (envelope) { event_id = sentry__envelope_get_event_id(envelope); } diff --git a/src/sentry_app_hang_monitor.c b/src/sentry_app_hang_monitor.c index dc2d2b672..da31c83f7 100644 --- a/src/sentry_app_hang_monitor.c +++ b/src/sentry_app_hang_monitor.c @@ -81,7 +81,7 @@ app_hang_capture(uint64_t hang_time_ms, uint64_t tid) return false; } sentry_value_t event = sentry__app_hang_make_event(ips, n, hang_time_ms); - sentry__capture_event(event, NULL); + sentry__capture_event(event, NULL, NULL); return true; } diff --git a/src/sentry_core.c b/src/sentry_core.c index af925b6de..b637fadc6 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -565,24 +565,26 @@ sentry_capture_event(sentry_value_t event) if (sentry__event_is_transaction(event)) { return sentry_uuid_nil(); } else { - return sentry__capture_event(event, NULL); + return sentry__capture_event(event, NULL, NULL); } } sentry_uuid_t -sentry_scope_capture_event(sentry_scope_t *scope, sentry_value_t event) +sentry_scope_capture_event( + sentry_scope_t *scope, sentry_value_t event, sentry_hint_t *hint) { if (sentry__event_is_transaction(event)) { + sentry__hint_free(hint); return sentry_uuid_nil(); } else { - return sentry__capture_event(event, scope); + return sentry__capture_event(event, scope, hint); } } sentry_uuid_t sentry_capture_event_with_scope(sentry_value_t event, sentry_scope_t *scope) { - return sentry_scope_capture_event(scope, event); + return sentry_scope_capture_event(scope, event, NULL); } #ifndef SENTRY_UNITTEST @@ -597,7 +599,8 @@ static } sentry_uuid_t -sentry__capture_event(sentry_value_t event, sentry_scope_t *local_scope) +sentry__capture_event( + sentry_value_t event, sentry_scope_t *local_scope, sentry_hint_t *hint) { // `event_id` is only used as an argument to pure output parameters. // Initialization only happens to prevent compiler warnings. @@ -612,8 +615,15 @@ sentry__capture_event(sentry_value_t event, sentry_scope_t *local_scope) if (sentry__event_is_transaction(event)) { envelope = sentry__prepare_transaction(options, event, &event_id); } else { + // Give the hook something to attach to when the caller passed no + // hint. + if (!hint && options->before_send_func) { + // A failed allocation is tolerated: operating on a NULL hint + // no-ops. + hint = sentry_hint_new(); + } envelope = sentry__prepare_event( - options, event, &event_id, true, local_scope); + options, event, &event_id, true, local_scope, hint); } if (envelope) { // Accept a racy read here, since SENTRY_WITH_OPTIONS only prevents @@ -663,6 +673,7 @@ sentry__capture_event(sentry_value_t event, sentry_scope_t *local_scope) sentry_value_decref(event); } sentry__scope_free_one_shot(local_scope); + sentry__hint_free(hint); return was_sent ? event_id : sentry_uuid_nil(); } @@ -708,10 +719,36 @@ static return send; } +static sentry_value_t +prepare_attachments(sentry_hint_t *hint, sentry_scope_t *local_scope) +{ + sentry_value_t attachments = sentry_value_new_null(); + if (hint) { + sentry__attachments_extend(&attachments, hint->attachments); + } + if (local_scope) { + sentry_value_t local_attachments + = sentry__scope_load_attachments(local_scope); + sentry__attachments_extend(&attachments, local_attachments); + sentry_value_decref(local_attachments); + } + SENTRY_WITH_SCOPE (scope) { + sentry_value_t global_attachments + = sentry__scope_load_attachments(scope); + sentry__attachments_extend(&attachments, global_attachments); + sentry_value_decref(global_attachments); + } + if (hint) { + sentry_value_decref(hint->attachments); + hint->attachments = sentry_value_incref(attachments); + } + return attachments; +} + sentry_envelope_t * sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry_uuid_t *event_id, bool invoke_before_send, - sentry_scope_t *local_scope) + sentry_scope_t *local_scope, sentry_hint_t *hint) { sentry_envelope_t *envelope = NULL; @@ -719,7 +756,6 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry__record_errors_on_current_session(1); } - sentry_value_t all_attachments = sentry_value_new_null(); if (local_scope) { SENTRY_DEBUG("merging local scope into event"); sentry_scope_mode_t mode = SENTRY_SCOPE_BREADCRUMBS; @@ -735,14 +771,17 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry__scope_apply_to_event(scope, options, event, mode); } + sentry_value_t all_attachments = prepare_attachments(hint, local_scope); + if (options->before_send_func && invoke_before_send) { SENTRY_DEBUG("invoking `before_send` hook"); event - = options->before_send_func(event, NULL, options->before_send_data); + = options->before_send_func(event, hint, options->before_send_data); if (sentry_value_is_null(event)) { SENTRY_DEBUG("event was discarded by the `before_send` hook"); sentry__client_report_discard(SENTRY_DISCARD_REASON_BEFORE_SEND, SENTRY_DATA_CATEGORY_ERROR, 1); + sentry_value_decref(all_attachments); return NULL; } } @@ -753,29 +792,11 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, goto fail; } - SENTRY_WITH_SCOPE (scope) { - sentry_value_t global_attachments - = sentry__scope_load_attachments(scope); - sentry_value_t attachments = global_attachments; - if (local_scope) { - sentry_value_t local_attachments - = sentry__scope_load_attachments(local_scope); - if (sentry_value_get_length(local_attachments) > 0) { - // all attachments merged from multiple scopes - sentry__attachments_extend(&all_attachments, local_attachments); - sentry__attachments_extend( - &all_attachments, global_attachments); - attachments = all_attachments; - } - sentry_value_decref(local_attachments); - } - // otherwise only global scope has attachments - sentry__envelope_add_attachments(envelope, attachments, options); - if (options->run) { - sentry__cache_attachment_refs(envelope, attachments, options, - options->run->cache_path, options->run->run_path); - } - sentry_value_decref(global_attachments); + sentry_value_t attachments = hint ? hint->attachments : all_attachments; + sentry__envelope_add_attachments(envelope, attachments, options); + if (options->run) { + sentry__cache_attachment_refs(envelope, attachments, options, + options->run->cache_path, options->run->run_path); } sentry_value_decref(all_attachments); @@ -873,6 +894,8 @@ prepare_user_feedback(const sentry_options_t *options, sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE); } + sentry_value_t all_attachments = prepare_attachments(hint, local_scope); + if (options->before_send_feedback_func) { SENTRY_DEBUG("invoking `before_send_feedback` hook"); event = options->before_send_feedback_func( @@ -882,6 +905,7 @@ prepare_user_feedback(const sentry_options_t *options, "feedback was discarded by the `before_send_feedback` hook"); sentry__client_report_discard(SENTRY_DISCARD_REASON_BEFORE_SEND, SENTRY_DATA_CATEGORY_FEEDBACK, 1); + sentry_value_decref(all_attachments); return NULL; } } @@ -893,39 +917,18 @@ prepare_user_feedback(const sentry_options_t *options, goto fail; } - sentry_value_t all_attachments = sentry_value_new_null(); - if (hint - && sentry_value_get_type(hint->attachments) == SENTRY_VALUE_TYPE_LIST - && sentry_value_get_length(hint->attachments) > 0) { - sentry__attachments_extend(&all_attachments, hint->attachments); - } - if (local_scope) { - sentry_value_t local_attachments - = sentry__scope_load_attachments(local_scope); - sentry__attachments_extend(&all_attachments, local_attachments); - sentry_value_decref(local_attachments); - } - - SENTRY_WITH_SCOPE (scope) { - sentry_value_t global_attachments - = sentry__scope_load_attachments(scope); - sentry_value_t attachments = global_attachments; - if (sentry_value_get_length(all_attachments) > 0) { - sentry__attachments_extend(&all_attachments, global_attachments); - attachments = all_attachments; - } - sentry__envelope_add_attachments(envelope, attachments, options); - if (options->run) { - sentry__cache_attachment_refs(envelope, attachments, options, - options->run->cache_path, options->run->run_path); - } - sentry_value_decref(global_attachments); + sentry_value_t attachments = hint ? hint->attachments : all_attachments; + sentry__envelope_add_attachments(envelope, attachments, options); + if (options->run) { + sentry__cache_attachment_refs(envelope, attachments, options, + options->run->cache_path, options->run->run_path); } sentry_value_decref(all_attachments); return envelope; fail: + sentry_value_decref(all_attachments); SENTRY_WARN("dropping user feedback"); sentry_envelope_free(envelope); sentry_value_decref(event); @@ -1526,7 +1529,7 @@ sentry_transaction_finish_ts( // This takes ownership of the transaction, generates an event ID, merges // scope - return sentry__capture_event(tx, NULL); + return sentry__capture_event(tx, NULL, NULL); } void @@ -2025,8 +2028,10 @@ capture_minidump(sentry_path_t *dump_path) sentry_value_t event = sentry_value_new_event(); sentry_value_set_by_key( event, "level", sentry__value_new_level(SENTRY_LEVEL_FATAL)); - sentry_envelope_t *envelope - = sentry__prepare_event(options, event, &event_id, true, NULL); + sentry_hint_t hint = { sentry_value_new_null() }; + sentry_envelope_t *envelope = sentry__prepare_event( + options, event, &event_id, true, NULL, &hint); + sentry_value_decref(hint.attachments); if (!envelope || sentry_uuid_is_nil(&event_id)) { sentry_envelope_free(envelope); diff --git a/src/sentry_core.h b/src/sentry_core.h index 3b04c7ad4..cc06c2b49 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -63,13 +63,13 @@ bool sentry__event_is_transaction(sentry_value_t event); */ sentry_envelope_t *sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry_uuid_t *event_id, bool invoke_before_send, - sentry_scope_t *local_scope); + sentry_scope_t *local_scope, sentry_hint_t *hint); /** * Sends a sentry event, regardless of its type. */ sentry_uuid_t sentry__capture_event( - sentry_value_t event, sentry_scope_t *local_scope); + sentry_value_t event, sentry_scope_t *local_scope, sentry_hint_t *hint); /** * Convert the given transaction into an envelope. This assumes that the diff --git a/src/sentry_hint.c b/src/sentry_hint.c index b819be762..b097f6a98 100644 --- a/src/sentry_hint.c +++ b/src/sentry_hint.c @@ -105,3 +105,21 @@ sentry_hint_attach_bytesw_n(sentry_hint_t *hint, const char *buf, sentry_attachment_from_bytesw_n(buf, buf_len, filename, filename_len)); } #endif + +void +sentry_hint_remove_attachment(sentry_hint_t *hint, sentry_uuid_t attachment_id) +{ + if (hint) { + sentry_value_decref( + sentry__attachments_remove(hint->attachments, &attachment_id)); + } +} + +void +sentry_hint_clear_attachments(sentry_hint_t *hint) +{ + if (hint) { + sentry_value_decref(hint->attachments); + hint->attachments = sentry_value_new_null(); + } +} diff --git a/tests/unit/test_app_hang.c b/tests/unit/test_app_hang.c index 176e8eee5..273a4bd89 100644 --- a/tests/unit/test_app_hang.c +++ b/tests/unit/test_app_hang.c @@ -122,7 +122,7 @@ fake_stackwalk(uint64_t tid, void **ips, size_t max) } static sentry_value_t -capture_before_send(sentry_value_t event, void *hint, void *data) +capture_before_send(sentry_value_t event, sentry_hint_t *hint, void *data) { (void)hint; (void)data; @@ -245,7 +245,7 @@ static long g_real_frames; static volatile long g_keep_spinning; static sentry_value_t -real_before_send(sentry_value_t event, void *hint, void *data) +real_before_send(sentry_value_t event, sentry_hint_t *hint, void *data) { (void)hint; (void)data; diff --git a/tests/unit/test_attachments.c b/tests/unit/test_attachments.c index 3ca5f8b84..d740b2014 100644 --- a/tests/unit/test_attachments.c +++ b/tests/unit/test_attachments.c @@ -599,15 +599,15 @@ typedef struct { } sentry_before_send_attachment_testdata_t; static sentry_value_t -before_send_attach_bytes(sentry_value_t event, void *UNUSED(hint), void *_data) +before_send_attach_bytes(sentry_value_t event, sentry_hint_t *hint, void *_data) { sentry_before_send_attachment_testdata_t *data = _data; data->called += 1; if (data->called == 1) { - sentry_attach_bytes("global", 6, ".before-send.txt"); + sentry_hint_attach_bytes(hint, "global", 6, ".before-send.txt"); } else { - sentry_attach_bytes("first", 5, ".before-send.txt"); + sentry_hint_attach_bytes(hint, "first", 5, ".before-send.txt"); } return event; @@ -657,7 +657,8 @@ SENTRY_TEST(attachments_before_send) sentry_scope_t *scope = sentry_local_scope_new(); sentry_scope_attach_file(scope, SENTRY_TEST_PATH_PREFIX ".local.txt"); sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_INFO, "root", "first")); + sentry_value_new_message_event(SENTRY_LEVEL_INFO, "root", "first"), + NULL); serialized = sentry_stringbuilder_take_string(&testdata.serialized_envelope); diff --git a/tests/unit/test_basic.c b/tests/unit/test_basic.c index 676fd0e22..d2d2f5abc 100644 --- a/tests/unit/test_basic.c +++ b/tests/unit/test_basic.c @@ -2,6 +2,7 @@ #include "sentry_backend.h" #include "sentry_core.h" #include "sentry_database.h" +#include "sentry_hint.h" #include "sentry_options.h" #include "sentry_scope.h" #include "sentry_string.h" @@ -81,7 +82,7 @@ counting_transport_func(sentry_envelope_t *envelope, void *data) } static sentry_value_t -before_send(sentry_value_t event, void *UNUSED(hint), void *data) +before_send(sentry_value_t event, sentry_hint_t *UNUSED(hint), void *data) { uint64_t *called = data; *called += 1; @@ -121,7 +122,8 @@ SENTRY_TEST(sampling_before_send) } static sentry_value_t -discarding_before_send(sentry_value_t event, void *UNUSED(hint), void *data) +discarding_before_send( + sentry_value_t event, sentry_hint_t *UNUSED(hint), void *data) { uint64_t *called = data; *called += 1; @@ -546,3 +548,189 @@ SENTRY_TEST(clear_options) // sentry__options_getref() must no longer expose the options. TEST_CHECK(options_ref == NULL); } + +static void +capture_envelope(sentry_envelope_t *envelope, void *data) +{ + sentry_envelope_t **captured = data; + TEST_CHECK(*captured == NULL); + *captured = envelope; +} + +static sentry_value_t +attach_before_send(sentry_value_t event, sentry_hint_t *hint, void *data) +{ + TEST_CHECK(hint != NULL); + if (data) { + TEST_CHECK(hint == data); + } + sentry_hint_attach_bytes(hint, "callback", 8, "callback.txt"); + return event; +} + +static sentry_value_t +discard_before_send(sentry_value_t event, sentry_hint_t *hint, void *data) +{ + TEST_CHECK(hint == data); + sentry_value_decref(event); + return sentry_value_new_null(); +} + +SENTRY_TEST(capture_event_hints) +{ + for (int mode = 0; mode < 4; mode++) { + sentry_envelope_t *captured = NULL; + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_auto_session_tracking(options, false); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_transport_t *transport = sentry_transport_new(capture_envelope); + sentry_transport_set_state(transport, &captured); + sentry_options_set_transport(options, transport); + sentry_hint_t *hint = mode ? sentry_hint_new() : NULL; + if (hint) { + sentry_hint_attach_bytes(hint, "hint", 4, "hint.txt"); + } + sentry_options_set_before_send(options, + mode == 3 ? discard_before_send : attach_before_send, hint); + TEST_CHECK_INT_EQUAL(sentry_init(options), 0); + sentry_attach_bytes("global", 6, "global.txt"); + sentry_uuid_t id; + if (mode == 2) { + sentry_scope_t *scope = sentry_scope_new(); + sentry_scope_attach_bytes(scope, "local", 5, "local.txt"); + id = sentry_scope_capture_event( + scope, sentry_value_new_event(), hint); + sentry_scope_free(scope); + } else if (mode == 0) { + id = sentry_capture_event(sentry_value_new_event()); + } else { + id = sentry_scope_capture_event( + NULL, sentry_value_new_event(), hint); + } + sentry_close(); + TEST_CHECK(sentry_uuid_is_nil(&id) == (mode == 3)); + if (mode == 3) { + TEST_CHECK(captured == NULL); + continue; + } + TEST_ASSERT(captured != NULL); + size_t size; + char *serialized = sentry_envelope_serialize(captured, &size); + TEST_ASSERT(serialized != NULL); + TEST_CHECK(strstr(serialized, "global.txt") != NULL); + TEST_CHECK(strstr(serialized, "callback.txt") != NULL); + TEST_CHECK((strstr(serialized, "hint.txt") != NULL) == (mode != 0)); + TEST_CHECK((strstr(serialized, "local.txt") != NULL) == (mode == 2)); + sentry_free(serialized); + sentry_envelope_free(captured); + } +} + +SENTRY_TEST(capture_hint_cleanup) +{ + sentry_hint_t *hint = sentry_hint_new(); + sentry_hint_attach_bytes(hint, "hint", 4, "hint.txt"); + sentry_uuid_t id + = sentry_scope_capture_event(NULL, sentry_value_new_event(), hint); + TEST_CHECK(sentry_uuid_is_nil(&id)); + sentry_value_t event = sentry_value_new_event(); + sentry_value_set_by_key( + event, "type", sentry_value_new_string("transaction")); + sentry_scope_t *scope = sentry_local_scope_new(); + id = sentry_scope_capture_event(scope, event, sentry_hint_new()); + TEST_CHECK(sentry_uuid_is_nil(&id)); + sentry_value_decref(event); + sentry_scope_free(scope); +} + +typedef struct { + sentry_uuid_t global; + sentry_uuid_t local; + bool clear; + bool discard; + int calls; +} attachment_filter_t; + +static sentry_value_t +filter_attachments(sentry_value_t event, sentry_hint_t *hint, void *data) +{ + attachment_filter_t *filter = data; + filter->calls++; + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint->attachments), 3); + sentry_hint_remove_attachment(hint, filter->global); + sentry_hint_remove_attachment(hint, filter->local); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint->attachments), 1); + if (filter->clear) { + sentry_hint_clear_attachments(hint); + } + sentry_hint_attach_bytes(hint, "callback", 8, "callback.txt"); + if (filter->discard) { + sentry_value_decref(event); + return sentry_value_new_null(); + } + return event; +} + +SENTRY_TEST(capture_filter_attachments) +{ + for (int feedback = 0; feedback < 2; feedback++) { + for (int mode = 0; mode < 3; mode++) { + sentry_envelope_t *captured = NULL; + attachment_filter_t filter = { 0 }; + filter.clear = mode == 1; + filter.discard = mode == 2; + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_auto_session_tracking(options, false); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_transport_t *transport + = sentry_transport_new(capture_envelope); + sentry_transport_set_state(transport, &captured); + sentry_options_set_transport(options, transport); + if (feedback) { + sentry_options_set_before_send_feedback( + options, filter_attachments, &filter); + } else { + sentry_options_set_before_send( + options, filter_attachments, &filter); + } + TEST_CHECK_INT_EQUAL(sentry_init(options), 0); + filter.global = sentry_attach_bytes("global", 6, "global.txt"); + sentry_scope_t *scope = sentry_scope_new(); + filter.local + = sentry_scope_attach_bytes(scope, "local", 5, "local.txt"); + for (int capture = 0; capture < 2; capture++) { + sentry_hint_t *hint = sentry_hint_new(); + sentry_hint_attach_bytes(hint, "hint", 4, "hint.txt"); + sentry_uuid_t id; + if (feedback) { + id = sentry_scope_capture_feedback(scope, + sentry_value_new_feedback("message", NULL, NULL, NULL), + hint); + } else { + id = sentry_scope_capture_event( + scope, sentry_value_new_event(), hint); + } + TEST_CHECK(sentry_uuid_is_nil(&id) == filter.discard); + if (filter.discard) { + TEST_CHECK(captured == NULL); + continue; + } + TEST_ASSERT(captured != NULL); + size_t size; + char *serialized = sentry_envelope_serialize(captured, &size); + TEST_ASSERT(serialized != NULL); + TEST_CHECK(strstr(serialized, "global.txt") == NULL); + TEST_CHECK(strstr(serialized, "local.txt") == NULL); + TEST_CHECK( + (strstr(serialized, "hint.txt") == NULL) == filter.clear); + TEST_CHECK(strstr(serialized, "callback.txt") != NULL); + sentry_free(serialized); + sentry_envelope_free(captured); + captured = NULL; + } + TEST_CHECK_INT_EQUAL(filter.calls, 2); + sentry_scope_free(scope); + sentry_close(); + } + } +} diff --git a/tests/unit/test_scope.c b/tests/unit/test_scope.c index 118212bc6..32e8714af 100644 --- a/tests/unit/test_scope.c +++ b/tests/unit/test_scope.c @@ -537,7 +537,8 @@ SENTRY_TEST(scope_fingerprint_n) } static sentry_value_t -before_send_capture_fingerprint(sentry_value_t event, void *hint, void *data) +before_send_capture_fingerprint( + sentry_value_t event, sentry_hint_t *hint, void *data) { (void)hint; char **fingerprint_json = data; @@ -562,7 +563,7 @@ SENTRY_TEST(scope_remove_fingerprint_capture) sentry_scope_set_fingerprint(local_scope, "local1", NULL); sentry_scope_remove_fingerprint(local_scope); sentry_scope_capture_event(local_scope, - sentry_value_new_message_event(SENTRY_LEVEL_INFO, NULL, "test")); + sentry_value_new_message_event(SENTRY_LEVEL_INFO, NULL, "test"), NULL); TEST_ASSERT(!!fingerprint_json); TEST_CHECK_STRING_EQUAL(fingerprint_json, "[\"global1\",\"global2\"]"); @@ -1126,7 +1127,7 @@ SENTRY_TEST(before_breadcrumb_passthrough) static sentry_value_t before_send_modify_scope_values( - sentry_value_t event, void *UNUSED(hint), void *UNUSED(data)) + sentry_value_t event, sentry_hint_t *UNUSED(hint), void *UNUSED(data)) { sentry_value_t contexts = sentry_value_get_by_key(event, "contexts"); sentry_value_t gpu = sentry_value_get_by_key(contexts, "gpu"); @@ -3077,7 +3078,7 @@ SENTRY_TEST(scope_capture_unlocked) static sentry_value_t conditionally_discard_event( - sentry_value_t event, void *UNUSED(hint), void *data) + sentry_value_t event, sentry_hint_t *UNUSED(hint), void *data) { if (*(bool *)data) { sentry_value_decref(event); @@ -3117,7 +3118,8 @@ SENTRY_TEST(scope_last_event_id) TEST_CHECK(sentry_uuid_is_nil(&last_event_id)); sentry_uuid_t scoped_event_id = sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_ERROR, NULL, "scoped")); + sentry_value_new_message_event(SENTRY_LEVEL_ERROR, NULL, "scoped"), + NULL); TEST_CHECK(!sentry_uuid_is_nil(&scoped_event_id)); last_event_id = sentry_scope_get_last_event_id(scope); TEST_CHECK_UUID_EQUAL(last_event_id, scoped_event_id); @@ -3130,7 +3132,8 @@ SENTRY_TEST(scope_last_event_id) discard = true; sentry_uuid_t discarded_event_id = sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_ERROR, NULL, "discarded")); + sentry_value_new_message_event(SENTRY_LEVEL_ERROR, NULL, "discarded"), + NULL); TEST_CHECK(sentry_uuid_is_nil(&discarded_event_id)); last_event_id = sentry_scope_get_last_event_id(scope); TEST_CHECK_UUID_EQUAL(last_event_id, scoped_event_id); @@ -3195,7 +3198,8 @@ SENTRY_TEST(scope_capture_user_owned) sentry_scope_set_tag(scope, "run", "first"); sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_INFO, "logger", "one")); + sentry_value_new_message_event(SENTRY_LEVEL_INFO, "logger", "one"), + NULL); // The scope was applied but not freed, so reading and reusing it is safe // (a use-after-free here would trip the sanitizers). @@ -3206,7 +3210,8 @@ SENTRY_TEST(scope_capture_user_owned) sentry_scope_set_tag(scope, "run", "second"); sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_INFO, "logger", "two")); + sentry_value_new_message_event(SENTRY_LEVEL_INFO, "logger", "two"), + NULL); sentry_scope_free(scope); @@ -3217,7 +3222,8 @@ SENTRY_TEST(scope_capture_user_owned) // Keeps the trace context the scopes produced and drops the event. static sentry_value_t -keep_trace_context(sentry_value_t event, void *UNUSED(hint), void *data) +keep_trace_context( + sentry_value_t event, sentry_hint_t *UNUSED(hint), void *data) { sentry_value_t *trace = data; sentry_value_decref(*trace); @@ -3244,7 +3250,8 @@ SENTRY_TEST(scope_bind_transaction_object) sentry_scope_set_transaction_object(scope, tx); sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_ERROR, "logger", "boom")); + sentry_value_new_message_event(SENTRY_LEVEL_ERROR, "logger", "boom"), + NULL); TEST_ASSERT(!sentry_value_is_null(trace)); TEST_CHECK_STRING_EQUAL( @@ -3254,7 +3261,8 @@ SENTRY_TEST(scope_bind_transaction_object) sentry_scope_set_transaction_object(scope, NULL); sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_ERROR, "logger", "boom")); + sentry_value_new_message_event(SENTRY_LEVEL_ERROR, "logger", "boom"), + NULL); // After unbinding, event falls back to the propagation context. TEST_ASSERT(!sentry_value_is_null(trace)); @@ -3309,7 +3317,8 @@ SENTRY_TEST(scope_bind_span) } sentry_scope_capture_event(scope, - sentry_value_new_message_event(SENTRY_LEVEL_ERROR, "logger", "boom")); + sentry_value_new_message_event(SENTRY_LEVEL_ERROR, "logger", "boom"), + NULL); TEST_ASSERT(!sentry_value_is_null(trace)); TEST_CHECK_STRING_EQUAL( diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 7f27131b0..5a3a44970 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -348,7 +348,7 @@ SENTRY_TEST(transport_sampling_transactions) } static sentry_value_t -before_send(sentry_value_t event, void *UNUSED(hint), void *data) +before_send(sentry_value_t event, sentry_hint_t *UNUSED(hint), void *data) { uint64_t *called = data; *called += 1; diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 98d78647f..a3d7268eb 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -95,6 +95,9 @@ XX(cache_symlink_run) XX(cache_write_minidump) XX(cache_write_raw_with_minidump) XX(callback_envelope_is_not_resent_without_backend) +XX(capture_event_hints) +XX(capture_filter_attachments) +XX(capture_hint_cleanup) XX(capture_minidump_basic) XX(capture_minidump_discard) XX(capture_minidump_invalid_path) From ce10a77281bc16b9444b961283a1f4ca1a65789d Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 17:49:52 +0200 Subject: [PATCH 09/13] sentry__invoke_before_send --- src/backends/sentry_backend_crashpad.cpp | 5 ++--- src/backends/sentry_backend_native.c | 4 +--- src/sentry_core.c | 26 +++++++++++++++++------- src/sentry_core.h | 9 ++++++++ 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 33a3d8a29..57c351a8d 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -564,9 +564,8 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) crash_event = options->on_crash_func( &uctx, crash_event, options->on_crash_data); } else if (options->before_send_func) { - SENTRY_DEBUG("invoking `before_send` hook"); - crash_event = options->before_send_func( - crash_event, nullptr, options->before_send_data); + crash_event + = sentry__invoke_before_send(options, crash_event, nullptr); } sentry__transport_suspend(options->transport); diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index e70f5502d..faf4ad971 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -1368,9 +1368,7 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) if (should_handle) { // Apply before_send hook if on_crash wasn't set if (!options->on_crash_func && options->before_send_func) { - SENTRY_DEBUG("invoking `before_send` hook"); - event = options->before_send_func( - event, NULL, options->before_send_data); + event = sentry__invoke_before_send(options, event, NULL); should_handle = !sentry_value_is_null(event); } diff --git a/src/sentry_core.c b/src/sentry_core.c index b637fadc6..6c9ad2305 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -745,6 +745,23 @@ prepare_attachments(sentry_hint_t *hint, sentry_scope_t *local_scope) return attachments; } +sentry_value_t +sentry__invoke_before_send( + const sentry_options_t *options, sentry_value_t event, sentry_hint_t *hint) +{ + if (!options->before_send_func) { + return event; + } + SENTRY_DEBUG("invoking `before_send` hook"); + event = options->before_send_func(event, hint, options->before_send_data); + if (sentry_value_is_null(event)) { + SENTRY_DEBUG("event was discarded by the `before_send` hook"); + sentry__client_report_discard( + SENTRY_DISCARD_REASON_BEFORE_SEND, SENTRY_DATA_CATEGORY_ERROR, 1); + } + return event; +} + sentry_envelope_t * sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry_uuid_t *event_id, bool invoke_before_send, @@ -773,14 +790,9 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry_value_t all_attachments = prepare_attachments(hint, local_scope); - if (options->before_send_func && invoke_before_send) { - SENTRY_DEBUG("invoking `before_send` hook"); - event - = options->before_send_func(event, hint, options->before_send_data); + if (invoke_before_send) { + event = sentry__invoke_before_send(options, event, hint); if (sentry_value_is_null(event)) { - SENTRY_DEBUG("event was discarded by the `before_send` hook"); - sentry__client_report_discard(SENTRY_DISCARD_REASON_BEFORE_SEND, - SENTRY_DATA_CATEGORY_ERROR, 1); sentry_value_decref(all_attachments); return NULL; } diff --git a/src/sentry_core.h b/src/sentry_core.h index cc06c2b49..2043f5b9d 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -47,6 +47,15 @@ bool sentry__should_skip_upload(void); */ bool sentry__event_is_transaction(sentry_value_t event); +/** + * Invokes the configured `before_send` callback, if any. + * + * Returns the callback result, or `event` unchanged when no callback is + * configured. Records a client report when the callback discards the event. + */ +sentry_value_t sentry__invoke_before_send( + const sentry_options_t *options, sentry_value_t event, sentry_hint_t *hint); + /** * Convert the given event into an envelope. This assumes that the event * being passed in is not a transaction. From 030edffbb2193734e4de3febbb45ed1b952529fb Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 21:18:40 +0200 Subject: [PATCH 10/13] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39fc0f4b6..90051c98b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,11 +19,11 @@ - Add `sentry_add_attachment`, `sentry_scope_add_attachment`, and `sentry_hint_add_attachment` for adding configured attachments to the global scope, a specific scope, or a hint. These functions consume and freeze the attachment value. ([#2079](https://github.com/getsentry/sentry-native/pull/2079), [#1974](https://github.com/getsentry/sentry-native/pull/1974)) - Add `sentry_start_new_trace()` as a clearer name for starting a new trace. ([#2095](https://github.com/getsentry/sentry-native/pull/2095)) - Add hint support to `sentry_scope_capture_event` to pass event-specific attachments. The hint is also passed to `before_send`, which can modify attachments before the event is sent. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) +- Add `sentry_hint_remove_attachment` and `sentry_hint_clear_attachments`. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) **Fixes**: - Reduce lock contention for multi-threaded log and metric capture by allowing concurrent reads of scope data, and reusing a single options reference throughout each capture. ([#2091](https://github.com/getsentry/sentry-native/pull/2091), [#2094](https://github.com/getsentry/sentry-native/pull/2094)) -- Add `sentry_hint_remove_attachment` and `sentry_hint_clear_attachments` to filter attachments without modifying scopes. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) **Other changes**: From 064b1cbff8b662d6fe4692d49f1ca3b0f3476a36 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 17 Sep 2026 14:58:01 +0200 Subject: [PATCH 11/13] feat!: add support for crash-time attachments via hint --- CHANGELOG.md | 3 +- examples/example.c | 25 ++++-- include/sentry.h | 7 +- src/backends/sentry_backend_breakpad.cpp | 19 ++++- src/backends/sentry_backend_crashpad.cpp | 56 +++++++++++++- src/backends/sentry_backend_inproc.c | 23 +++++- src/backends/sentry_backend_native.c | 49 ++++++++---- src/sentry_core.c | 96 ++++++++++++++---------- src/sentry_core.h | 29 +++++-- src/sentry_hint.c | 52 ++++++++++--- src/sentry_hint.h | 11 +++ tests/assertions.py | 5 ++ tests/fixtures/inproc_stress/main.c | 10 ++- tests/fixtures/stack_usage/stack_usage.c | 4 +- tests/test_integration_crashpad.py | 12 ++- tests/test_integration_native.py | 27 +++++++ tests/test_integration_stdout.py | 21 +++++- tests/unit/test_attachments.c | 45 +++++++++++ tests/unit/tests.inc | 1 + 19 files changed, 394 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90051c98b..368b5bcb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Attachment APIs now use `sentry_value_t` and `sentry_uuid_t` instead of `sentry_attachment_t *` handles. Most attachment APIs, function names and arguments, are otherwise unchanged. ([#1974](https://github.com/getsentry/sentry-native/pull/1974)) - `sentry_init()` now consumes `/last_crash` after caching its value, aligning crashed-last-run behavior with other Sentry SDKs. ([#2023](https://github.com/getsentry/sentry-native/pull/2023)) - Change the `hint` parameter of `before_send` callbacks (`sentry_event_function_t`) from `void *` to `sentry_hint_t *`. Update callbacks registered with `sentry_options_set_before_send` to use the new parameter type. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) +- Add a `sentry_hint_t *hint` argument to `on_crash` callbacks. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) - Add a `sentry_hint_t *hint` argument to `sentry_scope_capture_event`. Pass `NULL` if no hint is needed. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) - Remove `sentry_options_get/set_enable_logs` and `sentry_options_get/set_enable_metrics`. ([#1980](https://github.com/getsentry/sentry-native/pull/1980)) > Structured logs and metrics have been enabled by default since `0.13`, and the options were deprecated and made no-ops in `0.16`. @@ -18,7 +19,7 @@ - Add `sentry_attachment_from_file/bytes` (and their wide-string variants) for creating attachment values that can be fully configured before they are added. ([#2079](https://github.com/getsentry/sentry-native/pull/2079)) - Add `sentry_add_attachment`, `sentry_scope_add_attachment`, and `sentry_hint_add_attachment` for adding configured attachments to the global scope, a specific scope, or a hint. These functions consume and freeze the attachment value. ([#2079](https://github.com/getsentry/sentry-native/pull/2079), [#1974](https://github.com/getsentry/sentry-native/pull/1974)) - Add `sentry_start_new_trace()` as a clearer name for starting a new trace. ([#2095](https://github.com/getsentry/sentry-native/pull/2095)) -- Add hint support to `sentry_scope_capture_event` to pass event-specific attachments. The hint is also passed to `before_send`, which can modify attachments before the event is sent. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) +- Add hint support to `sentry_scope_capture_event` to pass event-specific attachments. The hint is also passed to `before_send` and `on_crash`, which can modify attachments before the event is sent. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) - Add `sentry_hint_remove_attachment` and `sentry_hint_clear_attachments`. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) **Fixes**: diff --git a/examples/example.c b/examples/example.c index 420d82786..b70f2e9a0 100644 --- a/examples/example.c +++ b/examples/example.c @@ -129,9 +129,13 @@ traces_sampler_callback(const sentry_transaction_context_t *transaction_ctx, static sentry_value_t before_send_callback(sentry_value_t event, sentry_hint_t *hint, void *user_data) { - (void)hint; (void)user_data; + sentry_hint_clear_attachments(hint); + sentry_hint_add_attachment(hint, + sentry_attachment_from_bytes( + "before_send", strlen("before_send"), "callback.txt")); + // make our mark on the event sentry_value_set_by_key( event, "adapted_by", sentry_value_new_string("before_send")); @@ -153,10 +157,11 @@ discarding_before_send_callback( } static sentry_value_t -discarding_on_crash_callback( - const sentry_ucontext_t *uctx, sentry_value_t event, void *user_data) +discarding_on_crash_callback(const sentry_ucontext_t *uctx, + sentry_value_t event, sentry_hint_t *hint, void *user_data) { (void)uctx; + (void)hint; (void)user_data; // discard event and signal backend to stop further processing @@ -165,12 +170,17 @@ discarding_on_crash_callback( } static sentry_value_t -on_crash_callback( - const sentry_ucontext_t *uctx, sentry_value_t event, void *user_data) +on_crash_callback(const sentry_ucontext_t *uctx, sentry_value_t event, + sentry_hint_t *hint, void *user_data) { (void)uctx; (void)user_data; + sentry_hint_clear_attachments(hint); + sentry_hint_add_attachment(hint, + sentry_attachment_from_bytes( + "on_crash", strlen("on_crash"), "callback.txt")); + // tell the backend to retain the event return event; } @@ -230,10 +240,11 @@ on_crashed_last_run_callback(const sentry_envelope_t *envelope, void *user_data) } static sentry_value_t -restart_on_crash( - const sentry_ucontext_t *uctx, sentry_value_t event, void *user_data) +restart_on_crash(const sentry_ucontext_t *uctx, sentry_value_t event, + sentry_hint_t *hint, void *user_data) { (void)uctx; + (void)hint; #ifdef SENTRY_PLATFORM_WINDOWS wchar_t **argv = user_data; diff --git a/include/sentry.h b/include/sentry.h index 8725f4e85..f2710c9da 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1540,6 +1540,9 @@ SENTRY_API void sentry_options_set_before_send( * `sentry_value_decref` on the provided event and return a * `sentry_value_new_null()` instead. * + * The hint is always provided and can be used to modify attachments on the + * event. + * * Only the `inproc` backend currently fills the passed-in event with crash * meta-data. Since both `breakpad` and `crashpad` use minidumps to capture the * crash state, the passed-in event is empty when using these backends. Changes @@ -1580,8 +1583,8 @@ SENTRY_API void sentry_options_set_before_send( * exception-handler, it will not be invoked when such a crash happened, even * though a crash report will be sent. */ -typedef sentry_value_t (*sentry_crash_function_t)( - const sentry_ucontext_t *uctx, sentry_value_t event, void *user_data); +typedef sentry_value_t (*sentry_crash_function_t)(const sentry_ucontext_t *uctx, + sentry_value_t event, sentry_hint_t *hint, void *user_data); /** * Sets the `on_crash` callback. diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 58b68b54b..acaaecfd6 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -8,6 +8,7 @@ extern "C" { #include "sentry_core.h" #include "sentry_database.h" #include "sentry_envelope.h" +#include "sentry_hint.h" #include "sentry_logger.h" #include "sentry_options.h" #ifdef SENTRY_PLATFORM_WINDOWS @@ -142,10 +143,14 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, sentry_value_t transaction = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); sentry_uuid_t event_id = sentry_uuid_nil(); + sentry_hint_t hint; + SENTRY__HINT_INIT(hint); bool should_handle = true; if (options->on_crash_func) { + sentry__hint_set_attachments( + &hint, sentry__merge_attachments(hint.attachments, nullptr)); sentry_ucontext_t *uctx = nullptr; #if defined(SENTRY_PLATFORM_DARWIN) \ @@ -162,7 +167,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, #endif SENTRY_SIGNAL_SAFE_LOG("DEBUG invoking `on_crash` hook"); - event = options->on_crash_func(uctx, event, options->on_crash_data); + event = options->on_crash_func( + uctx, event, &hint, options->on_crash_data); should_handle = !sentry_value_is_null(event); } @@ -183,8 +189,14 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, } #endif - sentry_envelope_t *envelope = sentry__prepare_event(options, event, - nullptr, !options->on_crash_func, nullptr, nullptr); + event = sentry__prepare_event(options, event, nullptr); + if (!options->on_crash_func) { + sentry__hint_set_attachments(&hint, + sentry__merge_attachments(hint.attachments, nullptr)); + event = sentry__invoke_before_send(options, event, &hint); + } + sentry_envelope_t *envelope = sentry__enclose_event( + options, event, nullptr, hint.attachments); if (envelope) { event_id = sentry__envelope_get_event_id(envelope); } @@ -280,6 +292,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, sentry_value_decref(event); sentry_value_decref(transaction); } + SENTRY__HINT_DEINIT(hint); // after capturing the crash event, try to dump all the in-flight // data of the previous transports diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 57c351a8d..a7699eff1 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -8,6 +8,7 @@ extern "C" { #include "sentry_cpu_relax.h" #include "sentry_database.h" #include "sentry_envelope.h" +#include "sentry_hint.h" #include "sentry_logger.h" #include "sentry_options.h" #ifdef SENTRY_PLATFORM_WINDOWS @@ -286,6 +287,44 @@ append_attachment(crashpad_state_t *state, const base::FilePath &path, : 1; } +#if defined(SENTRY_PLATFORM_LINUX) || defined(SENTRY_PLATFORM_WINDOWS) +static sentry_path_t *prepare_initial_attachment( + sentry_value_t attachment, const sentry_path_t *run_path); + +static void +write_attachment_manifest(crashpad_state_t *state, sentry_value_t attachments) +{ + sentry_path_t *manifest_path + = sentry__path_join_str(state->run_path, "__sentry-attachments"); + if (!manifest_path) { + return; + } + + sentry_value_t manifest = sentry_value_new_list(); + size_t len = sentry_value_get_length(attachments); + for (size_t i = 0; i < len; i++) { + sentry_value_t attachment = sentry_value_get_by_index(attachments, i); + sentry_path_t *path + = prepare_initial_attachment(attachment, state->run_path); + if (!path) { + continue; + } + sentry_value_t info = sentry__attachment_from_file(path->path); + sentry_attachment_set_filename( + info, sentry__attachment_get_filename(attachment)); + sentry_attachment_set_type( + info, sentry__attachment_get_type(attachment)); + sentry_attachment_set_content_type( + info, sentry__attachment_get_content_type(attachment)); + sentry_value_append(manifest, info); + sentry__path_free(path); + } + sentry__write_attachment_manifest(manifest_path, manifest); + sentry_value_decref(manifest); + sentry__path_free(manifest_path); +} +#endif + static void flush_scope_to_event(crashpad_state_t *state, const base::FilePath &event_path, const sentry_options_t *options, sentry_value_t crash_event) @@ -549,6 +588,14 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) = sentry__value_new_event_with_id(&state->crash_event_id); sentry_value_set_by_key( crash_event, "level", sentry__value_new_level(SENTRY_LEVEL_FATAL)); + sentry_hint_t hint; + SENTRY__HINT_INIT(hint); + + if (options->on_crash_func || options->before_send_func) { + sentry__hint_set_attachments( + &hint, sentry__merge_attachments(hint.attachments, nullptr)); + } + sentry_value_freeze(hint.attachments); if (options->on_crash_func) { sentry_ucontext_t uctx; @@ -562,10 +609,10 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) SENTRY_DEBUG("invoking `on_crash` hook"); crash_event = options->on_crash_func( - &uctx, crash_event, options->on_crash_data); + &uctx, crash_event, &hint, options->on_crash_data); } else if (options->before_send_func) { crash_event - = sentry__invoke_before_send(options, crash_event, nullptr); + = sentry__invoke_before_send(options, crash_event, &hint); } sentry__transport_suspend(options->transport); @@ -576,6 +623,9 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) should_dump = !sentry_value_is_null(crash_event); if (should_dump) { + if (!sentry_value_is_frozen(hint.attachments)) { + write_attachment_manifest(state, hint.attachments); + } sentry_value_incref(crash_event); flush_scope_from_handler(options, crash_event); sentry__write_crash_marker(options); @@ -606,6 +656,7 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) } else { SENTRY_DEBUG("event was discarded"); } + SENTRY__HINT_DEINIT(hint); sentry__transport_dump_queue(options->transport, options->run); } @@ -943,6 +994,7 @@ remove_attachment(void *state, sentry_value_t attachment) } sentry__path_free(path); } + #endif static int diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index a6e274760..963f4229b 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -7,6 +7,7 @@ #include "sentry_cpu_relax.h" #include "sentry_database.h" #include "sentry_envelope.h" +#include "sentry_hint.h" #include "sentry_logger.h" #include "sentry_options.h" #include "sentry_os.h" @@ -1076,10 +1077,17 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, sentry_value_t transaction = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); sentry_uuid_t event_id = sentry_uuid_nil(); + sentry_hint_t hint; + SENTRY__HINT_INIT(hint); + if (options->on_crash_func) { + sentry__hint_set_attachments( + &hint, sentry__merge_attachments(hint.attachments, NULL)); + } if (options->on_crash_func && !skip_hooks) { SENTRY_DEBUG("invoking `on_crash` hook"); - event = options->on_crash_func(uctx, event, options->on_crash_data); + event = options->on_crash_func( + uctx, event, &hint, options->on_crash_data); should_handle = !sentry_value_is_null(event); } else if (skip_hooks && options->on_crash_func) { SENTRY_DEBUG("skipping `on_crash` hook due to recursive crash"); @@ -1101,8 +1109,16 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, } #endif - sentry_envelope_t *envelope = sentry__prepare_event(options, event, - NULL, !options->on_crash_func && !skip_hooks, NULL, NULL); + event = sentry__prepare_event(options, event, NULL); + if (!options->on_crash_func) { + sentry__hint_set_attachments( + &hint, sentry__merge_attachments(hint.attachments, NULL)); + if (!skip_hooks) { + event = sentry__invoke_before_send(options, event, &hint); + } + } + sentry_envelope_t *envelope + = sentry__enclose_event(options, event, NULL, hint.attachments); if (envelope) { event_id = sentry__envelope_get_event_id(envelope); } @@ -1161,6 +1177,7 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, sentry_value_decref(event); sentry_value_decref(transaction); } + SENTRY__HINT_DEINIT(hint); // after capturing the crash event, dump all the envelopes to disk sentry__transport_dump_queue(options->transport, options->run); diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index faf4ad971..a913ad83c 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -31,6 +31,7 @@ #include "sentry_crash_ipc.h" #include "sentry_database.h" #include "sentry_envelope.h" +#include "sentry_hint.h" #include "sentry_json.h" #include "sentry_logger.h" #include "sentry_options.h" @@ -1168,17 +1169,20 @@ native_backend_free(sentry_backend_t *backend) sentry_free(state); } -// Writes the scope's attachment list to /__sentry-attachments so the -// crash daemon can locate and append them to the crash envelope. +// Writes attachments to /__sentry-attachments so the crash daemon can +// locate and append them to the crash envelope. static void -native_backend_write_attachments(const sentry_path_t *event_path) +native_backend_write_attachments( + const sentry_path_t *event_path, const sentry_hint_t *hint) { if (!event_path) { return; } SENTRY_WITH_SCOPE (scope) { - sentry_value_t attachments = sentry__scope_load_attachments(scope); - if (sentry_value_get_length(attachments) == 0) { + sentry_value_t attachments = hint + ? sentry_value_incref(hint->attachments) + : sentry__scope_load_attachments(scope); + if (sentry_value_get_length(attachments) == 0 && !hint) { sentry_value_decref(attachments); continue; } @@ -1236,17 +1240,11 @@ native_backend_flush_scope( sentry_backend_t *backend, const sentry_options_t *options) { native_backend_state_t *state = (native_backend_state_t *)backend->data; - if (!state || !state->event_path) { + if (!state || !state->event_path || sentry__atomic_fetch(&state->crashed)) { return; } - // Manifest writes must continue post-crash so attachments registered - // from on_crash/before_send reach the daemon - native_backend_write_attachments(state->event_path); - - if (sentry__atomic_fetch(&state->crashed)) { - return; - } + native_backend_write_attachments(state->event_path, NULL); // Create event with current scope sentry_value_t event = sentry_value_new_object(); @@ -1353,14 +1351,21 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) sentry_value_t event = sentry_value_new_event(); sentry_value_set_by_key( event, "level", sentry__value_new_level(SENTRY_LEVEL_FATAL)); + sentry_hint_t hint; + SENTRY__HINT_INIT(hint); bool should_handle = true; + if (options->on_crash_func || options->before_send_func) { + sentry__hint_set_attachments( + &hint, sentry__merge_attachments(hint.attachments, NULL)); + } + // Call on_crash hook if configured if (options->on_crash_func) { SENTRY_DEBUG("invoking `on_crash` hook"); - sentry_value_t result - = options->on_crash_func(uctx, event, options->on_crash_data); + sentry_value_t result = options->on_crash_func( + uctx, event, &hint, options->on_crash_data); should_handle = !sentry_value_is_null(result); event = result; } @@ -1368,11 +1373,22 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) if (should_handle) { // Apply before_send hook if on_crash wasn't set if (!options->on_crash_func && options->before_send_func) { - event = sentry__invoke_before_send(options, event, NULL); + event = sentry__invoke_before_send(options, event, &hint); should_handle = !sentry_value_is_null(event); } if (should_handle) { + if (options->on_crash_func || options->before_send_func) { + size_t len = sentry_value_get_length(hint.attachments); + for (size_t i = 0; i < len; i++) { + add_attachment(state, + sentry_value_get_by_index(hint.attachments, i)); + } + // Manifest writes must continue post-crash so attachments + // registered from on_crash/before_send reach the daemon + native_backend_write_attachments( + state ? state->event_path : NULL, &hint); + } // Apply scope to the event. The daemon assembles breadcrumbs // from the ring files SENTRY_WITH_SCOPE (scope) { @@ -1463,6 +1479,7 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) sentry_value_decref(event); sentry_value_decref(transaction); } + SENTRY__HINT_DEINIT(hint); } } diff --git a/src/sentry_core.c b/src/sentry_core.c index 6c9ad2305..22e66baa7 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -622,8 +622,21 @@ sentry__capture_event( // no-ops. hint = sentry_hint_new(); } - envelope = sentry__prepare_event( - options, event, &event_id, true, local_scope, hint); + event = sentry__prepare_event(options, event, local_scope); + sentry_value_t all_attachments = sentry__merge_attachments( + hint ? hint->attachments : sentry_value_new_null(), + local_scope); + if (hint) { + sentry__hint_set_attachments(hint, all_attachments); + } + event = sentry__invoke_before_send(options, event, hint); + sentry_value_t attachments + = hint ? hint->attachments : all_attachments; + envelope + = sentry__enclose_event(options, event, &event_id, attachments); + if (!hint) { + sentry_value_decref(all_attachments); + } } if (envelope) { // Accept a racy read here, since SENTRY_WITH_OPTIONS only prevents @@ -719,30 +732,25 @@ static return send; } -static sentry_value_t -prepare_attachments(sentry_hint_t *hint, sentry_scope_t *local_scope) +sentry_value_t +sentry__merge_attachments( + sentry_value_t attachments, sentry_scope_t *local_scope) { - sentry_value_t attachments = sentry_value_new_null(); - if (hint) { - sentry__attachments_extend(&attachments, hint->attachments); - } + sentry_value_t merged = sentry_value_new_null(); + sentry__attachments_extend(&merged, attachments); if (local_scope) { sentry_value_t local_attachments = sentry__scope_load_attachments(local_scope); - sentry__attachments_extend(&attachments, local_attachments); + sentry__attachments_extend(&merged, local_attachments); sentry_value_decref(local_attachments); } SENTRY_WITH_SCOPE (scope) { sentry_value_t global_attachments = sentry__scope_load_attachments(scope); - sentry__attachments_extend(&attachments, global_attachments); + sentry__attachments_extend(&merged, global_attachments); sentry_value_decref(global_attachments); } - if (hint) { - sentry_value_decref(hint->attachments); - hint->attachments = sentry_value_incref(attachments); - } - return attachments; + return merged; } sentry_value_t @@ -762,13 +770,10 @@ sentry__invoke_before_send( return event; } -sentry_envelope_t * +sentry_value_t sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, - sentry_uuid_t *event_id, bool invoke_before_send, - sentry_scope_t *local_scope, sentry_hint_t *hint) + sentry_scope_t *local_scope) { - sentry_envelope_t *envelope = NULL; - if (event_is_considered_error(event)) { sentry__record_errors_on_current_session(1); } @@ -787,16 +792,17 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, } sentry__scope_apply_to_event(scope, options, event, mode); } + return event; +} - sentry_value_t all_attachments = prepare_attachments(hint, local_scope); - - if (invoke_before_send) { - event = sentry__invoke_before_send(options, event, hint); - if (sentry_value_is_null(event)) { - sentry_value_decref(all_attachments); - return NULL; - } +sentry_envelope_t * +sentry__enclose_event(const sentry_options_t *options, sentry_value_t event, + sentry_uuid_t *event_id, sentry_value_t attachments) +{ + if (sentry_value_is_null(event)) { + return NULL; } + sentry_envelope_t *envelope = NULL; sentry__ensure_event_id(event, event_id); envelope = sentry__envelope_new(); @@ -804,18 +810,15 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, goto fail; } - sentry_value_t attachments = hint ? hint->attachments : all_attachments; sentry__envelope_add_attachments(envelope, attachments, options); if (options->run) { sentry__cache_attachment_refs(envelope, attachments, options, options->run->cache_path, options->run->run_path); } - sentry_value_decref(all_attachments); return envelope; fail: - sentry_value_decref(all_attachments); sentry_envelope_free(envelope); sentry_value_decref(event); return NULL; @@ -906,7 +909,11 @@ prepare_user_feedback(const sentry_options_t *options, sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE); } - sentry_value_t all_attachments = prepare_attachments(hint, local_scope); + sentry_value_t all_attachments = sentry__merge_attachments( + hint ? hint->attachments : sentry_value_new_null(), local_scope); + if (hint) { + sentry__hint_set_attachments(hint, all_attachments); + } if (options->before_send_feedback_func) { SENTRY_DEBUG("invoking `before_send_feedback` hook"); @@ -917,7 +924,9 @@ prepare_user_feedback(const sentry_options_t *options, "feedback was discarded by the `before_send_feedback` hook"); sentry__client_report_discard(SENTRY_DISCARD_REASON_BEFORE_SEND, SENTRY_DATA_CATEGORY_FEEDBACK, 1); - sentry_value_decref(all_attachments); + if (!hint) { + sentry_value_decref(all_attachments); + } return NULL; } } @@ -936,11 +945,15 @@ prepare_user_feedback(const sentry_options_t *options, options->run->cache_path, options->run->run_path); } - sentry_value_decref(all_attachments); + if (!hint) { + sentry_value_decref(all_attachments); + } return envelope; fail: - sentry_value_decref(all_attachments); + if (!hint) { + sentry_value_decref(all_attachments); + } SENTRY_WARN("dropping user feedback"); sentry_envelope_free(envelope); sentry_value_decref(event); @@ -2040,10 +2053,15 @@ capture_minidump(sentry_path_t *dump_path) sentry_value_t event = sentry_value_new_event(); sentry_value_set_by_key( event, "level", sentry__value_new_level(SENTRY_LEVEL_FATAL)); - sentry_hint_t hint = { sentry_value_new_null() }; - sentry_envelope_t *envelope = sentry__prepare_event( - options, event, &event_id, true, NULL, &hint); - sentry_value_decref(hint.attachments); + sentry_hint_t hint; + SENTRY__HINT_INIT(hint); + event = sentry__prepare_event(options, event, NULL); + sentry__hint_set_attachments( + &hint, sentry__merge_attachments(hint.attachments, NULL)); + event = sentry__invoke_before_send(options, event, &hint); + sentry_envelope_t *envelope = sentry__enclose_event( + options, event, &event_id, hint.attachments); + SENTRY__HINT_DEINIT(hint); if (!envelope || sentry_uuid_is_nil(&event_id)) { sentry_envelope_free(envelope); diff --git a/src/sentry_core.h b/src/sentry_core.h index 2043f5b9d..deab5cbbe 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -57,22 +57,35 @@ sentry_value_t sentry__invoke_before_send( const sentry_options_t *options, sentry_value_t event, sentry_hint_t *hint); /** - * Convert the given event into an envelope. This assumes that the event - * being passed in is not a transaction. + * Encloses the given event in an envelope. This assumes that the event being + * passed in is not a transaction. * * More specifically, it will do the following things: - * - apply the scope to it, - * - call the before_send hook on it (if invoke_before_send == true), * - add the event to a new envelope, - * - record errors on the current session, * - add any attachments to the envelope as well * * The function will ensure the event has a UUID and write it into the * `event_id` out-parameter. */ -sentry_envelope_t *sentry__prepare_event(const sentry_options_t *options, - sentry_value_t event, sentry_uuid_t *event_id, bool invoke_before_send, - sentry_scope_t *local_scope, sentry_hint_t *hint); +sentry_envelope_t *sentry__enclose_event(const sentry_options_t *options, + sentry_value_t event, sentry_uuid_t *event_id, sentry_value_t attachments); + +/** + * Prepares an event by recording errors on the current session and applying + * the local and global scopes. + * + * Returns `event` without transferring ownership. + */ +sentry_value_t sentry__prepare_event(const sentry_options_t *options, + sentry_value_t event, sentry_scope_t *local_scope); + +/** + * Merges attachments with local-scope and global-scope attachments. + * + * Returns an owned value that the caller must release or transfer. + */ +sentry_value_t sentry__merge_attachments( + sentry_value_t attachments, sentry_scope_t *local_scope); /** * Sends a sentry event, regardless of its type. diff --git a/src/sentry_hint.c b/src/sentry_hint.c index b097f6a98..427b804a2 100644 --- a/src/sentry_hint.c +++ b/src/sentry_hint.c @@ -13,17 +13,28 @@ sentry_hint_new(void) if (!hint) { return NULL; } - hint->attachments = sentry_value_new_null(); + SENTRY__HINT_INIT(*hint); return hint; } +void +sentry__hint_set_attachments(sentry_hint_t *hint, sentry_value_t attachments) +{ + if (hint) { + sentry_value_decref(hint->attachments); + hint->attachments = attachments; + } else { + sentry_value_decref(attachments); + } +} + void sentry__hint_free(sentry_hint_t *hint) { if (!hint) { return; } - sentry_value_decref(hint->attachments); + SENTRY__HINT_DEINIT(*hint); sentry_free(hint); } @@ -35,8 +46,20 @@ sentry_hint_add_attachment(sentry_hint_t *hint, sentry_value_t attachment) return sentry_uuid_nil(); } - sentry_value_t added - = sentry__attachments_add(&hint->attachments, attachment); + size_t len = sentry_value_get_length(hint->attachments); + sentry_value_t attachments = sentry_value_is_frozen(hint->attachments) + ? sentry__attachments_clone(hint->attachments) + : sentry_value_incref(hint->attachments); + if (len && sentry_value_is_null(attachments)) { + sentry_value_decref(attachment); + return sentry_uuid_nil(); + } + sentry_value_t added = sentry__attachments_add(&attachments, attachment); + if (sentry_value_get_length(attachments) != len) { + sentry__hint_set_attachments(hint, attachments); + } else { + sentry_value_decref(attachments); + } sentry_uuid_t attachment_id = sentry__attachment_get_id(added); sentry_value_decref(added); return attachment_id; @@ -110,16 +133,27 @@ void sentry_hint_remove_attachment(sentry_hint_t *hint, sentry_uuid_t attachment_id) { if (hint) { - sentry_value_decref( - sentry__attachments_remove(hint->attachments, &attachment_id)); + sentry_value_t attachments = sentry_value_is_frozen(hint->attachments) + ? sentry__attachments_clone(hint->attachments) + : sentry_value_incref(hint->attachments); + sentry_value_t removed + = sentry__attachments_remove(attachments, &attachment_id); + if (!sentry_value_is_null(removed)) { + sentry__hint_set_attachments(hint, attachments); + } else { + sentry_value_decref(attachments); + } + sentry_value_decref(removed); } } void sentry_hint_clear_attachments(sentry_hint_t *hint) { - if (hint) { - sentry_value_decref(hint->attachments); - hint->attachments = sentry_value_new_null(); + if (hint && sentry_value_get_length(hint->attachments)) { + sentry_value_t attachments = sentry_value_new_list(); + if (!sentry_value_is_null(attachments)) { + sentry__hint_set_attachments(hint, attachments); + } } } diff --git a/src/sentry_hint.h b/src/sentry_hint.h index 1b379d2ed..fbc1333bc 100644 --- a/src/sentry_hint.h +++ b/src/sentry_hint.h @@ -11,6 +11,17 @@ struct sentry_hint_s { sentry_value_t attachments; }; +#define SENTRY__HINT_INIT(Hint) \ + ((void)((Hint).attachments = sentry_value_new_null())) +#define SENTRY__HINT_DEINIT(Hint) \ + ((void)sentry_value_decref((Hint).attachments)) + +/** + * Replaces a hint's attachments, taking ownership of `attachments`. + */ +void sentry__hint_set_attachments( + sentry_hint_t *hint, sentry_value_t attachments); + /** * Frees a hint (internal use only). */ diff --git a/tests/assertions.py b/tests/assertions.py index 904429d99..b792e74d1 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -520,6 +520,7 @@ class CrashpadAttachments: cmake_cache: int bytes_bin: bytes = None minidump: bytes = None + callback: bytes = None def _unpack_breadcrumbs(payload): @@ -536,6 +537,7 @@ def _load_crashpad_attachments(msg): cmake_cache = -1 bytes_bin = None minidump = None + callback = None for part in msg.walk(): assert part.get_filename() != "__sentry-attachments" if part.get_filename() is not None: @@ -554,6 +556,8 @@ def _load_crashpad_attachments(msg): cmake_cache = len(part.get_payload(decode=True)) case "bytes.bin": bytes_bin = part.get_payload(decode=True) + case "callback.txt": + callback = part.get_payload(decode=True) if ( part.get_param("name", header="content-disposition") @@ -569,6 +573,7 @@ def _load_crashpad_attachments(msg): cmake_cache, bytes_bin, minidump, + callback, ) diff --git a/tests/fixtures/inproc_stress/main.c b/tests/fixtures/inproc_stress/main.c index 00e596cda..be60ffe44 100644 --- a/tests/fixtures/inproc_stress/main.c +++ b/tests/fixtures/inproc_stress/main.c @@ -173,11 +173,12 @@ stacktest_A_calls_B_no_frame_record(void) // on_crash callback that crashes via SIGSEGV: simulates buggy user code static sentry_value_t -crashing_on_crash_callback( - const sentry_ucontext_t *uctx, sentry_value_t event, void *closure) +crashing_on_crash_callback(const sentry_ucontext_t *uctx, sentry_value_t event, + sentry_hint_t *hint, void *closure) { (void)uctx; (void)event; + (void)hint; (void)closure; fprintf(stderr, "on_crash callback about to crash\n"); @@ -190,11 +191,12 @@ crashing_on_crash_callback( // on_crash callback that crashes via abort(): tests signal mask reset behavior static sentry_value_t -aborting_on_crash_callback( - const sentry_ucontext_t *uctx, sentry_value_t event, void *closure) +aborting_on_crash_callback(const sentry_ucontext_t *uctx, sentry_value_t event, + sentry_hint_t *hint, void *closure) { (void)uctx; (void)event; + (void)hint; (void)closure; fprintf(stderr, "on_crash callback about to abort\n"); diff --git a/tests/fixtures/stack_usage/stack_usage.c b/tests/fixtures/stack_usage/stack_usage.c index c4f313cd3..7a28c589d 100644 --- a/tests/fixtures/stack_usage/stack_usage.c +++ b/tests/fixtures/stack_usage/stack_usage.c @@ -135,9 +135,11 @@ stack_usage_integration_new(void) static void *invalid_mem = (void *)1; static sentry_value_t -on_crash(const sentry_ucontext_t *uctx, sentry_value_t event, void *data) +on_crash(const sentry_ucontext_t *uctx, sentry_value_t event, + sentry_hint_t *hint, void *data) { (void)uctx; + (void)hint; (void)data; return event; } diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index a356bc377..7e8702f81 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -601,11 +601,19 @@ def test_crashpad_dumping_crash(cmake, httpserver, run_args, build_args): envelope = Envelope.deserialize(session) assert_session(envelope, {"status": "crashed", "errors": 1}) + expect_attachments = not any( + arg in run_args for arg in ("before-send", "on-crash", "clear-attachments") + ) attachments = assert_crashpad_upload( multipart, - expect_attachment="clear-attachments" not in run_args, - expect_view_hierarchy="clear-attachments" not in run_args, + expect_attachment=expect_attachments, + expect_view_hierarchy=expect_attachments, + ) + callback = next( + (arg for arg in run_args if arg in ("before-send", "on-crash")), None ) + expected_callback = callback.replace("-", "_").encode() if callback else None + assert attachments.callback == expected_callback event_id = attachments.event["event_id"] if sys.platform == "win32": minidump = tmp_path / ".sentry-native" / "reports" / f"{event_id}.dmp" diff --git a/tests/test_integration_native.py b/tests/test_integration_native.py index 30940ea6e..423a6816e 100644 --- a/tests/test_integration_native.py +++ b/tests/test_integration_native.py @@ -80,6 +80,33 @@ def test_native_capture_crash(cmake, httpserver): assert_native_crash(envelope) +@pytest.mark.parametrize("callback", ["before-send", "on-crash"]) +def test_native_crash_hint_attachments(cmake, httpserver, callback): + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"}) + + httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data("OK") + + with httpserver.wait(timeout=10) as waiting: + run_crash( + tmp_path, + "sentry_example", + ["log", "attachment", callback, "crash"], + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + ) + assert waiting.result + + assert len(httpserver.log) >= 1 + envelope = Envelope.deserialize(httpserver.log[0][0].get_data()) + assert not any( + item.headers.get("filename") == "CMakeCache.txt" for item in envelope + ) + assert any( + item.headers.get("filename") == "callback.txt" + and item.payload.bytes == callback.replace("-", "_").encode() + for item in envelope + ) + + def test_native_on_crashed_last_run(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"}) httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data("OK") diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py index 45539a66c..bd221243e 100644 --- a/tests/test_integration_stdout.py +++ b/tests/test_integration_stdout.py @@ -175,6 +175,18 @@ def run_crash_stdout_for(backend, cmake, example_args): return run_stdout_for(backend, cmake, ["attachment", "crash"] + example_args) +def assert_crash_hint_attachments(envelope, callback): + assert not any( + item.headers.get("filename") in ("CMakeCache.txt", "bytes.bin") + for item in envelope + ) + assert any( + item.headers.get("filename") == "callback.txt" + and item.payload.bytes == callback.replace("-", "_").encode() + for item in envelope + ) + + def test_inproc_crash_stdout(cmake): tmp_path, output = run_crash_stdout_for("inproc", cmake, []) @@ -233,7 +245,7 @@ def test_inproc_crash_stdout_before_send(cmake): assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) - assert_attachment(envelope) + assert_crash_hint_attachments(envelope, "before-send") assert_inproc_crash(envelope) assert_before_send(envelope) @@ -248,6 +260,7 @@ def test_inproc_crash_stdout_discarding_on_crash(cmake): assert_no_crash_timestamp(has_files, tmp_path) +@pytest.mark.skipif(is_qemu, reason="unreliable under qemu-user") def test_inproc_crash_stdout_before_send_and_on_crash(cmake): tmp_path, output = run_crash_stdout_for( "inproc", cmake, ["before-send", "on-crash"] @@ -261,7 +274,7 @@ def test_inproc_crash_stdout_before_send_and_on_crash(cmake): assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) - assert_attachment(envelope) + assert_crash_hint_attachments(envelope, "on-crash") assert_inproc_crash(envelope) @@ -320,7 +333,7 @@ def test_breakpad_crash_stdout_before_send(cmake): assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) - assert_attachment(envelope) + assert_crash_hint_attachments(envelope, "before-send") assert_minidump(envelope) assert_before_send(envelope) assert_breakpad_crash(envelope) @@ -350,7 +363,7 @@ def test_breakpad_crash_stdout_before_send_and_on_crash(cmake): assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) - assert_attachment(envelope) + assert_crash_hint_attachments(envelope, "on-crash") assert_breakpad_crash(envelope) diff --git a/tests/unit/test_attachments.c b/tests/unit/test_attachments.c index d740b2014..09355d154 100644 --- a/tests/unit/test_attachments.c +++ b/tests/unit/test_attachments.c @@ -2,6 +2,7 @@ #include "sentry_attachment.h" #include "sentry_backend.h" #include "sentry_envelope.h" +#include "sentry_hint.h" #include "sentry_options.h" #include "sentry_path.h" #include "sentry_scope.h" @@ -893,3 +894,47 @@ SENTRY_TEST(attachment_manifest) sentry__path_free(manifest_path); sentry__path_free(run_path); } + +SENTRY_TEST(hint_frozen_attachments) +{ + for (int action = 0; action < 3; action++) { + sentry_hint_t hint; + SENTRY__HINT_INIT(hint); + sentry_hint_clear_attachments(&hint); + sentry_hint_remove_attachment(&hint, sentry_uuid_nil()); + sentry_hint_add_attachment(&hint, sentry_value_new_null()); + TEST_CHECK(sentry_value_is_frozen(hint.attachments)); + + sentry_uuid_t id + = sentry_hint_attach_bytes(&hint, "first", 5, "first.txt"); + sentry_value_freeze(hint.attachments); + sentry_value_t original = sentry_value_incref(hint.attachments); + sentry_hint_add_attachment( + &hint, sentry_value_get_by_index_owned(original, 0)); + sentry_hint_add_attachment(&hint, sentry_value_new_null()); + sentry_hint_remove_attachment(&hint, sentry_uuid_nil()); + TEST_CHECK(sentry_value_is_frozen(hint.attachments)); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint.attachments), 1); + + switch (action) { + case 0: + sentry_hint_attach_bytes(&hint, "second", 6, "second.txt"); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint.attachments), 2); + break; + case 1: + sentry_hint_remove_attachment(&hint, id); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint.attachments), 0); + break; + case 2: + sentry_hint_clear_attachments(&hint); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint.attachments), 0); + sentry_hint_clear_attachments(&hint); + break; + } + TEST_CHECK(!sentry_value_is_frozen(hint.attachments)); + TEST_CHECK(sentry_value_is_frozen(original)); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(original), 1); + sentry_value_decref(original); + SENTRY__HINT_DEINIT(hint); + } +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index a3d7268eb..46c4d9cda 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -210,6 +210,7 @@ XX(find_mem_range) XX(formatted_log_messages) XX(fuzz_json) XX(getenv_double) +XX(hint_frozen_attachments) XX(http_request_accessors_bodyless_request) XX(http_request_accessors_file_backed_body) XX(http_request_accessors_in_memory_body) From 6f6faf172918c945305271c1b23f6878833ec93b Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 19:09:20 +0200 Subject: [PATCH 12/13] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 368b5bcb8..e8159f53b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ - Attachment APIs now use `sentry_value_t` and `sentry_uuid_t` instead of `sentry_attachment_t *` handles. Most attachment APIs, function names and arguments, are otherwise unchanged. ([#1974](https://github.com/getsentry/sentry-native/pull/1974)) - `sentry_init()` now consumes `/last_crash` after caching its value, aligning crashed-last-run behavior with other Sentry SDKs. ([#2023](https://github.com/getsentry/sentry-native/pull/2023)) - Change the `hint` parameter of `before_send` callbacks (`sentry_event_function_t`) from `void *` to `sentry_hint_t *`. Update callbacks registered with `sentry_options_set_before_send` to use the new parameter type. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) -- Add a `sentry_hint_t *hint` argument to `on_crash` callbacks. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) +- Add a `sentry_hint_t *hint` argument to `on_crash` callbacks. ([#2112](https://github.com/getsentry/sentry-native/pull/2112)) - Add a `sentry_hint_t *hint` argument to `sentry_scope_capture_event`. Pass `NULL` if no hint is needed. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) - Remove `sentry_options_get/set_enable_logs` and `sentry_options_get/set_enable_metrics`. ([#1980](https://github.com/getsentry/sentry-native/pull/1980)) > Structured logs and metrics have been enabled by default since `0.13`, and the options were deprecated and made no-ops in `0.16`. @@ -19,7 +19,7 @@ - Add `sentry_attachment_from_file/bytes` (and their wide-string variants) for creating attachment values that can be fully configured before they are added. ([#2079](https://github.com/getsentry/sentry-native/pull/2079)) - Add `sentry_add_attachment`, `sentry_scope_add_attachment`, and `sentry_hint_add_attachment` for adding configured attachments to the global scope, a specific scope, or a hint. These functions consume and freeze the attachment value. ([#2079](https://github.com/getsentry/sentry-native/pull/2079), [#1974](https://github.com/getsentry/sentry-native/pull/1974)) - Add `sentry_start_new_trace()` as a clearer name for starting a new trace. ([#2095](https://github.com/getsentry/sentry-native/pull/2095)) -- Add hint support to `sentry_scope_capture_event` to pass event-specific attachments. The hint is also passed to `before_send` and `on_crash`, which can modify attachments before the event is sent. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) +- Add hint support to `sentry_scope_capture_event` to pass event-specific attachments. The hint is also passed to `before_send` and `on_crash`, which can modify attachments before the event is sent. ([#2099](https://github.com/getsentry/sentry-native/pull/2099), [#2112](https://github.com/getsentry/sentry-native/pull/2112)) - Add `sentry_hint_remove_attachment` and `sentry_hint_clear_attachments`. ([#2099](https://github.com/getsentry/sentry-native/pull/2099)) **Fixes**: From f9cc9af37fb34a9816a898897fa222602679e759 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 21 Sep 2026 18:14:10 +0200 Subject: [PATCH 13/13] fix!: apply scope before `on_crash` callback --- CHANGELOG.md | 1 + examples/example.c | 5 + include/sentry.h | 5 +- src/backends/sentry_backend_breakpad.cpp | 6 +- src/backends/sentry_backend_crashpad.cpp | 64 ++++-- src/backends/sentry_backend_inproc.c | 8 +- src/backends/sentry_backend_native.c | 16 +- src/sentry_core.c | 35 +++ src/sentry_core.h | 8 + src/sentry_scope.c | 263 +++++++++++++++++++++++ src/sentry_scope.h | 21 ++ tests/test_integration_stdout.py | 4 + tests/unit/test_basic.c | 60 ++++++ tests/unit/tests.inc | 1 + 14 files changed, 463 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8159f53b..ec7a8a980 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ **Fixes**: - Reduce lock contention for multi-threaded log and metric capture by allowing concurrent reads of scope data, and reusing a single options reference throughout each capture. ([#2091](https://github.com/getsentry/sentry-native/pull/2091), [#2094](https://github.com/getsentry/sentry-native/pull/2094)) +- Apply scope data before invoking `on_crash` while preserving scope changes made by the callback. ([#2119](https://github.com/getsentry/sentry-native/pull/2119)) **Other changes**: diff --git a/examples/example.c b/examples/example.c index b70f2e9a0..19d829517 100644 --- a/examples/example.c +++ b/examples/example.c @@ -181,6 +181,11 @@ on_crash_callback(const sentry_ucontext_t *uctx, sentry_value_t event, sentry_attachment_from_bytes( "on_crash", strlen("on_crash"), "callback.txt")); + sentry_value_t tags = sentry_value_get_by_key(event, "tags"); + sentry_value_set_by_key(event, "on_crash_scope_tag", + sentry_value_incref(sentry_value_get_by_key(tags, "expected-tag"))); + sentry_set_tag("test.on-crash", "added-by-on-crash"); + // tell the backend to retain the event return event; } diff --git a/include/sentry.h b/include/sentry.h index f2710c9da..6e188e9d3 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1543,9 +1543,12 @@ SENTRY_API void sentry_options_set_before_send( * The hint is always provided and can be used to modify attachments on the * event. * + * The current scope is applied before invoking the callback. Scope changes made + * from inside the callback are also applied to the returned event. + * * Only the `inproc` backend currently fills the passed-in event with crash * meta-data. Since both `breakpad` and `crashpad` use minidumps to capture the - * crash state, the passed-in event is empty when using these backends. Changes + * crash state, their events contain scope data but no crash meta-data. Changes * to the event from inside the hooks will be passed along, but in the case of * the minidump backends these changes might get overwritten during server-side * ingestion and processing. This primarily affects the exception payloads which diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index acaaecfd6..cc7caa776 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -166,9 +166,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, uctx = &uctx_data; #endif - SENTRY_SIGNAL_SAFE_LOG("DEBUG invoking `on_crash` hook"); - event = options->on_crash_func( - uctx, event, &hint, options->on_crash_data); + event = sentry__invoke_on_crash(options, uctx, event, &hint, true); should_handle = !sentry_value_is_null(event); } @@ -189,8 +187,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, } #endif - event = sentry__prepare_event(options, event, nullptr); if (!options->on_crash_func) { + event = sentry__prepare_event(options, event, nullptr); sentry__hint_set_attachments(&hint, sentry__merge_attachments(hint.attachments, nullptr)); event = sentry__invoke_before_send(options, event, &hint); diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index a7699eff1..1687f9114 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -326,15 +326,9 @@ write_attachment_manifest(crashpad_state_t *state, sentry_value_t attachments) #endif static void -flush_scope_to_event(crashpad_state_t *state, const base::FilePath &event_path, - const sentry_options_t *options, sentry_value_t crash_event) +flush_event(crashpad_state_t *state, const base::FilePath &event_path, + sentry_value_t crash_event) { - SENTRY_WITH_SCOPE (scope) { - // we want the scope without any modules or breadcrumbs - sentry__scope_apply_to_event( - scope, options, crash_event, SENTRY_SCOPE_NONE); - } - size_t mpack_size; char *mpack = sentry_value_to_msgpack(crash_event, &mpack_size); sentry_value_decref(crash_event); @@ -350,6 +344,18 @@ flush_scope_to_event(crashpad_state_t *state, const base::FilePath &event_path, } } +static void +flush_scope_to_event(crashpad_state_t *state, const base::FilePath &event_path, + const sentry_options_t *options, sentry_value_t crash_event) +{ + SENTRY_WITH_SCOPE (scope) { + // we want the scope without any modules or breadcrumbs + sentry__scope_apply_to_event( + scope, options, crash_event, SENTRY_SCOPE_NONE); + } + flush_event(state, event_path, crash_event); +} + // Prepares an envelope with DSN, event ID, and session if available, for an // external crash reporter. static void @@ -527,9 +533,8 @@ read_msgpack_stream_file(const sentry_path_t *path) } #if defined(SENTRY_PLATFORM_LINUX) || defined(SENTRY_PLATFORM_WINDOWS) -static void -flush_scope_from_handler( - const sentry_options_t *options, sentry_value_t crash_event) +static crashpad_state_t * +lock_scope_from_handler(const sentry_options_t *options) { auto state = static_cast(options->backend->data); @@ -551,14 +556,34 @@ flush_scope_from_handler( sentry__cpu_relax(); } - // now we are the sole flusher and can flush into the crash event - flush_scope_to_event(state, state->event_path, options, crash_event); + return state; +} + +static void +flush_event_from_handler(crashpad_state_t *state, + const sentry_options_t *options, sentry_value_t crash_event) +{ + flush_event(state, state->event_path, crash_event); if (!state->external_report_path.empty()) { flush_external_crash_report(state, state->external_report_path, options, &state->crash_event_id); } } +static void +flush_scope_from_handler( + const sentry_options_t *options, sentry_value_t crash_event) +{ + crashpad_state_t *state = lock_scope_from_handler(options); + + // now we are the sole flusher and can flush into the crash event + SENTRY_WITH_SCOPE (scope) { + sentry__scope_apply_to_event( + scope, options, crash_event, SENTRY_SCOPE_NONE); + } + flush_event_from_handler(state, options, crash_event); +} + # ifdef SENTRY_PLATFORM_WINDOWS static bool crashpad_handler(EXCEPTION_POINTERS *ExceptionInfo) @@ -597,7 +622,9 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) } sentry_value_freeze(hint.attachments); + crashpad_state_t *locked_state = nullptr; if (options->on_crash_func) { + locked_state = lock_scope_from_handler(options); sentry_ucontext_t uctx; # ifdef SENTRY_PLATFORM_WINDOWS uctx.exception_ptrs = *ExceptionInfo; @@ -607,9 +634,8 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) uctx.user_context = user_context; # endif - SENTRY_DEBUG("invoking `on_crash` hook"); - crash_event = options->on_crash_func( - &uctx, crash_event, &hint, options->on_crash_data); + crash_event = sentry__invoke_on_crash( + options, &uctx, crash_event, &hint, false); } else if (options->before_send_func) { crash_event = sentry__invoke_before_send(options, crash_event, &hint); @@ -627,7 +653,11 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) write_attachment_manifest(state, hint.attachments); } sentry_value_incref(crash_event); - flush_scope_from_handler(options, crash_event); + if (locked_state) { + flush_event_from_handler(locked_state, options, crash_event); + } else { + flush_scope_from_handler(options, crash_event); + } sentry__write_crash_marker(options); sentry__record_errors_on_current_session(1); diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 963f4229b..9a31a5f5d 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1085,9 +1085,7 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, &hint, sentry__merge_attachments(hint.attachments, NULL)); } if (options->on_crash_func && !skip_hooks) { - SENTRY_DEBUG("invoking `on_crash` hook"); - event = options->on_crash_func( - uctx, event, &hint, options->on_crash_data); + event = sentry__invoke_on_crash(options, uctx, event, &hint, true); should_handle = !sentry_value_is_null(event); } else if (skip_hooks && options->on_crash_func) { SENTRY_DEBUG("skipping `on_crash` hook due to recursive crash"); @@ -1109,7 +1107,9 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, } #endif - event = sentry__prepare_event(options, event, NULL); + if (!options->on_crash_func || skip_hooks) { + event = sentry__prepare_event(options, event, NULL); + } if (!options->on_crash_func) { sentry__hint_set_attachments( &hint, sentry__merge_attachments(hint.attachments, NULL)); diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index a913ad83c..22a795ccd 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -1363,9 +1363,8 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) // Call on_crash hook if configured if (options->on_crash_func) { - SENTRY_DEBUG("invoking `on_crash` hook"); - sentry_value_t result = options->on_crash_func( - uctx, event, &hint, options->on_crash_data); + sentry_value_t result + = sentry__invoke_on_crash(options, uctx, event, &hint, false); should_handle = !sentry_value_is_null(result); event = result; } @@ -1389,11 +1388,12 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) native_backend_write_attachments( state ? state->event_path : NULL, &hint); } - // Apply scope to the event. The daemon assembles breadcrumbs - // from the ring files - SENTRY_WITH_SCOPE (scope) { - sentry__scope_apply_to_event( - scope, options, event, SENTRY_SCOPE_NONE); + if (!options->on_crash_func) { + // The daemon assembles breadcrumbs from the ring files + SENTRY_WITH_SCOPE (scope) { + sentry__scope_apply_to_event( + scope, options, event, SENTRY_SCOPE_NONE); + } } #if defined(SENTRY_PLATFORM_WINDOWS) ensure_device_arch(event); diff --git a/src/sentry_core.c b/src/sentry_core.c index 22e66baa7..b96f472d7 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -795,6 +795,41 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, return event; } +sentry_value_t +sentry__invoke_on_crash(const sentry_options_t *options, + const sentry_ucontext_t *uctx, sentry_value_t event, sentry_hint_t *hint, + bool full_scope) +{ + if (!options->on_crash_func) { + return event; + } + + sentry_scope_change_observer_t *observer + = sentry__scope_change_observer_new(full_scope); + + SENTRY_WITH_SCOPE_MUT_NO_FLUSH (scope) { + bool observing + = observer && sentry__scope_add_observer(scope, &observer->base); + if (full_scope) { + event = sentry__prepare_event(options, event, NULL); + } else { + sentry__scope_apply_to_event( + scope, options, event, SENTRY_SCOPE_NONE); + } + SENTRY_SIGNAL_SAFE_LOG("DEBUG invoking `on_crash` hook"); + event + = options->on_crash_func(uctx, event, hint, options->on_crash_data); + if (observing) { + sentry__scope_change_observer_apply( + observer, event, options->max_breadcrumbs); + sentry__scope_change_observer_cleanup(observer); + sentry__scope_remove_observer(scope, &observer->base); + } + } + + return event; +} + sentry_envelope_t * sentry__enclose_event(const sentry_options_t *options, sentry_value_t event, sentry_uuid_t *event_id, sentry_value_t attachments) diff --git a/src/sentry_core.h b/src/sentry_core.h index deab5cbbe..986c76f65 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -87,6 +87,14 @@ sentry_value_t sentry__prepare_event(const sentry_options_t *options, sentry_value_t sentry__merge_attachments( sentry_value_t attachments, sentry_scope_t *local_scope); +/** + * Applies the scope, invokes the configured `on_crash` callback, and preserves + * scope changes made by the callback on the returned event. + */ +sentry_value_t sentry__invoke_on_crash(const sentry_options_t *options, + const sentry_ucontext_t *uctx, sentry_value_t event, sentry_hint_t *hint, + bool full_scope); + /** * Sends a sentry event, regardless of its type. */ diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 27c2c8dd6..af59411a8 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -569,6 +569,269 @@ sentry__scope_observer_new(void) return SENTRY_MAKE(sentry_scope_observer_t); } +static sentry_value_t +scope_change_observer_value(sentry_scope_change_observer_t *changes) +{ + if (sentry_value_is_null(changes->values)) { + changes->values = sentry_value_new_object(); + } + return changes->values; +} + +static void +scope_change_observer_set_value( + void *data, const char *key, sentry_value_t value) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key( + scope_change_observer_value(changes), key, sentry__value_clone(value)); +} + +static sentry_value_t +scope_change_observer_object( + sentry_scope_change_observer_t *changes, const char *key) +{ + sentry_value_t values = scope_change_observer_value(changes); + sentry_value_t object = sentry_value_get_by_key(values, key); + if (sentry_value_is_null(object)) { + object = sentry_value_new_object(); + sentry_value_set_by_key(values, key, object); + } + return object; +} + +static void +scope_change_observer_clear(void *data) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_decref(changes->values); + changes->values = sentry_value_new_null(); + changes->cleared = true; +} + +static void +scope_change_observer_set_string( + void *data, const char *key, sentry_value_t value) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_value(changes), key, + sentry_value_get_length(value) > 0 ? sentry__value_clone(value) + : sentry_value_new_null()); +} + +static void +scope_change_observer_set_release(void *data, sentry_value_t value) +{ + scope_change_observer_set_string(data, "release", value); +} + +static void +scope_change_observer_set_environment(void *data, sentry_value_t value) +{ + scope_change_observer_set_string(data, "environment", value); +} + +static void +scope_change_observer_set_transaction(void *data, sentry_value_t value) +{ + scope_change_observer_set_string(data, "transaction", value); +} + +static void +scope_change_observer_set_fingerprint(void *data, sentry_value_t value) +{ + scope_change_observer_set_value(data, "fingerprint", value); +} + +static void +scope_change_observer_set_level(void *data, sentry_level_t level) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_value(changes), "level", + sentry__value_new_level(level)); +} + +static void +scope_change_observer_set_user(void *data, sentry_value_t value) +{ + scope_change_observer_set_value(data, "user", value); +} + +static void +scope_change_observer_add_breadcrumb(void *data, sentry_value_t breadcrumb) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_t values = scope_change_observer_value(changes); + sentry_value_t breadcrumbs = sentry_value_get_by_key(values, "breadcrumbs"); + if (sentry_value_is_null(breadcrumbs)) { + breadcrumbs = sentry_value_new_list(); + sentry_value_set_by_key(values, "breadcrumbs", breadcrumbs); + } + sentry_value_append(breadcrumbs, sentry__value_clone(breadcrumb)); +} + +static void +scope_change_observer_set_tag(void *data, const char *key, const char *value) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_object(changes, "tags"), key, + sentry_value_new_string(value)); +} + +static void +scope_change_observer_remove_tag(void *data, const char *key) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_object(changes, "tags"), key, + sentry_value_new_null()); +} + +static void +scope_change_observer_set_extra( + void *data, const char *key, sentry_value_t value) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_object(changes, "extra"), key, + sentry__value_clone(value)); +} + +static void +scope_change_observer_remove_extra(void *data, const char *key) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_object(changes, "extra"), key, + sentry_value_new_null()); +} + +static void +scope_change_observer_set_context( + void *data, const char *key, sentry_value_t value) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_object(changes, "contexts"), + key, sentry__value_clone(value)); +} + +static void +scope_change_observer_remove_context(void *data, const char *key) +{ + sentry_scope_change_observer_t *changes = data; + sentry_value_set_by_key(scope_change_observer_object(changes, "contexts"), + key, sentry_value_new_null()); +} + +sentry_scope_change_observer_t * +sentry__scope_change_observer_new(bool include_breadcrumbs) +{ + sentry_scope_change_observer_t *changes + = SENTRY_MAKE(sentry_scope_change_observer_t); + if (!changes) { + return NULL; + } + + changes->values = sentry_value_new_null(); + changes->cleared = false; + changes->include_breadcrumbs = include_breadcrumbs; + + sentry_scope_observer_t *observer = &changes->base; + observer->data = changes; + observer->clear = scope_change_observer_clear; + observer->set_release = scope_change_observer_set_release; + observer->set_environment = scope_change_observer_set_environment; + observer->set_transaction = scope_change_observer_set_transaction; + observer->set_fingerprint = scope_change_observer_set_fingerprint; + observer->set_level = scope_change_observer_set_level; + observer->set_user = scope_change_observer_set_user; + observer->add_breadcrumb + = include_breadcrumbs ? scope_change_observer_add_breadcrumb : NULL; + observer->set_tag = scope_change_observer_set_tag; + observer->remove_tag = scope_change_observer_remove_tag; + observer->set_extra = scope_change_observer_set_extra; + observer->remove_extra = scope_change_observer_remove_extra; + observer->set_context = scope_change_observer_set_context; + observer->remove_context = scope_change_observer_remove_context; + return changes; +} + +static int +apply_scope_object_change(const char *key, sentry_value_t value, void *data) +{ + sentry_value_t object = *(sentry_value_t *)data; + if (sentry_value_is_null(value)) { + sentry_value_remove_by_key(object, key); + } else { + sentry_value_set_by_key(object, key, sentry__value_clone(value)); + } + return 0; +} + +typedef struct { + sentry_value_t event; + size_t max_breadcrumbs; +} sentry_scope_changes_apply_t; + +static int +apply_scope_change(const char *key, sentry_value_t value, void *data) +{ + sentry_scope_changes_apply_t *apply = data; + if (!strcmp(key, "tags") || !strcmp(key, "extra") + || !strcmp(key, "contexts")) { + sentry_value_t object = sentry_value_get_by_key(apply->event, key); + if (sentry_value_get_type(object) != SENTRY_VALUE_TYPE_OBJECT) { + object = sentry_value_new_object(); + sentry_value_set_by_key(apply->event, key, object); + } + sentry_value_foreach_key_value( + value, apply_scope_object_change, &object); + } else if (!strcmp(key, "breadcrumbs")) { + sentry_value_t breadcrumbs + = sentry_value_get_by_key(apply->event, "breadcrumbs"); + sentry_value_set_by_key(apply->event, "breadcrumbs", + sentry__value_merge_breadcrumbs( + breadcrumbs, value, apply->max_breadcrumbs)); + } else if (sentry_value_is_null(value)) { + sentry_value_remove_by_key(apply->event, key); + } else { + sentry_value_set_by_key(apply->event, key, sentry__value_clone(value)); + } + return 0; +} + +void +sentry__scope_change_observer_apply( + const sentry_scope_change_observer_t *changes, sentry_value_t event, + size_t max_breadcrumbs) +{ + if (sentry_value_is_null(event)) { + return; + } + if (changes->cleared) { + static const char *scope_keys[] + = { "release", "environment", "transaction", "fingerprint", "user", + "tags", "extra", "contexts", "sdk" }; + for (size_t i = 0; i < sizeof(scope_keys) / sizeof(scope_keys[0]); + i++) { + sentry_value_remove_by_key(event, scope_keys[i]); + } + if (changes->include_breadcrumbs) { + sentry_value_remove_by_key(event, "breadcrumbs"); + } + } + if (!sentry_value_is_null(changes->values)) { + sentry_scope_changes_apply_t apply + = { .event = event, .max_breadcrumbs = max_breadcrumbs }; + sentry_value_foreach_key_value( + changes->values, apply_scope_change, &apply); + } +} + +void +sentry__scope_change_observer_cleanup(sentry_scope_change_observer_t *changes) +{ + sentry_value_decref(changes->values); + changes->values = sentry_value_new_null(); +} + bool sentry__scope_add_observer( sentry_scope_t *scope, sentry_scope_observer_t *observer) diff --git a/src/sentry_scope.h b/src/sentry_scope.h index 84b1fc2a7..a32fcd5ad 100644 --- a/src/sentry_scope.h +++ b/src/sentry_scope.h @@ -50,6 +50,13 @@ typedef struct sentry_scope_observer_s { void (*remove_attachment)(void *data, sentry_value_t attachment); } sentry_scope_observer_t; +typedef struct { + sentry_scope_observer_t base; + sentry_value_t values; + bool cleared; + bool include_breadcrumbs; +} sentry_scope_change_observer_t; + typedef struct sentry_scope_data_s sentry_scope_data_t; /** @@ -238,6 +245,20 @@ void sentry__scope_set_trace_managed(sentry_scope_t *scope, bool managed); */ sentry_scope_observer_t *sentry__scope_observer_new(void); +/** + * Creates an observer that records scope changes for later application to an + * event. Register `base` with the scope and clean up the observer before + * removing it. + */ +sentry_scope_change_observer_t *sentry__scope_change_observer_new( + bool include_breadcrumbs); + +void sentry__scope_change_observer_apply( + const sentry_scope_change_observer_t *observer, sentry_value_t event, + size_t max_breadcrumbs); +void sentry__scope_change_observer_cleanup( + sentry_scope_change_observer_t *observer); + /** * Register a scope observer. * diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py index bd221243e..f09bb9782 100644 --- a/tests/test_integration_stdout.py +++ b/tests/test_integration_stdout.py @@ -276,6 +276,8 @@ def test_inproc_crash_stdout_before_send_and_on_crash(cmake): assert_breadcrumb(envelope) assert_crash_hint_attachments(envelope, "on-crash") assert_inproc_crash(envelope) + assert envelope.get_event()["on_crash_scope_tag"] == "some value" + assert envelope.get_event()["tags"]["test.on-crash"] == "added-by-on-crash" @pytest.mark.parametrize( @@ -365,6 +367,8 @@ def test_breakpad_crash_stdout_before_send_and_on_crash(cmake): assert_breadcrumb(envelope) assert_crash_hint_attachments(envelope, "on-crash") assert_breakpad_crash(envelope) + assert envelope.get_event()["on_crash_scope_tag"] == "some value" + assert envelope.get_event()["tags"]["test.on-crash"] == "added-by-on-crash" @pytest.mark.parametrize( diff --git a/tests/unit/test_basic.c b/tests/unit/test_basic.c index d2d2f5abc..478ab5746 100644 --- a/tests/unit/test_basic.c +++ b/tests/unit/test_basic.c @@ -158,6 +158,66 @@ SENTRY_TEST(discarding_before_send) TEST_CHECK_INT_EQUAL(called_beforesend, 1); } +static sentry_value_t +on_crash_modify_scope(const sentry_ucontext_t *UNUSED(uctx), + sentry_value_t event, sentry_hint_t *UNUSED(hint), void *data) +{ + bool *saw_scope = data; + sentry_value_t tags = sentry_value_get_by_key(event, "tags"); + *saw_scope = sentry__string_eq( + sentry_value_as_string(sentry_value_get_by_key(tags, "changed")), + "before"); + + sentry_set_tag("changed", "after"); + sentry_set_tag("added", "during-on-crash"); + sentry_remove_tag("removed"); + sentry_set_release("after-on-crash"); + sentry_set_user(sentry_value_new_user("after-on-crash", NULL, NULL, NULL)); + return event; +} + +SENTRY_TEST(on_crash_scope_changes) +{ + bool saw_scope = false; + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_backend(options, NULL); + sentry_options_set_release(options, "before"); + sentry_options_set_on_crash(options, on_crash_modify_scope, &saw_scope); + sentry_init(options); + + sentry_set_tag("changed", "before"); + sentry_set_tag("removed", "before"); + sentry_set_user(sentry_value_new_user("before", NULL, NULL, NULL)); + + sentry_value_t event = sentry_value_new_event(); + sentry_hint_t hint; + SENTRY__HINT_INIT(hint); + SENTRY_WITH_OPTIONS (held_options) { + event + = sentry__invoke_on_crash(held_options, NULL, event, &hint, false); + } + + TEST_CHECK(saw_scope); + sentry_value_t tags = sentry_value_get_by_key(event, "tags"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(tags, "changed")), + "after"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(tags, "added")), + "during-on-crash"); + TEST_CHECK(sentry_value_is_null(sentry_value_get_by_key(tags, "removed"))); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(event, "release")), + "after-on-crash"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(sentry_value_get_by_key( + sentry_value_get_by_key(event, "user"), "id")), + "after-on-crash"); + + sentry_value_decref(event); + SENTRY__HINT_DEINIT(hint); + sentry_close(); +} + SENTRY_TEST(crash_marker) { // We don't use sentry_init() in this test so we must create a database dir diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 46c4d9cda..3c4cc3085 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -285,6 +285,7 @@ XX(mpack_removed_tags) XX(multiple_inits) XX(multiple_transactions) XX(old_run_retry) +XX(on_crash_scope_changes) XX(on_crashed_last_run) XX(on_crashed_last_run_cache) XX(options_crash_reporting_mode_clamp)