Skip to content

Commit 91cca89

Browse files
committed
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 exception's 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 last seen stack frame that does not belong to a known testing or Java framework. This prevents the crash and correctly identifies the failure location. A regression test has been added to simulate this cross-thread exception scenario and verify the fix.
1 parent 83d3958 commit 91cca89

File tree

3 files changed

+75
-7
lines changed

3 files changed

+75
-7
lines changed

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

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,54 @@ private static void startRecording(TestDetails details, Recorder.Metadata metada
6363
RecordingSupport.startRecording(details, metadata);
6464
}
6565

66+
private static boolean isFrameworkClass(String className) {
67+
return className.startsWith("org.junit") ||
68+
className.startsWith("org.testng") ||
69+
className.startsWith("com.intellij") ||
70+
className.startsWith("java.") ||
71+
className.startsWith("jdk");
72+
}
73+
74+
/**
75+
* Finds the most relevant stack frame for a test failure.
76+
* <p>
77+
* The primary goal is to find the stack frame that corresponds to the test
78+
* class itself. However, in some scenarios (e.g., when an assertion fails on
79+
* a different thread), the test class may not be in the stack trace at all.
80+
* <p>
81+
* To handle this, we use a fallback heuristic: we find the last stack frame
82+
* that is not a known framework class. This is usually the entry point into
83+
* the user's code and the most likely source of the failure.
84+
*
85+
* @param self The test class instance
86+
* @param exception The exception that caused the failure
87+
* @return The most relevant stack frame
88+
* @throws InternalError if no suitable stack frame can be found
89+
*/
6690
static StackTraceElement findErrorFrame(Object self, Throwable exception) throws InternalError {
6791
String selfClass = self.getClass().getName();
68-
StackTraceElement errorFrame = null;
92+
StackTraceElement fallbackFrame = null;
93+
6994
for (StackTraceElement frame : exception.getStackTrace()) {
7095
if (frame.getClassName().equals(selfClass)) {
71-
errorFrame = frame;
72-
break;
96+
// This is the ideal case: we found the test class in the stack trace.
97+
return frame;
98+
}
99+
if (!isFrameworkClass(frame.getClassName())) {
100+
// This is a potential candidate for the fallback frame. We'll keep
101+
// track of the last one we see, as it's the most likely to be the
102+
// entry point into the user's code.
103+
fallbackFrame = frame;
73104
}
74105
}
75-
if (errorFrame == null) {
76-
throw new InternalError("no stack frame matched test class");
106+
107+
if (fallbackFrame != null) {
108+
// We didn't find the test class, but we have a good fallback.
109+
return fallbackFrame;
77110
}
78-
return errorFrame;
111+
112+
// We couldn't find any suitable frame.
113+
throw new InternalError("no stack frame matched test class");
79114
}
80115

81116
private static boolean hasTestAnnotation(ClassLoader cl, Method stackMethod) {

agent/test/test-frameworks/frameworks.bats

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ run_framework_test() {
5252

5353
assert_json_eq '.metadata.test_status' "failed"
5454
assert_json_contains '.metadata.test_failure.message' 'false is not true'
55-
assert_json_eq '.metadata.test_failure.location' "src/test/java/org/springframework/samples/petclinic/JunitTests.java:20"
55+
assert_json_eq '.metadata.test_failure.location' "src/test/java/org/springframework/samples/petclinic/JunitTests.java:21"
5656
}
5757

5858
@test "test status set for failed test in testng" {
@@ -104,4 +104,12 @@ run_framework_test() {
104104

105105
output="$(< tmp/appmap/testng/org_springframework_samples_petclinic_TestngTests_testItThrows.appmap.json)"
106106
assert_json_eq '.metadata.test_status' "succeeded"
107+
}
108+
109+
@test "No InternalError on different thread exception" {
110+
run_framework_test "junit" "JunitTests.offThreadExceptionTest"
111+
assert_failure
112+
# The test should fail with a RuntimeException, but not an InternalError
113+
assert_output --partial "java.lang.RuntimeException"
114+
refute_output --partial "java.lang.InternalError"
107115
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.springframework.samples.petclinic;
22

3+
import static java.util.concurrent.Executors.newSingleThreadExecutor;
34
import static org.junit.Assert.assertTrue;
45

56
import com.appland.appmap.annotation.NoAppMap;
@@ -38,4 +39,28 @@ public void testAnnotatedClassNotRecorded() {
3839
}
3940
}
4041

42+
private class ExecutorRunner {
43+
public Throwable run() {
44+
var executor = newSingleThreadExecutor();
45+
var future = executor.submit(() -> {
46+
throw new RuntimeException("Off-thread exception for testing");
47+
});
48+
try {
49+
future.get();
50+
} catch (java.util.concurrent.ExecutionException e) {
51+
return e.getCause();
52+
} catch (InterruptedException e) {
53+
Thread.currentThread().interrupt();
54+
} finally {
55+
executor.shutdown();
56+
}
57+
return null;
58+
}
59+
}
60+
61+
@Test
62+
public void offThreadExceptionTest() throws Throwable {
63+
var throwable = new ExecutorRunner().run();
64+
throw throwable;
65+
}
4166
}

0 commit comments

Comments
 (0)