fix: Prevent crash when test fails on a different thread

When a test fails due to an exception thrown on a different thread,
the test class itself may not appear in the exceptions stack trace.
The agent was previously unable to handle this scenario, crashing
with a `java.lang.InternalError` because it could not find a stack
frame matching the test class.

This commit introduces a more robust heuristic to find the most
likely source of the failure. If an exact match for the test class
is not found, the agent now falls back to the stack frame with the
longest common package prefix. This prevents the crash and correctly
identifies the failure location in a framework-agnostic way.

A regression test has been added to simulate this cross-thread
exception scenario and verify the fix.

Author: dividedmind

agent/src/main/java/com/appland/appmap/process/hooks/test/TestSupport.java

@@ -63,19 +63,61 @@ private static void startRecording(TestDetails details, Recorder.Metadata metada
     RecordingSupport.startRecording(details, metadata);
   }
 
+  /**
+   * Finds the most relevant stack frame for a test failure.
+   *
+   * The primary goal is to find the stack frame that corresponds to the test
+   * class itself. However, in some scenarios (e.g., when an assertion fails on
+   * a different thread), the test class may not be in the stack trace at all.
+   *
+   * To handle this, we use a fallback heuristic: we find the stack frame that
+   * has the longest common package prefix with the test class. This is usually
+   * the entry point into the user's code and the most likely source of the
+   * failure.
+   *
+   * @param self      The test class instance
+   * @param exception The exception that caused the failure
+   * @return The most relevant stack frame
+   * @throws InternalError if no suitable stack frame can be found
+   */
   static StackTraceElement findErrorFrame(Object self, Throwable exception) throws InternalError {
     String selfClass = self.getClass().getName();
-    StackTraceElement errorFrame = null;
+    StackTraceElement bestMatch = null;
+    int bestMatchLength = 0;
+
     for (StackTraceElement frame : exception.getStackTrace()) {
-      if (frame.getClassName().equals(selfClass)) {
-        errorFrame = frame;
-        break;
+      final String frameClassName = frame.getClassName();
+      if (frameClassName.equals(selfClass)) {
+        // This is the ideal case: we found the test class in the stack trace.
+        return frame;
       }
+
+      int commonPrefix = commonPrefixLength(selfClass, frameClassName);
+      if (commonPrefix >= bestMatchLength) {
+        // We use >= to get the last best match, which is the most likely to be
+        // the entry point into the user's code.
+        bestMatch = frame;
+        bestMatchLength = commonPrefix;
+      }
+    }
+
+    if (bestMatch != null) {
+      // We didn't find the test class, but we have a good fallback.
+      return bestMatch;
     }
-    if (errorFrame == null) {
-      throw new InternalError("no stack frame matched test class");
+
+    // This can happen if the exception has an empty stack trace, which is rare
+    // but possible.
+    throw new InternalError("no stack frame matched test class");
+  }
+
+  private static int commonPrefixLength(String s1, String s2) {
+    int len = Math.min(s1.length(), s2.length());
+    int i = 0;
+    while (i < len && s1.charAt(i) == s2.charAt(i)) {
+      i++;
     }
-    return errorFrame;
+    return i;
   }
 
   private static boolean hasTestAnnotation(ClassLoader cl, Method stackMethod) {

agent/test/test-frameworks/frameworks.bats

@@ -52,7 +52,7 @@ run_framework_test() {
 
   assert_json_eq '.metadata.test_status' "failed"
   assert_json_contains '.metadata.test_failure.message' 'false is not true'
-  assert_json_eq '.metadata.test_failure.location' "src/test/java/org/springframework/samples/petclinic/JunitTests.java:20"
+  assert_json_eq '.metadata.test_failure.location' "src/test/java/org/springframework/samples/petclinic/JunitTests.java:23"
 }
 
 @test "test status set for failed test in testng" {
@@ -104,4 +104,12 @@ run_framework_test() {
 
   output="$(< tmp/appmap/testng/org_springframework_samples_petclinic_TestngTests_testItThrows.appmap.json)"
   assert_json_eq '.metadata.test_status' "succeeded"
+}
+
+@test "No InternalError on different thread exception" {
+  run_framework_test "junit" "JunitTests.offThreadExceptionTest"
+  assert_failure
+  # The test should fail with a RuntimeException, but not an InternalError
+  assert_output --partial "java.lang.RuntimeException"
+  refute_output --partial "java.lang.InternalError"
 }

agent/test/test-frameworks/src/test/java/org/springframework/samples/petclinic/JunitTests.java

@@ -1,7 +1,10 @@
 package org.springframework.samples.petclinic;
 
+import static java.util.concurrent.Executors.newSingleThreadExecutor;
 import static org.junit.Assert.assertTrue;
 
+import java.util.concurrent.Future;
+import java.util.concurrent.ExecutorService;
 import com.appland.appmap.annotation.NoAppMap;
 import org.junit.Test;
 
@@ -38,4 +41,31 @@ public void testAnnotatedClassNotRecorded() {
     }
   }
 
+  private static class ExecutorRunner {
+    public Throwable run() {
+      ExecutorService executor = newSingleThreadExecutor();
+      Future<?> future = executor.submit(() -> {
+        throw new RuntimeException("Off-thread exception for testing");
+      });
+      try {
+        future.get();
+      } catch (java.util.concurrent.ExecutionException e) {
+        return e.getCause();
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+      } finally {
+        executor.shutdown();
+      }
+      return null;
+    }
+  }
+
+  @Test
+  public void offThreadExceptionTest() throws Throwable {
+    Throwable throwable = new ExecutorRunner().run();
+    if (throwable == null) {
+      throw new AssertionError("Expected exception from off-thread execution");
+    }
+    throw throwable;
+  }
 }