diff --git a/CHANGELOG.md b/CHANGELOG.md index d2738e444c..04fb54178c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Features**: +- Cache consent-revoked envelopes to disk when `cache_keep` or `http_retry` is enabled, instead of discarding them. With `http_retry`, the cached envelopes are sent automatically once consent is given. ([#1542](https://github.com/getsentry/sentry-native/pull/1542)) - Add `before_screenshot` hook. ([#1641](https://github.com/getsentry/sentry-native/pull/1641)) **Fixes**: diff --git a/examples/example.c b/examples/example.c index aadceb6ed5..505601b1b7 100644 --- a/examples/example.c +++ b/examples/example.c @@ -694,6 +694,9 @@ main(int argc, char **argv) if (has_arg(argc, argv, "log-attributes")) { sentry_options_set_logs_with_attributes(options, true); } + if (has_arg(argc, argv, "require-user-consent")) { + sentry_options_set_require_user_consent(options, true); + } if (has_arg(argc, argv, "cache-keep")) { sentry_options_set_cache_keep(options, true); sentry_options_set_cache_max_size(options, 4 * 1024 * 1024); // 4 MB @@ -758,6 +761,10 @@ main(int argc, char **argv) return EXIT_FAILURE; } + if (has_arg(argc, argv, "user-consent-revoke")) { + sentry_user_consent_revoke(); + } + if (has_arg(argc, argv, "set-global-attribute")) { sentry_set_attribute("global.attribute.bool", sentry_value_new_attribute(sentry_value_new_bool(true), NULL)); @@ -1049,6 +1056,10 @@ main(int argc, char **argv) } sentry_capture_event(event); } + if (has_arg(argc, argv, "user-consent-give")) { + sentry_user_consent_give(); + sentry_flush(10000); + } if (has_arg(argc, argv, "capture-exception")) { sentry_value_t exc = sentry_value_new_exception( "ParseIntError", "invalid digit found in string"); diff --git a/include/sentry.h b/include/sentry.h index 8d85de20f4..8b336a4feb 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1470,6 +1470,11 @@ SENTRY_API int sentry_options_get_auto_session_tracking( * This disables uploads until the user has given the consent to the SDK. * Consent itself is given with `sentry_user_consent_give` and * `sentry_user_consent_revoke`. + * + * When combined with `cache_keep` or `http_retry`, envelopes captured + * while consent is revoked are written to the cache directory instead + * of being discarded. With `http_retry` enabled, cached envelopes are + * sent automatically once consent is given. */ SENTRY_API void sentry_options_set_require_user_consent( sentry_options_t *opts, int val); @@ -1505,6 +1510,10 @@ SENTRY_API int sentry_options_get_symbolize_stacktraces( * subdirectory within the database directory. The cache is cleared on startup * based on the cache_max_items, cache_max_size, and cache_max_age options. * + * When combined with `sentry_options_set_require_user_consent`, envelopes + * captured while consent is revoked are also written to the cache. With + * `http_retry` enabled, they are sent once consent is given. + * * Only applicable for HTTP transports. * * Disabled by default. @@ -1954,6 +1963,9 @@ SENTRY_EXPERIMENTAL_API int sentry_reinstall_backend(void); /** * Gives user consent. + * + * Schedules a retry of any envelopes cached while consent was revoked, + * provided that `http_retry` is enabled. */ SENTRY_API void sentry_user_consent_give(void); diff --git a/src/sentry_core.c b/src/sentry_core.c index 4b617c3154..ce6a4aa8f2 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -427,6 +427,8 @@ set_user_consent(sentry_user_consent_t new_val) switch (new_val) { case SENTRY_USER_CONSENT_GIVEN: sentry__path_write_buffer(consent_path, "1\n", 2); + // flush any envelopes cached while consent was revoked + sentry_transport_retry(options->transport); break; case SENTRY_USER_CONSENT_REVOKED: sentry__path_write_buffer(consent_path, "0\n", 2); @@ -483,13 +485,24 @@ void sentry__capture_envelope( sentry_transport_t *transport, sentry_envelope_t *envelope) { - bool has_consent = !sentry__should_skip_upload(); - if (!has_consent) { - SENTRY_INFO("discarding envelope due to missing user consent"); - sentry_envelope_free(envelope); + if (!sentry__should_skip_upload()) { + sentry__transport_send_envelope(transport, envelope); return; } - sentry__transport_send_envelope(transport, envelope); + bool cached = false; + SENTRY_WITH_OPTIONS (options) { + if (options->cache_keep || options->http_retry) { + cached = sentry__run_write_cache(options->run, envelope, 0); + if (cached && !sentry__run_should_skip_upload(options->run)) { + // consent given meanwhile -> trigger retry to avoid waiting + // until the next retry poll + sentry_transport_retry(options->transport); + } + } + } + SENTRY_INFO(cached ? "caching envelope due to missing user consent" + : "discarding envelope due to missing user consent"); + sentry_envelope_free(envelope); } void diff --git a/src/sentry_database.c b/src/sentry_database.c index fe04d0440c..6c947a5917 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -458,7 +458,9 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) } } else if (sentry__path_ends_with(file, ".envelope")) { sentry_envelope_t *envelope = sentry__envelope_from_path(file); - sentry__capture_envelope(options->transport, envelope); + if (envelope) { + sentry__capture_envelope(options->transport, envelope); + } } sentry__path_remove(file); @@ -470,7 +472,9 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) } sentry__pathiter_free(db_iter); - sentry__capture_envelope(options->transport, session_envelope); + if (session_envelope) { + sentry__capture_envelope(options->transport, session_envelope); + } } // Cache Pruning below is based on prune_crash_reports.cc from Crashpad diff --git a/src/sentry_retry.c b/src/sentry_retry.c index 0bdbd9391b..59cb61b009 100644 --- a/src/sentry_retry.c +++ b/src/sentry_retry.c @@ -141,6 +141,10 @@ size_t sentry__retry_send(sentry_retry_t *retry, uint64_t before, sentry_retry_send_func_t send_cb, void *data) { + if (sentry__run_should_skip_upload(retry->run)) { + return 1; // keep the poll alive until consent is given + } + sentry_pathiter_t *piter = sentry__path_iter_directory(retry->run->cache_path); if (!piter) { diff --git a/tests/test_integration_cache.py b/tests/test_integration_cache.py index ba10a31e53..c0a072a3b1 100644 --- a/tests/test_integration_cache.py +++ b/tests/test_integration_cache.py @@ -263,3 +263,78 @@ def test_cache_max_items_with_retry(cmake, backend, unreachable_dsn): assert cache_dir.exists() cache_files = list(cache_dir.glob("*.envelope")) assert len(cache_files) <= 5 + + +def test_cache_consent_revoke(cmake, unreachable_dsn): + """With consent revoked and cache_keep, envelopes are cached to disk.""" + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + cache_dir = tmp_path.joinpath(".sentry-native/cache") + env = dict(os.environ, SENTRY_DSN=unreachable_dsn) + + run( + tmp_path, + "sentry_example", + [ + "log", + "cache-keep", + "require-user-consent", + "user-consent-revoke", + "capture-event", + "flush", + ], + env=env, + ) + + assert cache_dir.exists() + cache_files = list(cache_dir.glob("*.envelope")) + assert len(cache_files) == 1 + + +def test_cache_consent_discard(cmake, unreachable_dsn): + """With consent revoked but no cache_keep, envelopes are discarded.""" + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + cache_dir = tmp_path.joinpath(".sentry-native/cache") + env = dict(os.environ, SENTRY_DSN=unreachable_dsn) + + run( + tmp_path, + "sentry_example", + [ + "log", + "require-user-consent", + "user-consent-revoke", + "capture-event", + "flush", + ], + env=env, + ) + + assert not cache_dir.exists() or len(list(cache_dir.glob("*.envelope"))) == 0 + + +def test_cache_consent_flush(cmake, httpserver): + """Giving consent after capturing flushes cached envelopes immediately.""" + from . import make_dsn + + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"}) + cache_dir = tmp_path.joinpath(".sentry-native/cache") + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + run( + tmp_path, + "sentry_example", + [ + "log", + "http-retry", + "require-user-consent", + "user-consent-revoke", + "capture-event", + "user-consent-give", + ], + env=env, + ) + + assert len(httpserver.log) >= 1 + assert not cache_dir.exists() or len(list(cache_dir.glob("*.envelope"))) == 0 diff --git a/tests/unit/test_cache.c b/tests/unit/test_cache.c index f8e0083f70..a5b806718a 100644 --- a/tests/unit/test_cache.c +++ b/tests/unit/test_cache.c @@ -3,6 +3,7 @@ #include "sentry_envelope.h" #include "sentry_options.h" #include "sentry_path.h" +#include "sentry_retry.h" #include "sentry_string.h" #include "sentry_testsupport.h" #include "sentry_uuid.h" @@ -310,6 +311,83 @@ SENTRY_TEST(cache_max_items_with_retry) sentry_close(); } +SENTRY_TEST(cache_consent_revoked) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_cache_keep(options, true); + sentry_options_set_require_user_consent(options, true); + sentry_init(options); + sentry_user_consent_revoke(); + + sentry_path_t *cache_path + = sentry__path_join_str(options->database_path, "cache"); + TEST_ASSERT(!!cache_path); + sentry__path_remove_all(cache_path); + + sentry_capture_event( + sentry_value_new_message_event(SENTRY_LEVEL_INFO, "test", "revoked")); + + int count = 0; + bool is_retry_format = false; + sentry_pathiter_t *iter = sentry__path_iter_directory(cache_path); + const sentry_path_t *entry; + while (iter && (entry = sentry__pathiter_next(iter)) != NULL) { + if (sentry__path_ends_with(entry, ".envelope")) { + count++; + uint64_t ts; + int attempt; + const char *uuid; + is_retry_format = sentry__parse_cache_filename( + sentry__path_filename(entry), &ts, &attempt, &uuid); + } + } + sentry__pathiter_free(iter); + TEST_CHECK_INT_EQUAL(count, 1); + TEST_CHECK(is_retry_format); + + sentry__path_free(cache_path); + sentry_close(); +} + +SENTRY_TEST(cache_consent_revoked_nocache) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_cache_keep(options, false); + sentry_options_set_require_user_consent(options, true); + sentry_init(options); + sentry_user_consent_revoke(); + + sentry_path_t *cache_path + = sentry__path_join_str(options->database_path, "cache"); + TEST_ASSERT(!!cache_path); + sentry__path_remove_all(cache_path); + + sentry_capture_event( + sentry_value_new_message_event(SENTRY_LEVEL_INFO, "test", "revoked")); + + int count = 0; + sentry_pathiter_t *iter = sentry__path_iter_directory(cache_path); + const sentry_path_t *entry; + while (iter && (entry = sentry__pathiter_next(iter)) != NULL) { + if (sentry__path_ends_with(entry, ".envelope")) { + count++; + } + } + sentry__pathiter_free(iter); + TEST_CHECK_INT_EQUAL(count, 0); + + sentry__path_free(cache_path); + sentry_close(); +} + SENTRY_TEST(cache_max_size_and_age) { #if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) diff --git a/tests/unit/test_retry.c b/tests/unit/test_retry.c index 77cc1a993c..4dab3026ed 100644 --- a/tests/unit/test_retry.c +++ b/tests/unit/test_retry.c @@ -489,3 +489,49 @@ SENTRY_TEST(retry_trigger) sentry__retry_free(retry); sentry_close(); } + +SENTRY_TEST(retry_consent) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#endif + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_http_retry(options, true); + sentry_options_set_require_user_consent(options, true); + sentry_init(options); + sentry_user_consent_revoke(); + + sentry_retry_t *retry = sentry__retry_new(options); + TEST_ASSERT(!!retry); + + const sentry_path_t *cache_path = options->run->cache_path; + sentry__path_remove_all(cache_path); + sentry__path_create_dir_all(cache_path); + + uint64_t old_ts + = sentry__usec_time() / 1000 - 10 * sentry__retry_backoff(0); + sentry_uuid_t event_id = sentry_uuid_new_v4(); + write_retry_file(options->run, old_ts, 0, &event_id); + + TEST_CHECK_INT_EQUAL(count_envelope_files(cache_path), 1); + + // consent revoked: retry_send skips the round without calling send_cb, + // but returns non-zero to keep the poll alive until consent is given + retry_test_ctx_t ctx = { 200, 0 }; + size_t remaining = sentry__retry_send(retry, 0, test_send_cb, &ctx); + TEST_CHECK_INT_EQUAL(ctx.count, 0); + TEST_CHECK(remaining != 0); + TEST_CHECK_INT_EQUAL(count_envelope_files(cache_path), 1); + + // give consent: retry_send sends and removes the file + sentry_user_consent_give(); + ctx = (retry_test_ctx_t) { 200, 0 }; + remaining = sentry__retry_send(retry, UINT64_MAX, test_send_cb, &ctx); + TEST_CHECK_INT_EQUAL(ctx.count, 1); + TEST_CHECK_INT_EQUAL(remaining, 0); + TEST_CHECK_INT_EQUAL(count_envelope_files(cache_path), 0); + + sentry__retry_free(retry); + sentry_close(); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index a4d72ca20c..eaf305e5d7 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -40,6 +40,8 @@ XX(bgworker_flush) XX(bgworker_task_delay) XX(breadcrumb_without_type_or_message_still_valid) XX(build_id_parser) +XX(cache_consent_revoked) +XX(cache_consent_revoked_nocache) XX(cache_keep) XX(cache_max_age) XX(cache_max_items) @@ -208,6 +210,7 @@ XX(read_write_envelope_to_invalid_path) XX(recursive_paths) XX(retry_backoff) XX(retry_cache) +XX(retry_consent) XX(retry_filename) XX(retry_make_cache_path) XX(retry_result)