From 8f816033bbee4e86a780606ae7f3c997566fa083 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 5 Oct 2023 20:18:02 +1300 Subject: [PATCH 01/20] Conditionally include Ben.Demystifier only if IsTrimmable != true --- src/Sentry/Internal/DebugStackTrace.cs | 10 ++++++-- src/Sentry/Sentry.csproj | 32 +++++++++++++++----------- src/Sentry/SentryClient.cs | 4 ++++ 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/Sentry/Internal/DebugStackTrace.cs b/src/Sentry/Internal/DebugStackTrace.cs index 1d92182b50..9cd3570176 100644 --- a/src/Sentry/Internal/DebugStackTrace.cs +++ b/src/Sentry/Internal/DebugStackTrace.cs @@ -121,6 +121,7 @@ internal void MergeDebugImagesInto(SentryEvent @event) /// private IEnumerable CreateFrames(StackTrace stackTrace, bool isCurrentStackTrace) { +#if !IsTrimmable var frames = _options.StackTraceMode switch { StackTraceMode.Enhanced => EnhancedStackTrace.GetFrames(stackTrace).Select(p => p as StackFrame), @@ -130,7 +131,9 @@ private IEnumerable CreateFrames(StackTrace stackTrace, bool i .Where(f => f is not null) #endif }; - +#else + StackFrame[]? frames = null; +#endif // Not to throw on code that ignores nullability warnings. if (frames.IsNull()) { @@ -185,7 +188,7 @@ private SentryStackFrame InternalCreateFrame(StackFrame stackFrame, bool demangl { frame.Module = method.DeclaringType?.FullName ?? unknownRequiredField; frame.Package = method.DeclaringType?.Assembly.FullName; - +#if !IsTrimmable if (_options.StackTraceMode == StackTraceMode.Enhanced && stackFrame is EnhancedStackFrame enhancedStackFrame) { @@ -210,6 +213,9 @@ private SentryStackFrame InternalCreateFrame(StackFrame stackFrame, bool demangl { frame.Function = method.Name; } +#else + frame.Function = method.Name; +#endif // Originally we didn't skip methods from dynamic assemblies, so not to break compatibility: if (_options.StackTraceMode != StackTraceMode.Original && method.Module.Assembly.IsDynamic) diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index f790512a03..c662e32c29 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -5,6 +5,8 @@ $(NoWarn);RS0017 true true + true + $(DefineConstants);IsTrimmable @@ -22,21 +24,23 @@ - - - - %(RecursiveDir)\%(Filename)%(Extension) - - - + + + + + %(RecursiveDir)\%(Filename)%(Extension) + + + - - - - + + + + + diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index d0d52dfe7e..3f992296ac 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -166,8 +166,10 @@ public void CaptureTransaction(Transaction transaction, Hint? hint) } catch (Exception e) { + #if !IsTrimmable // Attempt to demystify exceptions before adding them as breadcrumbs. e.Demystify(); + #endif _options.LogError("The BeforeSendTransaction callback threw an exception. It will be added as breadcrumb and continue.", e); @@ -369,8 +371,10 @@ private bool CaptureEnvelope(Envelope envelope) } catch (Exception e) { +#if !IsTrimmable // Attempt to demystify exceptions before adding them as breadcrumbs. e.Demystify(); +#endif _options.LogError("The BeforeSend callback threw an exception. It will be added as breadcrumb and continue.", e); var data = new Dictionary From 8c8615e94492bd6766f7e0d508b4852b91b88dfd Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 9 Oct 2023 13:44:34 +1300 Subject: [PATCH 02/20] Review feedback --- Sentry.sln.DotSettings | 1 + src/Sentry/Internal/DebugStackTrace.cs | 6 +++--- src/Sentry/Sentry.csproj | 2 +- src/Sentry/SentryClient.cs | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Sentry.sln.DotSettings b/Sentry.sln.DotSettings index 817e3366d3..18ecf270d5 100644 --- a/Sentry.sln.DotSettings +++ b/Sentry.sln.DotSettings @@ -1,4 +1,5 @@  + UI True True True diff --git a/src/Sentry/Internal/DebugStackTrace.cs b/src/Sentry/Internal/DebugStackTrace.cs index 9cd3570176..39683364b2 100644 --- a/src/Sentry/Internal/DebugStackTrace.cs +++ b/src/Sentry/Internal/DebugStackTrace.cs @@ -121,7 +121,7 @@ internal void MergeDebugImagesInto(SentryEvent @event) /// private IEnumerable CreateFrames(StackTrace stackTrace, bool isCurrentStackTrace) { -#if !IsTrimmable +#if !TRIMMABLE var frames = _options.StackTraceMode switch { StackTraceMode.Enhanced => EnhancedStackTrace.GetFrames(stackTrace).Select(p => p as StackFrame), @@ -132,7 +132,7 @@ private IEnumerable CreateFrames(StackTrace stackTrace, bool i #endif }; #else - StackFrame[]? frames = null; + var frames = stackTrace.GetFrames(); #endif // Not to throw on code that ignores nullability warnings. if (frames.IsNull()) @@ -188,7 +188,7 @@ private SentryStackFrame InternalCreateFrame(StackFrame stackFrame, bool demangl { frame.Module = method.DeclaringType?.FullName ?? unknownRequiredField; frame.Package = method.DeclaringType?.Assembly.FullName; -#if !IsTrimmable +#if !TRIMMABLE if (_options.StackTraceMode == StackTraceMode.Enhanced && stackFrame is EnhancedStackFrame enhancedStackFrame) { diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index c662e32c29..5c6ce964aa 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -6,7 +6,7 @@ true true true - $(DefineConstants);IsTrimmable + $(DefineConstants);TRIMMABLE diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index 3f992296ac..e9a7825fd3 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -166,7 +166,7 @@ public void CaptureTransaction(Transaction transaction, Hint? hint) } catch (Exception e) { - #if !IsTrimmable + #if !TRIMMABLE // Attempt to demystify exceptions before adding them as breadcrumbs. e.Demystify(); #endif @@ -371,7 +371,7 @@ private bool CaptureEnvelope(Envelope envelope) } catch (Exception e) { -#if !IsTrimmable +#if !TRIMMABLE // Attempt to demystify exceptions before adding them as breadcrumbs. e.Demystify(); #endif From edb9e386872106c5fb88af4325220f14359482ac Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 9 Oct 2023 22:39:22 +1300 Subject: [PATCH 03/20] Removed automatic registration of WinUIUnhandledExceptionIntegration when IsTrimmable==true --- .../Integrations/WinUIUnhandledExceptionIntegration.cs | 8 ++++++++ src/Sentry/SentryOptions.cs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs b/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs index 6690217054..cb4e30e335 100644 --- a/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs +++ b/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs @@ -47,8 +47,10 @@ public void Register(IHub hub, SentryOptions options) _hub = hub; _options = options; +#if !TRIMMABLE // Hook the main event handler AttachEventHandler(); +#endif // First part of workaround for https://github.com/microsoft/microsoft-ui-xaml/issues/7160 AppDomain.CurrentDomain.FirstChanceException += (_, e) => _lastFirstChanceException = e.Exception; @@ -73,6 +75,11 @@ public void Register(IHub hub, SentryOptions options) }); } +#if !TRIMMABLE + /// + /// This method uses reflection to hook up an UnhandledExceptionHandler. When is true, users will have + /// follow our guidance to perform this initialization manually. + /// private void AttachEventHandler() { try @@ -92,6 +99,7 @@ private void AttachEventHandler() _options.LogError("Could not attach WinUIUnhandledExceptionHandler.", ex); } } +#endif private void WinUIUnhandledExceptionHandler(object sender, object e) { diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index 6b7f8ea8df..d3dae08012 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -1085,7 +1085,7 @@ public SentryOptions() #endif }; -#if NET5_0_OR_GREATER +#if NET5_0_OR_GREATER && !TRIMMABLE if (WinUIUnhandledExceptionIntegration.IsApplicable) { this.AddIntegration(new WinUIUnhandledExceptionIntegration()); From 5d2a51bfa7d2bc96b70758b133d2923ba413b694 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 10 Oct 2023 19:35:18 +1300 Subject: [PATCH 04/20] Merged DebugStackTrace from feat/4.0.0 --- src/Sentry/Internal/DebugStackTrace.cs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/Sentry/Internal/DebugStackTrace.cs b/src/Sentry/Internal/DebugStackTrace.cs index 351a3f485c..3ff51a3b6e 100644 --- a/src/Sentry/Internal/DebugStackTrace.cs +++ b/src/Sentry/Internal/DebugStackTrace.cs @@ -124,10 +124,11 @@ internal void MergeDebugImagesInto(SentryEvent @event) /// private IEnumerable CreateFrames(StackTrace stackTrace, bool isCurrentStackTrace) { -#if !TRIMMABLE var frames = _options.StackTraceMode switch { +#if !TRIMMABLE StackTraceMode.Enhanced => EnhancedStackTrace.GetFrames(stackTrace).Select(p => new RealStackFrame(p)), +#endif _ => stackTrace.GetFrames() // error CS8619: Nullability of reference types in value of type 'StackFrame?[]' doesn't match target type 'IEnumerable'. #if NETCOREAPP3_0 @@ -137,9 +138,7 @@ private IEnumerable CreateFrames(StackTrace stackTrace, bool i .Select(p => new RealStackFrame(p)) #endif }; -#else - var frames = stackTrace.GetFrames(); -#endif + // Not to throw on code that ignores nullability warnings. if (frames.IsNull()) { @@ -199,7 +198,7 @@ private IEnumerable CreateFrames(StackTrace stackTrace, bool i /// /// Native AOT implementation of CreateFrame. - /// Native frames have only limited method information at runtime (and even that can be disabled). + /// Native frames have only limited method information at runtime (and even that can be disabled). /// We try to parse that and also add addresses for server-side symbolication. /// private SentryStackFrame? TryCreateNativeAOTFrame(IStackFrame stackFrame) @@ -216,7 +215,7 @@ private IEnumerable CreateFrames(StackTrace stackTrace, bool i } // Method info is currently only exposed by ToString(), see https://github.com/dotnet/runtime/issues/92869 - // We only care about the case where the method is available (`StackTraceSupport` property is the default `true`): + // We only care about the case where the method is available (`StackTraceSupport` property is the default `true`): // https://github.com/dotnet/runtime/blob/254230253da143a082f47cfaf8711627c0bf2faf/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs#L42 internal static SentryStackFrame ParseNativeAOTToString(string info) { @@ -246,7 +245,6 @@ internal static SentryStackFrame ParseNativeAOTToString(string info) Module = method.DeclaringType?.FullName ?? unknownRequiredField, Package = method.DeclaringType?.Assembly.FullName }; - #if !TRIMMABLE if (stackFrame.Frame is EnhancedStackFrame enhancedStackFrame) { @@ -352,13 +350,7 @@ internal static SentryStackFrame ParseNativeAOTToString(string info) frame.ColumnNumber = colNo; } - if (stackFrame.Frame is not EnhancedStackFrame) - { - DemangleAsyncFunctionName(frame); - DemangleAnonymousFunction(frame); - DemangleLambdaReturnType(frame); - } - +#if !TRIMMABLE if (stackFrame.Frame is EnhancedStackFrame) { // In Enhanced mode, Module (which in this case is the Namespace) @@ -366,8 +358,13 @@ internal static SentryStackFrame ParseNativeAOTToString(string info) // Removing here at the end because this is used to resolve InApp=true/false // TODO what is this really about? we have already run ConfigureAppFrame() at this time... frame.Module = null; + return frame; } +#endif + DemangleAsyncFunctionName(frame); + DemangleAnonymousFunction(frame); + DemangleLambdaReturnType(frame); return frame; } From 9ccc5296ddd06a7aa77ac190f8204ba6531b9797 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 10 Oct 2023 19:52:42 +1300 Subject: [PATCH 05/20] Checkpoint --- src/Sentry/GraphQLRequestContent.cs | 5 ++ .../Internal/Extensions/JsonExtensions.cs | 56 +++++++++++++------ .../Internals/DebugStackTraceTests.verify.cs | 9 +++ test/Sentry.Tests/Sentry.Tests.csproj | 1 + 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/Sentry/GraphQLRequestContent.cs b/src/Sentry/GraphQLRequestContent.cs index 865e8d11b1..a7eda78e62 100644 --- a/src/Sentry/GraphQLRequestContent.cs +++ b/src/Sentry/GraphQLRequestContent.cs @@ -23,6 +23,10 @@ public GraphQLRequestContent(string? requestContent, SentryOptions? options = nu return; } +#if TRIMMABLE + // TODO: Work out how to make this trimmable + throw new NotImplementedException(); +#else try { var deserialized = JsonSerializer.Deserialize>(requestContent, SerializerOptions); @@ -59,6 +63,7 @@ public GraphQLRequestContent(string? requestContent, SentryOptions? options = nu { OperationType = "query"; } +#endif } internal string? RequestContent { get; } diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 440426d6f0..9e285098aa 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -14,6 +14,28 @@ internal static class JsonExtensions new UIntPtrNullableJsonConverter() }; + delegate object? UserDefinedReader(ref Utf8JsonReader reader, JsonSerializerOptions options); + delegate void UserDefinedWriter(Utf8JsonWriter writer, object value, JsonSerializerOptions options); + + private class JsonConverterWrapper + { + public required UserDefinedReader Read { get; set; } + public required UserDefinedWriter Write { get; set; } + } + + private static readonly ConcurrentDictionary UserDefinedConverters = new (); + public static void AddUserDefinedJsonConverter(JsonConverter converter) + { + Type typeToConvert = typeof(T); + var wrapper = new JsonConverterWrapper() + { + Read = (ref Utf8JsonReader reader, JsonSerializerOptions options) => + converter.Read(ref reader, typeToConvert, options), + Write = (writer, value, options) => converter.Write(writer, (T)value, options) + }; + UserDefinedConverters[typeof(T)] = wrapper; + } + internal static bool JsonPreserveReferences { get; set; } = true; private static JsonSerializerOptions SerializerOptions = null!; private static JsonSerializerOptions AltSerializerOptions = null!; @@ -479,26 +501,28 @@ public static void WriteDynamicValue( } else { - if (!JsonPreserveReferences) + if (UserDefinedConverters.TryGetValue(value.GetType(), out var converter)) { - JsonSerializer.Serialize(writer, value, SerializerOptions); + var options = JsonPreserveReferences ? AltSerializerOptions : SerializerOptions; + converter.Write(writer, value, SerializerOptions); return; } - try - { - // Use an intermediate temporary stream, so we can retry if serialization fails. - using var tempStream = new MemoryStream(); - using var tempWriter = new Utf8JsonWriter(tempStream, writer.Options); - JsonSerializer.Serialize(tempWriter, value, SerializerOptions); - tempWriter.Flush(); - writer.WriteRawValue(tempStream.ToArray()); - } - catch (JsonException) - { - // Retry, preserving references to avoid cyclical dependency. - JsonSerializer.Serialize(writer, value, AltSerializerOptions); - } + // TODO: Understand why this code was added previously and what the impact of removing it would be + // try + // { + // // Use an intermediate temporary stream, so we can retry if serialization fails. + // using var tempStream = new MemoryStream(); + // using var tempWriter = new Utf8JsonWriter(tempStream, writer.Options); + // JsonSerializer.Serialize(tempWriter, value, SerializerOptions); + // tempWriter.Flush(); + // writer.WriteRawValue(tempStream.ToArray()); + // } + // catch (JsonException) + // { + // } + + Debug.WriteLine("Failed to serialize object of type '{0}'.", value.GetType()); } } diff --git a/test/Sentry.Tests/Internals/DebugStackTraceTests.verify.cs b/test/Sentry.Tests/Internals/DebugStackTraceTests.verify.cs index 36d7b62162..04e51d8fb7 100644 --- a/test/Sentry.Tests/Internals/DebugStackTraceTests.verify.cs +++ b/test/Sentry.Tests/Internals/DebugStackTraceTests.verify.cs @@ -40,16 +40,25 @@ public void CreateSentryStackFrame_AppNamespaceExcluded_NotInAppFrame() Assert.False(actual?.InApp); } +#if !TRIMMABLE [Theory] [InlineData(true)] [InlineData(false)] public void CreateSentryStackFrame_SystemType_NotInAppFrame(bool useEnhancedStackTrace) +#else + [Fact] + public void CreateSentryStackFrame_SystemType_NotInAppFrame() +#endif { // Arrange var sut = _fixture.GetSut(); var exception = Assert.ThrowsAny(() => _ = Convert.FromBase64String("This will throw.")); var stackTrace = new StackTrace(exception); +#if !TRIMMABLE var frame = useEnhancedStackTrace ? EnhancedStackTrace.GetFrames(stackTrace)[0] : stackTrace.GetFrame(0); +#else + var frame = stackTrace.GetFrame(0); +#endif // Sanity Check Assert.NotNull(frame); diff --git a/test/Sentry.Tests/Sentry.Tests.csproj b/test/Sentry.Tests/Sentry.Tests.csproj index a0bdebbb3b..56aa58d6c3 100644 --- a/test/Sentry.Tests/Sentry.Tests.csproj +++ b/test/Sentry.Tests/Sentry.Tests.csproj @@ -1,6 +1,7 @@  + $(DefineConstants);TRIMMABLE net7.0;net6.0;netcoreapp3.1;net48 $(TargetFrameworks);net7.0-android $(TargetFrameworks);net7.0-ios From dcb7441a1c5c337bac18774ea170a338469c5c17 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 10 Oct 2023 22:07:04 +1300 Subject: [PATCH 06/20] Compiles (serialization verify tests still fail) --- .../Internal/Extensions/JsonExtensions.cs | 79 ++++++++++--------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 9e285098aa..7cc4914b93 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -14,31 +14,18 @@ internal static class JsonExtensions new UIntPtrNullableJsonConverter() }; - delegate object? UserDefinedReader(ref Utf8JsonReader reader, JsonSerializerOptions options); - delegate void UserDefinedWriter(Utf8JsonWriter writer, object value, JsonSerializerOptions options); + internal static bool JsonPreserveReferences { get; set; } = true; + private static JsonSerializerOptions SerializerOptions = null!; + private static JsonSerializerOptions AltSerializerOptions = null!; - private class JsonConverterWrapper - { - public required UserDefinedReader Read { get; set; } - public required UserDefinedWriter Write { get; set; } - } +#if NET6_0_OR_GREATER + private static JsonSerializerContext? CustomSerializerContext; - private static readonly ConcurrentDictionary UserDefinedConverters = new (); - public static void AddUserDefinedJsonConverter(JsonConverter converter) + internal static void AddJsonSerializerContext(JsonSerializerContext context) { - Type typeToConvert = typeof(T); - var wrapper = new JsonConverterWrapper() - { - Read = (ref Utf8JsonReader reader, JsonSerializerOptions options) => - converter.Read(ref reader, typeToConvert, options), - Write = (writer, value, options) => converter.Write(writer, (T)value, options) - }; - UserDefinedConverters[typeof(T)] = wrapper; + CustomSerializerContext = context; } - - internal static bool JsonPreserveReferences { get; set; } = true; - private static JsonSerializerOptions SerializerOptions = null!; - private static JsonSerializerOptions AltSerializerOptions = null!; +#endif static JsonExtensions() { @@ -47,6 +34,9 @@ static JsonExtensions() internal static void ResetSerializerOptions() { +#if NET6_0_OR_GREATER + CustomSerializerContext = null; +#endif SerializerOptions = new JsonSerializerOptions() .AddDefaultConverters(); @@ -57,6 +47,7 @@ internal static void ResetSerializerOptions() .AddDefaultConverters(); } + internal static void AddJsonConverter(JsonConverter converter) { // only add if we don't have this instance already @@ -501,28 +492,38 @@ public static void WriteDynamicValue( } else { - if (UserDefinedConverters.TryGetValue(value.GetType(), out var converter)) +#if !TRIMMABLE + if (!JsonPreserveReferences) { - var options = JsonPreserveReferences ? AltSerializerOptions : SerializerOptions; - converter.Write(writer, value, SerializerOptions); + JsonSerializer.Serialize(writer, value, SerializerOptions); return; } - // TODO: Understand why this code was added previously and what the impact of removing it would be - // try - // { - // // Use an intermediate temporary stream, so we can retry if serialization fails. - // using var tempStream = new MemoryStream(); - // using var tempWriter = new Utf8JsonWriter(tempStream, writer.Options); - // JsonSerializer.Serialize(tempWriter, value, SerializerOptions); - // tempWriter.Flush(); - // writer.WriteRawValue(tempStream.ToArray()); - // } - // catch (JsonException) - // { - // } - - Debug.WriteLine("Failed to serialize object of type '{0}'.", value.GetType()); + try + { + // Use an intermediate temporary stream, so we can retry if serialization fails. + using var tempStream = new MemoryStream(); + using var tempWriter = new Utf8JsonWriter(tempStream, writer.Options); + JsonSerializer.Serialize(tempWriter, value, SerializerOptions); + tempWriter.Flush(); + writer.WriteRawValue(tempStream.ToArray()); + } + catch (JsonException) + { + // Retry, preserving references to avoid cyclical dependency. + JsonSerializer.Serialize(writer, value, AltSerializerOptions); + } +#else + var valueType = value.GetType(); + #if NET6_0_OR_GREATER + if (CustomSerializerContext is { } jsonSerializerContext) + { + JsonSerializer.Serialize(writer, value, valueType, jsonSerializerContext); + return; + } + #endif + throw new Exception($"No JsonSerializerContext configured for {valueType}"); +#endif } } From 4f6644c0a327052a94fba9da4299eb8f615e9e0a Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 14:53:20 +1300 Subject: [PATCH 07/20] Added some tests for JsonText source generated serializers --- .../WinUIUnhandledExceptionIntegration.cs | 2 +- .../Internal/Extensions/JsonExtensions.cs | 106 +++++++++++------- .../Sentry.Tests/SerializationTests.verify.cs | 55 ++++++++- 3 files changed, 122 insertions(+), 41 deletions(-) diff --git a/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs b/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs index cb4e30e335..d5a82a33d1 100644 --- a/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs +++ b/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs @@ -77,7 +77,7 @@ public void Register(IHub hub, SentryOptions options) #if !TRIMMABLE /// - /// This method uses reflection to hook up an UnhandledExceptionHandler. When is true, users will have + /// This method uses reflection to hook up an UnhandledExceptionHandler. When IsTrimmed is true, users will have /// follow our guidance to perform this initialization manually. /// private void AttachEventHandler() diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 7cc4914b93..4b6631caea 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -15,43 +15,54 @@ internal static class JsonExtensions }; internal static bool JsonPreserveReferences { get; set; } = true; - private static JsonSerializerOptions SerializerOptions = null!; - private static JsonSerializerOptions AltSerializerOptions = null!; + + static JsonExtensions() + { + ResetSerializerOptions(); + } + + private static JsonSerializerOptions DefaultOptions(bool preserveReferences) => + preserveReferences + ? new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.Preserve } + .AddDefaultConverters() + : new JsonSerializerOptions().AddDefaultConverters(); #if NET6_0_OR_GREATER - private static JsonSerializerContext? CustomSerializerContext; + private static JsonSerializerContext SerializerContext = null!; + private static JsonSerializerContext AltSerializerContext = null!; - internal static void AddJsonSerializerContext(JsonSerializerContext context) + internal static void AddJsonSerializerContext(Func jsonSerializerContextFactory) + where T: JsonSerializerContext { - CustomSerializerContext = context; + SerializerContext = jsonSerializerContextFactory(DefaultOptions(false)); + AltSerializerContext = jsonSerializerContextFactory(DefaultOptions(true)); } -#endif - static JsonExtensions() + internal static void ResetSerializerOptions() { - ResetSerializerOptions(); + SerializerContext = new SentryJsonContext(DefaultOptions(false)); + AltSerializerContext = new SentryJsonContext(DefaultOptions(true)); } +#else + private static JsonSerializerOptions SerializerOptions = null!; + private static JsonSerializerOptions AltSerializerOptions = null!; internal static void ResetSerializerOptions() { -#if NET6_0_OR_GREATER - CustomSerializerContext = null; -#endif - SerializerOptions = new JsonSerializerOptions() - .AddDefaultConverters(); - - AltSerializerOptions = new JsonSerializerOptions - { - ReferenceHandler = ReferenceHandler.Preserve - } - .AddDefaultConverters(); + SerializerOptions = DefaultOptions(false); + AltSerializerOptions = DefaultOptions(true); } +#endif internal static void AddJsonConverter(JsonConverter converter) { // only add if we don't have this instance already +#if NET6_0_OR_GREATER + var converters = SerializerContext.Options.Converters; +#else var converters = SerializerOptions.Converters; +#endif if (converters.Contains(converter)) { return; @@ -59,8 +70,13 @@ internal static void AddJsonConverter(JsonConverter converter) try { +#if NET6_0_OR_GREATER + SerializerContext.Options.Converters.Add(converter); + AltSerializerContext.Options.Converters.Add(converter); +#else SerializerOptions.Converters.Add(converter); AltSerializerOptions.Converters.Add(converter); +#endif } catch (InvalidOperationException) { @@ -492,41 +508,46 @@ public static void WriteDynamicValue( } else { -#if !TRIMMABLE if (!JsonPreserveReferences) { - JsonSerializer.Serialize(writer, value, SerializerOptions); + InternalSerialize(writer, value, preserveReferences: false); return; } try { - // Use an intermediate temporary stream, so we can retry if serialization fails. - using var tempStream = new MemoryStream(); - using var tempWriter = new Utf8JsonWriter(tempStream, writer.Options); - JsonSerializer.Serialize(tempWriter, value, SerializerOptions); - tempWriter.Flush(); - writer.WriteRawValue(tempStream.ToArray()); + // Use an intermediate byte array, so we can retry if serialization fails. + var bytes = InternalSerializeToUtf8Bytes(value); + writer.WriteRawValue(bytes); } catch (JsonException) { // Retry, preserving references to avoid cyclical dependency. - JsonSerializer.Serialize(writer, value, AltSerializerOptions); - } -#else - var valueType = value.GetType(); - #if NET6_0_OR_GREATER - if (CustomSerializerContext is { } jsonSerializerContext) - { - JsonSerializer.Serialize(writer, value, valueType, jsonSerializerContext); - return; + InternalSerialize(writer, value, preserveReferences: true); } - #endif - throw new Exception($"No JsonSerializerContext configured for {valueType}"); -#endif } } +#if NET6_0_OR_GREATER + private static byte[] InternalSerializeToUtf8Bytes(object value) => + JsonSerializer.SerializeToUtf8Bytes(value, value.GetType(), SerializerContext); + + private static void InternalSerialize(Utf8JsonWriter writer, object value, bool preserveReferences = false) + { + var context = preserveReferences ? AltSerializerContext : SerializerContext; + JsonSerializer.Serialize(writer, value, value.GetType(), context); + } +#else + private static byte[] InternalSerializeToUtf8Bytes(object value) => + JsonSerializer.SerializeToUtf8Bytes(value, SerializerOptions); + + private static void InternalSerialize(Utf8JsonWriter writer, object value, bool preserveReferences = false) + { + var options = preserveReferences ? AltSerializerOptions : SerializerOptions; + JsonSerializer.Serialize(writer, value, options); + } +#endif + public static void WriteDynamic( this Utf8JsonWriter writer, string propertyName, @@ -816,3 +837,10 @@ public static void WriteString( } } } + +#if NET6_0_OR_GREATER +[JsonSerializable(typeof(BreadcrumbLevel))] +internal partial class SentryJsonContext : JsonSerializerContext +{ +} +#endif diff --git a/test/Sentry.Tests/SerializationTests.verify.cs b/test/Sentry.Tests/SerializationTests.verify.cs index 37edd86544..63d1f84123 100644 --- a/test/Sentry.Tests/SerializationTests.verify.cs +++ b/test/Sentry.Tests/SerializationTests.verify.cs @@ -18,6 +18,39 @@ public async Task Serialization(string name, object target) await Verify(json).UseParameters(name); } +#if NET7_0_OR_GREATER + internal class NestedStringClass { public string Value { get; set; } } + internal class NestedIntClass { public int Value { get; set; } } + internal class NestedNIntClass { public nint Value { get; set; } } + internal class NestedNuIntClass { public nuint Value { get; set; } } + internal class NestedIntPtrClass { public IntPtr Value { get; set; } } + internal class NestedNullableIntPtrClass { public IntPtr? Value { get; set; } } + internal class NestedUIntPtrClass { public UIntPtr Value { get; set; } } + internal class NestedNullableUIntPtrClass { public UIntPtr? Value { get; set; } } + + public static IEnumerable GetData() + { + yield return new object[] { "string", "string value" }; + yield return new object[] { "int", 5 }; + + JsonExtensions.ResetSerializerOptions(); + JsonExtensions.AddJsonSerializerContext(options => new SerializationTestsJsonContext(options)); + yield return new object[] { "nested string", new NestedStringClass { Value= "string value" } }; + yield return new object[] { "nested int", new NestedIntClass { Value= 5 } }; + yield return new object[] { "nested nint", new NestedNIntClass { Value= 5 } }; + yield return new object[] { "nested nuint", new NestedNuIntClass { Value= 5 } }; + yield return new object[] { "nested IntPtr", new NestedIntPtrClass { Value= (IntPtr)3 } }; + yield return new object[] { "nested nullable IntPtr", new NestedNullableIntPtrClass { Value= (IntPtr?)3 } }; + yield return new object[] { "nested UIntPtr", new NestedUIntPtrClass { Value= (UIntPtr)3 } }; + yield return new object[] { "nested nullable UIntPtr", new NestedNullableUIntPtrClass { Value= (UIntPtr?)3 } }; + + JsonExtensions.ResetSerializerOptions(); + JsonExtensions.AddJsonConverter(new CustomObjectConverter()); + JsonExtensions.AddJsonSerializerContext(options => new SerializationTestsJsonContext(options)); + yield return new object[] {"custom object with value", new CustomObject("test")}; + yield return new object[] {"custom object with null", new CustomObject(null)}; + } +#else public static IEnumerable GetData() { yield return new object[] {"string", "string value"}; @@ -36,15 +69,20 @@ public static IEnumerable GetData() yield return new object[] {"custom object with value", new CustomObject("test")}; yield return new object[] {"custom object with null", new CustomObject(null)}; } +#endif public class CustomObject { + public CustomObject() + { + } + public CustomObject(string value) { Value = value; } - internal string Value { get; } + public string Value { get; set; } } public class CustomObjectConverter : JsonConverter @@ -56,3 +94,18 @@ public override void Write(Utf8JsonWriter writer, CustomObject value, JsonSerial => writer.WriteStringValue(value.Value); } } + +#if NET7_0_OR_GREATER +[JsonSerializable(typeof(SerializationTests.CustomObject))] +[JsonSerializable(typeof(SerializationTests.NestedStringClass))] +[JsonSerializable(typeof(SerializationTests.NestedIntClass))] +[JsonSerializable(typeof(SerializationTests.NestedNIntClass))] +[JsonSerializable(typeof(SerializationTests.NestedNuIntClass))] +[JsonSerializable(typeof(SerializationTests.NestedIntPtrClass))] +[JsonSerializable(typeof(SerializationTests.NestedNullableIntPtrClass))] +[JsonSerializable(typeof(SerializationTests.NestedUIntPtrClass))] +[JsonSerializable(typeof(SerializationTests.NestedNullableUIntPtrClass))] +internal partial class SerializationTestsJsonContext : JsonSerializerContext +{ +} +#endif From 5479f1e4dd8dd943487ec00be3f8049ad24fdc22 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 15:05:19 +1300 Subject: [PATCH 08/20] Added SentryOptions.AddJsonSerializerContext --- .../Internal/Extensions/JsonExtensions.cs | 1 - src/Sentry/SentryOptions.cs | 26 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 4b6631caea..0803e2e9a3 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -54,7 +54,6 @@ internal static void ResetSerializerOptions() } #endif - internal static void AddJsonConverter(JsonConverter converter) { // only add if we don't have this instance already diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index e424f58864..60d1ef8da3 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -986,6 +986,32 @@ public void AddJsonConverter(JsonConverter converter) JsonExtensions.AddJsonConverter(converter); } +#if NET6_0_OR_GREATER + /// + /// Configures a custom to be used when serializing or deserializing + /// objects to JSON with this SDK. + /// + /// + /// A builder that takes and returns a + /// + /// + /// This currently modifies a static list, so will affect any instance of the Sentry SDK. + /// If that becomes problematic, we will have to refactor all serialization code to be + /// able to accept an instance of . + /// + public void AddJsonSerializerContext(Func contextBuilder) + where T: JsonSerializerContext + { + // protect against null because user may not have nullability annotations enabled + if (contextBuilder == null!) + { + throw new ArgumentNullException(nameof(contextBuilder)); + } + + JsonExtensions.AddJsonSerializerContext(contextBuilder); + } +#endif + /// /// When true, if an object being serialized to JSON contains references to other objects, and the /// serialized object graph exceed the maximum allowable depth, the object will instead be serialized using From 98a51c7973e4e1cc3dfe06579e6d0cacaaebcfe7 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 15:08:58 +1300 Subject: [PATCH 09/20] Update SerializationTests.verify.cs --- test/Sentry.Tests/SerializationTests.verify.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/Sentry.Tests/SerializationTests.verify.cs b/test/Sentry.Tests/SerializationTests.verify.cs index 63d1f84123..121aec28de 100644 --- a/test/Sentry.Tests/SerializationTests.verify.cs +++ b/test/Sentry.Tests/SerializationTests.verify.cs @@ -73,16 +73,12 @@ public static IEnumerable GetData() public class CustomObject { - public CustomObject() - { - } - public CustomObject(string value) { Value = value; } - public string Value { get; set; } + internal string Value { get; } } public class CustomObjectConverter : JsonConverter From dc2c52236c01e0115acb71f66bc9ca1683fb1e09 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 16:34:08 +1300 Subject: [PATCH 10/20] Update DebugStackTrace.cs --- src/Sentry/Internal/DebugStackTrace.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Sentry/Internal/DebugStackTrace.cs b/src/Sentry/Internal/DebugStackTrace.cs index 3ff51a3b6e..20cbbec663 100644 --- a/src/Sentry/Internal/DebugStackTrace.cs +++ b/src/Sentry/Internal/DebugStackTrace.cs @@ -243,7 +243,8 @@ internal static SentryStackFrame ParseNativeAOTToString(string info) var frame = new SentryStackFrame { Module = method.DeclaringType?.FullName ?? unknownRequiredField, - Package = method.DeclaringType?.Assembly.FullName + Package = method.DeclaringType?.Assembly.FullName, + Function = method.Name }; #if !TRIMMABLE if (stackFrame.Frame is EnhancedStackFrame enhancedStackFrame) @@ -265,12 +266,6 @@ internal static SentryStackFrame ParseNativeAOTToString(string info) : module; } } - else - { - frame.Function = method.Name; - } -#else - frame.Function = method.Name; #endif // Originally we didn't skip methods from dynamic assemblies, so not to break compatibility: From a2161162a7dcba9ca6e15fb93aa923710b5ff50c Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 20:13:20 +1300 Subject: [PATCH 11/20] Fixed JsonSerializerContext with CustomConverters --- .../Internal/Extensions/JsonExtensions.cs | 64 +++++++++---------- .../Sentry.Tests/SerializationTests.verify.cs | 4 ++ 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 0803e2e9a3..da98497437 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -14,6 +14,8 @@ internal static class JsonExtensions new UIntPtrNullableJsonConverter() }; + private static List CustomConverters = new List(); + internal static bool JsonPreserveReferences { get; set; } = true; static JsonExtensions() @@ -21,11 +23,24 @@ static JsonExtensions() ResetSerializerOptions(); } - private static JsonSerializerOptions DefaultOptions(bool preserveReferences) => - preserveReferences - ? new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.Preserve } - .AddDefaultConverters() - : new JsonSerializerOptions().AddDefaultConverters(); + private static JsonSerializerOptions BuildOptions(bool preserveReferences) + { + var options = new JsonSerializerOptions(); + if (preserveReferences) + { + options.ReferenceHandler = ReferenceHandler.Preserve; + } + foreach (var converter in DefaultConverters) + { + options.Converters.Add(converter); + } + foreach (var converter in CustomConverters) + { + options.Converters.Add(converter); + } + + return options; + } #if NET6_0_OR_GREATER private static JsonSerializerContext SerializerContext = null!; @@ -34,48 +49,39 @@ private static JsonSerializerOptions DefaultOptions(bool preserveReferences) => internal static void AddJsonSerializerContext(Func jsonSerializerContextFactory) where T: JsonSerializerContext { - SerializerContext = jsonSerializerContextFactory(DefaultOptions(false)); - AltSerializerContext = jsonSerializerContextFactory(DefaultOptions(true)); + SerializerContext = jsonSerializerContextFactory(BuildOptions(false)); + AltSerializerContext = jsonSerializerContextFactory(BuildOptions(true)); } internal static void ResetSerializerOptions() { - SerializerContext = new SentryJsonContext(DefaultOptions(false)); - AltSerializerContext = new SentryJsonContext(DefaultOptions(true)); + SerializerContext = new SentryJsonContext(BuildOptions(false)); + AltSerializerContext = new SentryJsonContext(BuildOptions(true)); } + #else private static JsonSerializerOptions SerializerOptions = null!; private static JsonSerializerOptions AltSerializerOptions = null!; internal static void ResetSerializerOptions() { - SerializerOptions = DefaultOptions(false); - AltSerializerOptions = DefaultOptions(true); + SerializerOptions = BuildOptions(false); + AltSerializerOptions = BuildOptions(true); } #endif internal static void AddJsonConverter(JsonConverter converter) { // only add if we don't have this instance already -#if NET6_0_OR_GREATER - var converters = SerializerContext.Options.Converters; -#else - var converters = SerializerOptions.Converters; -#endif - if (converters.Contains(converter)) + if (CustomConverters.Contains(converter)) { return; } try { -#if NET6_0_OR_GREATER - SerializerContext.Options.Converters.Add(converter); - AltSerializerContext.Options.Converters.Add(converter); -#else - SerializerOptions.Converters.Add(converter); - AltSerializerOptions.Converters.Add(converter); -#endif + CustomConverters.Add(converter); + ResetSerializerOptions(); } catch (InvalidOperationException) { @@ -90,16 +96,6 @@ internal static void AddJsonConverter(JsonConverter converter) } } - private static JsonSerializerOptions AddDefaultConverters(this JsonSerializerOptions options) - { - foreach (var converter in DefaultConverters) - { - options.Converters.Add(converter); - } - - return options; - } - public static void Deconstruct(this JsonProperty jsonProperty, out string name, out JsonElement value) { name = jsonProperty.Name; diff --git a/test/Sentry.Tests/SerializationTests.verify.cs b/test/Sentry.Tests/SerializationTests.verify.cs index 121aec28de..9f6a61f508 100644 --- a/test/Sentry.Tests/SerializationTests.verify.cs +++ b/test/Sentry.Tests/SerializationTests.verify.cs @@ -1,3 +1,7 @@ +#if NET7_0_OR_GREATER +using System.Text.Json.Serialization.Metadata; +#endif + namespace Sentry.Tests; [UsesVerify] From 3b24c7b0d29f217c52cd086243d0db5016eabb14 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 21:16:18 +1300 Subject: [PATCH 12/20] Update ContextsTests.cs --- test/Sentry.Tests/Protocol/Context/ContextsTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Sentry.Tests/Protocol/Context/ContextsTests.cs b/test/Sentry.Tests/Protocol/Context/ContextsTests.cs index 204cc264c0..4e4038917d 100644 --- a/test/Sentry.Tests/Protocol/Context/ContextsTests.cs +++ b/test/Sentry.Tests/Protocol/Context/ContextsTests.cs @@ -137,6 +137,7 @@ public void SerializeObject_SingleRuntimePropertySet_SerializeSingleProperty() Assert.Equal("""{"runtime":{"type":"runtime","version":"2.1.1.100"}}""", actualString); } +#if !TRIMMABLE [Fact] public void SerializeObject_AnonymousObject_SerializedCorrectly() { @@ -159,6 +160,7 @@ public void SerializeObject_AnonymousObject_SerializedCorrectly() ["Baz"] = "kek" }); } +#endif [Fact] public void SerializeObject_SortsContextKeys() From ae5643635a4ce65ce350d98816e1a5db3457c0a5 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 22:19:48 +1300 Subject: [PATCH 13/20] Update JsonExtensions.cs --- src/Sentry/Internal/Extensions/JsonExtensions.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index da98497437..3f2f9da4ed 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -46,17 +46,20 @@ private static JsonSerializerOptions BuildOptions(bool preserveReferences) private static JsonSerializerContext SerializerContext = null!; private static JsonSerializerContext AltSerializerContext = null!; - internal static void AddJsonSerializerContext(Func jsonSerializerContextFactory) + private static Func ContextBuilder = options + => new SentryJsonContext(options); + + internal static void AddJsonSerializerContext(Func jsonSerializerContextBuilder) where T: JsonSerializerContext { - SerializerContext = jsonSerializerContextFactory(BuildOptions(false)); - AltSerializerContext = jsonSerializerContextFactory(BuildOptions(true)); + ContextBuilder = jsonSerializerContextBuilder; + ResetSerializerOptions(); } internal static void ResetSerializerOptions() { - SerializerContext = new SentryJsonContext(BuildOptions(false)); - AltSerializerContext = new SentryJsonContext(BuildOptions(true)); + SerializerContext = ContextBuilder(BuildOptions(false)); + AltSerializerContext = ContextBuilder(BuildOptions(true)); } #else From bfb081886355c75016f3a2b587bfa7aebd343879 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 11 Oct 2023 22:52:11 +1300 Subject: [PATCH 14/20] Added support for a SentryJsonContext as well as a user defined JsonSerializerContext --- .../Internal/Extensions/JsonExtensions.cs | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 3f2f9da4ed..2f09cd45b5 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -43,23 +43,30 @@ private static JsonSerializerOptions BuildOptions(bool preserveReferences) } #if NET6_0_OR_GREATER - private static JsonSerializerContext SerializerContext = null!; - private static JsonSerializerContext AltSerializerContext = null!; + private static List DefaultSerializerContexts = new(); + private static List ReferencePreservingSerializerContexts = new(); - private static Func ContextBuilder = options - => new SentryJsonContext(options); + private static List> JsonSerializerContextBuilders = new() + { + options => new SentryJsonContext(options) + }; internal static void AddJsonSerializerContext(Func jsonSerializerContextBuilder) where T: JsonSerializerContext { - ContextBuilder = jsonSerializerContextBuilder; + JsonSerializerContextBuilders.Add(jsonSerializerContextBuilder); ResetSerializerOptions(); } internal static void ResetSerializerOptions() { - SerializerContext = ContextBuilder(BuildOptions(false)); - AltSerializerContext = ContextBuilder(BuildOptions(true)); + DefaultSerializerContexts.Clear(); + ReferencePreservingSerializerContexts.Clear(); + foreach (var builder in JsonSerializerContextBuilders) + { + DefaultSerializerContexts.Add(builder(BuildOptions(false))); + ReferencePreservingSerializerContexts.Add(builder(BuildOptions(true))); + } } #else @@ -527,12 +534,23 @@ public static void WriteDynamicValue( } #if NET6_0_OR_GREATER - private static byte[] InternalSerializeToUtf8Bytes(object value) => - JsonSerializer.SerializeToUtf8Bytes(value, value.GetType(), SerializerContext); + + private static JsonSerializerContext GetSerializerContext(Type type, bool preserveReferences = false) + { + var contexts = preserveReferences ? ReferencePreservingSerializerContexts : DefaultSerializerContexts; + return contexts.FirstOrDefault(c => c.GetTypeInfo(type) != null) + ?? contexts[0]; // If none of the contexts has type info, this gives us a proper exception message + } + + private static byte[] InternalSerializeToUtf8Bytes(object value) + { + var context = GetSerializerContext(value.GetType()); + return JsonSerializer.SerializeToUtf8Bytes(value, value.GetType(), context); + } private static void InternalSerialize(Utf8JsonWriter writer, object value, bool preserveReferences = false) { - var context = preserveReferences ? AltSerializerContext : SerializerContext; + var context = GetSerializerContext(value.GetType(), preserveReferences); JsonSerializer.Serialize(writer, value, value.GetType(), context); } #else @@ -837,7 +855,7 @@ public static void WriteString( } #if NET6_0_OR_GREATER -[JsonSerializable(typeof(BreadcrumbLevel))] +[JsonSerializable(typeof(Internal.GrowableArray))] internal partial class SentryJsonContext : JsonSerializerContext { } From 7322b6a106dd79992294cde3430f85aa8ebd0796 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 12 Oct 2023 12:36:00 +1300 Subject: [PATCH 15/20] Fixed almost all of the JsonTests --- test/Sentry.Tests/Internals/JsonTests.cs | 35 +++++++++++++++++-- .../Internals/SentryStackTraceFactoryTests.cs | 8 +++++ .../SentryStackTraceFactoryTests.verify.cs | 2 ++ test/Sentry.Tests/Sentry.Tests.csproj | 1 + 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/test/Sentry.Tests/Internals/JsonTests.cs b/test/Sentry.Tests/Internals/JsonTests.cs index 4214d759b4..8161869716 100644 --- a/test/Sentry.Tests/Internals/JsonTests.cs +++ b/test/Sentry.Tests/Internals/JsonTests.cs @@ -21,7 +21,7 @@ public static Exception GenerateException(string description) } } - private class DataAndNonSerializableObject + internal class DataAndNonSerializableObject { /// /// A class containing two objects that can be serialized and a third one that will have issues if serialized. @@ -60,7 +60,7 @@ private class ExceptionObjectMock public int? HResult { get; set; } } - private class DataWithSerializableObject : DataAndNonSerializableObject + internal class DataWithSerializableObject : DataAndNonSerializableObject { /// /// A class containing three objects that can be serialized. @@ -72,6 +72,9 @@ public DataWithSerializableObject(T obj) : base(obj) { } [Fact] public void WriteDynamicValue_ExceptionParameter_SerializedException() { +#if NET6_0_OR_GREATER + JsonExtensions.AddJsonSerializerContext(o => new JsonTestsJsonContext(o)); +#endif // Arrange var ex = GenerateException("Test"); ex.Data.Add("a", "b"); @@ -107,6 +110,9 @@ public void WriteDynamicValue_ExceptionParameter_SerializedException() public void WriteDynamicValue_ClassWithExceptionParameter_SerializedClassWithException() { // Arrange +#if NET6_0_OR_GREATER + JsonExtensions.AddJsonSerializerContext(o => new JsonTestsJsonContext(o)); +#endif var expectedMessage = "T est"; var expectedData = new KeyValuePair("a", "b"); var ex = GenerateException(expectedMessage); @@ -135,7 +141,11 @@ public void WriteDynamicValue_ClassWithExceptionParameter_SerializedClassWithExc [Fact] public void WriteDynamicValue_TypeParameter_FullNameTypeOutput() { +#if NET6_0_OR_GREATER + JsonExtensions.AddJsonSerializerContext(o => new JsonTestsJsonContext(o)); +#endif // Arrange + // TODO: Fix "Metadata for type 'System.RuntimeType' was not provided by TypeInfoResolver"... var type = typeof(Exception); var expectedValue = "\"System.Exception\""; @@ -150,6 +160,9 @@ public void WriteDynamicValue_TypeParameter_FullNameTypeOutput() public void WriteDynamicValue_ClassWithTypeParameter_ClassFormatted() { // Arrange +#if NET6_0_OR_GREATER + JsonExtensions.AddJsonSerializerContext(o => new JsonTestsJsonContext(o)); +#endif var type = typeof(List<>).GetGenericArguments()[0]; var data = new DataWithSerializableObject(type); @@ -165,6 +178,9 @@ public void WriteDynamicValue_ClassWithTypeParameter_ClassFormatted() public void WriteDynamicValue_ClassWithAssembly_SerializedClassWithNullAssembly() { // Arrange +#if NET6_0_OR_GREATER + JsonExtensions.AddJsonSerializerContext(o => new JsonTestsJsonContext(o)); +#endif var data = new DataAndNonSerializableObject(AppDomain.CurrentDomain.GetAssemblies()[0]); // Act @@ -205,6 +221,9 @@ public void WriteDynamic_ComplexObject_PreserveReferences() { // Arrange JsonExtensions.JsonPreserveReferences = true; +#if NET6_0_OR_GREATER + JsonExtensions.AddJsonSerializerContext(o => new JsonTestsJsonContext(o)); +#endif var testObject = new SelfReferencedObject(); using var stream = new MemoryStream(); using (var writer = new Utf8JsonWriter(stream)) @@ -242,3 +261,15 @@ public class SelfReferencedObject public SelfReferencedObject Object => this; } } + +#if NET6_0_OR_GREATER +[JsonSerializable(typeof(AccessViolationException))] +[JsonSerializable(typeof(Exception))] +[JsonSerializable(typeof(JsonTests.SelfReferencedObject))] +[JsonSerializable(typeof(JsonTests.DataAndNonSerializableObject))] +[JsonSerializable(typeof(JsonTests.DataWithSerializableObject))] +[JsonSerializable(typeof(JsonTests.DataWithSerializableObject))] +internal partial class JsonTestsJsonContext : JsonSerializerContext +{ +} +#endif diff --git a/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.cs b/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.cs index 665d2650f9..42631efe65 100644 --- a/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.cs +++ b/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.cs @@ -47,7 +47,9 @@ public void Create_NoExceptionAndAttachStackTraceOptionOnWithOriginalMode_Curren [Theory] [InlineData(StackTraceMode.Original, "AsyncWithWait_StackTrace { }")] +#if !TRIMMABLE [InlineData(StackTraceMode.Enhanced, "void SentryStackTraceFactoryTests.AsyncWithWait_StackTrace(StackTraceMode mode, string method)+() => { }")] +#endif public void AsyncWithWait_StackTrace(StackTraceMode mode, string method) { _fixture.SentryOptions.AttachStacktrace = true; @@ -69,7 +71,9 @@ public void AsyncWithWait_StackTrace(StackTraceMode mode, string method) [Theory] [InlineData(StackTraceMode.Original, "MoveNext")] // Should be "AsyncWithAwait_StackTrace { }", but see note in SentryStackTraceFactory +#if !TRIMMABLE [InlineData(StackTraceMode.Enhanced, "async Task SentryStackTraceFactoryTests.AsyncWithAwait_StackTrace(StackTraceMode mode, string method)+(?) => { }")] +#endif public async Task AsyncWithAwait_StackTrace(StackTraceMode mode, string method) { _fixture.SentryOptions.AttachStacktrace = true; @@ -87,6 +91,7 @@ public async Task AsyncWithAwait_StackTrace(StackTraceMode mode, string method) Assert.Equal(method, stackTrace.Frames.Last().Function); } +#if !TRIMMABLE [Fact] public void Create_NoExceptionAndAttachStackTraceOptionOnWithEnhancedMode_CurrentStackTrace() { @@ -108,6 +113,7 @@ public void Create_NoExceptionAndAttachStackTraceOptionOnWithEnhancedMode_Curren StringComparison.Ordinal ) == true); } +#endif [Fact] public void Create_WithExceptionAndDefaultAttachStackTraceOption_HasStackTrace() @@ -185,7 +191,9 @@ public void FileNameShouldBeRelative() [Theory] [InlineData(StackTraceMode.Original, "ByRefMethodThatThrows")] +#if !TRIMMABLE [InlineData(StackTraceMode.Enhanced, "(Fixture f, int b) SentryStackTraceFactoryTests.ByRefMethodThatThrows(int value, in int valueIn, ref int valueRef, out int valueOut)")] +#endif public void Create_InlineCase_IncludesAmpersandAfterParameterType(StackTraceMode mode, string method) { _fixture.SentryOptions.StackTraceMode = mode; diff --git a/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.verify.cs b/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.verify.cs index e701641303..49ccfbac35 100644 --- a/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.verify.cs +++ b/test/Sentry.Tests/Internals/SentryStackTraceFactoryTests.verify.cs @@ -9,7 +9,9 @@ public partial class SentryStackTraceFactoryTests { [SkippableTheory] [InlineData(StackTraceMode.Original)] +#if !TRIMMABLE [InlineData(StackTraceMode.Enhanced)] +#endif public Task MethodGeneric(StackTraceMode mode) { // TODO: Mono gives different results. Investigate why. diff --git a/test/Sentry.Tests/Sentry.Tests.csproj b/test/Sentry.Tests/Sentry.Tests.csproj index 56aa58d6c3..f5078c70ae 100644 --- a/test/Sentry.Tests/Sentry.Tests.csproj +++ b/test/Sentry.Tests/Sentry.Tests.csproj @@ -2,6 +2,7 @@ $(DefineConstants);TRIMMABLE + $(NoWarn);SYSLIB0012;SYSLIB0005 net7.0;net6.0;netcoreapp3.1;net48 $(TargetFrameworks);net7.0-android $(TargetFrameworks);net7.0-ios From bb41fbbe63c68cd8f4c4e1e60990dc77f3f8a3d8 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 12 Oct 2023 15:10:33 +1300 Subject: [PATCH 16/20] Resolved deserialization issues for GraphQLRequestContent --- Sentry.sln.DotSettings | 1 + src/Sentry/GraphQLRequestContent.cs | 71 +++++++++++++++++-- test/Sentry.Tests/SentryClientTests.verify.cs | 12 ++-- 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/Sentry.sln.DotSettings b/Sentry.sln.DotSettings index 18ecf270d5..7345707b8d 100644 --- a/Sentry.sln.DotSettings +++ b/Sentry.sln.DotSettings @@ -1,4 +1,5 @@  + QL UI True True diff --git a/src/Sentry/GraphQLRequestContent.cs b/src/Sentry/GraphQLRequestContent.cs index a7eda78e62..44d358bde8 100644 --- a/src/Sentry/GraphQLRequestContent.cs +++ b/src/Sentry/GraphQLRequestContent.cs @@ -23,14 +23,9 @@ public GraphQLRequestContent(string? requestContent, SentryOptions? options = nu return; } -#if TRIMMABLE - // TODO: Work out how to make this trimmable - throw new NotImplementedException(); -#else try { - var deserialized = JsonSerializer.Deserialize>(requestContent, SerializerOptions); - Items = (deserialized ?? new Dictionary()).AsReadOnly(); + Items = GraphQLRequestContentReader.Read(requestContent); } catch (Exception e) { @@ -63,7 +58,6 @@ public GraphQLRequestContent(string? requestContent, SentryOptions? options = nu { OperationType = "query"; } -#endif } internal string? RequestContent { get; } @@ -87,3 +81,66 @@ public GraphQLRequestContent(string? requestContent, SentryOptions? options = nu /// public string OperationTypeOrFallback() => OperationType ?? "graphql.operation"; } + +/// +/// Adapted from https://github.com/graphql-dotnet/graphql-dotnet/blob/42a299e77748ec588bf34c33334e985098563298/src/GraphQL.SystemTextJson/GraphQLRequestJsonConverter.cs#L64 +/// +internal static class GraphQLRequestContentReader +{ + /// + /// Name for the operation name parameter. + /// See https://github.com/graphql/graphql-over-http/blob/master/spec/GraphQLOverHTTP.md#request-parameters + /// + private const string OperationNameKey = "operationName"; + + /// + /// Name for the query parameter. + /// See https://github.com/graphql/graphql-over-http/blob/master/spec/GraphQLOverHTTP.md#request-parameters + /// + private const string QueryKey = "query"; + + + public static IReadOnlyDictionary Read(string requestContent) + { + Utf8JsonReader reader = new(Encoding.UTF8.GetBytes(requestContent)); + if (!reader.Read() || reader.TokenType != JsonTokenType.StartObject) + { + throw new JsonException("Expected start of object"); + } + + var request = new Dictionary(); + + while (reader.Read()) + { + if (reader.TokenType == JsonTokenType.EndObject) + { + return request; + } + + if (reader.TokenType != JsonTokenType.PropertyName) + { + throw new JsonException("Expected property name"); + } + + var key = reader.GetString()!; + + if (!reader.Read()) + { + throw new JsonException("unexpected end of data"); + } + + switch (key) + { + case QueryKey: + case OperationNameKey: + request[key] = reader.GetString()!; + break; + default: + reader.Skip(); + break; + } + } + + throw new JsonException("unexpected end of data"); + } +} diff --git a/test/Sentry.Tests/SentryClientTests.verify.cs b/test/Sentry.Tests/SentryClientTests.verify.cs index 47c3d0a2bb..6edb8c5629 100644 --- a/test/Sentry.Tests/SentryClientTests.verify.cs +++ b/test/Sentry.Tests/SentryClientTests.verify.cs @@ -6,31 +6,35 @@ public partial class SentryClientTests [Fact] public Task CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb() { + // Arrange var error = new Exception("Exception message!"); _fixture.SentryOptions.SetBeforeSend((_,_) => throw error); - var @event = new SentryEvent(); - var sut = _fixture.GetSut(); + + // Act _ = sut.CaptureEvent(@event); + // Assert return Verify(@event.Breadcrumbs); } [Fact] public Task CaptureTransaction_BeforeSendTransactionThrows_ErrorToEventBreadcrumb() { + // Arrange var error = new Exception("Exception message!"); _fixture.SentryOptions.SetBeforeSendTransaction((_, _) => throw error); - var transaction = new Transaction("name", "operation") { IsSampled = true }; - var sut = _fixture.GetSut(); + + // Act sut.CaptureTransaction(transaction); + // Assert return Verify(transaction.Breadcrumbs); } } From c37926ce4fdddb65891a228a79d9d6a91f5902ce Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 12 Oct 2023 22:25:42 +1300 Subject: [PATCH 17/20] Fixed MS Build issues --- .../Internal/Extensions/JsonExtensions.cs | 6 +++--- src/Sentry/Sentry.csproj | 8 +++++--- src/Sentry/SentryClient.cs | 4 ++-- src/Sentry/SentryOptions.cs | 2 +- .../Internals/DebugStackTraceTests.verify.cs | 4 ++-- test/Sentry.Tests/Internals/JsonTests.cs | 14 ++++++------- .../Internals/SentryStackTraceFactoryTests.cs | 8 ++++---- .../SentryStackTraceFactoryTests.verify.cs | 2 +- .../Protocol/Context/ContextsTests.cs | 2 +- test/Sentry.Tests/Sentry.Tests.csproj | 20 ++++++++++++++++--- 10 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 2f09cd45b5..989760f740 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -42,7 +42,7 @@ private static JsonSerializerOptions BuildOptions(bool preserveReferences) return options; } -#if NET6_0_OR_GREATER +#if TRIMMABLE private static List DefaultSerializerContexts = new(); private static List ReferencePreservingSerializerContexts = new(); @@ -533,7 +533,7 @@ public static void WriteDynamicValue( } } -#if NET6_0_OR_GREATER +#if TRIMMABLE private static JsonSerializerContext GetSerializerContext(Type type, bool preserveReferences = false) { @@ -854,7 +854,7 @@ public static void WriteString( } } -#if NET6_0_OR_GREATER +#if TRIMMABLE [JsonSerializable(typeof(Internal.GrowableArray))] internal partial class SentryJsonContext : JsonSerializerContext { diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index 5c6ce964aa..ec18456b43 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -5,8 +5,6 @@ $(NoWarn);RS0017 true true - true - $(DefineConstants);TRIMMABLE @@ -24,7 +22,11 @@ - + + true + $(DefineConstants);TRIMMABLE + + + + + + + + + + + + + From 29818c71d8ebeba34ba3977b8a9c45808663804c Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 12 Oct 2023 22:56:54 +1300 Subject: [PATCH 18/20] Update SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt --- ...vent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt b/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt index 6ce33e1e01..8ae43ba2f9 100644 --- a/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt +++ b/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt @@ -5,8 +5,8 @@ Data: { message: Exception message!, stackTrace: -at Task Sentry.Tests.SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb() -at SentryEvent Sentry.SentryClient.BeforeSend(...) +at Sentry.Tests.SentryClientTests.<>c__DisplayClass71_0.b__0(...) +at Sentry.SentryClient.BeforeSend(...) }, Category: SentryClient, Level: error From adabd5dabe3ce44b4a5ce0eeb8458dd40825fd73 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 12 Oct 2023 23:40:20 +1300 Subject: [PATCH 19/20] Fixed CapturesEventWithContextKey_Implementation test --- src/Sentry/Internal/Extensions/JsonExtensions.cs | 1 + test/Sentry.Tests/HubTests.cs | 12 +++++++++++- test/Sentry.Tests/Internals/JsonTests.cs | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 989760f740..cdf3e58bc8 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -856,6 +856,7 @@ public static void WriteString( #if TRIMMABLE [JsonSerializable(typeof(Internal.GrowableArray))] +[JsonSerializable(typeof(Dictionary))] internal partial class SentryJsonContext : JsonSerializerContext { } diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index e175eaf694..97cef588fa 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -275,7 +275,7 @@ public void CaptureEvent_ExceptionWithOpenSpan_SpanLinkedToEventContext() Assert.Equal(child.ParentSpanId, evt.Contexts.Trace.ParentSpanId); } - private class EvilContext + internal class EvilContext { // This property will throw an exception during serialization. // ReSharper disable once UnusedMember.Local @@ -300,6 +300,9 @@ await TestHelpers.RetryTestAsync( private async Task CapturesEventWithContextKey_Implementation(bool offlineCaching) { +#if NET7_0_OR_GREATER + JsonExtensions.AddJsonSerializerContext(o => new HubTestsJsonContext(o)); +#endif var tcs = new TaskCompletionSource(); var expectedMessage = Guid.NewGuid().ToString(); @@ -1523,3 +1526,10 @@ public async Task WithScopeAsyncT_Works() private static Scope GetCurrentScope(Hub hub) => hub.ScopeManager.GetCurrent().Key; } + +#if NET7_0_OR_GREATER +[JsonSerializable(typeof(HubTests.EvilContext))] +internal partial class HubTestsJsonContext : JsonSerializerContext +{ +} +#endif diff --git a/test/Sentry.Tests/Internals/JsonTests.cs b/test/Sentry.Tests/Internals/JsonTests.cs index b5a2f4b626..b250c219a6 100644 --- a/test/Sentry.Tests/Internals/JsonTests.cs +++ b/test/Sentry.Tests/Internals/JsonTests.cs @@ -72,10 +72,10 @@ public DataWithSerializableObject(T obj) : base(obj) { } [Fact] public void WriteDynamicValue_ExceptionParameter_SerializedException() { + // Arrange #if NET7_0_OR_GREATER JsonExtensions.AddJsonSerializerContext(o => new JsonTestsJsonContext(o)); #endif - // Arrange var ex = GenerateException("Test"); ex.Data.Add("a", "b"); From 89684a2e97198a13b7c62663af78ea5b7a765efd Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 12 Oct 2023 23:54:27 +1300 Subject: [PATCH 20/20] Update JsonExtensions.cs --- src/Sentry/Internal/Extensions/JsonExtensions.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index cdf3e58bc8..0f7d5cdbe3 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -511,6 +511,10 @@ public static void WriteDynamicValue( { writer.WriteStringValue(formattable.ToString(null, CultureInfo.InvariantCulture)); } + else if (value.GetType().ToString() == "System.RuntimeType") + { + writer.WriteStringValue(value.ToString()); + } else { if (!JsonPreserveReferences) @@ -855,7 +859,7 @@ public static void WriteString( } #if TRIMMABLE -[JsonSerializable(typeof(Internal.GrowableArray))] +[JsonSerializable(typeof(GrowableArray))] [JsonSerializable(typeof(Dictionary))] internal partial class SentryJsonContext : JsonSerializerContext {