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.

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

Feat: Error Cause Extractor #1198

Merged
merged 32 commits into from
Jan 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
88e39d2
sample on how to recursiveley extract errors
denrase Dec 19, 2022
ba00651
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 10, 2023
81394d1
introduce exception cause with exception and stacktrace
denrase Jan 10, 2023
620e917
move extractors to options
denrase Jan 10, 2023
1c7d835
extract exceptions and attach as sentry exceptions to sentry event
denrase Jan 10, 2023
2584bad
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 16, 2023
e8384ca
make internal field private
denrase Jan 16, 2023
b4fb217
make var final
denrase Jan 16, 2023
077aff4
make var final
denrase Jan 16, 2023
a1acbf3
add tests covering stacktrace attachments
denrase Jan 16, 2023
fcd1a38
check if correct stacktrace was captured
denrase Jan 16, 2023
34e9e8a
break circularity
denrase Jan 16, 2023
84f1104
add changelog entry
denrase Jan 16, 2023
0c15cd2
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 17, 2023
413ddd0
expose type in extractor
denrase Jan 17, 2023
4af4d07
export classes
denrase Jan 17, 2023
5159821
move recursivce extractor to exception factory
denrase Jan 17, 2023
983bdf3
format
denrase Jan 17, 2023
bde4df0
remove todo
denrase Jan 17, 2023
4bd8ad4
add throwable to sentry exception
denrase Jan 17, 2023
406e493
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 17, 2023
61900d4
apply new approach in dio event processor
denrase Jan 17, 2023
5d78242
update changelog
denrase Jan 17, 2023
b2abac7
dont remove stacktrace from DioError
denrase Jan 17, 2023
2e65a9e
preserve throwable mechanism
denrase Jan 17, 2023
4f37d19
search for first in list
denrase Jan 23, 2023
72cfb5b
search for dioexception in list
denrase Jan 23, 2023
db64952
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 23, 2023
dcbc09f
format changelog
denrase Jan 23, 2023
4938f3a
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 23, 2023
cce32de
format changelog
denrase Jan 23, 2023
22196e6
format
denrase Jan 23, 2023
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Features

- Error Cause Extractor ([#1198](https://github.com/getsentry/sentry-dart/pull/1198))
- Add `throwable` to `SentryException`

### Fixes

- Don't suppress error logs ([#1228](https://github.com/getsentry/sentry-dart/pull/1228))
Expand Down
3 changes: 3 additions & 0 deletions dart/lib/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ export 'src/utils/tracing_utils.dart';
export 'src/tracing.dart';
export 'src/hint.dart';
export 'src/type_check_hint.dart';
// exception extraction
export 'src/exception_cause_extractor.dart';
export 'src/exception_cause.dart';
6 changes: 6 additions & 0 deletions dart/lib/src/exception_cause.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class ExceptionCause {
denrase marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add docs for public classes and properties, that apply to some other files as well.

ExceptionCause(this.exception, this.stackTrace);

dynamic exception;
dynamic stackTrace;
}
40 changes: 40 additions & 0 deletions dart/lib/src/exception_cause_extractor.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import 'exception_cause.dart';
import 'sentry_options.dart';
import 'throwable_mechanism.dart';

abstract class ExceptionCauseExtractor<T> {
ExceptionCause? cause(T error);
Type get exceptionType => T;
}

class RecursiveExceptionCauseExtractor {
RecursiveExceptionCauseExtractor(this._options);

final SentryOptions _options;

List<ExceptionCause> flatten(exception, stackTrace) {
final allExceptionCauses = <ExceptionCause>[];
final circularityDetector = <dynamic>{};

var currentException = exception;
ExceptionCause? currentExceptionCause =
ExceptionCause(exception, stackTrace);

while (currentException != null &&
currentExceptionCause != null &&
circularityDetector.add(currentException)) {
allExceptionCauses.add(currentExceptionCause);

final extractionSourceSource = currentException is ThrowableMechanism
? currentException.throwable
: currentException;

final extractor =
_options.exceptionCauseExtractor(extractionSourceSource.runtimeType);

currentExceptionCause = extractor?.cause(extractionSourceSource);
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to wrap that with a try-catch, otherwise, we will silently swallow the exception in case the extract has a bug.
Logging the error too.

currentException = currentExceptionCause?.exception;
}
return allExceptionCauses;
}
}
5 changes: 5 additions & 0 deletions dart/lib/src/protocol/sentry_exception.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@ class SentryException {
/// Represents a [SentryThread.id].
final int? threadId;

final dynamic throwable;

const SentryException({
required this.type,
required this.value,
this.module,
this.stackTrace,
this.mechanism,
this.threadId,
this.throwable,
});

/// Deserializes a [SentryException] from JSON [Map].
Expand Down Expand Up @@ -68,6 +71,7 @@ class SentryException {
SentryStackTrace? stackTrace,
Mechanism? mechanism,
int? threadId,
dynamic throwable,
}) =>
SentryException(
type: type ?? this.type,
Expand All @@ -76,5 +80,6 @@ class SentryException {
stackTrace: stackTrace ?? this.stackTrace,
mechanism: mechanism ?? this.mechanism,
threadId: threadId ?? this.threadId,
throwable: throwable ?? this.throwable,
);
}
50 changes: 28 additions & 22 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -182,37 +182,43 @@ class SentryClient {
final isolateId = isolateName?.hashCode;

if (event.throwableMechanism != null) {
var sentryException = _exceptionFactory.getSentryException(
event.throwableMechanism,
stackTrace: stackTrace,
);
final extractedExceptions = _exceptionFactory.extractor
.flatten(event.throwableMechanism, stackTrace);

final sentryExceptions = <SentryException>[];
final sentryThreads = <SentryThread>[];

if (_options.platformChecker.isWeb) {
return event.copyWith(
exceptions: [
...?event.exceptions,
sentryException,
],
for (final extractedException in extractedExceptions) {
var sentryException = _exceptionFactory.getSentryException(
extractedException.exception,
stackTrace: extractedException.stackTrace,
);
}

SentryThread? thread;
SentryThread? sentryThread;

if (isolateName != null && _options.attachThreads) {
sentryException = sentryException.copyWith(threadId: isolateId);
thread = SentryThread(
id: isolateId,
name: isolateName,
crashed: true,
current: true,
);
if (!_options.platformChecker.isWeb &&
isolateName != null &&
_options.attachThreads) {
sentryException = sentryException.copyWith(threadId: isolateId);
sentryThread = SentryThread(
id: isolateId,
name: isolateName,
crashed: true,
current: true,
);
}

sentryExceptions.add(sentryException);
if (sentryThread != null) {
sentryThreads.add(sentryThread);
}
}

return event.copyWith(
exceptions: [...?event.exceptions, sentryException],
exceptions: [...?event.exceptions, ...sentryExceptions],
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
threads: [
...?event.threads,
if (thread != null) thread,
...sentryThreads,
],
);
}
Expand Down
4 changes: 4 additions & 0 deletions dart/lib/src/sentry_exception_factory.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'exception_cause_extractor.dart';
import 'protocol.dart';
import 'sentry_options.dart';
import 'sentry_stack_trace_factory.dart';
Expand All @@ -9,6 +10,8 @@ class SentryExceptionFactory {

SentryStackTraceFactory get _stacktraceFactory => _options.stackTraceFactory;

late final extractor = RecursiveExceptionCauseExtractor(_options);

SentryExceptionFactory(this._options);

SentryException getSentryException(
Expand Down Expand Up @@ -57,6 +60,7 @@ class SentryExceptionFactory {
value: throwable.toString(),
mechanism: mechanism,
stackTrace: sentryStackTrace,
throwable: throwable,
);
}
}
10 changes: 10 additions & 0 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,16 @@ class SentryOptions {
/// The default is 3 seconds.
Duration? idleTimeout = Duration(seconds: 3);

final _extractorsByType = <Type, ExceptionCauseExtractor>{};

ExceptionCauseExtractor? exceptionCauseExtractor(Type type) {
return _extractorsByType[type];
}

void addExceptionCauseExtractor(ExceptionCauseExtractor extractor) {
_extractorsByType[extractor.exceptionType] = extractor;
}

SentryOptions({this.dsn, PlatformChecker? checker}) {
if (checker != null) {
platformChecker = checker;
Expand Down
140 changes: 140 additions & 0 deletions dart/test/exception_cause_extractor_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import 'package:sentry/src/exception_cause.dart';
import 'package:sentry/src/exception_cause_extractor.dart';
import 'package:sentry/src/protocol/mechanism.dart';
import 'package:sentry/src/sentry_options.dart';
import 'package:sentry/src/throwable_mechanism.dart';
import 'package:test/test.dart';
import 'mocks.dart';

void main() {
late Fixture fixture;

setUp(() {
fixture = Fixture();
});

test('flatten', () {
final errorC = ExceptionC();
final errorB = ExceptionB(errorC);
final errorA = ExceptionA(errorB);

fixture.options.addExceptionCauseExtractor(
ExceptionACauseExtractor(),
);

fixture.options.addExceptionCauseExtractor(
ExceptionBCauseExtractor(),
);

final sut = fixture.getSut();

final flattened = sut.flatten(errorA, null);
final actual = flattened.map((exceptionCause) => exceptionCause.exception);
expect(actual, [errorA, errorB, errorC]);
});

test('flatten breaks circularity', () {
final a = ExceptionCircularA();
final b = ExceptionCircularB();
a.other = b;
b.other = a;

fixture.options.addExceptionCauseExtractor(
ExceptionCircularAExtractor(),
);

fixture.options.addExceptionCauseExtractor(
ExceptionCircularBExtractor(),
);

final sut = fixture.getSut();

final flattened = sut.flatten(a, null);
final actual = flattened.map((exceptionCause) => exceptionCause.exception);

expect(actual, [a, b]);
});

test('flatten preserves throwable mechanism', () {
final errorC = ExceptionC();
final errorB = ExceptionB(errorC);
final errorA = ExceptionA(errorB);

fixture.options.addExceptionCauseExtractor(
ExceptionACauseExtractor(),
);

fixture.options.addExceptionCauseExtractor(
ExceptionBCauseExtractor(),
);

final mechanism = Mechanism(type: "foo");
final throwableMechanism = ThrowableMechanism(mechanism, errorA);

final sut = fixture.getSut();
final flattened = sut.flatten(throwableMechanism, null);

final actual = flattened.map((exceptionCause) => exceptionCause.exception);
expect(actual, [throwableMechanism, errorB, errorC]);
});
}

class Fixture {
final options = SentryOptions(dsn: fakeDsn);

RecursiveExceptionCauseExtractor getSut() {
return RecursiveExceptionCauseExtractor(options);
}
}

class ExceptionA {
ExceptionA(this.other);
final ExceptionB? other;
}

class ExceptionB {
ExceptionB(this.anotherOther);
final ExceptionC? anotherOther;
}

class ExceptionC {
// I am empty inside
}

class ExceptionACauseExtractor extends ExceptionCauseExtractor<ExceptionA> {
@override
ExceptionCause? cause(ExceptionA error) {
return ExceptionCause(error.other, null);
}
}

class ExceptionBCauseExtractor extends ExceptionCauseExtractor<ExceptionB> {
@override
ExceptionCause? cause(ExceptionB error) {
return ExceptionCause(error.anotherOther, null);
}
}

class ExceptionCircularA {
ExceptionCircularB? other;
}

class ExceptionCircularB {
ExceptionCircularA? other;
}

class ExceptionCircularAExtractor
extends ExceptionCauseExtractor<ExceptionCircularA> {
@override
ExceptionCause? cause(ExceptionCircularA error) {
return ExceptionCause(error.other, null);
}
}

class ExceptionCircularBExtractor
extends ExceptionCauseExtractor<ExceptionCircularB> {
@override
ExceptionCause? cause(ExceptionCircularB error) {
return ExceptionCause(error.other, null);
}
}
Loading