Skip to content

Commit

Permalink
Fixes #2489 : Fixed issue related to exceptions thrown from the neste…
Browse files Browse the repository at this point in the history 
…d spies (#2546)

Behaviour of the filtering redundant method calls from stacktrace was changed. Now it removes only lines that have the same method name and line number

Co-authored-by: Andrey Kozel <andrey.kozel@kaseya.com>
  • Loading branch information
@andrey-kozel
andrey-kozel and Andrey Kozel committed Jan 22, 2022
1 parent faa6e92 commit 8cdf0cc
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 55 deletions.
61 changes: 26 additions & 35 deletions src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
View file
Expand Up @@ -19,6 +19,9 @@
import java.lang.reflect.Method;
import java.util.List;
import java.util.Map;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.concurrent.Callable;
import java.util.function.Predicate;

Expand Down Expand Up @@ -114,27 +117,6 @@ private static void exit(
}
}

static Throwable hideRecursiveCall(Throwable throwable, int current, Class<?> targetType) {
try {
StackTraceElement[] stack = throwable.getStackTrace();
int skip = 0;
StackTraceElement next;
do {
next = stack[stack.length - current - ++skip];
} while (!next.getClassName().equals(targetType.getName()));
int top = stack.length - current - skip;
StackTraceElement[] cleared = new StackTraceElement[stack.length - skip];
System.arraycopy(stack, 0, cleared, 0, top);
System.arraycopy(stack, top + skip, cleared, top, current);
throwable.setStackTrace(cleared);
return throwable;
} catch (RuntimeException ignored) {
// This should not happen unless someone instrumented or manipulated exception stack
// traces.
return throwable;
}
}

@Override
public Callable<?> handle(Object instance, Method origin, Object[] arguments) throws Throwable {
MockMethodInterceptor interceptor = interceptors.get(instance);
Expand Down Expand Up @@ -333,25 +315,34 @@ private static Object tryInvoke(Method origin, Object instance, Object[] argumen
return accessor.invoke(origin, instance, arguments);
} catch (InvocationTargetException exception) {
Throwable cause = exception.getCause();
StackTraceElement[] tmpStack = new Throwable().getStackTrace();

int skip = tmpStack.length;
// if there is a suitable instance, do not skip the root-cause for the exception
if (instance != null) {
skip = 0;
String causingClassName = instance.getClass().getName();
StackTraceElement stackFrame;
do {
stackFrame = tmpStack[skip++];
} while (stackFrame.getClassName().startsWith(causingClassName));
}

new ConditionalStackTraceFilter()
.filter(hideRecursiveCall(cause, skip, origin.getDeclaringClass()));
.filter(removeRecursiveCalls(cause, origin.getDeclaringClass()));
throw cause;
}
}

static Throwable removeRecursiveCalls(final Throwable cause, final Class<?> declaringClass) {
final List<String> uniqueStackTraceItems = new ArrayList<>();
final List<Integer> indexesToBeRemoved = new ArrayList<>();
for (StackTraceElement element : cause.getStackTrace()) {
final String key = element.getClassName() + element.getLineNumber();
final int elementIndex = uniqueStackTraceItems.lastIndexOf(key);
uniqueStackTraceItems.add(key);

if (elementIndex > -1 && declaringClass.getName().equals(element.getClassName())) {
indexesToBeRemoved.add(elementIndex);
}
}
final List<StackTraceElement> adjustedList =
new ArrayList<>(Arrays.asList(cause.getStackTrace()));
indexesToBeRemoved.stream()
.sorted(Comparator.reverseOrder())
.mapToInt(Integer::intValue)
.forEach(adjustedList::remove);
cause.setStackTrace(adjustedList.toArray(new StackTraceElement[] {}));
return cause;
}

private static class ReturnValueWrapper implements Callable<Object> {

private final Object returned;
Expand Down
83 changes: 63 additions & 20 deletions ...st/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java
View file
Expand Up @@ -9,19 +9,14 @@
import static net.bytebuddy.matcher.ElementMatchers.named;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.*;
import static org.junit.Assume.assumeTrue;

import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Observable;
import java.util.Observer;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.concurrent.Callable;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import net.bytebuddy.ByteBuddy;
import net.bytebuddy.ClassFileVersion;
Expand Down Expand Up @@ -253,48 +248,90 @@ public void should_leave_causing_stack() throws Exception {
mockMaker.createSpy(
settings, new MockHandlerImpl<>(settings), new ExceptionThrowingClass());

StackTraceElement[] returnedStack = null;
try {
proxy.get().throwException();
} catch (IOException ex) {
returnedStack = ex.getStackTrace();
}
StackTraceElement[] returnedStack =
assertThrows(IOException.class, () -> proxy.get().throwException()).getStackTrace();

assertNotNull("Stack trace from mockito expected", returnedStack);

assertEquals(ExceptionThrowingClass.class.getName(), returnedStack[0].getClassName());
assertEquals("internalThrowException", returnedStack[0].getMethodName());
List<StackTraceElement> exceptionClassElements =
Arrays.stream(returnedStack)
.filter(
element ->
element.getClassName()
.equals(ExceptionThrowingClass.class.getName()))
.collect(Collectors.toList());
assertEquals(3, exceptionClassElements.size());
assertEquals("internalThrowException", exceptionClassElements.get(0).getMethodName());
assertEquals("internalThrowException", exceptionClassElements.get(1).getMethodName());
assertEquals("throwException", exceptionClassElements.get(2).getMethodName());
}

@Test
public void should_leave_causing_stack_with_two_spies() throws Exception {
// given
MockSettingsImpl<ExceptionThrowingClass> settingsEx = new MockSettingsImpl<>();
settingsEx.setTypeToMock(ExceptionThrowingClass.class);
settingsEx.defaultAnswer(Answers.CALLS_REAL_METHODS);
Optional<ExceptionThrowingClass> proxyEx =
mockMaker.createSpy(
settingsEx,
new MockHandlerImpl<>(settingsEx),
new ExceptionThrowingClass());

MockSettingsImpl<WrapperClass> settingsWr = new MockSettingsImpl<>();
settingsWr.setTypeToMock(WrapperClass.class);
settingsWr.defaultAnswer(Answers.CALLS_REAL_METHODS);
Optional<WrapperClass> proxyWr =
mockMaker.createSpy(
settingsWr, new MockHandlerImpl<>(settingsWr), new WrapperClass());

// when
IOException ex =
assertThrows(IOException.class, () -> proxyWr.get().callWrapped(proxyEx.get()));
List<StackTraceElement> wrapperClassElements =
Arrays.stream(ex.getStackTrace())
.filter(
element ->
element.getClassName().equals(WrapperClass.class.getName()))
.collect(Collectors.toList());

// then
assertEquals(1, wrapperClassElements.size());
assertEquals("callWrapped", wrapperClassElements.get(0).getMethodName());
}

@Test
public void should_remove_recursive_self_call_from_stack_trace() throws Exception {
StackTraceElement[] stack =
new StackTraceElement[] {
new StackTraceElement("foo", "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
new StackTraceElement("qux", "", "", -1),
new StackTraceElement("bar", "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
new StackTraceElement("baz", "", "", -1)
};

Throwable throwable = new Throwable();
throwable.setStackTrace(stack);
throwable = MockMethodAdvice.hideRecursiveCall(throwable, 2, SampleInterface.class);
throwable = MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class);

assertThat(throwable.getStackTrace())
.isEqualTo(
new StackTraceElement[] {
new StackTraceElement("foo", "", "", -1),
new StackTraceElement("qux", "", "", -1),
new StackTraceElement("bar", "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
new StackTraceElement("baz", "", "", -1)
});
}

@Test
public void should_handle_missing_or_inconsistent_stack_trace() throws Exception {
public void should_handle_missing_or_inconsistent_stack_trace() {
Throwable throwable = new Throwable();
throwable.setStackTrace(new StackTraceElement[0]);
assertThat(MockMethodAdvice.hideRecursiveCall(throwable, 0, SampleInterface.class))
assertThat(MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class))
.isSameAs(throwable);
}

Expand Down Expand Up @@ -579,6 +616,12 @@ public T value() {
}
}

public static class WrapperClass {
public void callWrapped(ExceptionThrowingClass exceptionThrowingClass) throws IOException {
exceptionThrowingClass.throwException();
}
}

public static class GenericSubClass extends GenericClass<String> {}

public static class ExceptionThrowingClass {
Expand Down

0 comments on commit 8cdf0cc

Please sign in to comment.