diff --git a/CHANGELOG.md b/CHANGELOG.md index b86ba9199..fae4610d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,18 @@ ## Unreleased +**Breaking / Important behavior changes**: + +- `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)) + **Features**: - Add `sentry_event_set_level` for setting the level of an individual event. ([#2038](https://github.com/getsentry/sentry-native/pull/2038)) +**Deprecations**: + +- Deprecate `sentry_clear_crashed_last_run()` because `sentry_init()` now consumes the marker automatically. ([#2023](https://github.com/getsentry/sentry-native/pull/2023)) + ## 0.16.5 **Important behavior changes**: diff --git a/include/sentry.h b/include/sentry.h index f4e607566..cfa53f5d4 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -4321,7 +4321,7 @@ SENTRY_API void sentry_transaction_iter_headers(sentry_transaction_t *tx, * * Notes: * * The underlying value is set by sentry_init() - it must be called first. - * * Call sentry_clear_crashed_last_run() to reset for the next app run. + * * sentry_init() clears the persisted value for the next run. * * Possible return values: * 1 = the last run was a crash @@ -4331,9 +4331,7 @@ SENTRY_API void sentry_transaction_iter_headers(sentry_transaction_t *tx, SENTRY_EXPERIMENTAL_API int sentry_get_crashed_last_run(void); /** - * Clear the status of the "crashed-last-run". You should explicitly call - * this after sentry_init() if you're using sentry_get_crashed_last_run(). - * Otherwise, the same information is reported on any subsequent runs. + * Clear the persisted status of the "crashed-last-run". * * Notes: * * This doesn't change the value of sentry_get_crashed_last_run() yet. @@ -4342,6 +4340,7 @@ SENTRY_EXPERIMENTAL_API int sentry_get_crashed_last_run(void); * * Returns 0 on success, 1 on error. */ +SENTRY_DEPRECATED("The crash marker is cleared by `sentry_init()`.") SENTRY_EXPERIMENTAL_API int sentry_clear_crashed_last_run(void); /** diff --git a/ndk/lib/src/main/jni/sentry.c b/ndk/lib/src/main/jni/sentry.c index 30ecb6d55..a3d6445ac 100644 --- a/ndk/lib/src/main/jni/sentry.c +++ b/ndk/lib/src/main/jni/sentry.c @@ -377,6 +377,9 @@ send_envelope(sentry_envelope_t *envelope, void *data) // sentry_backend.h extern void sentry__backend_preload(void); +// sentry_database.h +extern void sentry__retain_crash_marker(sentry_options_t *options); + JNIEXPORT void JNICALL Java_io_sentry_ndk_SentryNdk_preloadSentryNative(JNIEnv *env, jclass cls) { @@ -436,6 +439,10 @@ Java_io_sentry_ndk_SentryNdk_initSentryNative( options = sentry_options_new(); ENSURE_OR_FAIL(options); + // Android SDK needs to retain the marker timestamp for session finalization + // and cleans it up when appropriate + sentry__retain_crash_marker(options); + // session tracking is enabled by default, but the Android SDK already // handles it sentry_options_set_auto_session_tracking(options, 0); diff --git a/src/sentry_core.c b/src/sentry_core.c index 9ed122b59..fd90c5b32 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -271,6 +271,9 @@ sentry_init(sentry_options_t *options) } g_last_crash = sentry__has_crash_marker(options); + if (g_last_crash && !options->retain_crash_marker) { + sentry__clear_crash_marker(options); + } g_options = options; // *after* setting the global options, trigger a scope and consent flush, diff --git a/src/sentry_database.c b/src/sentry_database.c index 45ff4b327..695c9f0b3 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -1172,6 +1172,12 @@ sentry__has_crash_marker(const sentry_options_t *options) return result; } +void +sentry__retain_crash_marker(sentry_options_t *options) +{ + options->retain_crash_marker = true; +} + bool sentry__clear_crash_marker(const sentry_options_t *options) { diff --git a/src/sentry_database.h b/src/sentry_database.h index ccd85434d..967509212 100644 --- a/src/sentry_database.h +++ b/src/sentry_database.h @@ -220,6 +220,13 @@ bool sentry__write_crash_marker(const sentry_options_t *options); */ bool sentry__has_crash_marker(const sentry_options_t *options); +/** + * Prevents sentry_init() from clearing `/last_crash`. + * + * Exported for the Android NDK integration. + */ +SENTRY_API void sentry__retain_crash_marker(sentry_options_t *options); + /** * This will remove the `/last_crash` file. */ diff --git a/src/sentry_options.h b/src/sentry_options.h index 9a274188c..0e88aebd5 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -102,6 +102,7 @@ struct sentry_options_s { sentry_session_t *session; sentry_integration_t **integrations; size_t num_integrations; + bool retain_crash_marker; long refcount; uint64_t shutdown_timeout; diff --git a/tests/assertions.py b/tests/assertions.py index c78584633..7071aca90 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -477,14 +477,17 @@ def assert_native_crash(envelope, exception_code=None): 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_no_crash_timestamp(has_files, tmp_path): + if has_files: + assert not (Path(tmp_path) / ".sentry-native" / "last_crash").exists() + + def assert_before_send(envelope): event = envelope.get_event() assert_matches(event, {"adapted_by": "before_send"}) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 8b5cbee64..bc046d054 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -21,7 +21,7 @@ lib_name, REPLAY_ID, ) -from .conditions import has_crashpad, has_oom +from .conditions import has_crashpad, has_files, has_oom from .proxy import ( setup_proxy_env_vars, cleanup_proxy_env_vars, @@ -30,6 +30,8 @@ ) from .assertions import ( assert_breadcrumb, + assert_crash_timestamp, + assert_no_crash_timestamp, assert_crashpad_upload, assert_meta, assert_minidump, @@ -100,6 +102,9 @@ def test_crashpad_on_crashed_last_run(cmake): stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) + # no first-chance handler nor crash marker on macOS + if sys.platform != "darwin": + assert_crash_timestamp(has_files, tmp_path) assert not list((tmp_path / ".sentry-native").glob("*.run/*.crash")) @@ -110,6 +115,9 @@ def test_crashpad_on_crashed_last_run(cmake): stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) + # no first-chance handler nor crash marker on macOS + if sys.platform != "darwin": + assert_no_crash_timestamp(has_files, tmp_path) callbacks = [ line for line in restarted.stdout.splitlines() diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 5cad93f42..ab2cf6e4b 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -33,6 +33,8 @@ assert_user_report, assert_minidump, assert_breakpad_crash, + assert_crash_timestamp, + assert_no_crash_timestamp, assert_gzip_content_encoding, assert_gzip_file_header, assert_attachment_view_hierarchy, @@ -1078,6 +1080,7 @@ def test_on_crashed_last_run(cmake, backend): stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) + assert_crash_timestamp(has_files, tmp_path) run_dirs = list((tmp_path / ".sentry-native").glob("*.run")) assert len(run_dirs) == 1 @@ -1092,6 +1095,7 @@ def test_on_crashed_last_run(cmake, backend): stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) + assert_no_crash_timestamp(has_files, tmp_path) callbacks = [ line for line in restarted.stdout.splitlines() diff --git a/tests/test_integration_native.py b/tests/test_integration_native.py index d7828645e..edd663671 100644 --- a/tests/test_integration_native.py +++ b/tests/test_integration_native.py @@ -28,6 +28,8 @@ ) from .assertions import ( assert_breadcrumb, + assert_crash_timestamp, + assert_no_crash_timestamp, assert_debug_meta_images_do_not_overlap, assert_meta, assert_native_crash, @@ -38,6 +40,7 @@ wait_for_file, assert_user_feedback, ) +from .conditions import has_files from .conditions import has_native, has_oom, is_asan, is_tsan, is_qemu, is_wine pytestmark = pytest.mark.skipif( @@ -97,6 +100,7 @@ def test_native_on_crashed_last_run(cmake, httpserver): crash_envelope = Envelope.deserialize(httpserver.log[0][0].get_data()) assert_native_crash(crash_envelope) event_id = crash_envelope.headers["event_id"] + assert_crash_timestamp(has_files, tmp_path) db_dir = tmp_path / ".sentry-native" run_dirs = list(db_dir.glob("*.run")) @@ -114,6 +118,7 @@ def test_native_on_crashed_last_run(cmake, httpserver): stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) + assert_no_crash_timestamp(has_files, tmp_path) callbacks = [ line for line in restarted.stdout.splitlines() diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py index ebb605b50..d978943d2 100644 --- a/tests/test_integration_stdout.py +++ b/tests/test_integration_stdout.py @@ -17,7 +17,7 @@ assert_minidump, assert_before_send, assert_no_before_send, - assert_crash_timestamp, + assert_no_crash_timestamp, assert_breakpad_crash, assert_exception, wait_for, @@ -174,7 +174,7 @@ def test_inproc_crash_stdout(cmake): envelope = Envelope.deserialize(output) - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) @@ -205,7 +205,7 @@ def test_abort_stdout(cmake, backend): envelope = Envelope.deserialize(output) - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration=backend) assert_breadcrumb(envelope) assert_attachment(envelope) @@ -224,7 +224,7 @@ def test_inproc_crash_stdout_before_send(cmake): envelope = Envelope.deserialize(output) - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) @@ -239,7 +239,7 @@ def test_inproc_crash_stdout_discarding_on_crash(cmake): # since the on_crash() handler discards further processing we expect an empty response assert len(output) == 0 - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) def test_inproc_crash_stdout_before_send_and_on_crash(cmake): @@ -252,7 +252,7 @@ def test_inproc_crash_stdout_before_send_and_on_crash(cmake): # 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_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) @@ -284,7 +284,7 @@ def test_inproc_stack_overflow_stdout(cmake, stack_size): envelope = Envelope.deserialize(output) - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) @@ -297,7 +297,7 @@ def test_breakpad_crash_stdout(cmake): envelope = Envelope.deserialize(output) - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) assert_attachment(envelope) @@ -311,7 +311,7 @@ def test_breakpad_crash_stdout_before_send(cmake): envelope = Envelope.deserialize(output) - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) assert_attachment(envelope) @@ -327,7 +327,7 @@ def test_breakpad_crash_stdout_discarding_on_crash(cmake): # since the on_crash() handler discards further processing we expect an empty response assert len(output) == 0 - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) @pytest.mark.skipif(not has_breakpad or is_qemu, reason="test needs breakpad backend") @@ -341,7 +341,7 @@ def test_breakpad_crash_stdout_before_send_and_on_crash(cmake): # 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_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) assert_attachment(envelope) @@ -380,7 +380,7 @@ def test_breakpad_stack_overflow_stdout(cmake, stack_size): envelope = Envelope.deserialize(output) - assert_crash_timestamp(has_files, tmp_path) + assert_no_crash_timestamp(has_files, tmp_path) assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) assert_attachment(envelope) diff --git a/tests/unit/test_basic.c b/tests/unit/test_basic.c index 0c2fd735b..bcdaed9d2 100644 --- a/tests/unit/test_basic.c +++ b/tests/unit/test_basic.c @@ -195,7 +195,8 @@ SENTRY_TEST(crash_marker) SENTRY_TEST(crashed_last_run) { // fails before init() is called - TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 1); + SENTRY_TEST_DEPRECATED( + TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 1)); // clear any leftover from previous test runs { @@ -229,9 +230,7 @@ SENTRY_TEST(crashed_last_run) TEST_CHECK_INT_EQUAL(sentry_init(options), 0); TEST_CHECK_INT_EQUAL(sentry_get_crashed_last_run(), 1); - - // clear the status and re-init - TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 0); + TEST_CHECK(!sentry__has_crash_marker(options)); sentry_close(); @@ -239,6 +238,25 @@ SENTRY_TEST(crashed_last_run) TEST_CHECK_INT_EQUAL(sentry_get_crashed_last_run(), 1); } + { + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn_n(options, dsn, sizeof(dsn)); + + // simulate a crash + TEST_CHECK(sentry__write_crash_marker(options)); + + sentry__retain_crash_marker(options); + TEST_CHECK_INT_EQUAL(sentry_init(options), 0); + + TEST_CHECK_INT_EQUAL(sentry_get_crashed_last_run(), 1); + TEST_CHECK(sentry__has_crash_marker(options)); + // explicit clearing remains supported on all platforms + SENTRY_TEST_DEPRECATED( + TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 0)); + + sentry_close(); + } + { SENTRY_TEST_OPTIONS_NEW(options); sentry_options_set_dsn_n(options, dsn, sizeof(dsn));