Feat: Error Cause Extractor (#1198)

getsentry · Jan 23, 2023 · 3bbfb14 · 3bbfb14
1 parent 6f5e2ff
commit 3bbfb14
Show file tree

Hide file tree

Showing 16 changed files with 594 additions and 144 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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))

diff --git a/dart/lib/sentry.dart b/dart/lib/sentry.dart
@@ -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';
diff --git a/dart/lib/src/exception_cause.dart b/dart/lib/src/exception_cause.dart
@@ -0,0 +1,6 @@
+class ExceptionCause {
+  ExceptionCause(this.exception, this.stackTrace);
+
+  dynamic exception;
+  dynamic stackTrace;
+}
diff --git a/dart/lib/src/exception_cause_extractor.dart b/dart/lib/src/exception_cause_extractor.dart
@@ -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);
+      currentException = currentExceptionCause?.exception;
+    }
+    return allExceptionCauses;
+  }
+}
diff --git a/dart/lib/src/protocol/sentry_exception.dart b/dart/lib/src/protocol/sentry_exception.dart
@@ -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].
@@ -68,6 +71,7 @@ class SentryException {
     SentryStackTrace? stackTrace,
     Mechanism? mechanism,
     int? threadId,
+    dynamic throwable,
   }) =>
       SentryException(
         type: type ?? this.type,
@@ -76,5 +80,6 @@ class SentryException {
         stackTrace: stackTrace ?? this.stackTrace,
         mechanism: mechanism ?? this.mechanism,
         threadId: threadId ?? this.threadId,
+        throwable: throwable ?? this.throwable,
       );
 }
diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart
@@ -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],
         threads: [
           ...?event.threads,
-          if (thread != null) thread,
+          ...sentryThreads,
         ],
       );
     }

diff --git a/dart/lib/src/sentry_exception_factory.dart b/dart/lib/src/sentry_exception_factory.dart
@@ -1,3 +1,4 @@
+import 'exception_cause_extractor.dart';
 import 'protocol.dart';
 import 'sentry_options.dart';
 import 'sentry_stack_trace_factory.dart';
@@ -9,6 +10,8 @@ class SentryExceptionFactory {
 
   SentryStackTraceFactory get _stacktraceFactory => _options.stackTraceFactory;
 
+  late final extractor = RecursiveExceptionCauseExtractor(_options);
+
   SentryExceptionFactory(this._options);
 
   SentryException getSentryException(
@@ -57,6 +60,7 @@ class SentryExceptionFactory {
       value: throwable.toString(),
       mechanism: mechanism,
       stackTrace: sentryStackTrace,
+      throwable: throwable,
     );
   }
 }
diff --git a/dart/lib/src/sentry_options.dart b/dart/lib/src/sentry_options.dart
@@ -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;

diff --git a/dart/test/exception_cause_extractor_test.dart b/dart/test/exception_cause_extractor_test.dart
@@ -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);
+  }
+}