Skip to content

Feature: Add threads for Android platform exceptions #853

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 12 commits into from
May 10, 2022
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

* Feat: Add Android thread to platform stacktraces (#853)

## 6.6.0-alpha.2

- Fix serialization of threads (#844)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:async';
import 'dart:isolate';

import 'package:flutter/services.dart';
import 'package:package_info_plus/package_info_plus.dart';
Expand All @@ -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);

Expand Down Expand Up @@ -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<SentryThread>? _markDartThreadsAsNonCrashed(
List<SentryThread>? 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
Expand Down Expand Up @@ -122,7 +156,8 @@ class _JvmExceptionFactory {

final String nativePackageName;

List<SentryException> fromJvmStackTrace(String exceptionAsString) {
List<MapEntry<SentryException, SentryThread>> fromJvmStackTrace(
String exceptionAsString) {
final jvmException = JvmException.parse(exceptionAsString);
final jvmExceptions = <JvmException>[
jvmException,
Expand All @@ -137,7 +172,8 @@ class _JvmExceptionFactory {
}

extension on JvmException {
SentryException toSentryException(String nativePackageName) {
MapEntry<SentryException, SentryThread> toSentryException(
String nativePackageName) {
String? exceptionType;
String? module;
final typeParts = type?.split('.');
Expand All @@ -152,14 +188,36 @@ extension on JvmException {
return entry.value.toSentryStackFrame(entry.key, nativePackageName);
}).toList(growable: false);

return SentryException(
var exception = SentryException(
value: description,
type: exceptionType,
module: module,
stackTrace: SentryStackTrace(
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);
}
}

Expand Down
98 changes: 78 additions & 20 deletions flutter/test/android_platform_exception_event_processor_test.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@TestOn('vm')
// ignore_for_file: invalid_use_of_internal_member

import 'package:flutter/services.dart';
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
});
});
}
Expand All @@ -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(
Expand Down