Skip to content

Commit 1f56e7e

Browse files
aritchiearitchie
authored
#4018 - Safety calls to/from Android Native Serialization (#4022)
1 parent cdb36a2 commit 1f56e7e

File tree

9 files changed

+188
-9
lines changed

9 files changed

+188
-9
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Fixes
6+
7+
- Prevent crashes from occurring on Android during OnBeforeSend ([#4022](https://github.com/getsentry/sentry-dotnet/pull/4022))
8+
59
### Dependencies
610

711
- Bump Native SDK from v0.8.0 to v0.8.1 ([#4014](https://github.com/getsentry/sentry-dotnet/pull/4014))

‎samples/Sentry.Samples.Android/MainActivity.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ protected override void OnCreate(Bundle? savedInstanceState)
2020
// https://docs.sentry.io/platforms/android/configuration/
2121
// Enable Native Android SDK ANR detection
2222
options.Native.AnrEnabled = true;
23+
24+
options.SetBeforeSend(evt =>
25+
{
26+
if (evt.Exception?.Message.Contains("Something you don't care want logged?") ?? false)
27+
{
28+
return null; // return null to filter out event
29+
}
30+
// or add additional data
31+
evt.SetTag("dotnet-Android-Native-Before", "Hello World");
32+
return evt;
33+
});
2334
});
2435

2536
// Here's an example of adding custom scope information.

‎samples/Sentry.Samples.Android/Sentry.Samples.Android.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3-
<TargetFramework>net8.0-android34.0</TargetFramework>
3+
<TargetFramework>net8.0-android</TargetFramework>
44
<SupportedOSPlatformVersion>21</SupportedOSPlatformVersion>
55
<OutputType>Exe</OutputType>
66
<Nullable>enable</Nullable>

‎samples/Sentry.Samples.Ios/AppDelegate.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,13 @@ public override bool FinishedLaunching(UIApplication application, NSDictionary l
2929

3030
options.CacheDirectoryPath = Path.GetTempPath();
3131

32-
options.SetBeforeSend((evt, _) =>
32+
options.SetBeforeSend(evt =>
3333
{
34+
if (evt.Exception?.Message.Contains("Something you don't care want logged?") ?? false)
35+
{
36+
return null; // return null to filter out event
37+
}
38+
// or add additional data
3439
evt.SetTag("dotnet-iOS-Native-Before", "Hello World");
3540
return evt;
3641
});

‎src/Sentry/Internal/Extensions/JsonExtensions.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,14 @@ public static void Deconstruct(this JsonProperty jsonProperty, out string name,
156156

157157
foreach (var (name, value) in json.EnumerateObject())
158158
{
159-
result[name] = value.GetString();
159+
if (value.ValueKind == JsonValueKind.String)
160+
{
161+
result[name] = value.GetString();
162+
}
163+
else
164+
{
165+
result[name] = value.ToString();
166+
}
160167
}
161168

162169
return result;

‎src/Sentry/Platforms/Android/Callbacks/BeforeSendCallback.cs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using Sentry.Android.Extensions;
2+
using Sentry.Extensibility;
23

34
namespace Sentry.Android.Callbacks;
45

@@ -22,10 +23,19 @@ public BeforeSendCallback(
2223
{
2324
// Note: Hint is unused due to:
2425
// https://github.com/getsentry/sentry-dotnet/issues/1469
25-
26-
var evnt = e.ToSentryEvent(_javaOptions);
27-
var hint = h.ToHint();
28-
var result = _beforeSend?.Invoke(evnt, hint);
29-
return result?.ToJavaSentryEvent(_options, _javaOptions);
26+
try
27+
{
28+
// because this can go out to user code, we want to prevent external crashing
29+
// also, native types tend to move before dotnet does, so serialization can fail
30+
var evnt = e.ToSentryEvent(_javaOptions);
31+
var hint = h.ToHint();
32+
var result = _beforeSend?.Invoke(evnt, hint);
33+
return result?.ToJavaSentryEvent(_options, _javaOptions);
34+
}
35+
catch (Exception exception)
36+
{
37+
_options.LogError(exception, "Before Send Error");
38+
return e;
39+
}
3040
}
3141
}

‎src/Sentry/Platforms/Android/Extensions/SentryEventExtensions.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using Sentry.Internal;
2+
13
namespace Sentry.Android.Extensions;
24

35
internal static class SentryEventExtensions
@@ -17,6 +19,12 @@ internal static class SentryEventExtensions
1719

1820
public static SentryEvent ToSentryEvent(this JavaSdk.SentryEvent sentryEvent, JavaSdk.SentryOptions javaOptions)
1921
{
22+
if (sentryEvent.Sdk != null)
23+
{
24+
// when we cast this serialize this over, this value must be set
25+
sentryEvent.Sdk.Name ??= Constants.SdkName;
26+
sentryEvent.Sdk.Version ??= SdkVersion.Instance.Version ?? "0.0.0";
27+
}
2028
using var stream = new MemoryStream();
2129
using var streamWriter = new JavaOutputStreamWriter(stream);
2230
using var jsonWriter = new JavaSdk.JsonObjectWriter(streamWriter, javaOptions.MaxDepth);
@@ -31,6 +39,11 @@ public static SentryEvent ToSentryEvent(this JavaSdk.SentryEvent sentryEvent, Ja
3139

3240
public static JavaSdk.SentryEvent ToJavaSentryEvent(this SentryEvent sentryEvent, SentryOptions options, JavaSdk.SentryOptions javaOptions)
3341
{
42+
if (sentryEvent.Sdk != null)
43+
{
44+
sentryEvent.Sdk.Name ??= Constants.SdkName;
45+
sentryEvent.Sdk.Version ??= SdkVersion.Instance.Version ?? "0.0.0";
46+
}
3447
using var stream = new MemoryStream();
3548
using var jsonWriter = new Utf8JsonWriter(stream);
3649
sentryEvent.WriteTo(jsonWriter, options.DiagnosticLogger);

‎src/Sentry/Platforms/Android/SentrySdk.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,10 @@ private static void InitSentryAndroidSdk(SentryOptions options)
100100
}
101101
}
102102

103-
o.BeforeSend = new BeforeSendCallback(BeforeSendWrapper(options), options, o);
103+
if (options.Android.SuppressSegfaults || (options.Native.EnableBeforeSend && options.BeforeSendInternal != null))
104+
{
105+
o.BeforeSend = new BeforeSendCallback(BeforeSendWrapper(options), options, o);
106+
}
104107

105108
// These options are from SentryAndroidOptions
106109
o.AttachScreenshot = options.Native.AttachScreenshot;

‎test/Sentry.Tests/Platforms/Android/JsonExtensionsTests.cs‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
using System;
2+
using System.Linq;
3+
using FluentAssertions;
4+
using Sentry.Android.Extensions;
5+
using Xunit;
6+
7+
namespace Sentry.Tests.Platforms.Android;
8+
9+
public class JsonExtensionsTests
10+
11+
{
12+
[Fact]
13+
public void ToJavaSentryEvent_Success()
14+
15+
{
16+
var evt = new SentryEvent(new Exception("Test Exception"));
17+
18+
evt.Level = SentryLevel.Debug;
19+
evt.ServerName = "test server name";
20+
evt.Distribution = "test distribution";
21+
evt.Logger = "test logger";
22+
evt.Release = "test release";
23+
evt.Environment = "test environment";
24+
evt.TransactionName = "test transaction name";
25+
evt.Message = new SentryMessage
26+
{
27+
Params = ["Test"]
28+
};
29+
30+
evt.SetTag("TestTagKey", "TestTagValue");
31+
evt.AddBreadcrumb(new Breadcrumb("test breadcrumb", "test type"));
32+
evt.SetExtra("TestExtraKey", "TestExtraValue");
33+
evt.User = new SentryUser
34+
{
35+
Id = "user id",
36+
Username = "test",
37+
Email = "[email protected]",
38+
IpAddress = "127.0.0.1"
39+
};
40+
41+
var native = evt.ToJavaSentryEvent(new SentryOptions(), new JavaSdk.SentryOptions());
42+
43+
AssertEqual(evt, native);
44+
}
45+
46+
47+
[Fact]
48+
public void ToSentryEvent_ConvertToManaged()
49+
{
50+
var native = new JavaSdk.SentryEvent();
51+
52+
native.Throwable = new Exception("Test Exception").ToThrowable();
53+
native.Timestamp = DateTimeOffset.UtcNow.ToJavaDate();
54+
native.Level = JavaSdk.SentryLevel.Debug;
55+
native.ServerName = "native server name";
56+
native.Dist = "native dist";
57+
native.Logger = "native logger";
58+
native.Release = "native release";
59+
native.Environment = "native env";
60+
native.Transaction = "native transaction";
61+
native.Message = new JavaSdk.Protocol.Message
62+
{
63+
Params = ["Test"]
64+
};
65+
native.SetTag("TestTagKey", "TestTagValue");
66+
native.SetExtra("TestExtraKey", "TestExtraValue");
67+
native.Breadcrumbs =
68+
[
69+
new JavaSdk.Breadcrumb
70+
{
71+
Category = "category",
72+
Level = JavaSdk.SentryLevel.Debug
73+
}
74+
];
75+
76+
native.User = new JavaSdk.Protocol.User
77+
{
78+
Id = "user id",
79+
Username = "test",
80+
Email = "[email protected]",
81+
IpAddress = "127.0.0.1"
82+
};
83+
84+
var managed = native.ToSentryEvent(new JavaSdk.SentryOptions());
85+
86+
AssertEqual(managed, native);
87+
}
88+
89+
90+
private static void AssertEqual(SentryEvent managed, JavaSdk.SentryEvent native)
91+
{
92+
native.ServerName.Should().Be(managed.ServerName, "Server Name");
93+
native.Dist.Should().Be(managed.Distribution, "Distribution");
94+
native.Logger.Should().Be(managed.Logger, "Logger");
95+
native.Release.Should().Be(managed.Release, "Release");
96+
native.Environment.Should().Be(managed.Environment, "Environment");
97+
native.Transaction.Should().Be(managed.TransactionName!, "Transaction");
98+
native.Level!.ToString().ToUpper().Should().Be(managed.Level!.ToString()!.ToUpper(), "Level");
99+
// native.Throwable.Message.Should().Be(managed.Exception!.Message, "Message should match");
100+
101+
// extras
102+
native.Extras.Should().NotBeNull("No extras found");
103+
native.Extras!.Count.Should().Be(1, "Extras should have 1 item");
104+
native.Extras!.Keys!.First().Should().Be(managed.Extra.Keys.First(), "Extras key should match");
105+
native.Extras!.Values!.First().ToString().Should().Be(managed.Extra.Values.First().ToString(), "Extra value should match");
106+
107+
// tags
108+
native.Tags.Should().NotBeNull("No tags found");
109+
native.Tags!.Count.Should().Be(1, "Tags should have 1 item");
110+
native.Tags!.Keys!.First().Should().Be(managed.Tags.Keys.First());
111+
native.Tags!.Values!.First().Should().Be(managed.Tags.Values.First());
112+
113+
// breadcrumbs
114+
native.Breadcrumbs.Should().NotBeNull("No breadcrumbs found");
115+
var nb = native.Breadcrumbs!.First();
116+
var mb = managed.Breadcrumbs!.First();
117+
nb.Message.Should().Be(mb.Message, "Breadcrumb message");
118+
nb.Type.Should().Be(mb.Type, "Breadcrumb type");
119+
120+
// user
121+
native.User!.Id.Should().Be(managed.User.Id, "UserId should match");
122+
native.User.Email.Should().Be(managed.User.Email, "Email should match");
123+
native.User.Username.Should().Be(managed.User.Username, "Username should match");
124+
native.User.IpAddress.Should().Be(managed.User.IpAddress, "IpAddress should match");
125+
}
126+
}

0 commit comments

Comments
 (0)