Remove AOP boilerplate from user visible stack traces.
We received complaints because AOP stack traces are difficult to follow. This removes almost everything. The weird thing about this change is that the stack traces contain gaps. Anyone tracing through a stack trace should recognize that proceed() or invoke() means that AOP is in play.
The stack trace of a method that was subject to two interceptors used to look like this:
java.lang.Exception: kaboom!
at com.publicobject.Interceptable.explode(Interceptable.java:203)
at com.publicobject.Interceptable$$EnhancerByGuice$$5ffccef9.CGLIB$explode$2(<generated>)
at com.publicobject.Interceptable$$EnhancerByGuice$$5ffccef9$$FastClassByGuice$$20c8faff.invoke(<generated>)
at net.sf.cglib.proxy.MethodProxy.invokeSuper(MethodProxy.java:228)
at com.google.inject.internal.InterceptorStackCallback$InterceptedMethodInvocation.proceed(InterceptorStackCallback.java:72)
at com.publicobject.InterceptorB.invoke(InterceptorB.java:57)
at com.google.inject.internal.InterceptorStackCallback$InterceptedMethodInvocation.proceed(InterceptorStackCallback.java:72)
at com.publicobject.InterceptorA.invoke(InterceptorA.java:50)
at com.google.inject.internal.InterceptorStackCallback$InterceptedMethodInvocation.proceed(InterceptorStackCallback.java:72)
at com.google.inject.internal.InterceptorStackCallback.intercept(InterceptorStackCallback.java:52)
at com.publicobject.MyTest$Interceptable$$EnhancerByGuice$$5ffccef9.explode(<generated>)
at com.publicobject.MyTest.testInterceptedMethodThrows(MyTest.java:181)
Now it looks like this:
java.lang.Exception: kaboom!
at com.publicobject.Interceptable.explode(Interceptable.java:203)
at com.publicobject.InterceptorB.invoke(InterceptorB.java:57)
at com.publicobject.InterceptorA.invoke(InterceptorA.java:50)
at com.publicobject.MyTest.testInterceptedMethodThrows(MyTest.java:181)
git-svn-id: https://google-guice.googlecode.com/svn/trunk@1481 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/core/src/com/google/inject/internal/InterceptorStackCallback.java b/core/src/com/google/inject/internal/InterceptorStackCallback.java
index c9a7633..387340c 100644
--- a/core/src/com/google/inject/internal/InterceptorStackCallback.java
+++ b/core/src/com/google/inject/internal/InterceptorStackCallback.java
@@ -16,9 +16,13 @@
package com.google.inject.internal;
+import com.google.inject.internal.util.Lists;
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import net.sf.cglib.proxy.MethodProxy;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
@@ -29,6 +33,10 @@
* @author crazybob@google.com (Bob Lee)
*/
final class InterceptorStackCallback implements net.sf.cglib.proxy.MethodInterceptor {
+ private static final Set<String> AOP_INTERNAL_CLASSES = new HashSet<String>(Arrays.asList(
+ InterceptorStackCallback.class.getName(),
+ InterceptedMethodInvocation.class.getName(),
+ MethodProxy.class.getName()));
final MethodInterceptor[] interceptors;
final Method method;
@@ -64,8 +72,10 @@
return index == interceptors.length
? methodProxy.invokeSuper(proxy, arguments)
: interceptors[index].invoke(this);
- }
- finally {
+ } catch (Throwable t) {
+ pruneStacktrace(t);
+ throw t;
+ } finally {
index--;
}
}
@@ -86,4 +96,20 @@
return getMethod();
}
}
+
+ /**
+ * Removes stacktrace elements related to AOP internal mechanics from the
+ * throwable's stack trace.
+ */
+ private void pruneStacktrace(Throwable throwable) {
+ StackTraceElement[] stackTrace = throwable.getStackTrace();
+ List<StackTraceElement> pruned = Lists.newArrayList();
+ for (StackTraceElement element : stackTrace) {
+ String className = element.getClassName();
+ if (!AOP_INTERNAL_CLASSES.contains(className) && !className.contains("$EnhancerByGuice$")) {
+ pruned.add(element);
+ }
+ }
+ throwable.setStackTrace(pruned.toArray(new StackTraceElement[pruned.size()]));
+ }
}
diff --git a/core/test/com/google/inject/MethodInterceptionTest.java b/core/test/com/google/inject/MethodInterceptionTest.java
index 70456bd..27c5db4 100644
--- a/core/test/com/google/inject/MethodInterceptionTest.java
+++ b/core/test/com/google/inject/MethodInterceptionTest.java
@@ -37,24 +37,24 @@
private AtomicInteger count = new AtomicInteger();
- private final MethodInterceptor countingInterceptor = new MethodInterceptor() {
+ private final class CountingInterceptor implements MethodInterceptor {
public Object invoke(MethodInvocation methodInvocation) throws Throwable {
count.incrementAndGet();
return methodInvocation.proceed();
}
- };
+ }
- private final MethodInterceptor returnNullInterceptor = new MethodInterceptor() {
+ private final class ReturnNullInterceptor implements MethodInterceptor {
public Object invoke(MethodInvocation methodInvocation) throws Throwable {
return null;
}
- };
+ }
public void testSharedProxyClasses() {
Injector injector = Guice.createInjector(new AbstractModule() {
protected void configure() {
bindInterceptor(Matchers.any(), Matchers.returns(only(Foo.class)),
- returnNullInterceptor);
+ new ReturnNullInterceptor());
}
});
@@ -72,18 +72,18 @@
protected void configure() {
bind(Interceptable.class);
bindInterceptor(Matchers.any(), Matchers.returns(only(Bar.class)),
- returnNullInterceptor);
+ new ReturnNullInterceptor());
}
});
Interceptable bothNull = nullFoosAndBarsInjector.getInstance(Interceptable.class);
assertNull(bothNull.bar());
assertNull(bothNull.foo());
-
+
assertSame("Child injectors should share proxy classes, otherwise memory leaks!",
nullFoos.getClass(), bothNull.getClass());
}
-
+
public void testGetThis() {
final AtomicReference<Object> lastTarget = new AtomicReference<Object>();
@@ -125,9 +125,11 @@
}
public void testSpiAccessToInterceptors() throws NoSuchMethodException {
+ final MethodInterceptor countingInterceptor = new CountingInterceptor();
+ final MethodInterceptor returnNullInterceptor = new ReturnNullInterceptor();
Injector injector = Guice.createInjector(new AbstractModule() {
protected void configure() {
- bindInterceptor(Matchers.any(), Matchers.returns(only(Foo.class)),
+ bindInterceptor(Matchers.any(),Matchers.returns(only(Foo.class)),
countingInterceptor);
bindInterceptor(Matchers.any(), Matchers.returns(only(Foo.class).or(only(Bar.class))),
returnNullInterceptor);
@@ -152,6 +154,27 @@
assertEquals("expected counting interceptor to be invoked first", 1, count.get());
}
+ public void testInterceptedMethodThrows() throws Exception {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ bindInterceptor(Matchers.any(), Matchers.any(), new CountingInterceptor());
+ bindInterceptor(Matchers.any(), Matchers.any(), new CountingInterceptor());
+ }
+ });
+
+ Interceptable interceptable = injector.getInstance(Interceptable.class);
+ try {
+ interceptable.explode();
+ fail();
+ } catch (Exception e) {
+ StackTraceElement[] stackTraceElement = e.getStackTrace();
+ assertEquals("explode", stackTraceElement[0].getMethodName());
+ assertEquals("invoke", stackTraceElement[1].getMethodName());
+ assertEquals("invoke", stackTraceElement[2].getMethodName());
+ assertEquals("testInterceptedMethodThrows", stackTraceElement[3].getMethodName());
+ }
+ }
+
static class Foo {}
static class Bar {}
@@ -162,7 +185,10 @@
public Bar bar() {
return new Bar() {};
}
+ public String explode() throws Exception {
+ throw new Exception("kaboom!");
+ }
}
-
+
public static final class NotInterceptable {}
}
