Remove the virtual method called from constructor & simulate a builder.

1. The virtual method to stop fillInStackTrace() from being called by the
constructor has been removed.
2. The public no argument constructor for MultiException puts the class
into a builder mode where it just lets you add exceptions.
MultiException created from the constructor should not be thrown because
fillInStackTrace() is not called.
3. In the `ifExceptionThrow*()` methods a new `MultiException` is
created, with the stack trace filled, before it is thrown.

Signed-off-by: Luke Butters <lbutters@funnelback.com>

jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java

@@ -26,49 +26,48 @@ import java.util.List;
  * Wraps multiple exceptions.
  *
  * Allows multiple exceptions to be thrown as a single exception.
+ * 
+ * The MultiException itself should not be thrown instead one of the
+ * ifExceptionThrow* methods should be called instead.
  */
 @SuppressWarnings("serial")
 public class MultiException extends Exception
 {
+    private static final String DEFAULT_MESSAGE = "Multiple exceptions";
+    
     private List<Throwable> nested;
-    boolean superConstructorCalled = false;
 
     /* ------------------------------------------------------------ */
     public MultiException()
     {
-        super("Multiple exceptions");
-        this.superConstructorCalled = true;
+        // Avoid filling in stack trace information.
+        super(DEFAULT_MESSAGE, null, false, false);
+        this.nested = new ArrayList<>();
     }
     
     /**
-     * We only want to fill in the stack trace if we have an exception to throw.
+     * Create a MultiException which may be thrown.
      * 
-     * This prevents the stack trace from being written when the constructor is
-     * called, yet will let as fill it in later on e.g. when we actually want
-     * to throw the multi exception.
+     * @param nested The nested exceptions which will be suppressed by this
+     * exception.
      */
-    @Override
-    public Throwable fillInStackTrace() {
-        if(superConstructorCalled) {
-            return super.fillInStackTrace();
+    private MultiException(List<Throwable> nested) {
+        super(DEFAULT_MESSAGE);
+        this.nested = new ArrayList<>(nested);
+        
+        if(nested.size() > 0) {
+            initCause(nested.get(0));
+        }
+        
+        for(Throwable t : nested) {
+            this.addSuppressed(t);
         }
-        return this;
     }
+    
 
     /* ------------------------------------------------------------ */
     public void add(Throwable e)
     {
-        if (e==null)
-            throw new IllegalArgumentException();
-
-        if(nested == null)
-        {
-            initCause(e);
-            nested = new ArrayList<>();
-        }
-        else
-            addSuppressed(e);
-        
         if (e instanceof MultiException)
         {
             MultiException me = (MultiException)e;
@@ -122,8 +121,7 @@ public class MultiException extends Exception
               if (th instanceof Exception)
                   throw (Exception)th;
           default:
-              this.fillInStackTrace();
-              throw this;
+              throw new MultiException(nested);
         }
     }
     
@@ -155,8 +153,7 @@ public class MultiException extends Exception
               else
                   throw new RuntimeException(th);
           default:
-              this.fillInStackTrace();
-              throw new RuntimeException(this);
+              throw new RuntimeException(new MultiException(nested));
         }
     }
     
@@ -175,8 +172,7 @@ public class MultiException extends Exception
         
         if (nested.size()>0)
         {
-            this.fillInStackTrace();
-            throw this;
+            throw new MultiException(nested);
         }
     }
 

jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java

@@ -21,6 +21,7 @@ package org.eclipse.jetty.util;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import org.junit.Assert;
 import org.junit.Test;
 
 import java.io.IOException;
@@ -67,7 +68,7 @@ public class MultiExceptionTest
         }
         catch(MultiException e)
         {
-            assertTrue(e==me);
+            assertTrue(e instanceof MultiException);
         }
 
         try
@@ -120,7 +121,7 @@ public class MultiExceptionTest
         }
         catch(MultiException e)
         {
-            assertTrue(e==me);
+            assertTrue(e instanceof MultiException);
             assertTrue(e.getStackTrace().length > 0);
         }
 
@@ -132,7 +133,7 @@ public class MultiExceptionTest
         }
         catch(MultiException e)
         {
-            assertTrue(e==me);
+            assertTrue(e instanceof MultiException);
             assertTrue(e.getStackTrace().length > 0);
         }
 
@@ -144,7 +145,7 @@ public class MultiExceptionTest
         }
         catch(RuntimeException e)
         {
-            assertTrue(e.getCause()==me);
+            assertTrue(e.getCause() instanceof MultiException);
             assertTrue(e.getStackTrace().length > 0);
         }
 
@@ -160,7 +161,8 @@ public class MultiExceptionTest
         }
         catch(RuntimeException e)
         {
-            assertTrue(e.getCause()==me);
+            assertTrue(e.getCause() instanceof MultiException);
+            Assert.assertEquals(e.getCause().getCause(), run);
             assertTrue(e.getStackTrace().length > 0);
         }
     }
@@ -174,7 +176,13 @@ public class MultiExceptionTest
         me.add(io);
         me.add(run);
 
-        assertEquals(2,me.size());
-        assertEquals(io,me.getCause());        
+        
+        try {
+            me.ifExceptionThrow();
+        } catch (MultiException e) {
+            assertEquals(io,e.getCause());
+            assertEquals(2,e.size());
+        }
+        
     }
 }