Skip to content

Commit f13b57b

Browse files
uemanueman
authored
Feature: Add threads for Android platform exceptions (#853)
1 parent dcf755c commit f13b57b

File tree

3 files changed

+149
-29
lines changed

3 files changed

+149
-29
lines changed

‎CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
* Feat: Add Android thread to platform stacktraces (#853)
6+
37
## 6.6.0-alpha.2
48

59
- Fix serialization of threads (#844)

‎flutter/lib/src/event_processor/android_platform_exception_event_processor.dart

+67-9
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import 'dart:async';
2+
import 'dart:isolate';
23

34
import 'package:flutter/services.dart';
45
import 'package:package_info_plus/package_info_plus.dart';
@@ -7,6 +8,8 @@ import '../jvm/jvm_exception.dart';
78
import '../jvm/jvm_frame.dart';
89

910
/// Transforms an Android PlatformException to a human readable SentryException
11+
// Relevant links:
12+
// - https://docs.flutter.dev/development/platform-integration/platform-channels?tab=ios-channel-objective-c-tab#channels-and-platform-threading
1013
class AndroidPlatformExceptionEventProcessor implements EventProcessor {
1114
const AndroidPlatformExceptionEventProcessor(this._options);
1215

@@ -62,12 +65,43 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
6265
exception,
6366
);
6467

65-
return event.copyWith(
66-
exceptions: [
67-
...?exceptions,
68-
...jvmException,
69-
],
70-
);
68+
final threads = _markDartThreadsAsNonCrashed(event.threads);
69+
70+
final jvmExceptions = jvmException.map((e) => e.key);
71+
72+
var jvmThreads = jvmException.map((e) => e.value).toList(growable: false);
73+
if (jvmThreads.isNotEmpty) {
74+
// filter potential duplicated threads
75+
final first = jvmThreads.first;
76+
jvmThreads = jvmThreads
77+
.skip(1)
78+
.where((element) => element.id != first.id)
79+
.toList(growable: true);
80+
jvmThreads.add(first);
81+
}
82+
83+
return event.copyWith(exceptions: [
84+
...?exceptions,
85+
...jvmExceptions,
86+
], threads: [
87+
...?threads,
88+
if (_options.attachThreads) ...jvmThreads,
89+
]);
90+
}
91+
92+
/// If the crash originated on Android, the Dart side didn't crash.
93+
/// Mark it accordingly.
94+
List<SentryThread>? _markDartThreadsAsNonCrashed(
95+
List<SentryThread>? threads,
96+
) {
97+
return threads
98+
?.map((e) => e.copyWith(
99+
crashed: false,
100+
// Isolate is safe to use directly,
101+
// because Android is only run in the dart:io context.
102+
current: e.name == Isolate.current.debugName,
103+
))
104+
.toList(growable: false);
71105
}
72106

73107
/// Remove the StackTrace from [PlatformException] so the message on Sentry
@@ -122,7 +156,8 @@ class _JvmExceptionFactory {
122156

123157
final String nativePackageName;
124158

125-
List<SentryException> fromJvmStackTrace(String exceptionAsString) {
159+
List<MapEntry<SentryException, SentryThread>> fromJvmStackTrace(
160+
String exceptionAsString) {
126161
final jvmException = JvmException.parse(exceptionAsString);
127162
final jvmExceptions = <JvmException>[
128163
jvmException,
@@ -137,7 +172,8 @@ class _JvmExceptionFactory {
137172
}
138173

139174
extension on JvmException {
140-
SentryException toSentryException(String nativePackageName) {
175+
MapEntry<SentryException, SentryThread> toSentryException(
176+
String nativePackageName) {
141177
String? exceptionType;
142178
String? module;
143179
final typeParts = type?.split('.');
@@ -152,14 +188,36 @@ extension on JvmException {
152188
return entry.value.toSentryStackFrame(entry.key, nativePackageName);
153189
}).toList(growable: false);
154190

155-
return SentryException(
191+
var exception = SentryException(
156192
value: description,
157193
type: exceptionType,
158194
module: module,
159195
stackTrace: SentryStackTrace(
160196
frames: stackFrames.reversed.toList(growable: false),
161197
),
162198
);
199+
200+
String threadName;
201+
if (thread != null) {
202+
// Needs to be prefixed with 'Android', otherwise this thread id might
203+
// clash with isolate names from the Dart side.
204+
threadName = 'Android: $thread';
205+
} else {
206+
// If there's no thread in the exception, we just indicate that it's
207+
// from Android
208+
threadName = 'Android';
209+
}
210+
final threadId = threadName.hashCode;
211+
212+
final sentryThread = SentryThread(
213+
crashed: true,
214+
current: false,
215+
name: threadName,
216+
id: threadId,
217+
);
218+
exception = exception.copyWith(threadId: threadId);
219+
220+
return MapEntry(exception, sentryThread);
163221
}
164222
}
165223

‎flutter/test/android_platform_exception_event_processor_test.dart

+78-20
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
@TestOn('vm')
12
// ignore_for_file: invalid_use_of_internal_member
23

34
import 'package:flutter/services.dart';
@@ -24,15 +25,8 @@ void main() {
2425

2526
group(AndroidPlatformExceptionEventProcessor, () {
2627
test('exception is correctly parsed', () async {
27-
final exception = fixture.options.exceptionFactory
28-
.getSentryException(testPlatformException);
29-
30-
final event = SentryEvent(
31-
exceptions: [exception],
32-
throwable: testPlatformException,
33-
);
34-
35-
final platformExceptionEvent = await fixture.processor.apply(event);
28+
final platformExceptionEvent =
29+
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);
3630

3731
final exceptions = platformExceptionEvent!.exceptions!;
3832
expect(exceptions.length, 2);
@@ -47,6 +41,49 @@ void main() {
4741
expect(platformException.stackTrace!.frames.length, 18);
4842
});
4943

44+
test(
45+
'Dart thread is current and not crashed if Android exception is present',
46+
() async {
47+
final platformExceptionEvent =
48+
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);
49+
50+
final exceptions = platformExceptionEvent!.exceptions!;
51+
expect(exceptions.length, 2);
52+
53+
expect(platformExceptionEvent.threads?.first.current, true);
54+
expect(platformExceptionEvent.threads?.first.crashed, false);
55+
});
56+
57+
test('platformexception has Android thread attached', () async {
58+
final platformExceptionEvent =
59+
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);
60+
61+
final exceptions = platformExceptionEvent!.exceptions!;
62+
expect(exceptions.length, 2);
63+
64+
final platformException = exceptions[1];
65+
final platformThread = platformExceptionEvent.threads?[1];
66+
67+
expect(platformException.threadId, platformThread?.id);
68+
expect(platformThread?.current, false);
69+
expect(platformThread?.crashed, true);
70+
expect(platformThread?.name, 'Android');
71+
});
72+
73+
test('platformexception has no Android thread attached if disabled',
74+
() async {
75+
fixture.options.attachThreads = false;
76+
final threadCount = fixture.eventWithPlatformStackTrace.threads?.length;
77+
78+
final platformExceptionEvent =
79+
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);
80+
81+
final exceptions = platformExceptionEvent!.exceptions!;
82+
expect(exceptions.length, 2);
83+
84+
expect(platformExceptionEvent.threads?.length, threadCount);
85+
});
86+
5087
test('does nothing if no PlatformException is there', () async {
5188
final exception = fixture.options.exceptionFactory
5289
.getSentryException(testPlatformException);
@@ -62,17 +99,10 @@ void main() {
6299
});
63100

64101
test('does nothing if PlatformException has no stackTrace', () async {
65-
final exception = fixture.options.exceptionFactory
66-
.getSentryException(emptyPlatformException);
102+
final platformExceptionEvent =
103+
await fixture.processor.apply(fixture.eventWithoutPlatformStackTrace);
67104

68-
final event = SentryEvent(
69-
exceptions: [exception],
70-
throwable: emptyPlatformException,
71-
);
72-
73-
final platformExceptionEvent = await fixture.processor.apply(event);
74-
75-
expect(event, platformExceptionEvent);
105+
expect(fixture.eventWithoutPlatformStackTrace, platformExceptionEvent);
76106
});
77107
});
78108
}
@@ -81,7 +111,35 @@ class Fixture {
81111
late AndroidPlatformExceptionEventProcessor processor =
82112
AndroidPlatformExceptionEventProcessor(options);
83113

84-
SentryFlutterOptions options = SentryFlutterOptions(dsn: fakeDsn);
114+
late SentryException withPlatformStackTrace = options.exceptionFactory
115+
.getSentryException(testPlatformException)
116+
.copyWith(threadId: 1);
117+
118+
late SentryException withoutPlatformStackTrace = options.exceptionFactory
119+
.getSentryException(emptyPlatformException)
120+
.copyWith(threadId: 1);
121+
122+
late SentryEvent eventWithPlatformStackTrace = SentryEvent(
123+
exceptions: [withPlatformStackTrace],
124+
throwable: testPlatformException,
125+
threads: [dartThread],
126+
);
127+
128+
late SentryEvent eventWithoutPlatformStackTrace = SentryEvent(
129+
exceptions: [withoutPlatformStackTrace],
130+
throwable: emptyPlatformException,
131+
threads: [dartThread],
132+
);
133+
134+
late SentryThread dartThread = SentryThread(
135+
crashed: true,
136+
current: true,
137+
id: 1,
138+
name: 'main',
139+
);
140+
141+
SentryFlutterOptions options = SentryFlutterOptions(dsn: fakeDsn)
142+
..attachThreads = true;
85143
}
86144

87145
final testPlatformException = PlatformException(

0 commit comments

Comments
 (0)