Revert "CompositeException does not mutate nested exceptions by attaching stacks, but instead"

tomrozb · tomrozb · commit 981ff6add164 · 2014-07-11T14:35:25.000+02:00
This reverts commit f4ae92a.
diff --git a/rxjava-core/src/main/java/rx/exceptions/CompositeException.java b/rxjava-core/src/main/java/rx/exceptions/CompositeException.java
@@ -15,44 +15,38 @@
  */
 package rx.exceptions;
 
-import java.io.PrintStream;
-import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedHashSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
 /**
  * Exception that is a composite of 1 or more other exceptions.
- * A CompositeException does not modify the structure of any exception it wraps, but at print-time
- * iterates through the list of contained Throwables to print them all.
- *
- * Its invariant is to contains an immutable, ordered (by insertion order), unique list of non-composite exceptions.
- * This list may be queried by {@code #getExceptions()}
+ * <p>
+ * Use <code>getMessage()</code> to retrieve a concatenation of the composite exceptions.
  */
 public final class CompositeException extends RuntimeException {
 
     private static final long serialVersionUID = 3026362227162912146L;
 
     private final List<Throwable> exceptions;
     private final String message;
+    private final Throwable cause;
 
     public CompositeException(String messagePrefix, Collection<Throwable> errors) {
-        Set<Throwable> deDupedExceptions = new LinkedHashSet<Throwable>();
         List<Throwable> _exceptions = new ArrayList<Throwable>();
-        for (Throwable ex: errors) {
-            if (ex instanceof CompositeException) {
-                deDupedExceptions.addAll(((CompositeException) ex).getExceptions());
-            } else {
-                deDupedExceptions.add(ex);
-            }
+        CompositeExceptionCausalChain _cause = new CompositeExceptionCausalChain();
+        int count = 0;
+        for (Throwable e : errors) {
+            count++;
+            attachCallingThreadStack(_cause, e);
+            _exceptions.add(e);
         }
-
-        _exceptions.addAll(deDupedExceptions);
         this.exceptions = Collections.unmodifiableList(_exceptions);
-        this.message = exceptions.size() + " exceptions occurred. See them in causal chain below.";
+        this.message = count + " exceptions occurred. See them in causal chain below.";
+        this.cause = _cause;
     }
 
     public CompositeException(Collection<Throwable> errors) {
@@ -76,106 +70,57 @@ public String getMessage() {
 
     @Override
     public synchronized Throwable getCause() {
-        return null;
-    }
-
-    /**
-     * All of the following printStackTrace functionality is derived from JDK Throwable printStackTrace.
-     * In particular, the PrintStreamOrWriter abstraction is copied wholesale.
-     *
-     * Changes from the official JDK implementation:
-     * * No infinite loop detection
-     * * Smaller critical section holding printStream lock
-     * * Explicit knowledge about exceptions List that this loops through
-     */
-    @Override
-    public void printStackTrace() {
-        printStackTrace(System.err);
-    }
-
-    @Override
-    public void printStackTrace(PrintStream s) {
-        printStackTrace(new WrappedPrintStream(s));
-    }
-
-    @Override
-    public void printStackTrace(PrintWriter s) {
-        printStackTrace(new WrappedPrintWriter(s));
-    }
-
-    /**
-     * Special handling for printing out a CompositeException
-     * Loop through all inner exceptions and print them out
-     * @param s stream to print to
-     */
-    private void printStackTrace(PrintStreamOrWriter s) {
-        StringBuilder bldr = new StringBuilder();
-        bldr.append(this).append("\n");
-        for (StackTraceElement myStackElement: getStackTrace()) {
-            bldr.append("\tat ").append(myStackElement).append("\n");
-        }
-        int i = 1;
-        for (Throwable ex: exceptions) {
-            bldr.append("  ComposedException ").append(i).append(" :").append("\n");
-            appendStackTrace(bldr, ex, "\t");
-            i++;
-        }
-        synchronized (s.lock()) {
-            s.println(bldr.toString());
-        }
+        return cause;
     }
 
-    private void appendStackTrace(StringBuilder bldr, Throwable ex, String prefix) {
-        bldr.append(prefix).append(ex).append("\n");
-        for (StackTraceElement stackElement: ex.getStackTrace()) {
-            bldr.append("\t\tat ").append(stackElement).append("\n");
-        }
-        if (ex.getCause() != null) {
-            bldr.append("\tCaused by: ");
-            appendStackTrace(bldr, ex.getCause(), "");
+    @SuppressWarnings("unused")
+    // useful when debugging but don't want to make part of publicly supported API
+    private static String getStackTraceAsString(StackTraceElement[] stack) {
+        StringBuilder s = new StringBuilder();
+        boolean firstLine = true;
+        for (StackTraceElement e : stack) {
+            if (e.toString().startsWith("java.lang.Thread.getStackTrace")) {
+                // we'll ignore this one
+                continue;
+            }
+            if (!firstLine) {
+                s.append("\n\t");
+            }
+            s.append(e.toString());
+            firstLine = false;
         }
+        return s.toString();
     }
 
-    private abstract static class PrintStreamOrWriter {
-        /** Returns the object to be locked when using this StreamOrWriter */
-        abstract Object lock();
-
-        /** Prints the specified string as a line on this StreamOrWriter */
-        abstract void println(Object o);
-    }
-
-    /**
-     * Same abstraction and implementation as in JDK to allow PrintStream and PrintWriter to share implementation
-     */
-    private static class WrappedPrintStream extends PrintStreamOrWriter {
-        private final PrintStream printStream;
-
-        WrappedPrintStream(PrintStream printStream) {
-            this.printStream = printStream;
-        }
+    /* package-private */ static void attachCallingThreadStack(Throwable e, Throwable cause) {
+        Set<Throwable> seenCauses = new HashSet<Throwable>();
 
-        Object lock() {
-            return printStream;
+        while (e.getCause() != null) {
+            e = e.getCause();
+            if (seenCauses.contains(e.getCause())) {
+                break;
+            } else {
+                seenCauses.add(e.getCause());
+            }
         }
-
-        void println(Object o) {
-            printStream.println(o);
+        // we now have 'e' as the last in the chain
+        try {
+            e.initCause(cause);
+        } catch (Throwable t) {
+            // ignore
+            // the javadocs say that some Throwables (depending on how they're made) will never
+            // let me call initCause without blowing up even if it returns null
         }
     }
 
-    private static class WrappedPrintWriter extends PrintStreamOrWriter {
-        private final PrintWriter printWriter;
+    /* package-private */ final static class CompositeExceptionCausalChain extends RuntimeException {
+        private static final long serialVersionUID = 3875212506787802066L;
+        /* package-private */ static String MESSAGE = "Chain of Causes for CompositeException In Order Received =>";
 
-        WrappedPrintWriter(PrintWriter printWriter) {
-            this.printWriter = printWriter;
-        }
-
-        Object lock() {
-            return printWriter;
-        }
-
-        void println(Object o) {
-            printWriter.println(o);
+        @Override
+        public String getMessage() {
+            return MESSAGE;
         }
     }
+
 }
diff --git a/rxjava-core/src/test/java/rx/exceptions/CompositeExceptionTest.java b/rxjava-core/src/test/java/rx/exceptions/CompositeExceptionTest.java
@@ -17,9 +17,6 @@
 
 import static org.junit.Assert.assertEquals;
 
-import java.io.ByteArrayOutputStream;
-import java.io.PrintStream;
-import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -33,6 +30,7 @@ public class CompositeExceptionTest {
     private final Throwable ex3 = new Throwable("Ex3", ex2);
 
     public CompositeExceptionTest() {
+        ex1.initCause(ex2);
     }
 
     private CompositeException getNewCompositeExceptionWithEx123() {
@@ -52,58 +50,46 @@ public void testMultipleWithSameCause() {
         CompositeException ce = new CompositeException("3 failures with same root cause", Arrays.asList(e1, e2, e3));
         
         assertEquals(3, ce.getExceptions().size());
-        assertNoCircularReferences(ce);
+        
     }
 
     @Test(timeout = 1000)
-    public void testCompositeExceptionFromParentThenChild() {
-        CompositeException cex = new CompositeException(Arrays.asList(ex1, ex2));
-        assertNoCircularReferences(cex);
-        assertEquals(2, cex.getExceptions().size());
+    public void testAttachCallingThreadStackParentThenChild() {
+        CompositeException.attachCallingThreadStack(ex1, ex2);
+        assertEquals("Ex2", ex1.getCause().getMessage());
     }
 
     @Test(timeout = 1000)
-    public void testCompositeExceptionFromChildThenParent() {
-        CompositeException cex = new CompositeException(Arrays.asList(ex2, ex1));
-        assertNoCircularReferences(cex);
-        assertEquals(2, cex.getExceptions().size());
+    public void testAttachCallingThreadStackChildThenParent() {
+        CompositeException.attachCallingThreadStack(ex2, ex1);
+        assertEquals("Ex1", ex2.getCause().getMessage());
     }
 
     @Test(timeout = 1000)
-    public void testCompositeExceptionFromChildAndComposite() {
-        CompositeException cex = new CompositeException(Arrays.asList(ex1, getNewCompositeExceptionWithEx123()));
-        assertNoCircularReferences(cex);
-        assertEquals(3, cex.getExceptions().size());
+    public void testAttachCallingThreadStackAddComposite() {
+        CompositeException.attachCallingThreadStack(ex1, getNewCompositeExceptionWithEx123());
+        assertEquals("Ex2", ex1.getCause().getMessage());
     }
 
     @Test(timeout = 1000)
-    public void testCompositeExceptionFromCompositeAndChild() {
-        CompositeException cex = new CompositeException(Arrays.asList(getNewCompositeExceptionWithEx123(), ex1));
-        assertNoCircularReferences(cex);
-        assertEquals(3, cex.getExceptions().size());
+    public void testAttachCallingThreadStackAddToComposite() {
+        CompositeException compositeEx = getNewCompositeExceptionWithEx123();
+        CompositeException.attachCallingThreadStack(compositeEx, ex1);
+        assertEquals(CompositeException.CompositeExceptionCausalChain.MESSAGE, compositeEx.getCause().getMessage());
     }
 
     @Test(timeout = 1000)
-    public void testCompositeExceptionFromTwoDuplicateComposites() {
-        List<Throwable> exs = new ArrayList<Throwable>();
-        exs.add(getNewCompositeExceptionWithEx123());
-        exs.add(getNewCompositeExceptionWithEx123());
-        CompositeException cex = new CompositeException(exs);
-        assertNoCircularReferences(cex);
-        assertEquals(3, cex.getExceptions().size());
+    public void testAttachCallingThreadStackAddCompositeToItself() {
+        CompositeException compositeEx = getNewCompositeExceptionWithEx123();
+        CompositeException.attachCallingThreadStack(compositeEx, compositeEx);
+        assertEquals(CompositeException.CompositeExceptionCausalChain.MESSAGE, compositeEx.getCause().getMessage());
     }
 
-    /**
-     * This hijacks the Throwable.printStackTrace() output and puts it in a string, where we can look for
-     * "CIRCULAR REFERENCE" (a String added by Throwable.printEnclosedStackTrace)
-     */
-    private static void assertNoCircularReferences(Throwable ex) {
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        PrintStream printStream = new PrintStream(baos);
-        StringWriter writer = new StringWriter();
-
-        ex.printStackTrace();
-        //ex.printStackTrace(printStream);
-        //assertFalse(baos.toString().contains("CIRCULAR REFERENCE"));
+    @Test(timeout = 1000)
+    public void testAttachCallingThreadStackAddExceptionsToEachOther() {
+        CompositeException.attachCallingThreadStack(ex1, ex2);
+        CompositeException.attachCallingThreadStack(ex2, ex1);
+        assertEquals("Ex2", ex1.getCause().getMessage());
+        assertEquals("Ex1", ex2.getCause().getMessage());
     }
 }
