From 7bbce0d16d5cff37f5ec5b54e8cf2b5027b12dc3 Mon Sep 17 00:00:00 2001 From: espkk Date: Sun, 12 Jun 2022 14:17:23 +0300 Subject: [PATCH] feat: invoke on_crash when handling a crash - don't invoke before_send if on_crash is set - if on_crash returns false crash report will be discarded --- include/sentry.h | 27 ++++++++ src/backends/sentry_backend_breakpad.cpp | 88 +++++++++++++++--------- src/backends/sentry_backend_crashpad.cpp | 64 ++++++++++------- src/backends/sentry_backend_inproc.c | 42 +++++++---- src/sentry_options.c | 8 +++ src/sentry_options.h | 2 + 6 files changed, 160 insertions(+), 71 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 3d33baad2..8032a3625 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -86,6 +86,7 @@ extern "C" { #include #include +#include #include /* context type dependencies */ @@ -794,6 +795,32 @@ 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 *data); +/** + * Type of the `on_crash` callback. + * + * Does not work with crashpad on macOS. + * The callback passes a pointer to sentry_ucontext_s structure when exception + * handler is invoked. For breakpad on Linux this pointer is NULL. + * + * If the callback returns false outgoing crash report will be discarded. + * + * 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`, see + * the documentation on SEH (structured exception handling) for more information + * https://docs.microsoft.com/en-us/windows/win32/debug/structured-exception-handling + */ +typedef bool (*sentry_crash_function_t)( + const sentry_ucontext_t *uctx, void *closure); + +/** + * Sets the `on_crash` callback. + * + * See the `sentry_crash_function_t` typedef above for more information. + */ +SENTRY_API void sentry_options_set_on_crash( + sentry_options_t *opts, sentry_crash_function_t func, void *data); + /** * Sets the DSN. */ diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index eaf596d17..07d64a0bf 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -40,7 +40,7 @@ extern "C" { static bool sentry__breakpad_backend_callback(const wchar_t *breakpad_dump_path, const wchar_t *minidump_id, void *UNUSED(context), - EXCEPTION_POINTERS *UNUSED(exinfo), MDRawAssertionInfo *UNUSED(assertion), + EXCEPTION_POINTERS *exinfo, MDRawAssertionInfo *UNUSED(assertion), bool succeeded) #elif defined(SENTRY_PLATFORM_DARWIN) static bool @@ -95,42 +95,64 @@ sentry__breakpad_backend_callback( SENTRY_WITH_OPTIONS (options) { sentry__write_crash_marker(options); - sentry_value_t event = sentry_value_new_event(); - sentry_envelope_t *envelope - = sentry__prepare_event(options, event, NULL); - // the event we just prepared is empty, so no error is recorded for it - sentry__record_errors_on_current_session(1); - sentry_session_t *session = sentry__end_current_session_with_status( - SENTRY_SESSION_STATUS_CRASHED); - sentry__envelope_add_session(envelope, session); - - // the minidump is added as an attachment, with type `event.minidump` - sentry_envelope_item_t *item - = sentry__envelope_add_from_path(envelope, dump_path, "attachment"); - if (item) { - sentry__envelope_item_set_header(item, "attachment_type", - sentry_value_new_string("event.minidump")); - - sentry__envelope_item_set_header(item, "filename", + bool should_handle = true; + + if (options->on_crash_func) { + sentry_ucontext_t *uctx = nullptr; + #ifdef SENTRY_PLATFORM_WINDOWS - sentry__value_new_string_from_wstr( -#else - sentry_value_new_string( + sentry_ucontext_t uctx_data; + uctx_data.exception_ptrs = *exinfo; + uctx = &uctx_data; #endif - sentry__path_filename(dump_path))); + + SENTRY_TRACE("invoking `on_crash` hook"); + should_handle + = options->on_crash_func(uctx, options->on_crash_data); } - // capture the envelope with the disk transport - sentry_transport_t *disk_transport - = sentry_new_disk_transport(options->run); - sentry__capture_envelope(disk_transport, envelope); - sentry__transport_dump_queue(disk_transport, options->run); - sentry_transport_free(disk_transport); - - // now that the envelope was written, we can remove the temporary - // minidump file - sentry__path_remove(dump_path); - sentry__path_free(dump_path); + if (should_handle) { + sentry_value_t event = sentry_value_new_event(); + sentry_envelope_t *envelope + = sentry__prepare_event(options, event, NULL); + // the event we just prepared is empty, + // so no error is recorded for it + sentry__record_errors_on_current_session(1); + sentry_session_t *session = sentry__end_current_session_with_status( + SENTRY_SESSION_STATUS_CRASHED); + sentry__envelope_add_session(envelope, session); + + // the minidump is added as an attachment, + // with type `event.minidump` + sentry_envelope_item_t *item = sentry__envelope_add_from_path( + envelope, dump_path, "attachment"); + if (item) { + sentry__envelope_item_set_header(item, "attachment_type", + sentry_value_new_string("event.minidump")); + + sentry__envelope_item_set_header(item, "filename", +#ifdef SENTRY_PLATFORM_WINDOWS + sentry__value_new_string_from_wstr( +#else + sentry_value_new_string( +#endif + sentry__path_filename(dump_path))); + } + + // capture the envelope with the disk transport + sentry_transport_t *disk_transport + = sentry_new_disk_transport(options->run); + sentry__capture_envelope(disk_transport, envelope); + sentry__transport_dump_queue(disk_transport, options->run); + sentry_transport_free(disk_transport); + + // now that the envelope was written, we can remove the temporary + // minidump file + sentry__path_remove(dump_path); + sentry__path_free(dump_path); + } else { + SENTRY_TRACE("event was discarded by the `on_crash` hook"); + } // 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 e65d6f235..bf8e55a62 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -122,42 +122,60 @@ sentry__crashpad_backend_flush_scope( #if defined(SENTRY_PLATFORM_LINUX) || defined(SENTRY_PLATFORM_WINDOWS) # ifdef SENTRY_PLATFORM_WINDOWS static bool -sentry__crashpad_handler(EXCEPTION_POINTERS *UNUSED(ExceptionInfo)) +sentry__crashpad_handler(EXCEPTION_POINTERS *ExceptionInfo) { # else static bool -sentry__crashpad_handler(int UNUSED(signum), siginfo_t *UNUSED(info), - ucontext_t *UNUSED(user_context)) +sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) { sentry__page_allocator_enable(); sentry__enter_signal_handler(); # endif SENTRY_DEBUG("flushing session and queue before crashpad handler"); + bool should_handle = true; + SENTRY_WITH_OPTIONS (options) { sentry__write_crash_marker(options); - sentry_value_t event = sentry_value_new_event(); - if (options->before_send_func) { + if (options->on_crash_func) { + sentry_ucontext_t uctx; +# ifdef SENTRY_PLATFORM_WINDOWS + uctx.exception_ptrs = *ExceptionInfo; +# else + uctx.signum = signum; + uctx.siginfo = info; + uctx.user_context = user_context; +# endif + + SENTRY_TRACE("invoking `on_crash` hook"); + should_handle + = options->on_crash_func(&uctx, options->on_crash_data); + } else if (options->before_send_func) { + sentry_value_t event = sentry_value_new_event(); SENTRY_TRACE("invoking `before_send` hook"); event = options->before_send_func( - event, NULL, options->before_send_data); + event, nullptr, options->before_send_data); + sentry_value_decref(event); } - sentry_value_decref(event); - - sentry__record_errors_on_current_session(1); - sentry_session_t *session = sentry__end_current_session_with_status( - SENTRY_SESSION_STATUS_CRASHED); - if (session) { - sentry_envelope_t *envelope = sentry__envelope_new(); - sentry__envelope_add_session(envelope, session); - - // capture the envelope with the disk transport - sentry_transport_t *disk_transport - = sentry_new_disk_transport(options->run); - sentry__capture_envelope(disk_transport, envelope); - sentry__transport_dump_queue(disk_transport, options->run); - sentry_transport_free(disk_transport); + + if (should_handle) { + sentry__record_errors_on_current_session(1); + sentry_session_t *session = sentry__end_current_session_with_status( + SENTRY_SESSION_STATUS_CRASHED); + if (session) { + sentry_envelope_t *envelope = sentry__envelope_new(); + sentry__envelope_add_session(envelope, session); + + // capture the envelope with the disk transport + sentry_transport_t *disk_transport + = sentry_new_disk_transport(options->run); + sentry__capture_envelope(disk_transport, envelope); + sentry__transport_dump_queue(disk_transport, options->run); + sentry_transport_free(disk_transport); + } + } else { + SENTRY_TRACE("event was discarded by the `on_crash` hook"); } sentry__transport_dump_queue(options->transport, options->run); @@ -167,8 +185,8 @@ sentry__crashpad_handler(int UNUSED(signum), siginfo_t *UNUSED(info), # ifndef SENTRY_PLATFORM_WINDOWS sentry__leave_signal_handler(); # endif - // we did not "handle" the signal, so crashpad should do that. - return false; + // further handling can be skipped via on_crash hook + return !should_handle; } #endif diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 0c5297b32..9e6e9ed7c 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -526,21 +526,33 @@ handle_ucontext(const sentry_ucontext_t *uctx) SENTRY_WITH_OPTIONS (options) { sentry__write_crash_marker(options); - sentry_envelope_t *envelope - = sentry__prepare_event(options, event, NULL); - // TODO(tracing): Revisit when investigating transaction flushing during - // hard crashes. - - sentry_session_t *session = sentry__end_current_session_with_status( - SENTRY_SESSION_STATUS_CRASHED); - sentry__envelope_add_session(envelope, session); - - // capture the envelope with the disk transport - sentry_transport_t *disk_transport - = sentry_new_disk_transport(options->run); - sentry__capture_envelope(disk_transport, envelope); - sentry__transport_dump_queue(disk_transport, options->run); - sentry_transport_free(disk_transport); + bool should_handle = true; + + if (options->on_crash_func) { + SENTRY_TRACE("invoking `on_crash` hook"); + should_handle + = options->on_crash_func(uctx, options->on_crash_data); + } + + if (should_handle) { + sentry_envelope_t *envelope + = sentry__prepare_event(options, event, NULL); + // TODO(tracing): Revisit when investigating transaction flushing + // during hard crashes. + + sentry_session_t *session = sentry__end_current_session_with_status( + SENTRY_SESSION_STATUS_CRASHED); + sentry__envelope_add_session(envelope, session); + + // capture the envelope with the disk transport + sentry_transport_t *disk_transport + = sentry_new_disk_transport(options->run); + sentry__capture_envelope(disk_transport, envelope); + sentry__transport_dump_queue(disk_transport, options->run); + sentry_transport_free(disk_transport); + } else { + SENTRY_TRACE("event was discarded by the `on_crash` hook"); + } // after capturing the crash event, dump all the envelopes to disk sentry__transport_dump_queue(options->transport, options->run); diff --git a/src/sentry_options.c b/src/sentry_options.c index 7588ad977..b83c304ab 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -119,6 +119,14 @@ sentry_options_set_before_send( opts->before_send_data = data; } +void +sentry_options_set_on_crash( + sentry_options_t *opts, sentry_crash_function_t func, void *data) +{ + opts->on_crash_func = func; + opts->on_crash_data = data; +} + void sentry_options_set_dsn(sentry_options_t *opts, const char *raw_dsn) { diff --git a/src/sentry_options.h b/src/sentry_options.h index 1ec3817c7..c6b3c10dc 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -54,6 +54,8 @@ typedef struct sentry_options_s { sentry_transport_t *transport; sentry_event_function_t before_send_func; void *before_send_data; + sentry_crash_function_t on_crash_func; + void *on_crash_data; /* Experimentally exposed */ double traces_sample_rate;