getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
575 stars 205 forks source link

[Android] Crash with `EnableBeforeSend` enabled #3465

Closed akravch closed 1 month ago

akravch commented 1 month ago

Package

Sentry

.NET Flavor

.NET

.NET Version

8.0.6

OS

Android

SDK Version

4.8.1

Self-Hosted Sentry Version

No response

Steps to Reproduce

  1. Create an Android app and install Sentry NuGet package
  2. Configure Sentry so that EnableBeforeSend is enabled and BeforeSend callback is set to something:

    SentrySdk.Init(options =>
    {
       options.Dsn = "https://****.ingest.us.sentry.io/****";
       options.Native.EnableBeforeSend = true;
       options.SetBeforeSend(@event => @event);
    }
  3. Crash the app with a native crash, e.g. using SentrySdk.CauseCrash(CrashType.Native)
  4. Restart the app

Expected Result

BeforeSend callback is fired, the crash is sent, the app works normally.

Actual Result

Crash with the following stack trace:

System.FormatException: One of the identified items was in an invalid format.
   at System.Text.Json.JsonElement.GetInt16()
   at Sentry.Protocol.Device.FromJson(JsonElement json)
   at Sentry.SentryContexts.FromJson(JsonElement json)
   at Sentry.Internal.Extensions.MiscExtensions.Pipe[JsonElement,SentryContexts](JsonElement input, Func`2 pipe)
   at Sentry.SentryEvent.FromJson(JsonElement json, Exception exception)
   at Sentry.Android.Extensions.SentryEventExtensions.ToSentryEvent(SentryEvent sentryEvent, SentryOptions javaOptions)
   at Sentry.Android.Callbacks.BeforeSendCallback.Execute(SentryEvent e, Hint h)
   at Sentry.JavaSdk.SentryOptions.IBeforeSendCallbackInvoker.n_Execute_Lio_sentry_SentryEvent_Lio_sentry_Hint_(IntPtr jnienv, IntPtr native__this, IntPtr native_p0, IntPtr native_p1)
   at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPLL_L(_JniMarshal_PPLL_L callback, IntPtr jnienv, IntPtr klazz, IntPtr p0, IntPtr p1) in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/Android.Runtime/JNINativeWrapper.g.cs:line 231

The reason is that the battery_level field is saved with a floating point, but the SDK expects short during deserialization:

{
    "device": {
        "manufacturer": "samsung",
        "brand": "samsung",
        "family": "SM-G991B",
        "model": "SM-G991B",
        "model_id": "UP1A.231005.007",
        "archs": [
            "arm64-v8a",
            "armeabi-v7a",
            "armeabi"
        ],
        "battery_level": 67.0,
        "charging": true,
        "orientation": "portrait",
        "simulator": false,
        "memory_size": 7550787584,
        "storage_size": 114057785344,
        "free_storage": 98203328512,
        "screen_width_pixels": 1080,
        "screen_height_pixels": 2176,
        "screen_density": 3.0,
        "screen_dpi": 480,
        "boot_time": "2024-07-05T13:10:13.757Z",
        "timezone": "Europe/Warsaw",
        "id": "e24d244e-0be7-4bdb-a744-bd7419c14eec",
        "language": "en",
        "battery_temperature": 30.6,
        "locale": "en_US",
        "processor_count": 8,
        "processor_frequency": 2912.0
    },
    "os": {
        "name": "Android",
        "version": "14",
        "build": "UP1A.231005.007.G991BXXS9FXBD",
        "kernel_version": "5.4.242-27760517-abG991BXXS9FXBD",
        "rooted": false
    },
    "os_linux": {
        "name": "Linux",
        "version": "5.4.242",
        "build": "27760517-abG991BXXS9FXBD"
    }
}
bitsandfoxes commented 1 month ago

Hey @akravch, sorry to see you run into issues. Based on your description it sounds like it should be fairly straight forward to reproduce the issue but I fail to do so locally. The FormatException happens during the restart?

akravch commented 1 month ago

Hi @bitsandfoxes, thanks for the quick reply!

Yeah, the exception is thrown during the restart, I guess when the SDK is deserializing the context to make it available in the BeforeSend callback.

Now as you said you couldn't reproduce it locally I went back and tried it again myself, and in my sample I can reproduce it reliably when I crash the app, and then start it with debugger attached. However, in our "big" real app it reproduces simply crashing and restarting, regardless of the debugger, even on the apk built by a remote build agent, so it doesn't look related to my local setup either.

Could it be related to the slower startup time for the real app compared to the sample, and the debugger adding up some time for the sample and making it reproducible?

I have uploaded the sample here just in case, although it's basically a dotnet new android + a few lines for Sentry.

bitsandfoxes commented 1 month ago

Thanks for the sample. I'll give that a try and see what happens!

bitsandfoxes commented 1 month ago

Ahh, now I got it! For some reason that one was tricky to repro. The exception gets swallowed in release builds? And the debugger refused to connect to the actual device most of the times. But I found the bugger, a fix is on its way. Thanks again for reaching out!

akravch commented 1 month ago

That's great, thanks for such a quick fix!