From 6ac1404dc6f46dd6388c3b9f40ca32b585f3ef58 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 27 Jul 2022 17:44:11 +0200 Subject: [PATCH] ref(on_crash): follow-up to make on_crash releasable (#734) * mentioned in the header docs `before_send` vs `on_crash` behavior * adapted `sentry__prepare_event` to provide a flag for backends on whether `before_send` should be invoked * adapted backends to prevent running `before_send` even if `on_crash` returns true * added `on_crash` vs `before_send` integration tests for `inproc` and `breakpad` (`crashpad` will follow) * changed the signature of `on_crash` to be closer to `before_send` * added remaining CHANGELOG entries for upcoming release --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 19 ++++ CONTRIBUTING.md | 4 + examples/example.c | 66 ++++++++++++ include/sentry.h | 46 +++++++-- src/backends/sentry_backend_breakpad.cpp | 12 ++- src/backends/sentry_backend_crashpad.cpp | 59 +++++++++-- src/backends/sentry_backend_inproc.c | 9 +- src/sentry_core.c | 6 +- src/sentry_core.h | 5 +- tests/assertions.py | 23 ++++- tests/test_integration_crashpad.py | 108 +++++++++++++++++++- tests/test_integration_stdout.py | 124 ++++++++++++++++++----- 13 files changed, 418 insertions(+), 65 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e778a7521..bc9919e74 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -136,7 +136,7 @@ jobs: - name: Expose llvm PATH for Mac if: ${{ runner.os == 'macOS' }} - run: echo $(brew --prefix llvm@13)/bin >> $GITHUB_PATH + run: echo $(brew --prefix llvm@14)/bin >> $GITHUB_PATH - name: Installing Android SDK Dependencies if: ${{ env['ANDROID_API'] }} diff --git a/CHANGELOG.md b/CHANGELOG.md index b042fe724..61f92a51e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,31 @@ ## Unreleased +**Features** + +- Provide `on_crash()` callback to allow clients to act on detected crashes. + Users often inquired about distinguishing between crashes and "normal" events in the `before_send()` hook. + `on_crash()` can be considered a replacement for `before_send()` for crash events, where the goal is to use + `before_send()` only for normal events, while `on_crash()` is only invoked for crashes. This change is backward + compatible for current users of `before_send()` and allows gradual migration to `on_crash()` + ([see the docs for details](https://docs.sentry.io/platforms/native/configuration/filtering/)). + ([#724](https://github.com/getsentry/sentry-native/pull/724), + [#734](https://github.com/getsentry/sentry-native/pull/734)) + **Fixes** +- Make Windows ModuleFinder more resilient to missing Debug Info + ([#732](https://github.com/getsentry/sentry-native/pull/732)) - Aligned pre-send event processing in `sentry_capture_event()` with the [cross-SDK session filter order](https://develop.sentry.dev/sdk/sessions/#filter-order) ([#729](https://github.com/getsentry/sentry-native/pull/729)) +**Thank you**: + +Features, fixes and improvements in this release have been contributed by: + +- [@espkk](https://github.com/espkk) + ## 0.4.18 **Features**: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 965683027..b6e81c408 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -140,3 +140,7 @@ The example currently supports the following commends: - `sleep`: Introduces a 10 second sleep. - `add-stacktrace`: Adds the current thread stacktrace to the captured event. - `disable-backend`: Disables the build-configured crash-handler backend. +- `before-send`: Installs a `before_send()` callback that retains the event. +- `discarding-before-send`: Installs a `before_send()` callback that retains the event. +- `on-crash`: Installs an `on_crash()` callback that retains the crash event. +- `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event. diff --git a/examples/example.c b/examples/example.c index 78d91ced5..d5d42cdad 100644 --- a/examples/example.c +++ b/examples/example.c @@ -23,6 +23,54 @@ # define sleep_s(SECONDS) sleep(SECONDS) #endif +static sentry_value_t +before_send_callback(sentry_value_t event, void *hint, void *closure) +{ + (void)hint; + (void)closure; + + // make our mark on the event + sentry_value_set_by_key( + event, "adapted_by", sentry_value_new_string("before_send")); + + // tell the backend to proceed with the event + return event; +} + +static sentry_value_t +discarding_before_send_callback(sentry_value_t event, void *hint, void *closure) +{ + (void)hint; + (void)closure; + + // discard event and signal backend to stop further processing + sentry_value_decref(event); + return sentry_value_new_null(); +} + +static sentry_value_t +discarding_on_crash_callback( + const sentry_ucontext_t *uctx, sentry_value_t event, void *closure) +{ + (void)uctx; + (void)closure; + + // discard event and signal backend to stop further processing + sentry_value_decref(event); + return sentry_value_new_null(); +} + +static sentry_value_t +on_crash_callback( + const sentry_ucontext_t *uctx, sentry_value_t event, void *closure) +{ + (void)uctx; + (void)closure; + + // tell the backend to retain the event + return event; +} + static void print_envelope(sentry_envelope_t *envelope, void *unused_state) { @@ -105,6 +153,24 @@ main(int argc, char **argv) sentry_options_set_max_spans(options, 5); } + if (has_arg(argc, argv, "before-send")) { + sentry_options_set_before_send(options, before_send_callback, NULL); + } + + if (has_arg(argc, argv, "discarding-before-send")) { + sentry_options_set_before_send( + options, discarding_before_send_callback, NULL); + } + + if (has_arg(argc, argv, "on-crash")) { + sentry_options_set_on_crash(options, on_crash_callback, NULL); + } + + if (has_arg(argc, argv, "discarding-on-crash")) { + sentry_options_set_on_crash( + options, discarding_on_crash_callback, NULL); + } + sentry_init(options); if (!has_arg(argc, argv, "no-setup")) { diff --git a/include/sentry.h b/include/sentry.h index 8032a3625..442e30b72 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -86,7 +86,6 @@ extern "C" { #include #include -#include #include /* context type dependencies */ @@ -771,6 +770,11 @@ SENTRY_API void sentry_options_set_transport( * call `sentry_value_decref` on the provided event, and return a * `sentry_value_new_null()` instead. * + * 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 + * in client-side pre-processing. + * * 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 @@ -798,20 +802,48 @@ SENTRY_API void sentry_options_set_before_send( /** * 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. + * The `on_crash` callback replaces the `before_send` callback for crash events. + * The interface is analogous to `before_send` in that the callback takes + * ownership of the `event`, and should usually return that same event. In case + * the event should be discarded, the callback needs to call + * `sentry_value_decref` on the provided event, and return a + * `sentry_value_new_null()` instead. * - * If the callback returns false outgoing crash report will be discarded. + * Only the `inproc` backend currently fills the passed-in event with useful + * data and processes any modifications to the return value. Since both + * `breakpad` and `crashpad` use minidumps to capture the crash state, the + * passed-in event is empty when using these backends, and they ignore any + * changes to the return value. + * + * If you set this callback in the options, it prevents a concurrently enabled + * `before_send` callback from being invoked in the crash case. This allows for + * better differentiation between crashes and other events and gradual migration + * from existing `before_send` implementations: + * + * - if you have a `before_send` implementation and do not define an `on_crash` + * callback your application will receive both normal and crash events as + * before + * - if you have a `before_send` implementation but only want to handle normal + * events with it, then you can define an `on_crash` callback that returns + * the passed-in event and does nothing else + * - if you are not interested in normal events, but only want to act on + * crashes (within the limits mentioned below), then only define an + * `on_crash` callback with the option to filter (on all backends) or enrich + * (only inproc) the crash event * * 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 + * + * Platform-specific behavior: + * + * - does not work with crashpad on macOS. + * - for breakpad on Linux the `uctx` parameter is always NULL. */ -typedef bool (*sentry_crash_function_t)( - const sentry_ucontext_t *uctx, void *closure); +typedef sentry_value_t (*sentry_crash_function_t)( + const sentry_ucontext_t *uctx, sentry_value_t event, void *closure); /** * Sets the `on_crash` callback. diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 07d64a0bf..1c22e43e9 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -91,6 +91,7 @@ sentry__breakpad_backend_callback( #else dump_path = sentry__path_new(descriptor.path()); #endif + sentry_value_t event = sentry_value_new_event(); SENTRY_WITH_OPTIONS (options) { sentry__write_crash_marker(options); @@ -107,14 +108,14 @@ sentry__breakpad_backend_callback( #endif SENTRY_TRACE("invoking `on_crash` hook"); - should_handle - = options->on_crash_func(uctx, options->on_crash_data); + sentry_value_t result + = options->on_crash_func(uctx, event, options->on_crash_data); + should_handle = !sentry_value_is_null(result); } if (should_handle) { - sentry_value_t event = sentry_value_new_event(); - sentry_envelope_t *envelope - = sentry__prepare_event(options, event, NULL); + sentry_envelope_t *envelope = sentry__prepare_event( + options, event, nullptr, !options->on_crash_func); // the event we just prepared is empty, // so no error is recorded for it sentry__record_errors_on_current_session(1); @@ -152,6 +153,7 @@ sentry__breakpad_backend_callback( sentry__path_free(dump_path); } else { SENTRY_TRACE("event was discarded by the `on_crash` hook"); + sentry_value_decref(event); } // after capturing the crash event, try to dump all the in-flight diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index bf8e55a62..5bbafe876 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -43,6 +43,7 @@ extern "C" { extern "C" { #ifdef SENTRY_PLATFORM_LINUX +# include # define SIGNAL_STACK_SIZE 65536 static stack_t g_signal_stack; @@ -133,10 +134,10 @@ sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) # endif SENTRY_DEBUG("flushing session and queue before crashpad handler"); - bool should_handle = true; + bool should_dump = true; + sentry_value_t event = sentry_value_new_event(); SENTRY_WITH_OPTIONS (options) { - sentry__write_crash_marker(options); if (options->on_crash_func) { sentry_ucontext_t uctx; @@ -149,17 +150,19 @@ sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) # endif SENTRY_TRACE("invoking `on_crash` hook"); - should_handle - = options->on_crash_func(&uctx, options->on_crash_data); + event + = options->on_crash_func(&uctx, event, 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, nullptr, options->before_send_data); - sentry_value_decref(event); } + should_dump = !sentry_value_is_null(event); + sentry_value_decref(event); + + if (should_dump) { + sentry__write_crash_marker(options); - if (should_handle) { sentry__record_errors_on_current_session(1); sentry_session_t *session = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); @@ -175,9 +178,8 @@ sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) sentry_transport_free(disk_transport); } } else { - SENTRY_TRACE("event was discarded by the `on_crash` hook"); + SENTRY_TRACE("event was discarded"); } - sentry__transport_dump_queue(options->transport, options->run); } @@ -185,8 +187,43 @@ sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) # ifndef SENTRY_PLATFORM_WINDOWS sentry__leave_signal_handler(); # endif - // further handling can be skipped via on_crash hook - return !should_handle; + + // If we __don't__ want a minidump produced by crashpad we need to either + // exit or longjmp at this point. The crashpad client handler which calls + // back here (SetFirstChanceExceptionHandler) does the same if the + // application is not shutdown via the crashpad_handler process. + // + // If we would return `true` here without changing any of the global signal- + // handling state or rectifying the cause of the signal, this would turn + // into a signal-handler/exception-filter loop, because some + // signals/exceptions (like SIGSEGV) are unrecoverable. + // + // Ideally the SetFirstChanceExceptionHandler would accept more than a + // boolean to differentiate between: + // + // * we accept our fate and want a minidump (currently returning `false`) + // * we accept our fate and don't want a minidump (currently not available) + // * we rectified the situation, so crashpads signal-handler can simply + // return, thereby letting the not-rectified signal-cause trigger a + // signal-handler/exception-filter again, which probably leads to us + // (currently returning `true`) + // + // TODO(supervacuus): + // * we need integration tests for more signal/exception types not only + // for unmapped memory access (which is the current crash in example.c). + // * we should adapt the SetFirstChanceExceptionHandler interface in + // crashpad + if (!should_dump) { +# ifdef SENTRY_PLATFORM_WINDOWS + // TerminateProcess(GetCurrentProcess(), kTerminationCodeCrashNoDump); + TerminateProcess(GetCurrentProcess(), 1); +# else + _exit(EXIT_FAILURE); +# endif + } + + // we did not "handle" the signal, so crashpad should do that. + return false; } #endif diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 9e6e9ed7c..32887a033 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -530,13 +530,13 @@ handle_ucontext(const sentry_ucontext_t *uctx) if (options->on_crash_func) { SENTRY_TRACE("invoking `on_crash` hook"); - should_handle - = options->on_crash_func(uctx, options->on_crash_data); + event = options->on_crash_func(uctx, event, options->on_crash_data); + should_handle = !sentry_value_is_null(event); } if (should_handle) { - sentry_envelope_t *envelope - = sentry__prepare_event(options, event, NULL); + sentry_envelope_t *envelope = sentry__prepare_event( + options, event, NULL, !options->on_crash_func); // TODO(tracing): Revisit when investigating transaction flushing // during hard crashes. @@ -552,6 +552,7 @@ handle_ucontext(const sentry_ucontext_t *uctx) sentry_transport_free(disk_transport); } else { SENTRY_TRACE("event was discarded by the `on_crash` hook"); + sentry_value_decref(event); } // after capturing the crash event, dump all the envelopes to disk diff --git a/src/sentry_core.c b/src/sentry_core.c index e05751c08..1a27fe06c 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -411,7 +411,7 @@ sentry__capture_event(sentry_value_t event) if (sentry__event_is_transaction(event)) { envelope = sentry__prepare_transaction(options, event, &event_id); } else { - envelope = sentry__prepare_event(options, event, &event_id); + envelope = sentry__prepare_event(options, event, &event_id, true); } if (envelope) { if (options->session) { @@ -460,7 +460,7 @@ sentry__should_send_transaction(sentry_value_t tx_cxt) sentry_envelope_t * sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, - sentry_uuid_t *event_id) + sentry_uuid_t *event_id, bool invoke_before_send) { sentry_envelope_t *envelope = NULL; @@ -477,7 +477,7 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry__scope_apply_to_event(scope, options, event, mode); } - if (options->before_send_func) { + if (options->before_send_func && invoke_before_send) { SENTRY_TRACE("invoking `before_send` hook"); event = options->before_send_func(event, NULL, options->before_send_data); diff --git a/src/sentry_core.h b/src/sentry_core.h index 18037fbf0..6ee6c387e 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -40,9 +40,8 @@ bool sentry__event_is_transaction(sentry_value_t event); * being passed in is not a transaction. * * More specifically, it will do the following things: - * - sample the event, possibly discarding it, * - apply the scope to it, - * - call the before_send hook on 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 @@ -51,7 +50,7 @@ bool sentry__event_is_transaction(sentry_value_t event); * `event_id` out-parameter. */ sentry_envelope_t *sentry__prepare_event(const sentry_options_t *options, - sentry_value_t event, sentry_uuid_t *event_id); + sentry_value_t event, sentry_uuid_t *event_id, bool invoke_before_send); /** * Sends a sentry event, regardless of its type. diff --git a/tests/assertions.py b/tests/assertions.py index c5d3174cb..491f2e31c 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -1,11 +1,11 @@ import datetime import email import gzip -import sys import platform import re -from .conditions import is_android +import sys +from .conditions import is_android VERSION_RE = re.compile(r"(\d+\.\d+\.\d+)(?:[-\.]?)(.*)") @@ -194,6 +194,25 @@ def assert_crash(envelope): assert_stacktrace(envelope, inside_exception=True, check_size=False) +def assert_crash_timestamp(has_files, tmp_path): + # The crash file should survive a `sentry_init` and should still be there + # even after restarts. + if has_files: + with open("{}/.sentry-native/last_crash".format(tmp_path)) as f: + crash_timestamp = f.read() + assert_timestamp(crash_timestamp) + + +def assert_before_send(envelope): + event = envelope.get_event() + assert_matches(event, {"adapted_by": "before_send"}) + + +def assert_no_before_send(envelope): + event = envelope.get_event() + assert ("adapted_by", "before_send") not in event.items() + + def assert_crashpad_upload(req): multipart = gzip.decompress(req.get_data()) msg = email.message_from_bytes(bytes(str(req.headers), encoding="utf8") + multipart) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index cd7363bed..8a5651edb 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -46,7 +46,7 @@ def test_crashpad_reinstall(cmake, httpserver): assert len(httpserver.log) == 1 -def test_crashpad_crash(cmake, httpserver): +def run_crashpad_dumping_crash(cmake, httpserver, custom_args): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) @@ -57,7 +57,8 @@ def test_crashpad_crash(cmake, httpserver): child = run( tmp_path, "sentry_example", - ["log", "start-session", "attachment", "overflow-breadcrumbs", "crash"], + ["log", "start-session", "attachment", "overflow-breadcrumbs", "crash"] + + custom_args, env=env, ) assert child.returncode # well, its a crash after all @@ -78,9 +79,112 @@ def test_crashpad_crash(cmake, httpserver): else (outputs[1].get_data(), outputs[0]) ) + return session, multipart + + +def run_crashpad_non_dumping_crash(cmake, httpserver, custom_args): + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + with httpserver.wait(timeout=5, raise_assertions=False) as waiting: + child = run( + tmp_path, + "sentry_example", + [ + "log", + "start-session", + "attachment", + "overflow-breadcrumbs", + "crash", + ] + + custom_args, + env=env, + ) + assert child.returncode # well, its a crash after all + + assert waiting.result is False + + # the session crash heuristic on mac uses timestamps, so make sure we have + # a small delay here + time.sleep(1) + + run(tmp_path, "sentry_example", ["log", "no-setup"], check=True, env=env) + + assert len(httpserver.log) == 1 + output = httpserver.log[0][0] + session = output.get_data() + + return session + + +def test_crashpad_crash(cmake, httpserver): + session, multipart = run_crashpad_dumping_crash(cmake, httpserver, []) + envelope = Envelope.deserialize(session) + assert_session(envelope, {"status": "crashed", "errors": 1}) + assert_crashpad_upload(multipart) + +@pytest.mark.skipif( + sys.platform == "darwin", + reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", +) +def test_crashpad_crash_before_send(cmake, httpserver): + session, multipart = run_crashpad_dumping_crash(cmake, httpserver, ["before-send"]) + + envelope = Envelope.deserialize(session) + + assert_session(envelope, {"status": "crashed", "errors": 1}) + assert_crashpad_upload(multipart) + + # TODO(supervacuus): we would expect to see a change coming from the + # before_send() hook, but in contrast to the other backends the crashpad + # backend currently only checks for null-value as a signal not to produce + # a minidump and after this decrefs the event. + + +@pytest.mark.skipif( + sys.platform == "darwin", + reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", +) +def test_crashpad_crash_discarding_before_send(cmake, httpserver): + session = run_crashpad_non_dumping_crash( + cmake, httpserver, ["discarding-before-send"] + ) + + envelope = Envelope.deserialize(session) + + assert_session(envelope, {"status": "abnormal", "errors": 0}) + + +@pytest.mark.skipif( + sys.platform == "darwin", + reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", +) +def test_crashpad_crash_discarding_on_crash(cmake, httpserver): + session = run_crashpad_non_dumping_crash(cmake, httpserver, ["discarding-on-crash"]) + + envelope = Envelope.deserialize(session) + + assert_session(envelope, {"status": "abnormal", "errors": 0}) + + +@pytest.mark.skipif( + sys.platform == "darwin", + reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", +) +def test_crashpad_crash_discarding_before_send_and_on_crash(cmake, httpserver): + session, multipart = run_crashpad_dumping_crash( + cmake, httpserver, ["discarding-before-send", "on-crash"] + ) + + envelope = Envelope.deserialize(session) + + # on_crash() is defined and overrules the discarding before_send() + assert_session(envelope, {"status": "crashed", "errors": 1}) assert_crashpad_upload(multipart) diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py index 710dea63a..ecb2c6f95 100644 --- a/tests/test_integration_stdout.py +++ b/tests/test_integration_stdout.py @@ -1,10 +1,11 @@ -import pytest +import os import subprocess import sys -import os import time + +import pytest + from . import check_output, run, Envelope -from .conditions import has_breakpad, has_files from .assertions import ( assert_attachment, assert_meta, @@ -14,8 +15,11 @@ assert_crash, assert_minidump, assert_timestamp, - assert_session, + assert_before_send, + assert_no_before_send, + assert_crash_timestamp, ) +from .conditions import has_breakpad, has_files def test_capture_stdout(cmake): @@ -111,52 +115,118 @@ def test_multi_process(cmake): assert len(runs) == 0 -def test_inproc_crash_stdout(cmake): +def run_crash_stdout_for(backend, cmake, example_args): tmp_path = cmake( ["sentry_example"], - {"SENTRY_BACKEND": "inproc", "SENTRY_TRANSPORT": "none"}, + {"SENTRY_BACKEND": backend, "SENTRY_TRANSPORT": "none"}, ) - child = run(tmp_path, "sentry_example", ["attachment", "crash"]) - assert child.returncode # well, its a crash after all + child = run(tmp_path, "sentry_example", ["attachment", "crash"] + example_args) + assert child.returncode # well, it's a crash after all + + return tmp_path, check_output(tmp_path, "sentry_example", ["stdout", "no-setup"]) + + +def test_inproc_crash_stdout(cmake): + tmp_path, output = run_crash_stdout_for("inproc", cmake, []) - output = check_output(tmp_path, "sentry_example", ["stdout", "no-setup"]) envelope = Envelope.deserialize(output) - # The crash file should survive a `sentry_init` and should still be there - # even after restarts. - if has_files: - with open("{}/.sentry-native/last_crash".format(tmp_path)) as f: - crash_timestamp = f.read() - assert_timestamp(crash_timestamp) + assert_crash_timestamp(has_files, tmp_path) + assert_meta(envelope, integration="inproc") + assert_breadcrumb(envelope) + assert_attachment(envelope) + assert_crash(envelope) + + +def test_inproc_crash_stdout_before_send(cmake): + tmp_path, output = run_crash_stdout_for("inproc", cmake, ["before-send"]) + + envelope = Envelope.deserialize(output) + assert_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) + assert_crash(envelope) + assert_before_send(envelope) + + +def test_inproc_crash_stdout_discarding_on_crash(cmake): + tmp_path, output = run_crash_stdout_for("inproc", cmake, ["discarding-on-crash"]) + + # since the on_crash() handler discards further processing we expect an empty response + assert len(output) == 0 + + assert_crash_timestamp(has_files, tmp_path) + + +def test_inproc_crash_stdout_before_send_and_on_crash(cmake): + tmp_path, output = run_crash_stdout_for( + "inproc", cmake, ["before-send", "on-crash"] + ) + + # the on_crash() hook retains the event + envelope = Envelope.deserialize(output) + # but we expect no event modification from before_send() since setting on_crash() disables before_send() + assert_no_before_send(envelope) + assert_crash_timestamp(has_files, tmp_path) + assert_meta(envelope, integration="inproc") + assert_breadcrumb(envelope) + assert_attachment(envelope) assert_crash(envelope) @pytest.mark.skipif(not has_breakpad, reason="test needs breakpad backend") def test_breakpad_crash_stdout(cmake): - tmp_path = cmake( - ["sentry_example"], - {"SENTRY_BACKEND": "breakpad", "SENTRY_TRANSPORT": "none"}, - ) + tmp_path, output = run_crash_stdout_for("breakpad", cmake, []) - child = run(tmp_path, "sentry_example", ["attachment", "crash"]) - assert child.returncode # well, its a crash after all + envelope = Envelope.deserialize(output) + + assert_crash_timestamp(has_files, tmp_path) + assert_meta(envelope, integration="breakpad") + assert_breadcrumb(envelope) + assert_attachment(envelope) + assert_minidump(envelope) - if has_files: - with open("{}/.sentry-native/last_crash".format(tmp_path)) as f: - crash_timestamp = f.read() - assert_timestamp(crash_timestamp) - output = check_output(tmp_path, "sentry_example", ["stdout", "no-setup"]) +@pytest.mark.skipif(not has_breakpad, reason="test needs breakpad backend") +def test_breakpad_crash_stdout_before_send(cmake): + tmp_path, output = run_crash_stdout_for("breakpad", cmake, ["before-send"]) + envelope = Envelope.deserialize(output) + assert_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) assert_attachment(envelope) - assert_minidump(envelope) + assert_before_send(envelope) + + +@pytest.mark.skipif(not has_breakpad, reason="test needs breakpad backend") +def test_breakpad_crash_stdout_discarding_on_crash(cmake): + tmp_path, output = run_crash_stdout_for("breakpad", cmake, ["discarding-on-crash"]) + + # since the on_crash() handler discards further processing we expect an empty response + assert len(output) == 0 + + assert_crash_timestamp(has_files, tmp_path) + + +@pytest.mark.skipif(not has_breakpad, reason="test needs breakpad backend") +def test_breakpad_crash_stdout_before_send_and_on_crash(cmake): + tmp_path, output = run_crash_stdout_for( + "breakpad", cmake, ["before-send", "on-crash"] + ) + + # the on_crash() hook retains the event + envelope = Envelope.deserialize(output) + # but we expect no event modification from before_send() since setting on_crash() disables before_send() + assert_no_before_send(envelope) + + assert_crash_timestamp(has_files, tmp_path) + assert_meta(envelope, integration="breakpad") + assert_breadcrumb(envelope) + assert_attachment(envelope)