Can't add instance of Exception to Extra or Contexts

[bruno-garcia]

This was previously possible with Newtonsoft.Json but now fails with System.Text.Json:

using System;
using Sentry;

using (SentrySdk.Init(o =>
{
    o.Debug = true;
    o.Dsn = "...";
}))
{
    try
    {
        throw new Exception("test");
    }
    catch (Exception e)
    {
        SentrySdk.CaptureEvent(new SentryEvent {Contexts = new Contexts() {["ex"] = e}});
    }
}

   Error: Error while processing event 96a7719b8c6d4a00a2b46f7c93bd0af8: System.NotSupportedException: Serialization and deserialization of 'System.Type' instances are not supported and should be avoided since they can lead to security issues. Path: $.TargetSite.DeclaringType.
 ---> System.NotSupportedException: Serialization and deserialization of 'System.Type' instances are not supported and should be avoided since they can lead to security issues.
   at System.Text.Json.Serialization.Converters.TypeConverter.Write(Utf8JsonWriter writer, Type value, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonPropertyInfo`1.GetMemberAndWriteJson(Object obj, WriteStack& state, Utf8JsonWriter writer)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonPropertyInfo`1.GetMemberAndWriteJson(Object obj, WriteStack& state, Utf8JsonWriter writer)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ThrowNotSupportedException(WriteStack& state, NotSupportedException ex)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCoreAsObject(Utf8JsonWriter writer, Object value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonSerializer.WriteCore[TValue](JsonConverter jsonConverter, Utf8JsonWriter writer, TValue& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonSerializer.WriteCore[TValue](Utf8JsonWriter writer, TValue& value, Type inputType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](Utf8JsonWriter writer, TValue& value, Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](Utf8JsonWriter writer, TValue value, JsonSerializerOptions options)
   at Sentry.JsonSerializableExtensions.WriteDynamicValue(Utf8JsonWriter writer, Object value) in /Users/bruno/git/sentry-dotnet/src/Sentry/IJsonSerializable.cs:line 96
   at Sentry.JsonSerializableExtensions.WriteDynamic(Utf8JsonWriter writer, String propertyName, Object value) in /Users/bruno/git/sentry-dotnet/src/Sentry/IJsonSerializable.cs:line 106
   at Sentry.Internal.Extensions.JsonExtensions.WriteDictionaryValue(Utf8JsonWriter writer, IEnumerable`1 dic) in /Users/bruno/git/sentry-dotnet/src/Sentry/Internal/Extensions/JsonExtensions.cs:line 27
   at Sentry.Contexts.WriteTo(Utf8JsonWriter writer) in /Users/bruno/git/sentry-dotnet/src/Sentry/Protocol/Contexts.cs:line 94
   at Sentry.JsonSerializableExtensions.WriteSerializableValue(Utf8JsonWriter writer, IJsonSerializable value) in /Users/bruno/git/sentry-dotnet/src/Sentry/IJsonSerializable.cs:line 30
   at Sentry.JsonSerializableExtensions.WriteSerializable(Utf8JsonWriter writer, String propertyName, IJsonSerializable value) in /Users/bruno/git/sentry-dotnet/src/Sentry/IJsonSerializable.cs:line 39
   at Sentry.SentryEvent.WriteTo(Utf8JsonWriter writer) in /Users/bruno/git/sentry-dotnet/src/Sentry/SentryEvent.cs:line 291
   at Sentry.Protocol.Envelopes.JsonSerializable.SerializeAsync(Stream stream, CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Envelopes/JsonSerializable.cs:line 27
   at Sentry.Protocol.Envelopes.JsonSerializable.SerializeAsync(Stream stream, CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Envelopes/JsonSerializable.cs:line 28
   at Sentry.Protocol.Envelopes.EnvelopeItem.BufferPayloadAsync(CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Envelopes/EnvelopeItem.cs:line 64
   at Sentry.Protocol.Envelopes.EnvelopeItem.SerializeAsync(Stream stream, CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Envelopes/EnvelopeItem.cs:line 101
   at Sentry.Protocol.Envelopes.Envelope.SerializeAsync(Stream stream, CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Envelopes/Envelope.cs:line 66
   at Sentry.Internal.Http.EnvelopeHttpContent.SerializeToStreamAsync(Stream stream, TransportContext context) in /Users/bruno/git/sentry-dotnet/src/Sentry/Internal/Http/EnvelopeHttpContent.cs:line 16
   at System.Net.Http.HttpContent.<CopyToAsync>g__WaitAsync|56_0(ValueTask copyTask)
   at Sentry.Internal.Http.GzipBufferedRequestBodyHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Internal/Http/GzipBufferedRequestBodyHandler.cs:line 58
   at Sentry.Internal.Http.RetryAfterHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Internal/Http/RetryAfterHandler.cs:line 64
   at System.Net.Http.HttpClient.SendAsyncCore(HttpRequestMessage request, HttpCompletionOption completionOption, Boolean async, Boolean emitTelemetryStartStop, CancellationToken cancellationToken)
   at Sentry.Internal.Http.HttpTransport.SendEnvelopeAsync(Envelope envelope, CancellationToken cancellationToken) in /Users/bruno/git/sentry-dotnet/src/Sentry/Internal/Http/HttpTransport.cs:line 124
   at Sentry.Internal.BackgroundWorker.WorkerAsync() in /Users/bruno/git/sentry-dotnet/src/Sentry/Internal/BackgroundWorker.cs:line 134. #1 in queue.

The culprit is on TargetSite which is of type MethodBase and not serializable without persisting some type information which can be exploited.

This limitations is by design. Related issues are #1, #2, dotnet/runtime#43026

The reasoning from folks from Microsoft:

We recently published guidance against (de)serializing the DataSet and DataTable types from untrusted input. In keeping with JsonSerializer's emphasis on security, we'll throw a clear NotSupportedException for these types, similar to the behavior for System.Type.

There's a level of regression with this feature gone. Even though it makes sense why the limitation exist.

One work around is to map the Exception instance to an anonymous type, for example:

try
{
    throw new Exception("test");
}
catch (Exception e)
{
    SentrySdk.CaptureEvent(new SentryEvent {Contexts = new Contexts
    {
        ["ex"] = new
        {
            e.Message,
            e.StackTrace,
            e.Data
        }
    }});
}

This successfully serializes the properties Message, StackTrace and Data. More compatible properties could be added in that matter.

Potential improvements:

• Don't drop the whole event if serializing Extra or Context: We can log the error and continue
• Type check for Exception and do the mapping manually for known types. Logging Warn for that case would tell the user this is happening and give them a chance to do the mapping upstream for example.
• Add to the documentation (on the migration path, and ideally on a troubleshooting page) Include the work around described above

[Tyrrrz]

Can add a special case for Exception and serialize only a subset of fields. Serialization won't be an issue, deserialization on the other hand I'm not sure. It may be that we won't be able to recover the exception in such case and it will be just a plain dictionary-like object. Which is fine I suppose, given that deserialization only happens as part of caching efforts and at that point the type information is not important, only the structural integrity is.

[bruno-garcia]

Agreed. We don't want customers to lose events because they added an instance of an Exception to Sentry's context particularly when that used to work on a previous version.
We can document/state that serialization exists as a way to cache the event before sending to Sentry. Not to restore the event to it's in-memory state given we can't guarantee all things we serialize can be deserialized back.