-
-
Notifications
You must be signed in to change notification settings - Fork 226
feat: Added SessionEndStatus.Unhandled
#4633
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.
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
Changes from all commits
c3d01c1
38ac24d
54d3b2f
f44a687
de5e1b5
b6c4c58
dec8f34
16179d8
3ecb136
d657e0b
f95bff3
fac1ab4
7a59034
0c47b3f
04d8889
4c24f51
bbefdd3
fe4e915
31416f9
1ad719c
4d6fb3f
624524f
a41b316
352fd01
a1500c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,8 +32,9 @@ public static void AddSentryContext(this Exception ex, string name, IReadOnlyDic | |
| /// <param name="type">A required short string that identifies the mechanism.</param> | ||
| /// <param name="description">An optional human-readable description of the mechanism.</param> | ||
| /// <param name="handled">An optional flag indicating whether the exception was handled by the mechanism.</param> | ||
| /// <param name="terminal">An optional flag indicating whether the exception is considered terminal.</param> | ||
| public static void SetSentryMechanism(this Exception ex, string type, string? description = null, | ||
| bool? handled = null) | ||
| bool? handled = null, bool? terminal = null) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only thing I don't like about this is that it allows six possible combinations, some of which are inconsistent... A nullable enum { Handled, Unhandled, Terminal } would constrain this to only consistent possibilities.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The protocol itself has The combinations do have legitimate use cases:
The terminal flag is also SDK internal and not getting sent to Sentry and I'd argue most users will never set it directly, so I'd rather opt towards SDK needs than to theoretical type safety that a nullable enum might bring. |
||
| { | ||
| ex.Data[Mechanism.MechanismKey] = type; | ||
|
|
||
|
|
@@ -54,5 +55,14 @@ public static void SetSentryMechanism(this Exception ex, string type, string? de | |
| { | ||
| ex.Data[Mechanism.HandledKey] = handled; | ||
| } | ||
|
|
||
| if (terminal == null) | ||
| { | ||
| ex.Data.Remove(Mechanism.TerminalKey); | ||
| } | ||
| else | ||
| { | ||
| ex.Data[Mechanism.TerminalKey] = terminal; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,10 +13,15 @@ public enum SessionEndStatus | |
| /// <summary> | ||
| /// Session ended with an unhandled exception. | ||
| /// </summary> | ||
| Unhandled, | ||
bitsandfoxes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /// <summary> | ||
| /// Session ended with a terminal unhandled exception. | ||
| /// </summary> | ||
| Crashed, | ||
|
|
||
| /// <summary> | ||
| /// Session ended abnormally (e.g. device lost power). | ||
| /// </summary> | ||
| Abnormal | ||
| Abnormal, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can/Would we ever set this in practice? I see it's used in the tests...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good point and I think we're abusing this in the Unity SDK. It's a bit outside of the scope of this PR but I'll check when taking a stab at properly shutting down sessions during application closure. |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.