Merge 1ee2738 into 70118e9

Author: supervacuus

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - Fix crash when unregistering `SystemEventsBroadcastReceiver` with try-catch block. ([#5106](https://github.com/getsentry/sentry-java/pull/5106))
+- Identify and correctly structure Java/Kotlin frames in mixed Tombstone stack traces. ([#5116](https://github.com/getsentry/sentry-java/pull/5116))
 
 ## 8.33.0
 

sentry-android-core/src/main/java/io/sentry/android/core/internal/tombstone/TombstoneParser.java

@@ -33,6 +33,20 @@ public class TombstoneParser implements Closeable {
   @Nullable private final String nativeLibraryDir;
   private final Map<String, String> excTypeValueMap = new HashMap<>();
 
+  private static boolean isJavaFrame(@NonNull final TombstoneProtos.BacktraceFrame frame) {
+    final String fileName = frame.getFileName();
+    return !fileName.endsWith(".so")
+        && !fileName.endsWith("app_process64")
+        && (fileName.endsWith(".jar")
+            || fileName.endsWith(".odex")
+            || fileName.endsWith(".vdex")
+            || fileName.endsWith(".oat")
+            || fileName.startsWith("[anon:dalvik-")
+            || fileName.startsWith("<anonymous:")
+            || fileName.startsWith("[anon_shmem:dalvik-")
+            || fileName.startsWith("/memfd:jit-cache"));
+  }
+
   private static String formatHex(long value) {
     return String.format("0x%x", value);
   }
@@ -108,7 +122,8 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneProtos.Thread
     final List<SentryStackFrame> frames = new ArrayList<>();
 
     for (TombstoneProtos.BacktraceFrame frame : thread.getCurrentBacktraceList()) {
-      if (frame.getFileName().endsWith("libart.so")) {
+      if (frame.getFileName().endsWith("libart.so")
+          || Objects.equals(frame.getFunctionName(), "art_jni_trampoline")) {
         // We ignore all ART frames for time being because they aren't actionable for app developers
         continue;
       }
@@ -118,9 +133,15 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneProtos.Thread
         continue;
       }
       final SentryStackFrame stackFrame = new SentryStackFrame();
-      stackFrame.setPackage(frame.getFileName());
-      stackFrame.setFunction(frame.getFunctionName());
-      stackFrame.setInstructionAddr(formatHex(frame.getPc()));
+      if (isJavaFrame(frame)) {
+        stackFrame.setPlatform("java");
+        stackFrame.setFunction(extractJavaFunctionName(frame.getFunctionName()));
+        stackFrame.setModule(extractJavaModuleName(frame.getFunctionName()));
+      } else {
+        stackFrame.setPackage(frame.getFileName());
+        stackFrame.setFunction(frame.getFunctionName());
+        stackFrame.setInstructionAddr(formatHex(frame.getPc()));
+      }
 
       // inAppIncludes/inAppExcludes filter by Java/Kotlin package names, which don't overlap
       // with native C/C++ function names (e.g., "crash", "__libc_init"). For native frames,
@@ -159,6 +180,22 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneProtos.Thread
     return stacktrace;
   }
 
+  private static @Nullable String extractJavaModuleName(String fqFunctionName) {
+    if (fqFunctionName.contains(".")) {
+      return fqFunctionName.substring(0, fqFunctionName.lastIndexOf("."));
+    } else {
+      return "";
+    }
+  }
+
+  private static @Nullable String extractJavaFunctionName(String fqFunctionName) {
+    if (fqFunctionName.contains(".")) {
+      return fqFunctionName.substring(fqFunctionName.lastIndexOf(".") + 1);
+    } else {
+      return fqFunctionName;
+    }
+  }
+
   @NonNull
   private List<SentryException> createException(@NonNull TombstoneProtos.Tombstone tombstone) {
     final SentryException exception = new SentryException();
@@ -296,7 +333,7 @@ private DebugMeta createDebugMeta(@NonNull final TombstoneProtos.Tombstone tombs
         // Check for duplicated mappings: On Android, the same ELF can have multiple
         // mappings at offset 0 with different permissions (r--p, r-xp, r--p).
         // If it's the same file as the current module, just extend it.
-        if (currentModule != null && mappingName.equals(currentModule.mappingName)) {
+        if (currentModule != null && Objects.equals(mappingName, currentModule.mappingName)) {
           currentModule.extendTo(mapping.getEndAddress());
           continue;
         }
@@ -311,7 +348,7 @@ private DebugMeta createDebugMeta(@NonNull final TombstoneProtos.Tombstone tombs
 
         // Start a new module
         currentModule = new ModuleAccumulator(mapping);
-      } else if (currentModule != null && mappingName.equals(currentModule.mappingName)) {
+      } else if (currentModule != null && Objects.equals(mappingName, currentModule.mappingName)) {
         // Extend the current module with this mapping (same file, continuation)
         currentModule.extendTo(mapping.getEndAddress());
       }

sentry-android-core/src/test/java/io/sentry/android/core/internal/tombstone/TombstoneParserTest.kt

@@ -101,14 +101,20 @@ class TombstoneParserTest {
 
       for (frame in thread.stacktrace!!.frames!!) {
         assertNotNull(frame.function)
-        assertNotNull(frame.`package`)
-        assertNotNull(frame.instructionAddr)
+        if (frame.platform == "java") {
+          // Java frames have module instead of package/instructionAddr
+          assertNotNull(frame.module)
+        } else {
+          assertNotNull(frame.`package`)
+          assertNotNull(frame.instructionAddr)
+        }
 
         if (thread.id == crashedThreadId) {
           if (frame.isInApp!!) {
             assert(
-              frame.function!!.startsWith(inAppIncludes[0]) ||
-                frame.`package`!!.startsWith(nativeLibraryDir)
+              frame.module?.startsWith(inAppIncludes[0]) == true ||
+                frame.function!!.startsWith(inAppIncludes[0]) ||
+                frame.`package`?.startsWith(nativeLibraryDir) == true
             )
           }
         }
@@ -429,6 +435,35 @@ class TombstoneParserTest {
     }
   }
 
+  @Test
+  fun `java frames snapshot test for all threads`() {
+    val tombstoneStream =
+      GZIPInputStream(TombstoneParserTest::class.java.getResourceAsStream("/tombstone.pb.gz"))
+    val parser = TombstoneParser(tombstoneStream, inAppIncludes, inAppExcludes, nativeLibraryDir)
+    val event = parser.parse()
+
+    val logger = mock<ILogger>()
+    val writer = StringWriter()
+    val jsonWriter = JsonObjectWriter(writer, 100)
+    jsonWriter.beginObject()
+    for (thread in event.threads!!) {
+      val javaFrames = thread.stacktrace!!.frames!!.filter { it.platform == "java" }
+      if (javaFrames.isEmpty()) continue
+      jsonWriter.name(thread.id.toString())
+      jsonWriter.beginArray()
+      for (frame in javaFrames) {
+        frame.serialize(jsonWriter, logger)
+      }
+      jsonWriter.endArray()
+    }
+    jsonWriter.endObject()
+
+    val actualJson = writer.toString()
+    val expectedJson = readGzippedResourceFile("/tombstone_java_frames.json.gz")
+
+    assertEquals(expectedJson, actualJson)
+  }
+
   private fun serializeDebugMeta(debugMeta: DebugMeta): String {
     val logger = mock<ILogger>()
     val writer = StringWriter()