-
-
Notifications
You must be signed in to change notification settings - Fork 253
v9: Parent-child relationship for the PlatformExceptions and Cause #2803
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
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
bd65dec
add source to exception cause, populate source from sentry exception …
denrase 8488663
Merge branch 'main' into enha/rel-platfomr-cause
denrase b950043
add source for osError
denrase 3486958
revert sentry exception cause factory changes
denrase fd73eaf
extract soruce from android platform exceptions
denrase 07087aa
add ExceptionGroupEventProcessor
denrase 2e189f2
format
denrase 27ab973
reverse order according to rfc
denrase c888e40
iterate seperatley
denrase d317c84
update test
denrase 9a23c4a
introduce parent/child relationship in exceptions, flatten exceptions…
denrase 83081be
Merge branch 'main' into enha/rel-platfomr-cause
denrase 576672c
mark methods as internal
denrase 28a7e06
add cl entry
denrase d74c081
fix integration test
denrase 4f6a74f
fix cl
denrase 5e0236b
don’t try grouping if there are no children
denrase 31c1a24
sentry client builds correct hierarchy for exception causes
denrase 277bc21
reumove unused test
denrase a57fa2b
attach missing thread to root exception
denrase a3e6104
Merge branch 'main' into enha/rel-platfomr-cause
denrase 23d5f24
move flatten to event processor
denrase 972a9b7
Merge branch 'main' into enha/rel-platfomr-cause
denrase 6fdbf5b
format
denrase 4adb170
Merge branch 'main' into enha/rel-platfomr-cause
denrase a19c9aa
handle copyWith usage
denrase 3a6af68
update exception grouping
denrase e26f4d4
Merge branch 'main' into enha/rel-platfomr-cause
denrase cd9e4ae
Update dart/lib/src/sentry_exception_factory.dart
denrase 80e7f9e
Merge branch 'main' into enha/rel-platfomr-cause
buenaflor File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletions
54
dart/lib/src/event_processor/exception/exception_group_event_processor.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import '../../event_processor.dart'; | ||
import '../../protocol.dart'; | ||
import '../../hint.dart'; | ||
|
||
/// Group exceptions into a flat list with references to hierarchy. | ||
class ExceptionGroupEventProcessor implements EventProcessor { | ||
@override | ||
SentryEvent? apply(SentryEvent event, Hint hint) { | ||
final sentryExceptions = event.exceptions ?? []; | ||
if (sentryExceptions.isEmpty) { | ||
return event; | ||
} | ||
final firstException = sentryExceptions.first; | ||
|
||
if (sentryExceptions.length > 1 || firstException.exceptions == null) { | ||
// If already a list or no child exceptions, no grouping possible/needed. | ||
return event; | ||
} else { | ||
event.exceptions = | ||
firstException.flatten().reversed.toList(growable: false); | ||
return event; | ||
} | ||
} | ||
} | ||
|
||
extension _SentryExceptionFlatten on SentryException { | ||
List<SentryException> flatten({int? parentId, int id = 0}) { | ||
final exceptions = this.exceptions ?? []; | ||
|
||
final newMechanism = mechanism ?? Mechanism(type: "generic"); | ||
newMechanism | ||
..type = id > 0 ? "chained" : newMechanism.type | ||
..parentId = parentId | ||
..exceptionId = id | ||
..isExceptionGroup = exceptions.isNotEmpty ? true : null; | ||
|
||
mechanism = newMechanism; | ||
|
||
var all = <SentryException>[]; | ||
all.add(this); | ||
|
||
if (exceptions.isNotEmpty) { | ||
final parentId = id; | ||
for (var exception in exceptions) { | ||
id++; | ||
final flattenedExceptions = | ||
exception.flatten(parentId: parentId, id: id); | ||
id = flattenedExceptions.lastOrNull?.mechanism?.exceptionId ?? id; | ||
all.addAll(flattenedExceptions); | ||
} | ||
} | ||
return all.toList(growable: false); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
/// Holds inner exception and stackTrace combinations contained in other exceptions | ||
class ExceptionCause { | ||
ExceptionCause(this.exception, this.stackTrace); | ||
ExceptionCause(this.exception, this.stackTrace, {this.source}); | ||
|
||
dynamic exception; | ||
dynamic stackTrace; | ||
String? source; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the copyWith?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the tests, yeah. There we use copy with and without this, we'd just share the same instance of stackTrace/mecahnism between multiple tests, now that it's immutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to fix this? since copyWith is deprecated we will remove it at some point
but if it's too much work it's fine, we can do it when we eventually remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave it for now and handle this some other time