Prune all the billions of c.g.i.servlet.Filter{ChainInvocation,Definition}.doFilter elements from the stack traces.

Revision created by MOE tool push_codebase.
MOE_MIGRATION=3587
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
index 262d263..36599a6 100755
--- a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
@@ -15,7 +15,14 @@
  */
 package com.google.inject.servlet;
 
+import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+
 import java.io.IOException;
+import java.util.List;
+import java.util.Set;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -37,12 +44,19 @@
  * @since 1.0
  */
 class FilterChainInvocation implements FilterChain {
+  
+  private static final Set<String> SERVLET_INTERNAL_METHODS = ImmutableSet.of(
+      FilterDefinition.class.getName() + ".doFilter",
+      FilterChainInvocation.class.getName() + ".doFilter");
+  
   private final FilterDefinition[] filterDefinitions;
   private final FilterChain proceedingChain;
   private final ManagedServletPipeline servletPipeline;
 
   //state variable tracks current link in filterchain
   private int index = -1;
+  // whether or not we've caught an exception & cleaned up stack traces
+  private boolean cleanedStacks = false;
 
   public FilterChainInvocation(FilterDefinition[] filterDefinitions,
       ManagedServletPipeline servletPipeline, FilterChain proceedingChain) {
@@ -75,8 +89,37 @@
           proceedingChain.doFilter(servletRequest, servletResponse);
         }
       }
+    } catch (Throwable t) {
+      // Only clean on the first pass through -- one exception deep in a filter
+      // will propogate upward & hit this catch clause multiple times.  We don't
+      // want to iterate through the stack elements for every filter.
+      if (!cleanedStacks) {
+        cleanedStacks = true;
+        pruneStacktrace(t);
+      }
+      Throwables.propagateIfInstanceOf(t, ServletException.class);
+      Throwables.propagateIfInstanceOf(t, IOException.class);
+      throw Throwables.propagate(t);
     } finally {
       GuiceFilter.localContext.set(previous);
     }
   }
+  
+  /**
+   * Removes stacktrace elements related to AOP internal mechanics from the
+   * throwable's stack trace and any causes it may have.
+   */
+  private void pruneStacktrace(Throwable throwable) {
+    for (Throwable t = throwable; t != null; t = t.getCause()) {
+      StackTraceElement[] stackTrace = t.getStackTrace();
+      List<StackTraceElement> pruned = Lists.newArrayList();
+      for (StackTraceElement element : stackTrace) {
+        String name = element.getClassName() + "." + element.getMethodName();
+        if (!SERVLET_INTERNAL_METHODS.contains(name)) {
+          pruned.add(element);
+        }
+      }
+      t.setStackTrace(pruned.toArray(new StackTraceElement[pruned.size()]));
+    }
+  }
 }
diff --git a/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java b/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
index 81354cc..6feb82e 100644
--- a/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
@@ -1,6 +1,7 @@
 package com.google.inject.servlet;
 
 import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST;
+import static com.google.inject.servlet.ServletTestUtils.newFakeHttpServletRequest;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.expectLastCall;
 
@@ -21,6 +22,8 @@
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
+import javax.servlet.Servlet;
+import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
@@ -234,4 +237,87 @@
       service((HttpServletRequest) servletRequest, (HttpServletResponse) servletResponse);
     }
   }
+  
+  public final void testFilterExceptionPrunesStack() throws Exception {
+    Injector injector = Guice.createInjector(new ServletModule() {
+      @Override
+      protected void configureServlets() {
+        filter("/").through(TestFilter.class);
+        filter("/nothing").through(TestFilter.class);
+        filter("/").through(ThrowingFilter.class);
+      }
+    });
+
+    HttpServletRequest request = newFakeHttpServletRequest();    
+    FilterPipeline pipeline = injector.getInstance(FilterPipeline.class);
+    pipeline.initPipeline(null);    
+    try {
+      pipeline.dispatch(request, null, null);
+      fail("expected exception");
+    } catch(ServletException ex) {
+      for (StackTraceElement element : ex.getStackTrace()) {
+        String className = element.getClassName();
+        assertTrue("was: " + element,
+            !className.equals(FilterChainInvocation.class.getName())
+            && !className.equals(FilterDefinition.class.getName()));
+      }
+    }
+  }
+  
+  public final void testServletExceptionPrunesStack() throws Exception {
+    Injector injector = Guice.createInjector(new ServletModule() {
+      @Override
+      protected void configureServlets() {
+        filter("/").through(TestFilter.class);
+        filter("/nothing").through(TestFilter.class);
+        serve("/").with(ThrowingServlet.class);
+      }
+    });
+
+    HttpServletRequest request = newFakeHttpServletRequest();    
+    FilterPipeline pipeline = injector.getInstance(FilterPipeline.class);
+    pipeline.initPipeline(null);    
+    try {
+      pipeline.dispatch(request, null, null);
+      fail("expected exception");
+    } catch(ServletException ex) {
+      for (StackTraceElement element : ex.getStackTrace()) {
+        String className = element.getClassName();
+        assertTrue("was: " + element,
+            !className.equals(FilterChainInvocation.class.getName())
+            && !className.equals(FilterDefinition.class.getName()));
+      }
+    }
+  }
+  
+  @Singleton
+  private static class ThrowingServlet extends HttpServlet {
+
+    @Override
+    protected void service(HttpServletRequest req, HttpServletResponse resp)
+        throws ServletException, IOException {
+      throw new ServletException("failure!");
+    }
+    
+  }
+
+
+  @Singleton
+  private static class ThrowingFilter implements Filter {
+    @Override
+    public void destroy() {
+    }
+    
+    @Override
+    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+        throws ServletException {
+      throw new ServletException("we failed!");
+    }
+    
+    @Override
+    public void init(FilterConfig filterConfig) {
+
+    }
+    
+  }
 }
diff --git a/extensions/servlet/test/com/google/inject/servlet/ServletTest.java b/extensions/servlet/test/com/google/inject/servlet/ServletTest.java
index 06197f8..ae1750b 100644
--- a/extensions/servlet/test/com/google/inject/servlet/ServletTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/ServletTest.java
@@ -17,7 +17,8 @@
 package com.google.inject.servlet;
 
 import static com.google.inject.Asserts.reserialize;
-import static com.google.inject.servlet.ServletScopes.NullObject;
+import static com.google.inject.servlet.ServletTestUtils.newFakeHttpServletRequest;
+import static com.google.inject.servlet.ServletTestUtils.newFakeHttpServletResponse;
 import static java.lang.annotation.ElementType.FIELD;
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.ElementType.PARAMETER;
@@ -25,7 +26,6 @@
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
 import com.google.inject.AbstractModule;
 import com.google.inject.BindingAnnotation;
 import com.google.inject.CreationException;
@@ -35,6 +35,7 @@
 import com.google.inject.Key;
 import com.google.inject.Module;
 import com.google.inject.Provider;
+import com.google.inject.servlet.ServletScopes.NullObject;
 import com.google.inject.util.Providers;
 
 import junit.framework.TestCase;
@@ -43,9 +44,6 @@
 import java.io.Serializable;
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
-import java.lang.reflect.InvocationHandler;
-import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
 import java.util.Map;
 
 import javax.servlet.Filter;
@@ -412,88 +410,6 @@
     }
   }
 
-  private static class ThrowingInvocationHandler implements InvocationHandler {
-    @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-      throw new UnsupportedOperationException("No methods are supported on this object");
-    }
-  }
-  
-  /**
-   * Returns a fake, HttpServletRequest which stores attributes in a HashMap.
-   */
-  private HttpServletRequest newFakeHttpServletRequest() {
-    HttpServletRequest delegate = (HttpServletRequest) Proxy.newProxyInstance(
-        HttpServletRequest.class.getClassLoader(),
-        new Class[] { HttpServletRequest.class }, new ThrowingInvocationHandler());
-    
-    return new HttpServletRequestWrapper(delegate) {
-      final Map<String, Object> attributes = Maps.newHashMap(); 
-      final HttpSession session = newFakeHttpSession();
-
-      @Override public String getMethod() {
-        return "GET";
-      }
-
-      @Override public Object getAttribute(String name) {
-        return attributes.get(name);
-      }
-      
-      @Override public void setAttribute(String name, Object value) {
-        attributes.put(name, value);
-      }
-      
-      @Override public Map getParameterMap() {
-        return ImmutableMap.of();
-      }
-      
-      @Override public String getRequestURI() {
-        return "/";
-      }
-      
-      @Override public String getContextPath() {
-        return "";
-      }
-      
-      @Override public HttpSession getSession() {
-        return session;
-      }
-    };
-  }
-  
-  /**
-   * Returns a fake, HttpServletResponse which throws an exception if any of its
-   * methods are called.
-   */
-  private HttpServletResponse newFakeHttpServletResponse() {
-    return (HttpServletResponse) Proxy.newProxyInstance(
-        HttpServletResponse.class.getClassLoader(),
-        new Class[] { HttpServletResponse.class }, new ThrowingInvocationHandler());
-  }  
-  
-  private static class FakeHttpSessionHandler implements InvocationHandler, Serializable {
-    final Map<String, Object> attributes = Maps.newHashMap();
-
-    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-      String name = method.getName();
-      if ("setAttribute".equals(name)) {
-        attributes.put((String) args[0], args[1]);
-        return null;
-      } else if ("getAttribute".equals(name)) {
-        return attributes.get(args[0]);
-      } else {
-        throw new UnsupportedOperationException();
-      }
-    }
-  }
-
-  /**
-   * Returns a fake, serializable HttpSession which stores attributes in a HashMap.
-   */
-  private HttpSession newFakeHttpSession() {
-    return (HttpSession) Proxy.newProxyInstance(HttpSession.class.getClassLoader(),
-        new Class[] { HttpSession.class }, new FakeHttpSessionHandler());
-  }
-
   private Injector createInjector(Module... modules) throws CreationException {
     return Guice.createInjector(Lists.<Module>asList(new AbstractModule() {
       @Override
diff --git a/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java b/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
new file mode 100644
index 0000000..8657d38
--- /dev/null
+++ b/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
@@ -0,0 +1,110 @@
+// Copyright 2011 Google Inc. All Rights Reserved.
+
+package com.google.inject.servlet;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+
+import java.io.Serializable;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+
+/**
+ * Utilities for servlet tests.
+ * 
+ * @author sameb@google.com (Sam Berlin)
+ */
+public class ServletTestUtils {
+  
+  private ServletTestUtils() {}
+
+  private static class ThrowingInvocationHandler implements InvocationHandler {
+    @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+      throw new UnsupportedOperationException("No methods are supported on this object");
+    }
+  }
+  
+  /**
+   * Returns a fake, HttpServletRequest which stores attributes in a HashMap.
+   */
+  public static HttpServletRequest newFakeHttpServletRequest() {
+    HttpServletRequest delegate = (HttpServletRequest) Proxy.newProxyInstance(
+        HttpServletRequest.class.getClassLoader(),
+        new Class[] { HttpServletRequest.class }, new ThrowingInvocationHandler());
+    
+    return new HttpServletRequestWrapper(delegate) {
+      final Map<String, Object> attributes = Maps.newHashMap(); 
+      final HttpSession session = newFakeHttpSession();
+
+      @Override public String getMethod() {
+        return "GET";
+      }
+
+      @Override public Object getAttribute(String name) {
+        return attributes.get(name);
+      }
+      
+      @Override public void setAttribute(String name, Object value) {
+        attributes.put(name, value);
+      }
+      
+      @Override public Map getParameterMap() {
+        return ImmutableMap.of();
+      }
+      
+      @Override public String getRequestURI() {
+        return "/";
+      }
+      
+      @Override public String getContextPath() {
+        return "";
+      }
+      
+      @Override public HttpSession getSession() {
+        return session;
+      }
+    };
+  }
+  
+  /**
+   * Returns a fake, HttpServletResponse which throws an exception if any of its
+   * methods are called.
+   */
+  public static HttpServletResponse newFakeHttpServletResponse() {
+    return (HttpServletResponse) Proxy.newProxyInstance(
+        HttpServletResponse.class.getClassLoader(),
+        new Class[] { HttpServletResponse.class }, new ThrowingInvocationHandler());
+  }  
+  
+  private static class FakeHttpSessionHandler implements InvocationHandler, Serializable {
+    final Map<String, Object> attributes = Maps.newHashMap();
+
+    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+      String name = method.getName();
+      if ("setAttribute".equals(name)) {
+        attributes.put((String) args[0], args[1]);
+        return null;
+      } else if ("getAttribute".equals(name)) {
+        return attributes.get(args[0]);
+      } else {
+        throw new UnsupportedOperationException();
+      }
+    }
+  }
+
+  /**
+   * Returns a fake, serializable HttpSession which stores attributes in a HashMap.
+   */
+  public static HttpSession newFakeHttpSession() {
+    return (HttpSession) Proxy.newProxyInstance(HttpSession.class.getClassLoader(),
+        new Class[] { HttpSession.class }, new FakeHttpSessionHandler());
+  }
+
+}