diff --git a/CHANGELOG.md b/CHANGELOG.md index c8bab372c..1c5554ce8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Internal:** - Support downstream Xbox SDK specifying networking initialization mechanism ([#1359](https://github.com/getsentry/sentry-native/pull/1359)) +- Add before crash hook to allow modifying other callbacks before crash handling ([#1366](https://github.com/getsentry/sentry-native/pull/1366)) ## 0.10.1 diff --git a/include/sentry.h b/include/sentry.h index 9a7ada89e..e61d3f396 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -989,6 +989,36 @@ typedef sentry_value_t (*sentry_event_function_t)( SENTRY_API void sentry_options_set_before_send( sentry_options_t *opts, sentry_event_function_t func, void *user_data); +/** + * Type of the `before_crash` callback. + * + * The `before_crash` callback is invoked immediately when a crash is detected, + * before any crash processing begins. This provides an opportunity to perform + * cleanup or disable logging operations at the earliest possible moment during + * crash handling. + * + * Unlike `on_crash`, this callback does not receive an event object and is not + * intended for event filtering or modification. It is purely for performing + * early crash-time operations like flushing logs, closing files, or other + * cleanup tasks. + * + * The callback has a void return type as it is not intended to filter or + * modify crash events. + * + * This function may be invoked inside of a signal handler and must be safe for + * that purpose, see https://man7.org/linux/man-pages/man7/signal-safety.7.html. + * On Windows, it may be called from inside of a `UnhandledExceptionFilter`. + */ +typedef void (*sentry_before_crash_function_t)(void *user_data); + +/** + * Sets the `before_crash` callback. + * + * See the `sentry_before_crash_function_t` typedef above for more information. + */ +SENTRY_API void sentry_options_set_before_crash(sentry_options_t *opts, + sentry_before_crash_function_t func, void *user_data); + /** * Type of the `on_crash` callback. * diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 89be05165..51736febe 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -74,6 +74,12 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, void *UNUSED(context), bool succeeded) #endif { + SENTRY_WITH_OPTIONS (options) { + if (options->before_crash_func) { + options->before_crash_func(options->before_crash_data); + } + } + SENTRY_INFO("entering breakpad minidump callback"); // this is a bit strange, according to docs, `succeeded` should be true when diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index c3552d475..f78e411a5 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -280,6 +280,13 @@ sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) { sentry__page_allocator_enable(); # endif + + SENTRY_WITH_OPTIONS (options) { + if (options->before_crash_func) { + options->before_crash_func(options->before_crash_data); + } + } + SENTRY_INFO("flushing session and queue before crashpad handler"); bool should_dump = true; diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index c2ee95fa0..723db751d 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -523,6 +523,12 @@ make_signal_event( static void handle_ucontext(const sentry_ucontext_t *uctx) { + SENTRY_WITH_OPTIONS (options) { + if (options->before_crash_func) { + options->before_crash_func(options->before_crash_data); + } + } + SENTRY_INFO("entering signal handler"); const struct signal_slot *sig_slot = NULL; diff --git a/src/sentry_options.c b/src/sentry_options.c index 23eaef2b4..9b791674f 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -137,6 +137,14 @@ sentry_options_set_before_send( opts->before_send_data = user_data; } +void +sentry_options_set_before_crash(sentry_options_t *opts, + sentry_before_crash_function_t func, void *user_data) +{ + opts->before_crash_func = func; + opts->before_crash_data = user_data; +} + void sentry_options_set_on_crash( sentry_options_t *opts, sentry_crash_function_t func, void *user_data) diff --git a/src/sentry_options.h b/src/sentry_options.h index 5316b69e5..c34051017 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -52,6 +52,8 @@ struct sentry_options_s { void *on_crash_data; sentry_transaction_function_t before_transaction_func; void *before_transaction_data; + sentry_before_crash_function_t before_crash_func; + void *before_crash_data; /* Experimentally exposed */ double traces_sample_rate; diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 9add26e97..f8f746357 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -23,6 +23,7 @@ add_executable(sentry_test_unit sentry_testsupport.h test_attachments.c test_basic.c + test_before_crash.c test_consent.c test_concurrency.c test_embedded_info.c diff --git a/tests/unit/test_before_crash.c b/tests/unit/test_before_crash.c new file mode 100644 index 000000000..9318a95de --- /dev/null +++ b/tests/unit/test_before_crash.c @@ -0,0 +1,110 @@ +#include "sentry_core.h" +#include "sentry_options.h" +#include "sentry_testsupport.h" +#include + +static volatile bool g_before_crash_called = false; +static volatile int g_before_crash_call_count = 0; +static volatile void *g_before_crash_user_data = NULL; + +static void +test_before_crash_func(void *user_data) +{ + g_before_crash_called = true; + g_before_crash_call_count++; + g_before_crash_user_data = user_data; +} + +static void +reset_before_crash_state(void) +{ + g_before_crash_called = false; + g_before_crash_call_count = 0; + g_before_crash_user_data = NULL; +} + +SENTRY_TEST(before_crash_func_call) +{ + void *user_data = (void *)1; + reset_before_crash_state(); + + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_before_crash(options, test_before_crash_func, user_data); + sentry_options_set_auto_session_tracking(options, false); + + TEST_CHECK_INT_EQUAL(sentry_init(options), 0); + + // Verify before_crash_func is set correctly in options + TEST_CHECK(options->before_crash_func == test_before_crash_func); + TEST_CHECK(options->before_crash_data == user_data); + + // Call the before_crash_func directly to test it + if (options->before_crash_func) { + options->before_crash_func(options->before_crash_data); + } + + sentry_close(); + + // Verify the before_crash_func was called correctly + TEST_CHECK(g_before_crash_called); + TEST_CHECK_INT_EQUAL(g_before_crash_call_count, 1); + TEST_CHECK(g_before_crash_user_data == user_data); +} + +SENTRY_TEST(before_crash_func_not_set) +{ + reset_before_crash_state(); + + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + // Do not set before_crash_func + sentry_options_set_auto_session_tracking(options, false); + + TEST_CHECK_INT_EQUAL(sentry_init(options), 0); + + // Verify before_crash_func is not set + TEST_CHECK(options->before_crash_func == NULL); + + sentry_close(); + + // Verify the before_crash_func was not called + TEST_CHECK(!g_before_crash_called); + TEST_CHECK_INT_EQUAL(g_before_crash_call_count, 0); +} + +SENTRY_TEST(before_crash_func_change) +{ + reset_before_crash_state(); + + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + + // Set initial before_crash_func + void *user_data1 = (void *)2; + sentry_options_set_before_crash( + options, test_before_crash_func, user_data1); + + // Change to different user data + void *user_data2 = (void *)3; + sentry_options_set_before_crash( + options, test_before_crash_func, user_data2); + + sentry_options_set_auto_session_tracking(options, false); + TEST_CHECK_INT_EQUAL(sentry_init(options), 0); + + // Verify the updated user_data is set + TEST_CHECK(options->before_crash_func == test_before_crash_func); + TEST_CHECK(options->before_crash_data == user_data2); + + // Call the before_crash_func to test it + if (options->before_crash_func) { + options->before_crash_func(options->before_crash_data); + } + + sentry_close(); + + // Verify the updated user_data was used + TEST_CHECK(g_before_crash_called); + TEST_CHECK(g_before_crash_user_data == user_data2); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 152bd293c..6a5b17e11 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -21,6 +21,9 @@ XX(basic_spans) XX(basic_tracing_context) XX(basic_transaction) XX(basic_write_envelope_to_file) +XX(before_crash_func_call) +XX(before_crash_func_change) +XX(before_crash_func_not_set) XX(bgworker_flush) XX(breadcrumb_without_type_or_message_still_valid) XX(build_id_parser) @@ -67,6 +70,10 @@ XX(dsn_with_ending_forward_slash_will_be_cleaned) XX(dsn_with_non_http_scheme_is_invalid) XX(dsn_without_project_id_is_invalid) XX(dsn_without_url_scheme_is_invalid) +XX(embedded_info_basic) +XX(embedded_info_disabled) +XX(embedded_info_format) +XX(embedded_info_sentry_version) XX(empty_transport) XX(event_with_id) XX(exception_without_type_or_value_still_valid) @@ -204,7 +211,3 @@ XX(value_unicode) XX(value_user) XX(value_wrong_type) XX(write_raw_envelope_to_file) -XX(embedded_info_basic) -XX(embedded_info_disabled) -XX(embedded_info_format) -XX(embedded_info_sentry_version)