Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Parse message field in PlatformException.message #1716

Merged
merged 3 commits into from
Nov 13, 2023
Merged

Parse message field in PlatformException.message #1716

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cba4f2a
Parse message field
ueman Nov 9, 2023
d6b5e7f
changelog
ueman Nov 9, 2023
a7c53b6
Update CHANGELOG.md
ueman Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# Changelog

Check failure on line 1 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / danger / danger 

Please consider adding a changelog entry for the next release.

Check notice on line 1 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / danger / danger 

### Instructions and example for changelog

Please add an entry to `CHANGELOG.md` to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

```markdown
## Unreleased

- Parse message field in `PlatformException.message` ([#1716](https://github.com/getsentry/sentry-dart/pull/1716))
```

If none of the above apply, you can opt out of this check by adding `#skip-changelog` to the PR description.

## Unreleased

### Features

- StackTraces in `PlatformException.message` will get nicely formatted too
ueman marked this conversation as resolved.
Show resolved Hide resolved
- Breadcrumbs for database operations ([#1656](https://github.com/getsentry/sentry-dart/pull/1656))

## 7.12.0
Expand Down
135 changes: 53 additions & 82 deletions flutter/lib/src/event_processor/android_platform_exception_event_processor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {

final SentryFlutterOptions _options;

// Because of obfuscation, we need to dynamically get the name
static final _platformExceptionType = (PlatformException).toString();

@override
Future<SentryEvent?> apply(SentryEvent event, {Hint? hint}) async {
if (event is SentryTransaction) {
Expand All @@ -29,19 +26,23 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
return event;
}

final nativeStackTrace = plaformException.stacktrace;
if (nativeStackTrace == null) {
return event;
}

try {
// PackageInfo has an internal cache, so no need to do it ourselves.
final packageInfo = await PackageInfo.fromPlatform();

final nativeStackTrace =
_tryParse(plaformException.stacktrace, packageInfo.packageName);
final messageStackTrace =
_tryParse(plaformException.message, packageInfo.packageName);

if (nativeStackTrace == null && messageStackTrace == null) {
return event;
}

return _processPlatformException(
event,
plaformException,
nativeStackTrace,
packageInfo.packageName,
messageStackTrace,
);
} catch (e, stackTrace) {
_options.logger(
Expand All @@ -55,25 +56,35 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
}
}

SentryEvent _processPlatformException(
SentryEvent event,
PlatformException exception,
String nativeStackTrace,
List<MapEntry<SentryException, SentryThread>>? _tryParse(
String? potentialStackTrace,
String packageName,
) {
final jvmException =
_JvmExceptionFactory(packageName).fromJvmStackTrace(nativeStackTrace);
if (potentialStackTrace == null) {
return null;
}

final exceptions = _removePlatformExceptionStackTraceFromValue(
event.exceptions,
exception,
);
return _JvmExceptionFactory(packageName)
.fromJvmStackTrace(potentialStackTrace);
}

SentryEvent _processPlatformException(
SentryEvent event,
List<MapEntry<SentryException, SentryThread>>? nativeStackTrace,
List<MapEntry<SentryException, SentryThread>>? messageStackTrace,
) {
final threads = _markDartThreadsAsNonCrashed(event.threads);

final jvmExceptions = jvmException.map((e) => e.key);
final jvmExceptions = [
...?nativeStackTrace?.map((e) => e.key),
...?messageStackTrace?.map((e) => e.key)
];

var jvmThreads = [
...?nativeStackTrace?.map((e) => e.value),
...?messageStackTrace?.map((e) => e.value),
];

var jvmThreads = jvmException.map((e) => e.value).toList(growable: false);
if (jvmThreads.isNotEmpty) {
// filter potential duplicated threads
final first = jvmThreads.first;
Expand All @@ -84,13 +95,16 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
jvmThreads.add(first);
}

return event.copyWith(exceptions: [
...?exceptions,
...jvmExceptions,
], threads: [
...?threads,
if (_options.attachThreads) ...jvmThreads,
]);
return event.copyWith(
exceptions: [
...?event.exceptions,
...jvmExceptions,
],
threads: [
...?threads,
if (_options.attachThreads) ...jvmThreads,
],
);
}

/// If the crash originated on Android, the Dart side didn't crash.
Expand All @@ -99,60 +113,16 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
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,
))
?.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
/// looks much better.
List<SentryException>? _removePlatformExceptionStackTraceFromValue(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this part of the code as it breaks PlatformException subclasses.

List<SentryException>? exceptions,
PlatformException platformException,
) {
if (exceptions == null || exceptions.isEmpty) {
return null;
}
final exceptionCopy = List<SentryException>.from(exceptions);

final sentryExceptions = exceptionCopy
.where((element) => element.type == _platformExceptionType);
if (sentryExceptions.isEmpty) {
return null;
}
var sentryException = sentryExceptions.first;

final exceptionIndex = exceptionCopy.indexOf(sentryException);
exceptionCopy.remove(sentryException);

// Remove stacktrace, so that the PlatformException value doesn't
// include the chained exception.
// PlatformException.stackTrace is an empty string so that
// PlatformException.toString() results in
// `PlatformException(error, Exception Message, null, )`
// instead of
// `PlatformException(error, Exception Message, null, null)`.
// While `null` for `PlatformException.stackTrace` is technically correct
// it's semantically wrong.
platformException = PlatformException(
code: platformException.code,
details: platformException.details,
message: platformException.message,
stacktrace: '',
);

sentryException = sentryException.copyWith(
value: platformException.toString(),
);

exceptionCopy.insert(exceptionIndex, sentryException);

return exceptionCopy;
}
}

class _JvmExceptionFactory {
Expand All @@ -161,7 +131,8 @@ class _JvmExceptionFactory {
final String nativePackageName;

List<MapEntry<SentryException, SentryThread>> fromJvmStackTrace(
String exceptionAsString) {
String exceptionAsString,
) {
final jvmException = JvmException.parse(exceptionAsString);
final jvmExceptions = <JvmException>[
jvmException,
Expand Down
43 changes: 27 additions & 16 deletions flutter/test/android_platform_exception_event_processor_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void main() {
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);

final exceptions = platformExceptionEvent!.exceptions!;
expect(exceptions.length, 2);
expect(exceptions.length, 3);

final platformException = exceptions[1];

Expand All @@ -40,6 +40,15 @@ void main() {
"Unsupported value: '[Ljava.lang.StackTraceElement;@ba6feed' of type 'class [Ljava.lang.StackTraceElement;'",
);
expect(platformException.stackTrace!.frames.length, 18);

final platformException_2 = exceptions[2];

expect(platformException_2.type, 'IllegalArgumentException');
expect(
platformException_2.value,
"Unsupported value: '[Ljava.lang.StackTraceElement;@ba6feed' of type 'class [Ljava.lang.StackTraceElement;'",
);
expect(platformException_2.stackTrace!.frames.length, 18);
});

test(
Expand All @@ -49,7 +58,7 @@ void main() {
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);

final exceptions = platformExceptionEvent!.exceptions!;
expect(exceptions.length, 2);
expect(exceptions.length, 3);

expect(platformExceptionEvent.threads?.first.current, true);
expect(platformExceptionEvent.threads?.first.crashed, false);
Expand All @@ -60,7 +69,7 @@ void main() {
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);

final exceptions = platformExceptionEvent!.exceptions!;
expect(exceptions.length, 2);
expect(exceptions.length, 3);

final platformException = exceptions[1];
final platformThread = platformExceptionEvent.threads?[1];
Expand All @@ -80,7 +89,7 @@ void main() {
await fixture.processor.apply(fixture.eventWithPlatformStackTrace);

final exceptions = platformExceptionEvent!.exceptions!;
expect(exceptions.length, 2);
expect(exceptions.length, 3);

expect(platformExceptionEvent.threads?.length, threadCount);
});
Expand Down Expand Up @@ -144,12 +153,23 @@ class Fixture {
}

final testPlatformException = PlatformException(
code: 'error',
details:
"Unsupported value: '[Ljava.lang.StackTraceElement;@fa902f1' of type 'class [Ljava.lang.StackTraceElement;'",
message: _jvmStackTrace,
stacktrace: _jvmStackTrace,
);

final emptyPlatformException = PlatformException(
code: 'error',
details:
"Unsupported value: '[Ljava.lang.StackTraceElement;@fa902f1' of type 'class [Ljava.lang.StackTraceElement;'",
message: null,
stacktrace:
"""java.lang.IllegalArgumentException: Unsupported value: '[Ljava.lang.StackTraceElement;@ba6feed' of type 'class [Ljava.lang.StackTraceElement;'
stacktrace: null,
);

const _jvmStackTrace =
"""java.lang.IllegalArgumentException: Unsupported value: '[Ljava.lang.StackTraceElement;@ba6feed' of type 'class [Ljava.lang.StackTraceElement;'
at io.flutter.plugin.common.StandardMessageCodec.writeValue(StandardMessageCodec.java:292)
at io.flutter.plugin.common.StandardMethodCodec.encodeSuccessEnvelope(StandardMethodCodec.java:59)
at io.flutter.plugin.common.MethodChannel\$IncomingMethodCallHandler\$1.success(MethodChannel.java:267)
Expand All @@ -167,13 +187,4 @@ final testPlatformException = PlatformException(
at android.app.ActivityThread.main(ActivityThread.java:8138)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit\$MethodAndArgsCaller.run(RuntimeInit.java:556)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1037)""",
);

final emptyPlatformException = PlatformException(
code: 'error',
details:
"Unsupported value: '[Ljava.lang.StackTraceElement;@fa902f1' of type 'class [Ljava.lang.StackTraceElement;'",
message: null,
stacktrace: null,
);
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1037)""";
Loading