diff --git a/CHANGELOG.md b/CHANGELOG.md index d7e3139f65..4573d1874e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Sentry.addFeatureFlag('my-feature', true); ### Behavioral changes - Set log level to `warning` by default when `debug = true` ([#2836](https://github.com/getsentry/sentry-dart/pull/2836)) +- Parent-child relationship for the PlatformExceptions and Cause ([#2803](https://github.com/getsentry/sentry-dart/pull/2803)) + - Improves and changes exception grouping ### API Changes diff --git a/dart/lib/src/event_processor/exception/exception_group_event_processor.dart b/dart/lib/src/event_processor/exception/exception_group_event_processor.dart new file mode 100644 index 0000000000..6141452315 --- /dev/null +++ b/dart/lib/src/event_processor/exception/exception_group_event_processor.dart @@ -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 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 = []; + 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); + } +} diff --git a/dart/lib/src/event_processor/exception/io_exception_event_processor.dart b/dart/lib/src/event_processor/exception/io_exception_event_processor.dart index 1a16015902..40aa5fa9a9 100644 --- a/dart/lib/src/event_processor/exception/io_exception_event_processor.dart +++ b/dart/lib/src/event_processor/exception/io_exception_event_processor.dart @@ -43,16 +43,27 @@ class IoExceptionEventProcessor implements ExceptionEventProcessor { SocketException exception, SentryEvent event, ) { - final address = exception.address; final osError = exception.osError; + SentryException? osException; + List? exceptions = event.exceptions; + if (osError != null) { + // OSError is the underlying error + // https://api.dart.dev/stable/dart-io/SocketException/osError.html + // https://api.dart.dev/stable/dart-io/OSError-class.html + osException = _sentryExceptionFromOsError(osError); + final exception = event.exceptions?.firstOrNull; + if (exception != null) { + exception.addException(osException); + } else { + exceptions = [osException]; + } + } else { + exceptions = event.exceptions; + } + + final address = exception.address; if (address == null) { - event.exceptions = [ - // OSError is the underlying error - // https://api.dart.dev/stable/dart-io/SocketException/osError.html - // https://api.dart.dev/stable/dart-io/OSError-class.html - if (osError != null) _sentryExceptionfromOsError(osError), - ...?event.exceptions, - ]; + event.exceptions = exceptions; return event; } SentryRequest? request; @@ -71,15 +82,9 @@ class IoExceptionEventProcessor implements ExceptionEventProcessor { } } - event.request = event.request ?? request; - event.exceptions = [ - // OSError is the underlying error - // https://api.dart.dev/stable/dart-io/SocketException/osError.html - // https://api.dart.dev/stable/dart-io/OSError-class.html - if (osError != null) _sentryExceptionfromOsError(osError), - ...?event.exceptions, - ]; - return event; + return event + ..request = event.request ?? request + ..exceptions = exceptions; } // https://api.dart.dev/stable/dart-io/FileSystemException-class.html @@ -88,18 +93,24 @@ class IoExceptionEventProcessor implements ExceptionEventProcessor { SentryEvent event, ) { final osError = exception.osError; - event.exceptions = [ + + if (osError != null) { // OSError is the underlying error - // https://api.dart.dev/stable/dart-io/FileSystemException/osError.html + // https://api.dart.dev/stable/dart-io/SocketException/osError.html // https://api.dart.dev/stable/dart-io/OSError-class.html - if (osError != null) _sentryExceptionfromOsError(osError), - ...?event.exceptions, - ]; + final osException = _sentryExceptionFromOsError(osError); + final exception = event.exceptions?.firstOrNull; + if (exception != null) { + exception.addException(osException); + } else { + event.exceptions = [osException]; + } + } return event; } } -SentryException _sentryExceptionfromOsError(OSError osError) { +SentryException _sentryExceptionFromOsError(OSError osError) { return SentryException( type: osError.runtimeType.toString(), value: osError.toString(), @@ -110,6 +121,7 @@ SentryException _sentryExceptionfromOsError(OSError osError) { meta: { 'errno': {'number': osError.errorCode}, }, + source: 'osError', ), ); } diff --git a/dart/lib/src/exception_cause.dart b/dart/lib/src/exception_cause.dart index 522244eb93..60bf12e31e 100644 --- a/dart/lib/src/exception_cause.dart +++ b/dart/lib/src/exception_cause.dart @@ -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; } diff --git a/dart/lib/src/protocol/sentry_exception.dart b/dart/lib/src/protocol/sentry_exception.dart index f4c1bc3a39..a27d86c140 100644 --- a/dart/lib/src/protocol/sentry_exception.dart +++ b/dart/lib/src/protocol/sentry_exception.dart @@ -26,7 +26,9 @@ class SentryException { dynamic throwable; @internal - final Map? unknown; + Map? unknown; + + List? _exceptions; SentryException({ required this.type, @@ -86,10 +88,25 @@ class SentryException { type: type ?? this.type, value: value ?? this.value, module: module ?? this.module, - stackTrace: stackTrace ?? this.stackTrace, - mechanism: mechanism ?? this.mechanism, + stackTrace: stackTrace ?? this.stackTrace?.copyWith(), + mechanism: mechanism ?? this.mechanism?.copyWith(), threadId: threadId ?? this.threadId, throwable: throwable ?? this.throwable, unknown: unknown, ); + + @internal + List? get exceptions => + _exceptions != null ? List.unmodifiable(_exceptions!) : null; + + @internal + set exceptions(List? value) { + _exceptions = value; + } + + @internal + void addException(SentryException exception) { + _exceptions ??= []; + _exceptions!.add(exception); + } } diff --git a/dart/lib/src/recursive_exception_cause_extractor.dart b/dart/lib/src/recursive_exception_cause_extractor.dart index 5636af84a5..360044f0bc 100644 --- a/dart/lib/src/recursive_exception_cause_extractor.dart +++ b/dart/lib/src/recursive_exception_cause_extractor.dart @@ -16,8 +16,10 @@ class RecursiveExceptionCauseExtractor { final circularityDetector = {}; var currentException = exception; - ExceptionCause? currentExceptionCause = - ExceptionCause(exception, stackTrace); + ExceptionCause? currentExceptionCause = ExceptionCause( + exception, + stackTrace, + ); while (currentException != null && currentExceptionCause != null && diff --git a/dart/lib/src/sentry.dart b/dart/lib/src/sentry.dart index b0e3b8b93c..93d2d9e1ad 100644 --- a/dart/lib/src/sentry.dart +++ b/dart/lib/src/sentry.dart @@ -7,6 +7,7 @@ import 'environment/environment_variables.dart'; import 'event_processor/deduplication_event_processor.dart'; import 'event_processor/enricher/enricher_event_processor.dart'; import 'event_processor/exception/exception_event_processor.dart'; +import 'event_processor/exception/exception_group_event_processor.dart'; import 'hint.dart'; import 'hub.dart'; import 'hub_adapter.dart'; @@ -113,6 +114,9 @@ class Sentry { options.addEventProcessor(DeduplicationEventProcessor(options)); options.prependExceptionTypeIdentifier(DartExceptionTypeIdentifier()); + + // Added last to ensure all error events have correct parent/child relationships + options.addEventProcessor(ExceptionGroupEventProcessor()); } /// This method reads available environment variables and uses them diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart index 2de11de3fc..a3198e7524 100644 --- a/dart/lib/src/sentry_client.dart +++ b/dart/lib/src/sentry_client.dart @@ -211,12 +211,13 @@ class SentryClient { SentryEvent _prepareEvent(SentryEvent event, Hint hint, {dynamic stackTrace}) { - event.serverName = event.serverName ?? _options.serverName; - event.dist = event.dist ?? _options.dist; - event.environment = event.environment ?? _options.environment; - event.release = event.release ?? _options.release; - event.sdk = event.sdk ?? _options.sdk; - event.platform = event.platform ?? sdkPlatform(_options.platform.isWeb); + event + ..serverName = event.serverName ?? _options.serverName + ..dist = event.dist ?? _options.dist + ..environment = event.environment ?? _options.environment + ..release = event.release ?? _options.release + ..sdk = event.sdk ?? _options.sdk + ..platform = event.platform ?? sdkPlatform(_options.platform.isWeb); if (event is SentryTransaction) { return event; @@ -235,18 +236,26 @@ class SentryClient { final isolateId = isolateName?.hashCode; if (event.throwableMechanism != null) { - final extractedExceptions = _exceptionFactory.extractor + final extractedExceptionCauses = _exceptionFactory.extractor .flatten(event.throwableMechanism, stackTrace); - final sentryExceptions = []; + SentryException? rootException; + SentryException? currentException; final sentryThreads = []; - for (final extractedException in extractedExceptions) { + for (final extractedExceptionCause in extractedExceptionCauses) { var sentryException = _exceptionFactory.getSentryException( - extractedException.exception, - stackTrace: extractedException.stackTrace, + extractedExceptionCause.exception, + stackTrace: extractedExceptionCause.stackTrace, removeSentryFrames: hint.get(TypeCheckHint.currentStackTrace), ); + if (extractedExceptionCause.source != null) { + var mechanism = + sentryException.mechanism ?? Mechanism(type: "generic"); + + mechanism.source = extractedExceptionCause.source; + sentryException.mechanism = mechanism; + } SentryThread? sentryThread; @@ -262,21 +271,25 @@ class SentryClient { ); } - sentryExceptions.add(sentryException); + rootException ??= sentryException; + currentException?.addException(sentryException); + currentException = sentryException; + if (sentryThread != null) { sentryThreads.add(sentryThread); } } - event.exceptions = [ - ...?event.exceptions, - ...sentryExceptions, - ]; - event.threads = [ - ...?event.threads, - ...sentryThreads, - ]; - return event; + final exceptions = [...?event.exceptions]; + if (rootException != null) { + exceptions.add(rootException); + } + return event + ..exceptions = exceptions + ..threads = [ + ...?event.threads, + ...sentryThreads, + ]; } // The stacktrace is not part of an exception, diff --git a/dart/test/event_processor/exception/exception_group_event_processor_test.dart b/dart/test/event_processor/exception/exception_group_event_processor_test.dart new file mode 100644 index 0000000000..32332ab8f5 --- /dev/null +++ b/dart/test/event_processor/exception/exception_group_event_processor_test.dart @@ -0,0 +1,408 @@ +import 'package:sentry/sentry.dart'; +import 'package:test/test.dart'; + +import 'package:sentry/src/event_processor/exception/exception_group_event_processor.dart'; + +void main() { + group(ExceptionGroupEventProcessor, () { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + test('applies grouping to exception with children', () { + final throwableA = Exception('ExceptionA'); + final exceptionA = SentryException( + type: 'ExceptionA', + value: 'ExceptionA', + throwable: throwableA, + ); + final throwableB = Exception('ExceptionB'); + final exceptionB = SentryException( + type: 'ExceptionB', + value: 'ExceptionB', + throwable: throwableB, + ); + exceptionA.addException(exceptionB); + + var event = SentryEvent( + throwable: throwableA, + exceptions: [exceptionA], + ); + + final sut = fixture.getSut(); + event = (sut.apply(event, Hint()))!; + + final sentryExceptionB = event.exceptions![0]; + final sentryExceptionA = event.exceptions![1]; + + expect(sentryExceptionB.throwable, throwableB); + expect(sentryExceptionB.mechanism?.type, "chained"); + expect(sentryExceptionB.mechanism?.isExceptionGroup, isNull); + expect(sentryExceptionB.mechanism?.exceptionId, 1); + expect(sentryExceptionB.mechanism?.parentId, 0); + + expect(sentryExceptionA.throwable, throwableA); + expect(sentryExceptionA.mechanism?.type, "generic"); + expect(sentryExceptionA.mechanism?.isExceptionGroup, isTrue); + expect(sentryExceptionA.mechanism?.exceptionId, 0); + expect(sentryExceptionA.mechanism?.parentId, isNull); + }); + + test('applies no grouping if there is no exception', () { + final event = SentryEvent(); + final sut = fixture.getSut(); + + final result = sut.apply(event, Hint()); + + expect(result, event); + expect(event.throwable, isNull); + expect(event.exceptions, isNull); + }); + + test('applies no grouping if there is already a list of exceptions', () { + final event = SentryEvent( + exceptions: [ + SentryException( + type: 'ExceptionA', + value: 'ExceptionA', + throwable: Exception('ExceptionA')), + SentryException( + type: 'ExceptionB', + value: 'ExceptionB', + throwable: Exception('ExceptionB')), + ], + ); + final sut = fixture.getSut(); + + final result = sut.apply(event, Hint()); + + final sentryExceptionA = result?.exceptions![0]; + final sentryExceptionB = result?.exceptions![1]; + + expect(sentryExceptionA?.type, 'ExceptionA'); + expect(sentryExceptionB?.type, 'ExceptionB'); + }); + }); + + group('flatten', () { + late Fixture fixture; + + final sentryException = SentryException( + type: 'type', + value: 'value', + module: 'module', + stackTrace: SentryStackTrace(frames: [SentryStackFrame(absPath: 'abs')]), + mechanism: Mechanism(type: 'type'), + threadId: 1, + ); + + setUp(() { + fixture = Fixture(); + }); + + test('will flatten exception with nested chained exceptions', () { + // ignore: deprecated_member_use_from_same_package + final origin = sentryException.copyWith( + value: 'origin', + ); + // ignore: deprecated_member_use_from_same_package + final originChild = sentryException.copyWith( + value: 'originChild', + ); + origin.addException(originChild); + + // ignore: deprecated_member_use_from_same_package + final originChildChild = sentryException.copyWith( + value: 'originChildChild', + ); + originChild.addException(originChildChild); + + final sut = fixture.getSut(); + var event = SentryEvent(exceptions: [origin]); + event = sut.apply(event, Hint())!; + final flattened = event.exceptions ?? []; + + expect(flattened.length, 3); + + expect(flattened[2].value, 'origin'); + expect(flattened[2].mechanism?.isExceptionGroup, isTrue); + expect(flattened[2].mechanism?.source, isNull); + expect(flattened[2].mechanism?.exceptionId, 0); + expect(flattened[2].mechanism?.parentId, null); + + expect(flattened[1].value, 'originChild'); + expect(flattened[1].mechanism?.isExceptionGroup, isTrue); + expect(flattened[1].mechanism?.source, isNull); + expect(flattened[1].mechanism?.exceptionId, 1); + expect(flattened[1].mechanism?.parentId, 0); + + expect(flattened[0].value, 'originChildChild'); + expect(flattened[0].mechanism?.isExceptionGroup, isNull); + expect(flattened[0].mechanism?.source, isNull); + expect(flattened[0].mechanism?.exceptionId, 2); + expect(flattened[0].mechanism?.parentId, 1); + }); + + test('will flatten exception with nested parallel exceptions', () { + // ignore: deprecated_member_use_from_same_package + final origin = sentryException.copyWith( + value: 'origin', + ); + // ignore: deprecated_member_use_from_same_package + final originChild = sentryException.copyWith( + value: 'originChild', + ); + origin.addException(originChild); + // ignore: deprecated_member_use_from_same_package + final originChild2 = sentryException.copyWith( + value: 'originChild2', + ); + origin.addException(originChild2); + + final sut = fixture.getSut(); + var event = SentryEvent(exceptions: [origin]); + event = sut.apply(event, Hint())!; + final flattened = event.exceptions ?? []; + + expect(flattened.length, 3); + + expect(flattened[2].value, 'origin'); + expect(flattened[2].mechanism?.isExceptionGroup, true); + expect(flattened[2].mechanism?.source, isNull); + expect(flattened[2].mechanism?.exceptionId, 0); + expect(flattened[2].mechanism?.parentId, null); + + expect(flattened[1].value, 'originChild'); + expect(flattened[1].mechanism?.source, isNull); + expect(flattened[1].mechanism?.exceptionId, 1); + expect(flattened[1].mechanism?.parentId, 0); + + expect(flattened[0].value, 'originChild2'); + expect(flattened[0].mechanism?.source, isNull); + expect(flattened[0].mechanism?.exceptionId, 2); + expect(flattened[0].mechanism?.parentId, 0); + }); + + test('will flatten rfc example', () { + // try: + // raise RuntimeError("something") + // except: + // raise ExceptionGroup("nested", + // [ + // ValueError(654), + // ExceptionGroup("imports", + // [ + // ImportError("no_such_module"), + // ModuleNotFoundError("another_module"), + // ] + // ), + // TypeError("int"), + // ] + // ) + + // https://github.com/getsentry/rfcs/blob/main/text/0079-exception-groups.md#example-event + // In the example, the runtime error is inserted as the first exception in the outer exception group. + + // ignore: deprecated_member_use_from_same_package + final exceptionGroupNested = sentryException.copyWith( + value: 'ExceptionGroup', + ); + // ignore: deprecated_member_use_from_same_package + final runtimeError = sentryException.copyWith( + value: 'RuntimeError', + // ignore: deprecated_member_use_from_same_package + mechanism: sentryException.mechanism?.copyWith(source: '__source__'), + ); + exceptionGroupNested.addException(runtimeError); + // ignore: deprecated_member_use_from_same_package + final valueError = sentryException.copyWith( + value: 'ValueError', + // ignore: deprecated_member_use_from_same_package + mechanism: sentryException.mechanism?.copyWith(source: 'exceptions[0]'), + ); + exceptionGroupNested.addException(valueError); + + // ignore: deprecated_member_use_from_same_package + final exceptionGroupImports = sentryException.copyWith( + value: 'ExceptionGroup', + // ignore: deprecated_member_use_from_same_package + mechanism: sentryException.mechanism?.copyWith(source: 'exceptions[1]'), + ); + exceptionGroupNested.addException(exceptionGroupImports); + + // ignore: deprecated_member_use_from_same_package + final importError = sentryException.copyWith( + value: 'ImportError', + // ignore: deprecated_member_use_from_same_package + mechanism: sentryException.mechanism?.copyWith(source: 'exceptions[0]'), + ); + exceptionGroupImports.addException(importError); + + // ignore: deprecated_member_use_from_same_package + final moduleNotFoundError = sentryException.copyWith( + value: 'ModuleNotFoundError', + // ignore: deprecated_member_use_from_same_package + mechanism: sentryException.mechanism?.copyWith(source: 'exceptions[1]'), + ); + exceptionGroupImports.addException(moduleNotFoundError); + + // ignore: deprecated_member_use_from_same_package + final typeError = sentryException.copyWith( + value: 'TypeError', + // ignore: deprecated_member_use_from_same_package + mechanism: sentryException.mechanism?.copyWith(source: 'exceptions[2]'), + ); + exceptionGroupNested.addException(typeError); + + final sut = fixture.getSut(); + var event = SentryEvent(exceptions: [exceptionGroupNested]); + event = sut.apply(event, Hint())!; + final flattened = event.exceptions ?? []; + + expect(flattened.length, 7); + + // { + // "exception": { + // "values": [ + // { + // "type": "TypeError", + // "value": "int", + // "mechanism": { + // "type": "chained", + // "source": "exceptions[2]", + // "exception_id": 6, + // "parent_id": 0 + // } + // }, + // { + // "type": "ModuleNotFoundError", + // "value": "another_module", + // "mechanism": { + // "type": "chained", + // "source": "exceptions[1]", + // "exception_id": 5, + // "parent_id": 3 + // } + // }, + // { + // "type": "ImportError", + // "value": "no_such_module", + // "mechanism": { + // "type": "chained", + // "source": "exceptions[0]", + // "exception_id": 4, + // "parent_id": 3 + // } + // }, + // { + // "type": "ExceptionGroup", + // "value": "imports", + // "mechanism": { + // "type": "chained", + // "source": "exceptions[1]", + // "is_exception_group": true, + // "exception_id": 3, + // "parent_id": 0 + // } + // }, + // { + // "type": "ValueError", + // "value": "654", + // "mechanism": { + // "type": "chained", + // "source": "exceptions[0]", + // "exception_id": 2, + // "parent_id": 0 + // } + // }, + // { + // "type": "RuntimeError", + // "value": "something", + // "mechanism": { + // "type": "chained", + // "source": "__context__", + // "exception_id": 1, + // "parent_id": 0 + // } + // }, + // { + // "type": "ExceptionGroup", + // "value": "nested", + // "mechanism": { + // "type": "exceptionhook", + // "handled": false, + // "is_exception_group": true, + // "exception_id": 0 + // } + // }, + // ] + // } + // } + + expect(flattened[0].value, 'TypeError'); + expect(flattened[0].mechanism?.source, 'exceptions[2]'); + expect(flattened[0].mechanism?.exceptionId, 6); + expect(flattened[0].mechanism?.parentId, 0); + expect(flattened[0].mechanism?.type, 'chained'); + + expect(flattened[1].value, 'ModuleNotFoundError'); + expect(flattened[1].mechanism?.source, 'exceptions[1]'); + expect(flattened[1].mechanism?.exceptionId, 5); + expect(flattened[1].mechanism?.parentId, 3); + expect(flattened[1].mechanism?.type, 'chained'); + + expect(flattened[2].value, 'ImportError'); + expect(flattened[2].mechanism?.source, 'exceptions[0]'); + expect(flattened[2].mechanism?.exceptionId, 4); + expect(flattened[2].mechanism?.parentId, 3); + expect(flattened[2].mechanism?.type, 'chained'); + + expect(flattened[3].value, 'ExceptionGroup'); + expect(flattened[3].mechanism?.source, 'exceptions[1]'); + expect(flattened[3].mechanism?.isExceptionGroup, true); + expect(flattened[3].mechanism?.exceptionId, 3); + expect(flattened[3].mechanism?.parentId, 0); + expect(flattened[3].mechanism?.type, 'chained'); + + expect(flattened[4].value, 'ValueError'); + expect(flattened[4].mechanism?.source, 'exceptions[0]'); + expect(flattened[4].mechanism?.exceptionId, 2); + expect(flattened[4].mechanism?.parentId, 0); + expect(flattened[4].mechanism?.type, 'chained'); + + expect(flattened[5].value, 'RuntimeError'); + expect(flattened[5].mechanism?.exceptionId, 1); + expect(flattened[5].mechanism?.parentId, 0); + expect(flattened[5].mechanism?.type, 'chained'); + + expect(flattened[6].value, 'ExceptionGroup'); + expect(flattened[6].mechanism?.isExceptionGroup, true); + expect(flattened[6].mechanism?.exceptionId, 0); + expect(flattened[6].mechanism?.parentId, isNull); + expect( + flattened[6].mechanism?.type, exceptionGroupNested.mechanism?.type); + }); + }); +} + +class Fixture { + ExceptionGroupEventProcessor getSut() { + return ExceptionGroupEventProcessor(); + } +} + +class ExceptionA { + ExceptionA(this.other); + final ExceptionB? other; +} + +class ExceptionB { + ExceptionB(this.anotherOther); + final ExceptionC? anotherOther; +} + +class ExceptionC { + // I am empty inside +} diff --git a/dart/test/event_processor/exception/io_exception_event_processor_test.dart b/dart/test/event_processor/exception/io_exception_event_processor_test.dart index 335f45ab2b..109b07b8eb 100644 --- a/dart/test/event_processor/exception/io_exception_event_processor_test.dart +++ b/dart/test/event_processor/exception/io_exception_event_processor_test.dart @@ -6,6 +6,9 @@ import 'dart:io'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/event_processor/exception/io_exception_event_processor.dart'; import 'package:test/test.dart'; +import 'package:sentry/src/sentry_exception_factory.dart'; + +import '../../test_utils.dart'; void main() { group(IoExceptionEventProcessor, () { @@ -46,17 +49,22 @@ void main() { test('adds $SentryRequest for $SocketException with addresses', () { final enricher = fixture.getSut(); + final throwable = SocketException( + 'Exception while connecting', + osError: OSError('Connection reset by peer', 54), + port: 12345, + address: InternetAddress( + '127.0.0.1', + type: InternetAddressType.IPv4, + ), + ); + final sentryException = + fixture.exceptionFactory.getSentryException(throwable); + final event = enricher.apply( SentryEvent( - throwable: SocketException( - 'Exception while connecting', - osError: OSError('Connection reset by peer', 54), - port: 12345, - address: InternetAddress( - '127.0.0.1', - type: InternetAddressType.IPv4, - ), - ), + throwable: throwable, + exceptions: [sentryException], ), Hint(), ); @@ -64,44 +72,60 @@ void main() { expect(event?.request, isNotNull); expect(event?.request?.url, '127.0.0.1'); - // Due to the test setup, there's no SentryException for the SocketException. - // And thus only one entry for the added OSError - expect(event?.exceptions?.first.type, 'OSError'); - expect( - event?.exceptions?.first.value, - 'OS Error: Connection reset by peer, errno = 54', - ); - expect(event?.exceptions?.first.mechanism?.type, 'OSError'); - expect(event?.exceptions?.first.mechanism?.meta['errno']['number'], 54); + final rootException = event?.exceptions?.first; + expect(rootException, sentryException); + + final childException = rootException?.exceptions?.first; + expect(childException?.type, 'OSError'); + expect(childException?.value, + 'OS Error: Connection reset by peer, errno = 54'); + expect(childException?.mechanism?.type, 'OSError'); + expect(childException?.mechanism?.meta['errno']['number'], 54); + expect(childException?.mechanism?.source, 'osError'); }); test('adds OSError SentryException for $FileSystemException', () { final enricher = fixture.getSut(); + final throwable = FileSystemException( + 'message', + 'path', + OSError('Oh no :(', 42), + ); + final sentryException = + fixture.exceptionFactory.getSentryException(throwable); + final event = enricher.apply( SentryEvent( - throwable: FileSystemException( - 'message', - 'path', - OSError('Oh no :(', 42), - ), + throwable: throwable, + exceptions: [sentryException], ), Hint(), ); + final rootException = event?.exceptions?.first; + expect(rootException, sentryException); + + final childException = rootException?.exceptions?.firstOrNull; // Due to the test setup, there's no SentryException for the FileSystemException. // And thus only one entry for the added OSError - expect(event?.exceptions?.first.type, 'OSError'); + expect(childException?.type, 'OSError'); expect( - event?.exceptions?.first.value, + childException?.value, 'OS Error: Oh no :(, errno = 42', ); - expect(event?.exceptions?.first.mechanism?.type, 'OSError'); - expect(event?.exceptions?.first.mechanism?.meta['errno']['number'], 42); + expect(childException?.mechanism?.type, 'OSError'); + expect(childException?.mechanism?.meta['errno']['number'], 42); + expect(childException?.mechanism?.source, 'osError'); }); }); } class Fixture { + final SentryOptions options = defaultTestOptions(); + + // ignore: invalid_use_of_internal_member + SentryExceptionFactory get exceptionFactory => options.exceptionFactory; + IoExceptionEventProcessor getSut() { return IoExceptionEventProcessor(SentryOptions.empty()); } diff --git a/dart/test/recursive_exception_cause_extractor_test.dart b/dart/test/recursive_exception_cause_extractor_test.dart index b2da696998..9eeaf63697 100644 --- a/dart/test/recursive_exception_cause_extractor_test.dart +++ b/dart/test/recursive_exception_cause_extractor_test.dart @@ -33,6 +33,27 @@ void main() { expect(actual, [errorA, errorB, errorC]); }); + test('parent (source) references', () { + final errorC = ExceptionC(); + final errorB = ExceptionB(errorC); + final errorA = ExceptionA(errorB); + + fixture.options.addExceptionCauseExtractor( + ExceptionACauseExtractor(false), + ); + + fixture.options.addExceptionCauseExtractor( + ExceptionBCauseExtractor(), + ); + + final sut = fixture.getSut(); + + final flattened = sut.flatten(errorA, null); + final actual = + flattened.map((exceptionCause) => exceptionCause.source).toList(); + expect(actual, [null, "other", "anotherOther"]); + }); + test('flatten breaks circularity', () { final a = ExceptionCircularA(); final b = ExceptionCircularB(); @@ -132,14 +153,14 @@ class ExceptionACauseExtractor extends ExceptionCauseExtractor { if (throwing) { throw StateError("Unexpected exception"); } - return ExceptionCause(error.other, null); + return ExceptionCause(error.other, null, source: "other"); } } class ExceptionBCauseExtractor extends ExceptionCauseExtractor { @override ExceptionCause? cause(ExceptionB error) { - return ExceptionCause(error.anotherOther, null); + return ExceptionCause(error.anotherOther, null, source: "anotherOther"); } } diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart index 9d33f02900..729b5349df 100644 --- a/dart/test/sentry_client_test.dart +++ b/dart/test/sentry_client_test.dart @@ -15,8 +15,9 @@ import 'package:sentry/src/transport/data_category.dart'; import 'package:sentry/src/transport/noop_transport.dart'; import 'package:sentry/src/transport/spotlight_http_transport.dart'; import 'package:sentry/src/utils/iterable_utils.dart'; -import 'package:test/test.dart'; +import 'package:sentry/src/event_processor/exception/exception_group_event_processor.dart'; +import 'package:test/test.dart'; import 'mocks.dart'; import 'mocks/mock_client_report_recorder.dart'; import 'mocks/mock_hub.dart'; @@ -256,14 +257,29 @@ void main() { final cause = Object(); exception = ExceptionWithCause(cause, null); - final client = fixture.getSut(); + final client = fixture.getSut( + eventProcessor: ExceptionGroupEventProcessor(), + ); await client.captureException(exception, stackTrace: null); final capturedEnvelope = (fixture.transport).envelopes.first; final capturedEvent = await eventFromEnvelope(capturedEnvelope); - expect(capturedEvent.exceptions?[0] is SentryException, true); - expect(capturedEvent.exceptions?[1] is SentryException, true); + expect(capturedEvent.exceptions?.length, 2); + + final firstException = capturedEvent.exceptions?[0]; + expect(firstException is SentryException, true); + expect(firstException?.mechanism?.source, "cause"); + expect(firstException?.mechanism?.parentId, 0); + expect(firstException?.mechanism?.exceptionId, 1); + expect(firstException?.mechanism?.isExceptionGroup, isNull); + + final secondException = capturedEvent.exceptions?[1]; + expect(secondException is SentryException, true); + expect(secondException?.mechanism?.source, null); + expect(secondException?.mechanism?.parentId, null); + expect(secondException?.mechanism?.exceptionId, 0); + expect(secondException?.mechanism?.isExceptionGroup, isTrue); }); test('should capture cause stacktrace', () async { @@ -280,17 +296,20 @@ void main() { exception = ExceptionWithCause(cause, stackTrace); - final client = fixture.getSut(attachStacktrace: true); + final client = fixture.getSut( + attachStacktrace: true, + eventProcessor: ExceptionGroupEventProcessor(), + ); await client.captureException(exception, stackTrace: null); final capturedEnvelope = (fixture.transport).envelopes.first; final capturedEvent = await eventFromEnvelope(capturedEnvelope); - expect(capturedEvent.exceptions?[1].stackTrace, isNotNull); - expect(capturedEvent.exceptions?[1].stackTrace!.frames.first.fileName, + expect(capturedEvent.exceptions?[0].stackTrace, isNotNull); + expect(capturedEvent.exceptions?[0].stackTrace!.frames.first.fileName, 'test.dart'); - expect(capturedEvent.exceptions?[1].stackTrace!.frames.first.lineNo, 46); - expect(capturedEvent.exceptions?[1].stackTrace!.frames.first.colNo, 9); + expect(capturedEvent.exceptions?[0].stackTrace!.frames.first.lineNo, 46); + expect(capturedEvent.exceptions?[0].stackTrace!.frames.first.colNo, 9); }); test('should capture custom stacktrace', () async { @@ -306,7 +325,10 @@ void main() { exception = ExceptionWithStackTrace(stackTrace); - final client = fixture.getSut(attachStacktrace: true); + final client = fixture.getSut( + attachStacktrace: true, + eventProcessor: ExceptionGroupEventProcessor(), + ); await client.captureException(exception, stackTrace: null); final capturedEnvelope = (fixture.transport).envelopes.first; @@ -328,13 +350,16 @@ void main() { final cause = Object(); exception = ExceptionWithCause(cause, null); - final client = fixture.getSut(attachStacktrace: false); + final client = fixture.getSut( + attachStacktrace: false, + eventProcessor: ExceptionGroupEventProcessor(), + ); await client.captureException(exception, stackTrace: null); final capturedEnvelope = (fixture.transport).envelopes.first; final capturedEvent = await eventFromEnvelope(capturedEnvelope); - expect(capturedEvent.exceptions?[1].stackTrace, isNull); + expect(capturedEvent.exceptions?[0].stackTrace, isNull); }); test( @@ -347,13 +372,16 @@ void main() { final cause = Object(); exception = ExceptionWithCause(cause, StackTrace.empty); - final client = fixture.getSut(attachStacktrace: false); + final client = fixture.getSut( + attachStacktrace: false, + eventProcessor: ExceptionGroupEventProcessor(), + ); await client.captureException(exception, stackTrace: null); final capturedEnvelope = (fixture.transport).envelopes.first; final capturedEvent = await eventFromEnvelope(capturedEnvelope); - expect(capturedEvent.exceptions?[1].stackTrace, isNull); + expect(capturedEvent.exceptions?[0].stackTrace, isNull); }); test('should capture cause exception with Stackframe.current', () async { @@ -364,13 +392,16 @@ void main() { final cause = Object(); exception = ExceptionWithCause(cause, null); - final client = fixture.getSut(attachStacktrace: true); + final client = fixture.getSut( + attachStacktrace: true, + eventProcessor: ExceptionGroupEventProcessor(), + ); await client.captureException(exception, stackTrace: null); final capturedEnvelope = (fixture.transport).envelopes.first; final capturedEvent = await eventFromEnvelope(capturedEnvelope); - expect(capturedEvent.exceptions?[1].stackTrace, isNotNull); + expect(capturedEvent.exceptions?[0].stackTrace, isNotNull); }); test('should capture sentry frames exception', () async { @@ -387,13 +418,16 @@ void main() { '''; exception = ExceptionWithCause(cause, stackTrace); - final client = fixture.getSut(attachStacktrace: true); + final client = fixture.getSut( + attachStacktrace: true, + eventProcessor: ExceptionGroupEventProcessor(), + ); await client.captureException(exception, stackTrace: null); final capturedEnvelope = (fixture.transport).envelopes.first; final capturedEvent = await eventFromEnvelope(capturedEnvelope); - final sentryFramesCount = capturedEvent.exceptions?[1].stackTrace!.frames + final sentryFramesCount = capturedEvent.exceptions?[0].stackTrace!.frames .where((frame) => frame.package == 'sentry') .length; @@ -2468,7 +2502,7 @@ class ExceptionWithCauseExtractor extends ExceptionCauseExtractor { @override ExceptionCause? cause(ExceptionWithCause error) { - return ExceptionCause(error.cause, error.stackTrace); + return ExceptionCause(error.cause, error.stackTrace, source: "cause"); } } diff --git a/dio/lib/src/dio_error_extractor.dart b/dio/lib/src/dio_error_extractor.dart index 6accbc17c7..3509cbea9a 100644 --- a/dio/lib/src/dio_error_extractor.dart +++ b/dio/lib/src/dio_error_extractor.dart @@ -16,6 +16,7 @@ class DioErrorExtractor extends ExceptionCauseExtractor { // A custom [ExceptionStackTraceExtractor] can be // used to extract the inner stacktrace in other cases cause is Error ? cause.stackTrace : null, + source: 'error', ); } } diff --git a/dio/test/dio_error_extractor_test.dart b/dio/test/dio_error_extractor_test.dart index 63dc4a46ee..bee490d196 100644 --- a/dio/test/dio_error_extractor_test.dart +++ b/dio/test/dio_error_extractor_test.dart @@ -29,6 +29,7 @@ void main() { expect(cause?.exception, error); expect(cause?.stackTrace, error.stackTrace); + expect(cause?.source, 'error'); }); test('extracts exception', () { @@ -43,6 +44,7 @@ void main() { expect(cause?.exception, 'Some error'); expect(cause?.stackTrace, isNull); + expect(cause?.source, 'error'); }); test('extracts nothing with missing cause', () { diff --git a/flutter/example/integration_test/integration_test.dart b/flutter/example/integration_test/integration_test.dart index a1100d3f12..f8c8478b7d 100644 --- a/flutter/example/integration_test/integration_test.dart +++ b/flutter/example/integration_test/integration_test.dart @@ -67,7 +67,9 @@ void main() { try { throw SentryException( - type: 'StarError', value: 'I have a bad feeling about this...'); + type: 'StarError', + value: 'I have a bad feeling about this...', + ); } catch (exception, stacktrace) { final sentryId = await Sentry.captureException(exception, stackTrace: stacktrace); diff --git a/flutter/lib/src/event_processor/android_platform_exception_event_processor.dart b/flutter/lib/src/event_processor/android_platform_exception_event_processor.dart index b04e3dab6c..60dbadc378 100644 --- a/flutter/lib/src/event_processor/android_platform_exception_event_processor.dart +++ b/flutter/lib/src/event_processor/android_platform_exception_event_processor.dart @@ -30,16 +30,22 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor { // PackageInfo has an internal cache, so no need to do it ourselves. final packageInfo = await PackageInfo.fromPlatform(); - final nativeStackTrace = - _tryParse(platformException.stacktrace, packageInfo.packageName); + final nativeStackTrace = _tryParse( + platformException.stacktrace, + packageInfo.packageName, + "stackTrace", + ); final details = platformException.details; String? detailsString; if (details is String) { detailsString = details; } - final detailsStackTrace = - _tryParse(detailsString, packageInfo.packageName); + final detailsStackTrace = _tryParse( + detailsString, + packageInfo.packageName, + "details", + ); if (nativeStackTrace == null && detailsStackTrace == null) { return event; @@ -65,34 +71,43 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor { } } - List>? _tryParse( + MapEntry>? _tryParse( String? potentialStackTrace, String packageName, + String source, ) { if (potentialStackTrace == null) { return null; } - return _JvmExceptionFactory(packageName) - .fromJvmStackTrace(potentialStackTrace); + .fromJvmStackTrace(potentialStackTrace, source); } SentryEvent _processPlatformException( SentryEvent event, - List>? nativeStackTrace, - List>? detailsStackTrace, + MapEntry>? nativeStackTrace, + MapEntry>? detailsStackTrace, ) { _markDartThreadsAsNonCrashed(event.threads); + final exception = event.exceptions?.firstOrNull; - final jvmExceptions = [ - ...?nativeStackTrace?.map((e) => e.key), - ...?detailsStackTrace?.map((e) => e.key) - ]; + // Assumption is that the first exception is the original exception and there is only one. + if (exception == null) { + return event; + } - var jvmThreads = [ - ...?nativeStackTrace?.map((e) => e.value), - ...?detailsStackTrace?.map((e) => e.value), - ]; + var jvmThreads = []; + if (nativeStackTrace != null) { + // ignore: invalid_use_of_internal_member + exception.addException(nativeStackTrace.key); + jvmThreads.addAll(nativeStackTrace.value); + } + + if (detailsStackTrace != null) { + // ignore: invalid_use_of_internal_member + exception.addException(detailsStackTrace.key); + jvmThreads.addAll(detailsStackTrace.value); + } if (jvmThreads.isNotEmpty) { // filter potential duplicated threads @@ -103,10 +118,8 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor { .toList(growable: true); jvmThreads.add(first); } - event.exceptions = [ - ...?event.exceptions, - ...jvmExceptions, - ]; + + event.exceptions = [exception]; event.threads = [ ...?event.threads, if (_options.attachThreads) ...jvmThreads, @@ -133,25 +146,65 @@ class _JvmExceptionFactory { final String nativePackageName; - List> fromJvmStackTrace( + MapEntry> fromJvmStackTrace( String exceptionAsString, + String source, ) { final jvmException = JvmException.parse(exceptionAsString); - final jvmExceptions = [ - jvmException, - ...?jvmException.causes, - ...?jvmException.suppressed, - ]; - return jvmExceptions.map((exception) { - return exception.toSentryException(nativePackageName); - }).toList(growable: false); + List sentryThreads = []; + + final sentryException = jvmException.toSentryException(nativePackageName); + final sentryThread = jvmException.toSentryThread(); + sentryThreads.add(sentryThread); + + final mechanism = sentryException.mechanism ?? Mechanism(type: "generic"); + mechanism.source = source; + sentryException.threadId = sentryThread.id; + sentryException.mechanism = mechanism; + + int causeIndex = 0; + for (final cause in jvmException.causes ?? []) { + var causeSentryException = cause.toSentryException(nativePackageName); + final causeSentryThread = cause.toSentryThread(); + sentryThreads.add(causeSentryThread); + + final causeMechanism = + causeSentryException.mechanism ?? Mechanism(type: "generic"); + causeMechanism.source = 'causes[$causeIndex]'; + + causeSentryException.threadId = causeSentryThread.id; + causeSentryException.mechanism = causeMechanism; + + // ignore: invalid_use_of_internal_member + sentryException.addException(causeSentryException); + causeIndex++; + } + + int suppressedIndex = 0; + for (final suppressed in jvmException.suppressed ?? []) { + var suppressedSentryException = + suppressed.toSentryException(nativePackageName); + final suppressedSentryThread = suppressed.toSentryThread(); + sentryThreads.add(suppressedSentryThread); + + final suppressedMechanism = + suppressedSentryException.mechanism ?? Mechanism(type: "generic"); + suppressedMechanism.source = 'suppressed[$suppressedIndex]'; + + suppressedSentryException.threadId = suppressedSentryThread.id; + suppressedSentryException.mechanism = suppressedMechanism; + + // ignore: invalid_use_of_internal_member + sentryException.addException(suppressedSentryException); + suppressedIndex++; + } + return MapEntry(sentryException, sentryThreads.toList(growable: false)); } } extension on JvmException { - MapEntry toSentryException( - String nativePackageName) { + SentryException toSentryException(String nativePackageName) { String? exceptionType; String? module; final typeParts = type?.split('.'); @@ -169,7 +222,7 @@ extension on JvmException { return entry.value.toSentryStackFrame(entry.key, nativePackageName); }).toList(growable: false); - var exception = SentryException( + return SentryException( value: description, type: exceptionType, module: module, @@ -177,7 +230,9 @@ extension on JvmException { frames: stackFrames.reversed.toList(growable: false), ), ); + } + SentryThread toSentryThread() { String threadName; if (thread != null) { // Needs to be prefixed with 'Android', otherwise this thread id might @@ -190,14 +245,12 @@ extension on JvmException { } final threadId = threadName.hashCode; - final sentryThread = SentryThread( + return SentryThread( crashed: true, current: false, name: threadName, id: threadId, ); - exception.threadId = threadId; - return MapEntry(exception, sentryThread); } } diff --git a/flutter/test/android_platform_exception_event_processor_test.dart b/flutter/test/android_platform_exception_event_processor_test.dart index e190dd921e..477a384ad9 100644 --- a/flutter/test/android_platform_exception_event_processor_test.dart +++ b/flutter/test/android_platform_exception_event_processor_test.dart @@ -12,6 +12,7 @@ import 'mocks.dart'; void main() { late Fixture fixture; + setUp(() { fixture = Fixture(); @@ -32,9 +33,12 @@ void main() { .apply(fixture.eventWithPlatformDetailsAndStackTrace, Hint()); final exceptions = platformExceptionEvent!.exceptions!; - expect(exceptions.length, 3); + expect(exceptions.length, 1); + + final exception = exceptions[0]; + expect(exception.mechanism?.source, isNull); - final platformException_1 = exceptions[1]; + final platformException_1 = exception.exceptions![0]; expect(platformException_1.type, 'IllegalArgumentException'); expect( @@ -42,8 +46,9 @@ void main() { "Unsupported value: '[Ljava.lang.StackTraceElement;@ba6feed' of type 'class [Ljava.lang.StackTraceElement;'", ); expect(platformException_1.stackTrace!.frames.length, 18); + expect(platformException_1.mechanism?.source, "stackTrace"); - final platformException_2 = exceptions[2]; + final platformException_2 = exception.exceptions![1]; expect(platformException_2.type, 'IllegalArgumentException'); expect( @@ -51,6 +56,7 @@ void main() { "Unsupported value: '[Ljava.lang.StackTraceElement;@ba6feed' of type 'class [Ljava.lang.StackTraceElement;'", ); expect(platformException_2.stackTrace!.frames.length, 18); + expect(platformException_2.mechanism?.source, "details"); }); test('platform exception with details correctly parsed', () async { @@ -58,9 +64,12 @@ void main() { .apply(fixture.eventWithPlatformDetails, Hint()); final exceptions = platformExceptionEvent!.exceptions!; - expect(exceptions.length, 2); + expect(exceptions.length, 1); - final platformException_1 = exceptions[1]; + final exception = exceptions[0]; + expect(exception.mechanism?.source, isNull); + + final platformException_1 = exception.exceptions![0]; expect(platformException_1.type, 'Resources\$NotFoundException'); expect(platformException_1.module, 'android.content.res'); @@ -69,6 +78,7 @@ void main() { "Unable to find resource ID #0x7f14000d", ); expect(platformException_1.stackTrace!.frames.length, 19); + expect(platformException_1.mechanism?.source, "details"); }); test('platform exception with stackTrace correctly parsed', () async { @@ -76,9 +86,12 @@ void main() { .apply(fixture.eventWithPlatformStackTrace, Hint()); final exceptions = platformExceptionEvent!.exceptions!; - expect(exceptions.length, 2); + expect(exceptions.length, 1); + + final exception = exceptions[0]; + expect(exception.mechanism?.source, isNull); - final platformException_1 = exceptions[1]; + final platformException_1 = exception.exceptions![0]; expect(platformException_1.type, 'IllegalArgumentException'); expect(platformException_1.module, 'java.lang'); @@ -87,6 +100,7 @@ void main() { "Not supported, use openfile", ); expect(platformException_1.stackTrace!.frames.length, 22); + expect(platformException_1.mechanism?.source, "stackTrace"); }); test( @@ -96,7 +110,7 @@ void main() { .apply(fixture.eventWithPlatformDetailsAndStackTrace, Hint()); final exceptions = platformExceptionEvent!.exceptions!; - expect(exceptions.length, 3); + expect(exceptions.length, 1); expect(platformExceptionEvent.threads?.first.current, true); expect(platformExceptionEvent.threads?.first.crashed, false); @@ -107,9 +121,10 @@ void main() { .apply(fixture.eventWithPlatformDetailsAndStackTrace, Hint()); final exceptions = platformExceptionEvent!.exceptions!; - expect(exceptions.length, 3); + expect(exceptions.length, 1); - final platformException = exceptions[1]; + final exception = exceptions[0]; + final platformException = exception.exceptions![0]; final platformThread = platformExceptionEvent.threads?[1]; expect(platformException.threadId, platformThread?.id); @@ -128,7 +143,7 @@ void main() { .apply(fixture.eventWithPlatformDetailsAndStackTrace, Hint()); final exceptions = platformExceptionEvent!.exceptions!; - expect(exceptions.length, 3); + expect(exceptions.length, 1); expect(platformExceptionEvent.threads?.length, threadCount); });