diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c08e150a..c0f6b9fa5 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. ([#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`. @@ -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), [#2112](https://github.com/getsentry/sentry-native/pull/2112)) - Add `sentry_hint_get_attachments`, `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 3d984e04d..efa0e4f24 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..655fc59ed 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 @@ -143,6 +144,9 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, = 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) { @@ -161,8 +165,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, options->on_crash_data); + event = sentry__invoke_on_crash(options, uctx, event, &hint); should_handle = !sentry_value_is_null(event); } @@ -183,8 +186,15 @@ 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) { + event = sentry__invoke_before_send(options, event, &hint); + } + sentry_value_t attachments + = sentry__hint_resolve_attachments(&hint); + sentry_envelope_t *envelope + = sentry__enclose_event(options, event, nullptr, attachments); + sentry_value_decref(attachments); if (envelope) { event_id = sentry__envelope_get_event_id(envelope); } @@ -280,6 +290,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..f99b48791 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 @@ -398,22 +399,22 @@ flush_scope_attachments(crashpad_state_t *data, const sentry_options_t *options) } static sentry_path_t * -prepare_initial_attachment( - sentry_value_t attachment, const sentry_path_t *run_path) +prepare_attachment(sentry_value_t attachment, const sentry_path_t *run_path) { - size_t bytes_len = 0; - const char *bytes = sentry__attachment_get_bytes(attachment, &bytes_len); sentry_path_t *path = sentry__attachment_make_run_path(run_path, attachment); if (!path) { return nullptr; } + + size_t bytes_len = 0; + const char *bytes = sentry__attachment_get_bytes(attachment, &bytes_len); if (bytes) { sentry_path_t *dir = sentry__path_dir(path); int rv = dir ? sentry__path_create_dir_all(dir) : 1; sentry__path_free(dir); if (rv != 0 || sentry__path_write_buffer(path, bytes, bytes_len) != 0) { - SENTRY_WARN("failed to prepare initial scope attachment"); + SENTRY_WARN("failed to prepare crashpad attachment"); sentry__path_remove(path); sentry__path_free(path); return nullptr; @@ -422,6 +423,28 @@ prepare_initial_attachment( return path; } +#if defined(SENTRY_PLATFORM_LINUX) || defined(SENTRY_PLATFORM_WINDOWS) +static void +write_attachment_manifest(crashpad_state_t *state, sentry_value_t attachments) +{ + sentry_path_t *path + = sentry__path_join_str(state->run_path, "__sentry-attachments"); + if (!path) { + return; + } + + 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__attachment_get_path(attachment)) { + sentry__path_free(prepare_attachment(attachment, state->run_path)); + } + } + sentry__write_attachment_manifest(path, attachments); + sentry__path_free(path); +} +#endif + static void preload_scope_breadcrumbs( sentry_backend_t *backend, const sentry_options_t *options) @@ -549,6 +572,8 @@ 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) { sentry_ucontext_t uctx; @@ -560,12 +585,11 @@ 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, options->on_crash_data); + crash_event + = sentry__invoke_on_crash(options, &uctx, crash_event, &hint); } 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 +600,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__hint_is_modified(&hint)) { + 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 +633,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); } @@ -889,29 +917,10 @@ add_attachment(void *state, sentry_value_t attachment) return; } - size_t bytes_len = 0; - const char *bytes = sentry__attachment_get_bytes(attachment, &bytes_len); - sentry_path_t *path - = sentry__attachment_make_run_path(data->run_path, attachment); + sentry_path_t *path = prepare_attachment(attachment, data->run_path); if (!path) { - const char *filename = sentry__attachment_get_filename(attachment); - SENTRY_WARNF("failed to create path for crashpad attachment \"%s\"", - filename ? filename : ""); return; } - - if (bytes) { - sentry_path_t *dir = sentry__path_dir(path); - int rv = dir ? sentry__path_create_dir_all(dir) : 1; - sentry__path_free(dir); - if (rv != 0 || sentry__path_write_buffer(path, bytes, bytes_len) != 0) { - SENTRY_WARNF( - "failed to write crashpad attachment \"%s\"", path->path); - sentry__path_remove(path); - sentry__path_free(path); - return; - } - } data->client->AddAttachment(base::FilePath(SENTRY_PATH_PLATFORM_STR(path))); sentry__path_free(path); } @@ -1013,7 +1022,7 @@ crashpad_backend_startup( sentry_value_t attachment = sentry_value_get_by_index(scope_attachments, i); sentry_path_t *path - = prepare_initial_attachment(attachment, current_run_folder); + = prepare_attachment(attachment, current_run_folder); if (path) { attachments.emplace_back(SENTRY_PATH_PLATFORM_STR(path)); sentry__path_free(path); diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index a6e274760..46c02083b 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" @@ -1077,9 +1078,11 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, = 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 && !skip_hooks) { - SENTRY_DEBUG("invoking `on_crash` hook"); - event = options->on_crash_func(uctx, event, options->on_crash_data); + event = sentry__invoke_on_crash(options, uctx, event, &hint); 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 +1104,15 @@ 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 && !skip_hooks) { + event = sentry__invoke_before_send(options, event, &hint); + } + sentry_value_t attachments + = sentry__hint_resolve_attachments(&hint); + sentry_envelope_t *envelope + = sentry__enclose_event(options, event, NULL, attachments); + sentry_value_decref(attachments); if (envelope) { event_id = sentry__envelope_get_event_id(envelope); } @@ -1161,6 +1171,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..21c0ae706 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 +// Writes the attachment list 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,15 @@ 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; // 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__invoke_on_crash(options, uctx, event, &hint); should_handle = !sentry_value_is_null(result); event = result; } @@ -1368,11 +1367,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) { + bool modified = sentry__hint_is_modified(&hint); + if (modified) { + 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)); + } + } + // Write modified hint attachments or reload the current scope + native_backend_write_attachments( + state ? state->event_path : NULL, modified ? &hint : NULL); // Apply scope to the event. The daemon assembles breadcrumbs // from the ring files SENTRY_WITH_SCOPE (scope) { @@ -1463,6 +1473,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..e7a7c5726 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -587,6 +587,9 @@ sentry_capture_event_with_scope(sentry_value_t event, sentry_scope_t *scope) return sentry_scope_capture_event(scope, event, NULL); } +static sentry_value_t prepare_attachments( + sentry_hint_t *hint, sentry_scope_t *local_scope); + #ifndef SENTRY_UNITTEST static #endif @@ -622,8 +625,12 @@ 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 attachments = prepare_attachments(hint, local_scope); + event = sentry__invoke_before_send(options, event, hint); + envelope = sentry__enclose_event(options, event, &event_id, + hint ? hint->attachments : attachments); + sentry_value_decref(attachments); } if (envelope) { // Accept a racy read here, since SENTRY_WITH_OPTIONS only prevents @@ -739,12 +746,22 @@ prepare_attachments(sentry_hint_t *hint, sentry_scope_t *local_scope) sentry_value_decref(global_attachments); } if (hint) { - sentry_value_decref(hint->attachments); - hint->attachments = sentry_value_incref(attachments); + sentry__hint_set_attachments(hint, sentry_value_incref(attachments)); } return attachments; } +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) +{ + if (!options->on_crash_func) { + return event; + } + SENTRY_SIGNAL_SAFE_LOG("DEBUG invoking `on_crash` hook"); + return options->on_crash_func(uctx, event, hint, options->on_crash_data); +} + sentry_value_t sentry__invoke_before_send( const sentry_options_t *options, sentry_value_t event, sentry_hint_t *hint) @@ -762,13 +779,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 +801,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 +819,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; @@ -922,6 +934,11 @@ prepare_user_feedback(const sentry_options_t *options, } } + if (hint && sentry__hint_is_modified(hint)) { + sentry_value_decref(all_attachments); + all_attachments = sentry_value_incref(hint->attachments); + } + sentry__ensure_event_id(event, event_id); sentry_envelope_t *envelope = sentry__envelope_new(); @@ -929,10 +946,9 @@ prepare_user_feedback(const sentry_options_t *options, goto fail; } - sentry_value_t attachments = hint ? hint->attachments : all_attachments; - sentry__envelope_add_attachments(envelope, attachments, options); + sentry__envelope_add_attachments(envelope, all_attachments, options); if (options->run) { - sentry__cache_attachment_refs(envelope, attachments, options, + sentry__cache_attachment_refs(envelope, all_attachments, options, options->run->cache_path, options->run->run_path); } @@ -2040,10 +2056,13 @@ 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); + 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..b3aa24c02 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 `on_crash` callback, if any. + * + * Returns the callback result, or `event` unchanged when no callback is + * configured. + */ +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); + /** * Invokes the configured `before_send` callback, if any. * @@ -57,22 +66,27 @@ 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. + * 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); + +/** + * 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); /** * Sends a sentry event, regardless of its type. diff --git a/src/sentry_hint.c b/src/sentry_hint.c index 922dd629a..bfebee0ac 100644 --- a/src/sentry_hint.c +++ b/src/sentry_hint.c @@ -2,6 +2,7 @@ #include "sentry_alloc.h" #include "sentry_attachment.h" +#include "sentry_scope.h" #include "sentry_string.h" #include @@ -14,16 +15,67 @@ sentry_hint_new(void) return NULL; } hint->attachments = sentry_value_new_null(); + hint->modified = false; return hint; } +void +sentry__hint_init(sentry_hint_t *hint) +{ + hint->attachments = sentry_value_new_null(); + SENTRY_WITH_SCOPE (scope) { + sentry_value_decref(hint->attachments); + hint->attachments = sentry__scope_load_attachments(scope); + } + hint->modified = false; +} + +void +sentry__hint_deinit(sentry_hint_t *hint) +{ + sentry_value_decref(hint->attachments); +} + +bool +sentry__hint_is_modified(const sentry_hint_t *hint) +{ + return hint && hint->modified; +} + +sentry_value_t +sentry__hint_resolve_attachments(const sentry_hint_t *hint) +{ + if (sentry__hint_is_modified(hint)) { + return sentry_value_incref(hint->attachments); + } + + sentry_value_t attachments = sentry_value_new_null(); + SENTRY_WITH_SCOPE (scope) { + sentry_value_decref(attachments); + attachments = sentry__scope_load_attachments(scope); + } + return attachments; +} + +void +sentry__hint_set_attachments(sentry_hint_t *hint, sentry_value_t attachments) +{ + if (hint) { + sentry_value_decref(hint->attachments); + hint->attachments = attachments; + hint->modified = false; + } 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 +87,12 @@ sentry_hint_add_attachment(sentry_hint_t *hint, sentry_value_t attachment) return sentry_uuid_nil(); } + size_t len = sentry_value_get_length(hint->attachments); sentry_value_t added = sentry__attachments_add(&hint->attachments, attachment); + if (sentry_value_get_length(hint->attachments) != len) { + hint->modified = true; + } sentry_uuid_t attachment_id = sentry__attachment_get_id(added); sentry_value_decref(added); return attachment_id; @@ -116,16 +172,21 @@ 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 removed + = sentry__attachments_remove(hint->attachments, &attachment_id); + if (!sentry_value_is_null(removed)) { + hint->modified = true; + } + sentry_value_decref(removed); } } void sentry_hint_clear_attachments(sentry_hint_t *hint) { - if (hint) { + if (hint && sentry_value_get_length(hint->attachments)) { sentry_value_decref(hint->attachments); hint->attachments = sentry_value_new_null(); + hint->modified = true; } } diff --git a/src/sentry_hint.h b/src/sentry_hint.h index 1b379d2ed..8047a103d 100644 --- a/src/sentry_hint.h +++ b/src/sentry_hint.h @@ -9,8 +9,36 @@ */ struct sentry_hint_s { sentry_value_t attachments; + bool modified; }; +/** + * Initializes a hint with a snapshot of the global scope's attachments. + */ +void sentry__hint_init(sentry_hint_t *hint); + +/** + * Releases resources owned by a hint. + */ +void sentry__hint_deinit(sentry_hint_t *hint); + +/** + * Returns whether the hint's attachments differ from its baseline. + */ +bool sentry__hint_is_modified(const sentry_hint_t *hint); + +/** + * Returns modified hint attachments or the current scope attachments as an + * owned value. + */ +sentry_value_t sentry__hint_resolve_attachments(const sentry_hint_t *hint); + +/** + * Replaces a hint's attachment baseline, 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..6f3d0be44 100644 --- a/tests/unit/test_attachments.c +++ b/tests/unit/test_attachments.c @@ -1,7 +1,9 @@ #include "sentry_alloc.h" #include "sentry_attachment.h" #include "sentry_backend.h" +#include "sentry_core.h" #include "sentry_envelope.h" +#include "sentry_hint.h" #include "sentry_options.h" #include "sentry_path.h" #include "sentry_scope.h" @@ -893,3 +895,56 @@ SENTRY_TEST(attachment_manifest) sentry__path_free(manifest_path); sentry__path_free(run_path); } + +SENTRY_TEST(hint_attachments) +{ + sentry_hint_t *empty = sentry_hint_new(); + sentry_hint_clear_attachments(empty); + sentry_hint_remove_attachment(empty, sentry_uuid_nil()); + sentry_hint_add_attachment(empty, sentry_value_new_null()); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(empty->attachments), 0); + TEST_CHECK(!sentry__hint_is_modified(empty)); + sentry__hint_free(empty); + + SENTRY_TEST_OPTIONS_NEW(options); + TEST_CHECK_INT_EQUAL(sentry_init(options), 0); + sentry_uuid_t id = sentry_attach_bytes("first", 5, "first.txt"); + TEST_CHECK(!sentry_uuid_is_nil(&id)); + + for (int action = 0; action < 3; action++) { + sentry_hint_t hint; + sentry__hint_init(&hint); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint.attachments), 1); + TEST_CHECK(!sentry__hint_is_modified(&hint)); + + sentry_hint_remove_attachment(&hint, sentry_uuid_nil()); + sentry_hint_add_attachment(&hint, sentry_value_new_null()); + TEST_CHECK(!sentry__hint_is_modified(&hint)); + + sentry_hint_add_attachment( + &hint, sentry_value_get_by_index_owned(hint.attachments, 0)); + sentry_hint_add_attachment(&hint, sentry_value_new_null()); + sentry_hint_remove_attachment(&hint, sentry_uuid_nil()); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(hint.attachments), 1); + TEST_CHECK(!sentry__hint_is_modified(&hint)); + + 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__hint_is_modified(&hint)); + sentry__hint_deinit(&hint); + } + sentry_close(); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index a3d7268eb..d344da20e 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_attachments) XX(http_request_accessors_bodyless_request) XX(http_request_accessors_file_backed_body) XX(http_request_accessors_in_memory_body)