From 26f3a393b3a6a63fbc887aa3bbc08a98ccc60e43 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 19 Feb 2024 13:12:59 +0100 Subject: [PATCH 1/3] Reverse order for Android RuntimeExceptions --- .../core/DefaultAndroidEventProcessor.java | 48 +++++++++++ .../core/DefaultAndroidEventProcessorTest.kt | 86 +++++++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 8dcfe196c2..97d5a042a5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -3,6 +3,7 @@ import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; +import androidx.annotation.NonNull; import io.sentry.DateUtils; import io.sentry.EventProcessor; import io.sentry.Hint; @@ -15,11 +16,16 @@ import io.sentry.android.core.performance.TimeSpan; import io.sentry.protocol.App; import io.sentry.protocol.OperatingSystem; +import io.sentry.protocol.SentryException; +import io.sentry.protocol.SentryStackFrame; +import io.sentry.protocol.SentryStackTrace; import io.sentry.protocol.SentryThread; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; import io.sentry.util.HintUtils; import io.sentry.util.Objects; +import java.util.Collections; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.concurrent.ExecutorService; @@ -68,9 +74,51 @@ public DefaultAndroidEventProcessor( setCommons(event, true, applyScopeData); + fixExceptionOrder(event); + return event; } + /** + * The last exception is usually used for picking the issue title, but the convention is to send + * inner exceptions first, e.g. [inner, outer] This doesn't work very well on Android, as some + * hooks like Application.onCreate is wrapped by Android framework with a RuntimeException. Thus, + * if the last exception is a RuntimeInit$MethodAndArgsCaller, reverse the order to get a better + * issue title. This is a quick fix, for more details see: #64074 #59679 #64088 + * + * @param event the event to process + */ + private static void fixExceptionOrder(final @NonNull SentryEvent event) { + boolean reverseExceptions = false; + + final @Nullable List exceptions = event.getExceptions(); + if (exceptions != null && exceptions.size() > 1) { + final @NotNull SentryException lastException = exceptions.get(exceptions.size() - 1); + if ("java.lang".equals(lastException.getModule())) { + final @Nullable SentryStackTrace stacktrace = lastException.getStacktrace(); + if (stacktrace != null) { + final @Nullable List frames = stacktrace.getFrames(); + if (frames != null) { + for (SentryStackFrame frame : frames) { + if ("com.android.internal.os.RuntimeInit$MethodAndArgsCaller" + .equals(frame.getModule())) { + reverseExceptions = true; + break; + } + } + } + } + } + } + + if (reverseExceptions) { + Collections.reverse(exceptions); + } + } + private void setCommons( final @NotNull SentryBaseEvent event, final boolean errorEvent, diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index 70b20e2ab2..80954f67a5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -16,6 +16,9 @@ import io.sentry.TypeCheckHint.SENTRY_DART_SDK_NAME import io.sentry.android.core.internal.util.CpuInfoUtils import io.sentry.protocol.OperatingSystem import io.sentry.protocol.SdkVersion +import io.sentry.protocol.SentryException +import io.sentry.protocol.SentryStackFrame +import io.sentry.protocol.SentryStackTrace import io.sentry.protocol.SentryThread import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User @@ -573,4 +576,87 @@ class DefaultAndroidEventProcessorTest { assertNull(thread.isMain) } } + + @Test + fun `the exception list is reversed in case there's an RuntimeException`() { + val sut = fixture.getSut(context) + val event = SentryEvent().apply { + exceptions = listOf( + SentryException().apply { + type = "IllegalStateException" + module = "com.example" + stacktrace = SentryStackTrace( + listOf( + SentryStackFrame().apply { + function = "onCreate" + module = "com.example.Application" + filename = "Application.java" + } + ) + ) + }, + SentryException().apply { + type = "RuntimeException" + value = "Unable to create application com.example.Application: java.lang.IllegalStateException" + module = "java.lang" + stacktrace = SentryStackTrace( + listOf( + SentryStackFrame().apply { + function = "run" + module = "com.android.internal.os.RuntimeInit\$MethodAndArgsCaller" + filename = "RuntimeInit.java" + } + ) + ) + } + ) + } + val processedEvent = sut.process(event, Hint()) + assertNotNull(processedEvent) { + assertEquals(2, it.exceptions!!.size) + assertEquals("RuntimeException", it.exceptions!![0].type) + assertEquals("IllegalStateException", it.exceptions!![1].type) + } + } + + @Test + fun `the exception list is kept as-is in case there's no RuntimeException`() { + val sut = fixture.getSut(context) + val event = SentryEvent().apply { + exceptions = listOf( + SentryException().apply { + type = "IllegalStateException" + module = "com.example" + stacktrace = SentryStackTrace( + listOf( + SentryStackFrame().apply { + function = "onCreate" + module = "com.example.Application" + filename = "Application.java" + } + ) + ) + }, + SentryException().apply { + type = "IllegalArgumentException" + module = "com.example" + stacktrace = SentryStackTrace( + listOf( + SentryStackFrame().apply { + function = "onCreate" + module = "com.example.Application" + filename = "Application.java" + } + ) + ) + } + ) + } + val processedEvent = sut.process(event, Hint()) + assertNotNull(processedEvent) { + assertEquals(2, it.exceptions!!.size) + assertEquals("IllegalStateException", it.exceptions!![0].type) + assertEquals("IllegalArgumentException", it.exceptions!![1].type) + } + } } From db1e48b210072b8263b9da0ef4eb5d19b775f607 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 20 Feb 2024 09:09:48 +0100 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 754646ddc5..a9c00ea92f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Don't wait on main thread when SDK restarts ([#3200](https://github.com/getsentry/sentry-java/pull/3200)) - Fix Jetpack Compose widgets are not being correctly identified for user interaction tracing ([#3209](https://github.com/getsentry/sentry-java/pull/3209)) +- Fix issue title on Android when a wrapped RuntimeException is thrown by the system ([#3212](https://github.com/getsentry/sentry-java/pull/3212)) ## 7.3.0 From 38e4f313b3c081b353f8283e7acb5cb143d0ad85 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 20 Feb 2024 11:27:04 +0100 Subject: [PATCH 3/3] Address PR feedback --- CHANGELOG.md | 3 ++- .../io/sentry/android/core/DefaultAndroidEventProcessor.java | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9c00ea92f..2ce997f3b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,8 @@ - Don't wait on main thread when SDK restarts ([#3200](https://github.com/getsentry/sentry-java/pull/3200)) - Fix Jetpack Compose widgets are not being correctly identified for user interaction tracing ([#3209](https://github.com/getsentry/sentry-java/pull/3209)) -- Fix issue title on Android when a wrapped RuntimeException is thrown by the system ([#3212](https://github.com/getsentry/sentry-java/pull/3212)) +- Fix issue title on Android when a wrapping `RuntimeException` is thrown by the system ([#3212](https://github.com/getsentry/sentry-java/pull/3212)) + - This will change grouping of the issues that were previously titled `RuntimeInit$MethodAndArgsCaller` to have them split up properly by the original root cause exception ## 7.3.0 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 97d5a042a5..45e4b78787 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -3,7 +3,6 @@ import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; -import androidx.annotation.NonNull; import io.sentry.DateUtils; import io.sentry.EventProcessor; import io.sentry.Hint; @@ -91,7 +90,7 @@ public DefaultAndroidEventProcessor( * * @param event the event to process */ - private static void fixExceptionOrder(final @NonNull SentryEvent event) { + private static void fixExceptionOrder(final @NotNull SentryEvent event) { boolean reverseExceptions = false; final @Nullable List exceptions = event.getExceptions(); @@ -102,7 +101,7 @@ private static void fixExceptionOrder(final @NonNull SentryEvent event) { if (stacktrace != null) { final @Nullable List frames = stacktrace.getFrames(); if (frames != null) { - for (SentryStackFrame frame : frames) { + for (final @NotNull SentryStackFrame frame : frames) { if ("com.android.internal.os.RuntimeInit$MethodAndArgsCaller" .equals(frame.getModule())) { reverseExceptions = true;