Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose EnumerateChainedExceptions #1733

Merged
merged 6 commits into from
Jun 23, 2022
Merged

Expose EnumerateChainedExceptions #1733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e144927
expose EnumerateChainedExceptions
SimonCropp Jun 18, 2022
5e7c87c
Update SentryExceptionExtensions.cs
SimonCropp Jun 18, 2022
67fb4bb
Update CHANGELOG.md
SimonCropp Jun 18, 2022
87080f2
Merge branch 'main' into expose-EnumerateChainedExceptions
bruno-garcia Jun 23, 2022
878e19b
Update src/Sentry/SentryExceptionExtensions.cs
SimonCropp Jun 23, 2022
c77e432
Update SentryExceptionExtensions.cs
SimonCropp Jun 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Expose EnumerateChainedExceptions ([#1733](https://github.com/getsentry/sentry-dotnet/pull/1733))
- Android Scope Sync ([#1737](https://github.com/getsentry/sentry-dotnet/pull/1737))
- Enable logging in MAUI ([#1738](https://github.com/getsentry/sentry-dotnet/pull/1738))

Expand Down
43 changes: 14 additions & 29 deletions src/Sentry/Internal/MainExceptionProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,49 +82,34 @@ keyValue.Value is string tagValue &&

internal IEnumerable<SentryException> CreateSentryException(Exception exception)
{
if (exception is AggregateException ae)
{
foreach (var inner in ae.InnerExceptions.SelectMany(CreateSentryException))
{
yield return inner;
}

if (!_options.KeepAggregateException)
{
yield break;
}
}
else if (exception.InnerException != null)
{
foreach (var inner in CreateSentryException(exception.InnerException))
{
yield return inner;
}
}
return exception.EnumerateChainedExceptions(_options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we could refactor out this class to have a base class, so that the code that follows it doesn't need to iterate on the exceptions once again:

https://github.com/getsentry/sentry-unity/pull/683/files#diff-d7c6f55c576cbd905ba5affcc1c578fd120655cdfcdc520994c837efeeebccc2R33

Then a third time to have the original + the Sentry exception side by side:

https://github.com/getsentry/sentry-unity/pull/683/files#diff-d7c6f55c576cbd905ba5affcc1c578fd120655cdfcdc520994c837efeeebccc2R47

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bruno-garcia i dont really understand what you are proposing here. can we sync my tuesday morning?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought of re-thinking these hooks but didn't get that far, #1734

.Select(BuildSentryException);
}

private SentryException BuildSentryException(Exception innerException)
{
var sentryEx = new SentryException
{
Type = exception.GetType()?.FullName,
Module = exception.GetType()?.Assembly?.FullName,
Value = exception.Message,
Type = innerException.GetType()?.FullName,
Module = innerException.GetType()?.Assembly?.FullName,
Value = innerException.Message,
ThreadId = Environment.CurrentManagedThreadId,
Mechanism = GetMechanism(exception)
Mechanism = GetMechanism(innerException)
};

if (exception.Data.Count != 0)
if (innerException.Data.Count != 0)
{
foreach (var key in exception.Data.Keys)
foreach (var key in innerException.Data.Keys)
{
if (key is string keyString)
{
sentryEx.Data[keyString] = exception.Data[key];
sentryEx.Data[keyString] = innerException.Data[key];
}
}
}

sentryEx.Stacktrace = SentryStackTraceFactoryAccessor().Create(exception);

yield return sentryEx;
sentryEx.Stacktrace = SentryStackTraceFactoryAccessor().Create(innerException);
return sentryEx;
}

internal static Mechanism GetMechanism(Exception exception)
Expand Down
38 changes: 38 additions & 0 deletions src/Sentry/SentryExceptionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using Sentry;
using Sentry.Internal;

/// <summary>
Expand All @@ -18,6 +20,42 @@ public static class SentryExceptionExtensions
public static void AddSentryTag(this Exception ex, string name, string value)
=> ex.Data.Add($"{MainExceptionProcessor.ExceptionDataTagKey}{name}", value);

/// <summary>
/// Recursively enumerates all <see cref="AggregateException.InnerExceptions"/> and <see cref="Exception.InnerException"/>
/// Not for public use.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public static IEnumerable<Exception> EnumerateChainedExceptions(this Exception exception, SentryOptions options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This not being for public use but part of the global namespace don't go well together.
Should we remove the note on the xml doc?

Or perhaps this is best not being an extension method of Exception? If it's under a different namespace it will already help not show up on the user's intellisense. The goal is to share the built-in behavior with the Unity IL2CPP support

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with it as is, since it includes a SentryOptions parameter - we're not likely to step on anyone. We can hide it from intellisense with EditorBrowsable

SimonCropp marked this conversation as resolved.
Show resolved Hide resolved
{
if (exception is AggregateException aggregateException)
{
foreach (var inner in EnumerateInner(options, aggregateException))
{
yield return inner;
}

if (!options.KeepAggregateException)
{
yield break;
}
}
else if (exception.InnerException != null)
{
foreach (var inner in exception.InnerException.EnumerateChainedExceptions(options))
{
yield return inner;
}
}

yield return exception;
}

private static IEnumerable<Exception> EnumerateInner(SentryOptions options, AggregateException aggregateException)
{
return aggregateException.InnerExceptions
.SelectMany(exception => exception.EnumerateChainedExceptions(options));
}

/// <summary>
/// Set a Sentry's structured Context to the Exception.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions test/Sentry.DiagnosticSource.Tests/ApiApprovalTests.Run.Core3_1.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,4 +1403,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.DiagnosticSource.Tests/ApiApprovalTests.Run.DotNet5_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,4 +1403,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.DiagnosticSource.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,4 +1403,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.Tests/ApiApprovalTests.Run.Core2_1.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,4 +1402,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.Tests/ApiApprovalTests.Run.Core3_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,4 +1402,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,4 +1403,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_6.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,4 +1403,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.Tests/ApiApprovalTests.Run.DotNet5_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,4 +1403,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}
1 change: 1 addition & 0 deletions test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,4 +1403,5 @@ public static class SentryExceptionExtensions
{
public static void AddSentryContext(this System.Exception ex, string name, System.Collections.Generic.IReadOnlyDictionary<string, object> data) { }
public static void AddSentryTag(this System.Exception ex, string name, string value) { }
public static System.Collections.Generic.IEnumerable<System.Exception> EnumerateChainedExceptions(this System.Exception exception, Sentry.SentryOptions options) { }
}