diff --git a/CHANGELOG.md b/CHANGELOG.md index 09583f6171..6dcb06d64e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +* Feat: Add Android thread to platform stacktraces (#853) + ## 6.6.0-alpha.2 - Fix serialization of threads (#844) 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 6bc7f51b8b..907e4335bd 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 @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:isolate'; import 'package:flutter/services.dart'; import 'package:package_info_plus/package_info_plus.dart'; @@ -7,6 +8,8 @@ import '../jvm/jvm_exception.dart'; import '../jvm/jvm_frame.dart'; /// Transforms an Android PlatformException to a human readable SentryException +// Relevant links: +// - https://docs.flutter.dev/development/platform-integration/platform-channels?tab=ios-channel-objective-c-tab#channels-and-platform-threading class AndroidPlatformExceptionEventProcessor implements EventProcessor { const AndroidPlatformExceptionEventProcessor(this._options); @@ -62,12 +65,43 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor { exception, ); - return event.copyWith( - exceptions: [ - ...?exceptions, - ...jvmException, - ], - ); + final threads = _markDartThreadsAsNonCrashed(event.threads); + + final jvmExceptions = jvmException.map((e) => e.key); + + var jvmThreads = jvmException.map((e) => e.value).toList(growable: false); + if (jvmThreads.isNotEmpty) { + // filter potential duplicated threads + final first = jvmThreads.first; + jvmThreads = jvmThreads + .skip(1) + .where((element) => element.id != first.id) + .toList(growable: true); + jvmThreads.add(first); + } + + return event.copyWith(exceptions: [ + ...?exceptions, + ...jvmExceptions, + ], threads: [ + ...?threads, + if (_options.attachThreads) ...jvmThreads, + ]); + } + + /// If the crash originated on Android, the Dart side didn't crash. + /// Mark it accordingly. + List? _markDartThreadsAsNonCrashed( + List? threads, + ) { + return threads + ?.map((e) => e.copyWith( + crashed: false, + // Isolate is safe to use directly, + // because Android is only run in the dart:io context. + current: e.name == Isolate.current.debugName, + )) + .toList(growable: false); } /// Remove the StackTrace from [PlatformException] so the message on Sentry @@ -122,7 +156,8 @@ class _JvmExceptionFactory { final String nativePackageName; - List fromJvmStackTrace(String exceptionAsString) { + List> fromJvmStackTrace( + String exceptionAsString) { final jvmException = JvmException.parse(exceptionAsString); final jvmExceptions = [ jvmException, @@ -137,7 +172,8 @@ class _JvmExceptionFactory { } extension on JvmException { - SentryException toSentryException(String nativePackageName) { + MapEntry toSentryException( + String nativePackageName) { String? exceptionType; String? module; final typeParts = type?.split('.'); @@ -152,7 +188,7 @@ extension on JvmException { return entry.value.toSentryStackFrame(entry.key, nativePackageName); }).toList(growable: false); - return SentryException( + var exception = SentryException( value: description, type: exceptionType, module: module, @@ -160,6 +196,28 @@ extension on JvmException { frames: stackFrames.reversed.toList(growable: false), ), ); + + String threadName; + if (thread != null) { + // Needs to be prefixed with 'Android', otherwise this thread id might + // clash with isolate names from the Dart side. + threadName = 'Android: $thread'; + } else { + // If there's no thread in the exception, we just indicate that it's + // from Android + threadName = 'Android'; + } + final threadId = threadName.hashCode; + + final sentryThread = SentryThread( + crashed: true, + current: false, + name: threadName, + id: threadId, + ); + exception = exception.copyWith(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 f37a38fef9..77a85724da 100644 --- a/flutter/test/android_platform_exception_event_processor_test.dart +++ b/flutter/test/android_platform_exception_event_processor_test.dart @@ -1,3 +1,4 @@ +@TestOn('vm') // ignore_for_file: invalid_use_of_internal_member import 'package:flutter/services.dart'; @@ -24,15 +25,8 @@ void main() { group(AndroidPlatformExceptionEventProcessor, () { test('exception is correctly parsed', () async { - final exception = fixture.options.exceptionFactory - .getSentryException(testPlatformException); - - final event = SentryEvent( - exceptions: [exception], - throwable: testPlatformException, - ); - - final platformExceptionEvent = await fixture.processor.apply(event); + final platformExceptionEvent = + await fixture.processor.apply(fixture.eventWithPlatformStackTrace); final exceptions = platformExceptionEvent!.exceptions!; expect(exceptions.length, 2); @@ -47,6 +41,49 @@ void main() { expect(platformException.stackTrace!.frames.length, 18); }); + test( + 'Dart thread is current and not crashed if Android exception is present', + () async { + final platformExceptionEvent = + await fixture.processor.apply(fixture.eventWithPlatformStackTrace); + + final exceptions = platformExceptionEvent!.exceptions!; + expect(exceptions.length, 2); + + expect(platformExceptionEvent.threads?.first.current, true); + expect(platformExceptionEvent.threads?.first.crashed, false); + }); + + test('platformexception has Android thread attached', () async { + final platformExceptionEvent = + await fixture.processor.apply(fixture.eventWithPlatformStackTrace); + + final exceptions = platformExceptionEvent!.exceptions!; + expect(exceptions.length, 2); + + final platformException = exceptions[1]; + final platformThread = platformExceptionEvent.threads?[1]; + + expect(platformException.threadId, platformThread?.id); + expect(platformThread?.current, false); + expect(platformThread?.crashed, true); + expect(platformThread?.name, 'Android'); + }); + + test('platformexception has no Android thread attached if disabled', + () async { + fixture.options.attachThreads = false; + final threadCount = fixture.eventWithPlatformStackTrace.threads?.length; + + final platformExceptionEvent = + await fixture.processor.apply(fixture.eventWithPlatformStackTrace); + + final exceptions = platformExceptionEvent!.exceptions!; + expect(exceptions.length, 2); + + expect(platformExceptionEvent.threads?.length, threadCount); + }); + test('does nothing if no PlatformException is there', () async { final exception = fixture.options.exceptionFactory .getSentryException(testPlatformException); @@ -62,17 +99,10 @@ void main() { }); test('does nothing if PlatformException has no stackTrace', () async { - final exception = fixture.options.exceptionFactory - .getSentryException(emptyPlatformException); + final platformExceptionEvent = + await fixture.processor.apply(fixture.eventWithoutPlatformStackTrace); - final event = SentryEvent( - exceptions: [exception], - throwable: emptyPlatformException, - ); - - final platformExceptionEvent = await fixture.processor.apply(event); - - expect(event, platformExceptionEvent); + expect(fixture.eventWithoutPlatformStackTrace, platformExceptionEvent); }); }); } @@ -81,7 +111,35 @@ class Fixture { late AndroidPlatformExceptionEventProcessor processor = AndroidPlatformExceptionEventProcessor(options); - SentryFlutterOptions options = SentryFlutterOptions(dsn: fakeDsn); + late SentryException withPlatformStackTrace = options.exceptionFactory + .getSentryException(testPlatformException) + .copyWith(threadId: 1); + + late SentryException withoutPlatformStackTrace = options.exceptionFactory + .getSentryException(emptyPlatformException) + .copyWith(threadId: 1); + + late SentryEvent eventWithPlatformStackTrace = SentryEvent( + exceptions: [withPlatformStackTrace], + throwable: testPlatformException, + threads: [dartThread], + ); + + late SentryEvent eventWithoutPlatformStackTrace = SentryEvent( + exceptions: [withoutPlatformStackTrace], + throwable: emptyPlatformException, + threads: [dartThread], + ); + + late SentryThread dartThread = SentryThread( + crashed: true, + current: true, + id: 1, + name: 'main', + ); + + SentryFlutterOptions options = SentryFlutterOptions(dsn: fakeDsn) + ..attachThreads = true; } final testPlatformException = PlatformException(