diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a5d03e81a..13c748df8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,7 +121,7 @@ jobs: submodules: recursive - uses: actions/setup-python@v4 with: - python-version: '3.10' + python-version: '3.11' cache: 'pip' - name: Installing Linux Dependencies diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc2ccb01..ea9f639d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 0.6.5 + +**Fixes**: + +- Remove deadlock pattern in dynamic sdk-name assignment ([#858](https://github.com/getsentry/sentry-native/pull/858)) + ## 0.6.4 **Fixes**: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 61bc6f297..2af86e966 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,6 +144,7 @@ The example currently supports the following commands: - `discarding-before-send`: Installs a `before_send()` callback that discards 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. +- `override-sdk-name`: Changes the SDK name via the options at runtime. Only on Windows using crashpad with its WER handler module: diff --git a/examples/example.c b/examples/example.c index ad9e594f9..7e1b71b0c 100644 --- a/examples/example.c +++ b/examples/example.c @@ -218,6 +218,10 @@ main(int argc, char **argv) options, discarding_on_crash_callback, NULL); } + if (has_arg(argc, argv, "override-sdk-name")) { + sentry_options_set_sdk_name(options, "sentry.native.android.flutter"); + } + sentry_init(options); if (!has_arg(argc, argv, "no-setup")) { diff --git a/include/sentry.h b/include/sentry.h index 61b363f18..180a22bc2 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -30,7 +30,7 @@ extern "C" { # define SENTRY_SDK_NAME "sentry.native" # endif #endif -#define SENTRY_SDK_VERSION "0.6.4" +#define SENTRY_SDK_VERSION "0.6.5" #define SENTRY_SDK_USER_AGENT SENTRY_SDK_NAME "/" SENTRY_SDK_VERSION /* common platform detection */ diff --git a/src/sentry_core.c b/src/sentry_core.c index 8cbb39a03..43309b67f 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -163,11 +163,16 @@ sentry_init(sentry_options_t *options) g_options = options; // *after* setting the global options, trigger a scope and consent flush, - // since at least crashpad needs that. - // the only way to get a reference to the scope is by locking it, the macro - // does all that at once, including invoking the backends scope flush hook + // since at least crashpad needs that. At this point we also freeze the + // `client_sdk` in the `scope` because some downstream SDKs want to override + // it at runtime via the options interface. SENTRY_WITH_SCOPE_MUT (scope) { - (void)scope; + if (options->sdk_name) { + sentry_value_t sdk_name + = sentry_value_new_string(options->sdk_name); + sentry_value_set_by_key(scope->client_sdk, "name", sdk_name); + } + sentry_value_freeze(scope->client_sdk); } if (backend && backend->user_consent_changed_func) { backend->user_consent_changed_func(backend); diff --git a/src/sentry_scope.c b/src/sentry_scope.c index a98a583ba..ba246ab2e 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -29,15 +29,9 @@ get_client_sdk(void) { sentry_value_t client_sdk = sentry_value_new_object(); - SENTRY_WITH_OPTIONS (options) { - sentry_value_t sdk_name = sentry_value_new_string(options->sdk_name); - sentry_value_set_by_key(client_sdk, "name", sdk_name); - } - // in case the SDK is not initialized yet, fallback to build-time value - if (sentry_value_is_null(sentry_value_get_by_key(client_sdk, "name"))) { - sentry_value_t sdk_name = sentry_value_new_string(SENTRY_SDK_NAME); - sentry_value_set_by_key(client_sdk, "name", sdk_name); - } + // the SDK is not initialized yet, fallback to build-time value + sentry_value_t sdk_name = sentry_value_new_string(SENTRY_SDK_NAME); + sentry_value_set_by_key(client_sdk, "name", sdk_name); sentry_value_t version = sentry_value_new_string(SENTRY_SDK_VERSION); sentry_value_set_by_key(client_sdk, "version", version); @@ -61,7 +55,6 @@ get_client_sdk(void) sentry_value_set_by_key(client_sdk, "integrations", integrations); #endif - sentry_value_freeze(client_sdk); return client_sdk; } diff --git a/tests/assertions.py b/tests/assertions.py index 6622c788f..65bbd98b5 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -1,9 +1,12 @@ -import datetime import email import gzip import platform import re import sys +from dataclasses import dataclass +from datetime import datetime + +import msgpack from .conditions import is_android @@ -55,9 +58,9 @@ def assert_meta( } expected_sdk = { "name": "sentry.native", - "version": "0.6.4", + "version": "0.6.5", "packages": [ - {"name": "github:getsentry/sentry-native", "version": "0.6.4"}, + {"name": "github:getsentry/sentry-native", "version": "0.6.5"}, ], } if is_android: @@ -161,7 +164,7 @@ def assert_minidump(envelope): assert minidump.payload.bytes.startswith(b"MDMP") -def assert_timestamp(ts, now=datetime.datetime.utcnow()): +def assert_timestamp(ts, now=datetime.utcnow()): assert ts[:11] == now.isoformat()[:11] @@ -176,6 +179,14 @@ def assert_event(envelope): assert_timestamp(event["timestamp"]) +def assert_breakpad_crash(envelope): + event = envelope.get_event() + expected = { + "level": "fatal", + } + assert_matches(event, expected) + + def assert_exception(envelope): event = envelope.get_event() exception = { @@ -186,7 +197,7 @@ def assert_exception(envelope): assert_timestamp(event["timestamp"]) -def assert_crash(envelope): +def assert_inproc_crash(envelope): event = envelope.get_event() assert_matches(event, {"level": "fatal"}) # depending on the unwinder, we currently don’t get any stack frames from @@ -213,16 +224,62 @@ def assert_no_before_send(envelope): assert ("adapted_by", "before_send") not in event.items() +@dataclass(frozen=True) +class CrashpadAttachments: + event: dict + breadcrumb1: list + breadcrumb2: list + + +def _unpack_breadcrumbs(payload): + unpacker = msgpack.Unpacker() + unpacker.feed(payload) + return [unpacked for unpacked in unpacker] + + +def _load_crashpad_attachments(msg): + event = {} + breadcrumb1 = [] + breadcrumb2 = [] + for part in msg.walk(): + match part.get_filename(): + case "__sentry-event": + event = msgpack.unpackb(part.get_payload(decode=True)) + case "__sentry-breadcrumb1": + breadcrumb1 = _unpack_breadcrumbs(part.get_payload(decode=True)) + case "__sentry-breadcrumb2": + breadcrumb2 = _unpack_breadcrumbs(part.get_payload(decode=True)) + + return CrashpadAttachments(event, breadcrumb1, breadcrumb2) + + +def is_valid_timestamp(timestamp): + try: + datetime.fromisoformat(timestamp) + return True + except ValueError: + return False + + +def _validate_breadcrumb_seq(seq, breadcrumb_func): + for i in seq: + breadcrumb = breadcrumb_func(i) + assert breadcrumb["message"] == str(i) + assert is_valid_timestamp(breadcrumb["timestamp"]) + + def assert_crashpad_upload(req): multipart = gzip.decompress(req.get_data()) msg = email.message_from_bytes(bytes(str(req.headers), encoding="utf8") + multipart) - files = [part.get_filename() for part in msg.walk()] + attachments = _load_crashpad_attachments(msg) + + if len(attachments.breadcrumb1) > 3: + _validate_breadcrumb_seq(range(97), lambda i: attachments.breadcrumb1[3 + i]) + _validate_breadcrumb_seq( + range(97, 101), lambda i: attachments.breadcrumb2[i - 97] + ) - # TODO: - # Actually assert that we get a correct event/breadcrumbs payload - assert "__sentry-breadcrumb1" in files - assert "__sentry-breadcrumb2" in files - assert "__sentry-event" in files + assert attachments.event["level"] == "fatal" assert any( b'name="upload_file_minidump"' in part.as_bytes() diff --git a/tests/requirements.txt b/tests/requirements.txt index 4d181eb9d..c90370ba0 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,3 +1,4 @@ black==23.3.0 -pytest==7.2.2 -pytest-httpserver==1.0.6 +pytest==7.4.0 +pytest-httpserver==1.0.8 +msgpack==1.0.5 diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index c304f8441..bfc87552b 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -1,11 +1,12 @@ -import pytest +import itertools +import json import os import time -import itertools import uuid -import json + +import pytest + from . import make_dsn, run, Envelope -from .conditions import has_http, has_breakpad, has_files from .assertions import ( assert_attachment, assert_meta, @@ -13,15 +14,17 @@ assert_stacktrace, assert_event, assert_exception, - assert_crash, + assert_inproc_crash, assert_session, assert_minidump, + assert_breakpad_crash, ) +from .conditions import has_http, has_breakpad, has_files pytestmark = pytest.mark.skipif(not has_http, reason="tests need http") auth_header = ( - "Sentry sentry_key=uiaeosnrtdy, sentry_version=7, sentry_client=sentry.native/0.6.4" + "Sentry sentry_key=uiaeosnrtdy, sentry_version=7, sentry_client=sentry.native/0.6.5" ) @@ -233,7 +236,7 @@ def test_inproc_crash_http(cmake, httpserver): assert_breadcrumb(envelope) assert_attachment(envelope) - assert_crash(envelope) + assert_inproc_crash(envelope) def test_inproc_reinstall(cmake, httpserver): @@ -318,6 +321,7 @@ def test_breakpad_crash_http(cmake, httpserver): assert_breadcrumb(envelope) assert_attachment(envelope) + assert_breakpad_crash(envelope) assert_minidump(envelope) diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py index ecb2c6f95..e44a90ead 100644 --- a/tests/test_integration_stdout.py +++ b/tests/test_integration_stdout.py @@ -12,12 +12,12 @@ assert_breadcrumb, assert_stacktrace, assert_event, - assert_crash, + assert_inproc_crash, assert_minidump, - assert_timestamp, assert_before_send, assert_no_before_send, assert_crash_timestamp, + assert_breakpad_crash, ) from .conditions import has_breakpad, has_files @@ -46,6 +46,26 @@ def test_capture_stdout(cmake): assert_event(envelope) +def test_dynamic_sdk_name_override(cmake): + tmp_path = cmake( + ["sentry_example"], + { + "SENTRY_BACKEND": "none", + "SENTRY_TRANSPORT": "none", + }, + ) + + output = check_output( + tmp_path, + "sentry_example", + ["stdout", "override-sdk-name", "capture-event"], + ) + envelope = Envelope.deserialize(output) + + assert_meta(envelope, sdk_override="sentry.native.android.flutter") + assert_event(envelope) + + def test_sdk_name_override(cmake): sdk_name = "cUsToM.SDK" tmp_path = cmake( @@ -92,9 +112,9 @@ def test_multi_process(cmake): # while the processes are running, we expect two runs runs = [ - run - for run in os.listdir(os.path.join(cwd, ".sentry-native")) - if run.endswith(".run") + db_run + for db_run in os.listdir(os.path.join(cwd, ".sentry-native")) + if db_run.endswith(".run") ] assert len(runs) == 2 @@ -108,9 +128,9 @@ def test_multi_process(cmake): subprocess.run([cmd], cwd=cwd) runs = [ - run - for run in os.listdir(os.path.join(cwd, ".sentry-native")) - if run.endswith(".run") or run.endswith(".lock") + db_run + for db_run in os.listdir(os.path.join(cwd, ".sentry-native")) + if db_run.endswith(".run") or db_run.endswith(".lock") ] assert len(runs) == 0 @@ -136,7 +156,7 @@ def test_inproc_crash_stdout(cmake): assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) - assert_crash(envelope) + assert_inproc_crash(envelope) def test_inproc_crash_stdout_before_send(cmake): @@ -148,7 +168,7 @@ def test_inproc_crash_stdout_before_send(cmake): assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) - assert_crash(envelope) + assert_inproc_crash(envelope) assert_before_send(envelope) @@ -175,7 +195,7 @@ def test_inproc_crash_stdout_before_send_and_on_crash(cmake): assert_meta(envelope, integration="inproc") assert_breadcrumb(envelope) assert_attachment(envelope) - assert_crash(envelope) + assert_inproc_crash(envelope) @pytest.mark.skipif(not has_breakpad, reason="test needs breakpad backend") @@ -189,6 +209,7 @@ def test_breakpad_crash_stdout(cmake): assert_breadcrumb(envelope) assert_attachment(envelope) assert_minidump(envelope) + assert_breakpad_crash(envelope) @pytest.mark.skipif(not has_breakpad, reason="test needs breakpad backend") @@ -203,6 +224,7 @@ def test_breakpad_crash_stdout_before_send(cmake): assert_attachment(envelope) assert_minidump(envelope) assert_before_send(envelope) + assert_breakpad_crash(envelope) @pytest.mark.skipif(not has_breakpad, reason="test needs breakpad backend") @@ -230,3 +252,4 @@ def test_breakpad_crash_stdout_before_send_and_on_crash(cmake): assert_meta(envelope, integration="breakpad") assert_breadcrumb(envelope) assert_attachment(envelope) + assert_breakpad_crash(envelope) diff --git a/tests/unit/test_concurrency.c b/tests/unit/test_concurrency.c index 946081dba..47fc691a9 100644 --- a/tests/unit/test_concurrency.c +++ b/tests/unit/test_concurrency.c @@ -43,7 +43,7 @@ SENTRY_TEST(multiple_inits) SENTRY_LEVEL_INFO, "root", "Hello World!")); sentry_value_t obj = sentry_value_new_object(); - // something that is not a uuid, as this will be forcibly changed + // something that is not a UUID, as this will be forcibly changed sentry_value_set_by_key(obj, "event_id", sentry_value_new_int32(1234)); sentry_capture_event(obj); @@ -64,7 +64,7 @@ thread_worker(void *called) SENTRY_LEVEL_INFO, "root", "Hello World!")); sentry_value_t obj = sentry_value_new_object(); - // something that is not a uuid, as this will be forcibly changed + // something that is not a UUID, as this will be forcibly changed sentry_value_set_by_key(obj, "event_id", sentry_value_new_int32(1234)); sentry_capture_event(obj); @@ -75,7 +75,7 @@ SENTRY_TEST(concurrent_init) { long called = 0; -#define THREADS_NUM 10 +#define THREADS_NUM 100 sentry_threadid_t threads[THREADS_NUM]; for (size_t i = 0; i < THREADS_NUM; i++) { diff --git a/tests/unit/test_options.c b/tests/unit/test_options.c index 529b91472..c9115a434 100644 --- a/tests/unit/test_options.c +++ b/tests/unit/test_options.c @@ -43,7 +43,7 @@ SENTRY_TEST(options_sdk_name_invalid) const char *sdk_name = NULL; const int result = sentry_options_set_sdk_name(options, sdk_name); - // then the value should should be ignored + // then the value should be ignored TEST_CHECK_INT_EQUAL(result, 1); TEST_CHECK_STRING_EQUAL( sentry_options_get_sdk_name(options), SENTRY_SDK_NAME);