test: Add more testcases for previously untested code (#317)

# Tests added: * Object cloning and overwriting keys. * Serializing more msgpack values. * Forcing captured events to have a uuid event_id. * JSON escapes and surrogate parsing, also fixing a bug for that. * Discarding events when missing consent. * Test and fix recording errors on current session. * Sessions with username did. * Lots of abnormal sessions on startup. # Bugs fixed: * Wrong JSON decoding with surrogate pairs. * Memory Leak parsing unmatched surrogate pairs. * Never recorded error counts on sessions.
getsentry · Jul 6, 2020 · 577c9f4 · 577c9f4
1 parent 4adf9c3
commit 577c9f4
Show file tree

Hide file tree

Showing 11 changed files with 222 additions and 54 deletions.
diff --git a/examples/example.c b/examples/example.c
@@ -149,6 +149,25 @@ main(int argc, char **argv)
         }
         sentry_capture_event(event);
     }
+    if (has_arg(argc, argv, "capture-exception")) {
+        // TODO: Create a convenience API to create a new exception object,
+        // and to attach a stacktrace to the exception.
+        // See also https://github.com/getsentry/sentry-native/issues/235
+        sentry_value_t event = sentry_value_new_event();
+        sentry_value_t exception = sentry_value_new_object();
+        // for example:
+        sentry_value_set_by_key(
+            exception, "type", sentry_value_new_string("ParseIntError"));
+        sentry_value_set_by_key(exception, "value",
+            sentry_value_new_string("invalid digit found in string"));
+        sentry_value_t exceptions = sentry_value_new_list();
+        sentry_value_append(exceptions, exception);
+        sentry_value_t values = sentry_value_new_object();
+        sentry_value_set_by_key(values, "values", exceptions);
+        sentry_value_set_by_key(event, "exception", values);
+
+        sentry_capture_event(event);
+    }
 
     // make sure everything flushes
     sentry_shutdown();

diff --git a/src/sentry_core.c b/src/sentry_core.c
@@ -311,7 +311,7 @@ sentry_capture_event(sentry_value_t event)
                     sentry__path_filename(attachment->path)));
         }
 
-        if (event_is_considered_error(sentry_envelope_get_event(envelope))) {
+        if (event_is_considered_error(event)) {
             sentry__record_errors_on_current_session(1);
         }
         sentry__add_current_session_to_envelope(envelope);
@@ -362,8 +362,7 @@ sentry_value_t
 sentry__ensure_event_id(sentry_value_t event, sentry_uuid_t *uuid_out)
 {
     sentry_value_t event_id = sentry_value_get_by_key(event, "event_id");
-    const char *uuid_str = sentry_value_as_string(event_id);
-    sentry_uuid_t uuid = sentry_uuid_from_string(uuid_str);
+    sentry_uuid_t uuid = sentry__value_as_uuid(event_id);
     if (sentry_uuid_is_nil(&uuid)) {
         uuid = sentry__new_event_id();
         event_id = sentry__value_new_uuid(&uuid);

diff --git a/src/sentry_json.c b/src/sentry_json.c
@@ -370,6 +370,7 @@ decode_string_inplace(char *buf)
                     || !sentry__is_trail_surrogate(trail)) {
                     return false;
                 }
+                input += 4;
                 uchar = sentry__surrogate_value(lead, trail);
             } else if (sentry__is_trail_surrogate(uchar)) {
                 return false;
@@ -445,6 +446,7 @@ tokens_to_value(jsmntok_t *tokens, size_t token_count, const char *buf,
         if (decode_string_inplace(string)) {
             rv = sentry__value_new_string_owned(string);
         } else {
+            sentry_free(string);
             rv = sentry_value_new_null();
         }
         break;

diff --git a/tests/assertions.py b/tests/assertions.py
@@ -108,6 +108,17 @@ def assert_event(envelope):
     assert_timestamp(event["timestamp"])
 
 
+def assert_exception(envelope):
+    event = envelope.get_event()
+    exception = {
+        "type": "ParseIntError",
+        "value": "invalid digit found in string",
+    }
+    expected = {"exception": {"values": [exception]}}
+    assert matches(event, expected)
+    assert_timestamp(event["timestamp"])
+
+
 def assert_crash(envelope):
     event = envelope.get_event()
     assert matches(event, {"level": "fatal"})

diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py
@@ -2,14 +2,17 @@
 import subprocess
 import sys
 import os
+import itertools
+import json
 from . import make_dsn, check_output, run, Envelope
-from .conditions import has_http, has_inproc, has_breakpad
+from .conditions import has_http, has_inproc, has_breakpad, has_files
 from .assertions import (
     assert_attachment,
     assert_meta,
     assert_breadcrumb,
     assert_stacktrace,
     assert_event,
+    assert_exception,
     assert_crash,
     assert_session,
     assert_minidump,
@@ -109,6 +112,83 @@ def test_capture_and_session_http(cmake, httpserver):
     assert_session(envelope, {"status": "exited", "errors": 0})
 
 
+def test_exception_and_session_http(cmake, httpserver):
+    tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"})
+
+    httpserver.expect_request(
+        "/api/123456/envelope/", headers={"x-sentry-auth": auth_header},
+    ).respond_with_data("OK")
+
+    run(
+        tmp_path,
+        "sentry_example",
+        ["log", "start-session", "capture-exception"],
+        check=True,
+        env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)),
+    )
+
+    assert len(httpserver.log) == 2
+    output = httpserver.log[0][0].get_data()
+    envelope = Envelope.deserialize(output)
+
+    assert_exception(envelope)
+    assert_session(envelope, {"init": True, "status": "ok", "errors": 1})
+
+    output = httpserver.log[1][0].get_data()
+    envelope = Envelope.deserialize(output)
+    assert_session(envelope, {"status": "exited", "errors": 1})
+
+
+@pytest.mark.skipif(not has_files, reason="test needs a local filesystem")
+def test_abnormal_session(cmake, httpserver):
+    tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "none"},)
+
+    httpserver.expect_request(
+        "/api/123456/envelope/", headers={"x-sentry-auth": auth_header},
+    ).respond_with_data("OK")
+
+    # create a bogus session file
+    session = json.dumps(
+        {
+            "sid": "00000000-0000-0000-0000-000000000000",
+            "did": "42",
+            "status": "started",
+            "errors": 0,
+            "started": "2020-06-02T10:04:53.680Z",
+            "duration": 10,
+            "attrs": {"release": "test-example-release", "environment": "development"},
+        }
+    )
+    db_dir = tmp_path.joinpath(".sentry-native")
+    db_dir.mkdir(exist_ok=True)
+    # 15 exceeds the max envelope items
+    for i in range(15):
+        run_dir = db_dir.joinpath(f"foo-{i}.run")
+        run_dir.mkdir()
+        with open(run_dir.joinpath("session.json"), "w") as session_file:
+            session_file.write(session)
+
+    run(
+        tmp_path,
+        "sentry_example",
+        ["log", "no-setup"],
+        check=True,
+        env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)),
+    )
+
+    assert len(httpserver.log) == 2
+    envelope1 = Envelope.deserialize(httpserver.log[0][0].get_data())
+    envelope2 = Envelope.deserialize(httpserver.log[1][0].get_data())
+
+    session_count = 0
+    for item in itertools.chain(envelope1, envelope2):
+        if item.headers.get("type") == "session":
+            session_count += 1
+    assert session_count == 15
+
+    assert_session(envelope1, {"status": "abnormal", "errors": 0, "duration": 10})
+
+
 @pytest.mark.skipif(not has_inproc, reason="test needs inproc backend")
 def test_inproc_crash_http(cmake, httpserver):
     tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "inproc"})

diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py
@@ -3,7 +3,6 @@
 import sys
 import os
 import time
-import json
 from . import check_output, run, Envelope
 from .conditions import has_inproc, has_breakpad, has_files
 from .assertions import (
@@ -85,40 +84,6 @@ def test_multi_process(cmake):
     assert len(runs) == 0
 
 
-@pytest.mark.skipif(not has_files, reason="test needs a local filesystem")
-def test_abnormal_session(cmake):
-    tmp_path = cmake(
-        ["sentry_example"], {"SENTRY_BACKEND": "none", "SENTRY_TRANSPORT": "none"},
-    )
-
-    # create a bogus session file
-    db_dir = tmp_path.joinpath(".sentry-native")
-    db_dir.mkdir(exist_ok=True)
-    run_dir = db_dir.joinpath("foobar.run")
-    run_dir.mkdir()
-    with open(run_dir.joinpath("session.json"), "w") as session_file:
-        json.dump(
-            {
-                "sid": "00000000-0000-0000-0000-000000000000",
-                "did": "42",
-                "status": "started",
-                "errors": 0,
-                "started": "2020-06-02T10:04:53.680Z",
-                "duration": 10,
-                "attrs": {
-                    "release": "test-example-release",
-                    "environment": "development",
-                },
-            },
-            session_file,
-        )
-
-    output = check_output(tmp_path, "sentry_example", ["stdout", "no-setup"])
-    envelope = Envelope.deserialize(output)
-
-    assert_session(envelope, {"status": "abnormal", "errors": 0, "duration": 10})
-
-
 @pytest.mark.skipif(not has_inproc, reason="test needs inproc backend")
 def test_inproc_crash_stdout(cmake):
     tmp_path = cmake(

diff --git a/tests/unit/test_basic.c b/tests/unit/test_basic.c
@@ -13,15 +13,18 @@ send_envelope(const sentry_envelope_t *envelope, void *data)
     const char *event_id
         = sentry_value_as_string(sentry_value_get_by_key(event, "event_id"));
     TEST_CHECK_STRING_EQUAL(event_id, "4c035723-8638-4c3a-923f-2ab9d08b4018");
-    const char *msg = sentry_value_as_string(sentry_value_get_by_key(
-        sentry_value_get_by_key(event, "message"), "formatted"));
-    TEST_CHECK_STRING_EQUAL(msg, "Hello World!");
-    const char *release
-        = sentry_value_as_string(sentry_value_get_by_key(event, "release"));
-    TEST_CHECK_STRING_EQUAL(release, "prod");
-    const char *trans
-        = sentry_value_as_string(sentry_value_get_by_key(event, "transaction"));
-    TEST_CHECK_STRING_EQUAL(trans, "demo-trans");
+
+    if (*called == 1) {
+        const char *msg = sentry_value_as_string(sentry_value_get_by_key(
+            sentry_value_get_by_key(event, "message"), "formatted"));
+        TEST_CHECK_STRING_EQUAL(msg, "Hello World!");
+        const char *release
+            = sentry_value_as_string(sentry_value_get_by_key(event, "release"));
+        TEST_CHECK_STRING_EQUAL(release, "prod");
+        const char *trans = sentry_value_as_string(
+            sentry_value_get_by_key(event, "transaction"));
+        TEST_CHECK_STRING_EQUAL(trans, "demo-trans");
+    }
 }
 
 SENTRY_TEST(basic_function_transport)
@@ -33,16 +36,30 @@ SENTRY_TEST(basic_function_transport)
     sentry_options_set_transport(
         options, sentry_new_function_transport(send_envelope, &called));
     sentry_options_set_release(options, "prod");
+    sentry_options_set_require_user_consent(options, true);
     sentry_init(options);
 
     sentry_set_transaction("demo-trans");
 
+    sentry_capture_event(sentry_value_new_message_event(
+        SENTRY_LEVEL_INFO, "root", "not captured due to missing consent"));
+    sentry_user_consent_give();
+
     sentry_capture_event(sentry_value_new_message_event(
         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
+    sentry_value_set_by_key(obj, "event_id", sentry_value_new_int32(1234));
+    sentry_capture_event(obj);
+
+    sentry_user_consent_revoke();
+    sentry_capture_event(sentry_value_new_message_event(SENTRY_LEVEL_INFO,
+        "root", "not captured either due to revoked consent"));
+
     sentry_shutdown();
 
-    TEST_CHECK_INT_EQUAL(called, 1);
+    TEST_CHECK_INT_EQUAL(called, 2);
 }
 
 static sentry_value_t

diff --git a/tests/unit/test_mpack.c b/tests/unit/test_mpack.c
@@ -11,6 +11,10 @@ SENTRY_TEST(mpack_removed_tags)
     sentry_set_tag("baz", "baz");
     sentry_set_tag("qux", "qux");
     sentry_remove_tag("bar");
+    sentry_set_extra("null", sentry_value_new_null());
+    sentry_set_extra("bool", sentry_value_new_bool(true));
+    sentry_set_extra("int", sentry_value_new_int32(1234));
+    sentry_set_extra("double", sentry_value_new_double(12.34));
 
     SENTRY_WITH_SCOPE (scope) {
         sentry__scope_apply_to_event(scope, obj, SENTRY_SCOPE_NONE);

diff --git a/tests/unit/test_session.c b/tests/unit/test_session.c
@@ -30,7 +30,7 @@ send_envelope(const sentry_envelope_t *envelope, void *data)
         "exited");
     TEST_CHECK_STRING_EQUAL(
         sentry_value_as_string(sentry_value_get_by_key(session, "did")),
-        "foo@blabla.invalid");
+        *called == 1 ? "foo@blabla.invalid" : "swatinem");
     TEST_CHECK_INT_EQUAL(
         sentry_value_as_int32(sentry_value_get_by_key(session, "errors")), 0);
     TEST_CHECK_INT_EQUAL(
@@ -71,7 +71,15 @@ SENTRY_TEST(session_basics)
         user, "email", sentry_value_new_string("foo@blabla.invalid"));
     sentry_set_user(user);
 
+    sentry_end_session();
+    sentry_start_session();
+
+    user = sentry_value_new_object();
+    sentry_value_set_by_key(
+        user, "username", sentry_value_new_string("swatinem"));
+    sentry_set_user(user);
+
     sentry_shutdown();
 
-    TEST_CHECK_INT_EQUAL(called, 1);
+    TEST_CHECK_INT_EQUAL(called, 2);
 }