diff --git a/rxjava/src/main/java/rx/exceptions/CompositeException.java b/rxjava/src/main/java/rx/exceptions/CompositeException.java index d4c91e29d4..e2d9a97527 100644 --- a/rxjava/src/main/java/rx/exceptions/CompositeException.java +++ b/rxjava/src/main/java/rx/exceptions/CompositeException.java @@ -15,17 +15,29 @@ */ 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.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; /** * Exception that is a composite of 1 or more other exceptions. - *

- * Use getMessage() to retrieve a concatenation of the composite 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()} + * + * The `printStackTrace()` implementation does custom handling of the StackTrace instead of using `getCause()` so it + * can avoid circular references. + * + * If `getCause()` is invoked, it will lazily create the causal chain but stop if it finds any Throwable in the chain + * that it has already seen. */ public final class CompositeException extends RuntimeException { @@ -33,36 +45,31 @@ public final class CompositeException extends RuntimeException { private final List exceptions; private final String message; - private final Throwable cause; - public CompositeException(String messagePrefix, Collection errors) { + public CompositeException(String messagePrefix, Collection errors) { + Set deDupedExceptions = new LinkedHashSet(); List _exceptions = new ArrayList(); - CompositeExceptionCausalChain _cause = new CompositeExceptionCausalChain(); - int count = errors.size(); - errors = removeDuplicatedCauses(errors); - for (Throwable e : errors) { - attachCallingThreadStack(_cause, e); - _exceptions.add(e); + for (Throwable ex : errors) { + if (ex instanceof CompositeException) { + deDupedExceptions.addAll(((CompositeException) ex).getExceptions()); + } else { + deDupedExceptions.add(ex); + } } + + _exceptions.addAll(deDupedExceptions); this.exceptions = Collections.unmodifiableList(_exceptions); - - String msg = count + " exceptions occurred. See them in causal chain below."; - if(messagePrefix != null) { - msg = messagePrefix + " " + msg; - } - this.message = msg; - this.cause = _cause; + this.message = exceptions.size() + " exceptions occurred. "; } - public CompositeException(Collection errors) { + public CompositeException(Collection errors) { this(null, errors); } /** * Retrieves the list of exceptions that make up the {@code CompositeException} * - * @return the exceptions that make up the {@code CompositeException}, as a {@link List} of - * {@link Throwable}s + * @return the exceptions that make up the {@code CompositeException}, as a {@link List} of {@link Throwable}s */ public List getExceptions() { return exceptions; @@ -73,77 +80,154 @@ public String getMessage() { return message; } + private Throwable cause = null; + @Override public synchronized Throwable getCause() { - return cause; - } - - private Collection removeDuplicatedCauses(Collection errors) { - Set duplicated = new HashSet(); - for (Throwable cause : errors) { - for (Throwable error : errors) { - if(cause == error || duplicated.contains(error)) { + if (cause == null) { + // we lazily generate this causal chain if this is called + CompositeExceptionCausalChain _cause = new CompositeExceptionCausalChain(); + Set seenCauses = new HashSet(); + + Throwable chain = _cause; + for (Throwable e : exceptions) { + if (seenCauses.contains(e)) { + // already seen this outer Throwable so skip continue; } - while (error.getCause() != null) { - error = error.getCause(); - if (error == cause) { - duplicated.add(cause); - break; + seenCauses.add(e); + + List listOfCauses = getListOfCauses(e); + // check if any of them have been seen before + for(Throwable child : listOfCauses) { + if (seenCauses.contains(child)) { + // already seen this outer Throwable so skip + e = new RuntimeException("Duplicate found in causal chain so cropping to prevent loop ..."); + continue; } + seenCauses.add(child); } + + // we now have 'e' as the last in the chain + try { + chain.initCause(e); + } 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 + } + chain = chain.getCause(); } + cause = _cause; + } + return cause; + } + + /** + * 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"); } - if (!duplicated.isEmpty()) { - errors = new ArrayList(errors); - errors.removeAll(duplicated); + 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 errors; } - @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; + 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(), ""); } - return s.toString(); } - /* package-private */ static void attachCallingThreadStack(Throwable e, Throwable cause) { - Set seenCauses = new HashSet(); + private abstract static class PrintStreamOrWriter { + /** Returns the object to be locked when using this StreamOrWriter */ + abstract Object lock(); - while (e.getCause() != null) { - e = e.getCause(); - if (seenCauses.contains(e.getCause())) { - break; - } else { - seenCauses.add(e.getCause()); - } + /** 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; + } + + Object lock() { + return printStream; + } + + void println(Object o) { + printStream.println(o); + } + } + + private static class WrappedPrintWriter extends PrintStreamOrWriter { + private final PrintWriter printWriter; + + WrappedPrintWriter(PrintWriter printWriter) { + this.printWriter = printWriter; } - // 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 + + Object lock() { + return printWriter; + } + + void println(Object o) { + printWriter.println(o); } } - /* package-private */ final static class CompositeExceptionCausalChain extends RuntimeException { + /* 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 =>"; + /* package-private */static String MESSAGE = "Chain of Causes for CompositeException In Order Received =>"; @Override public String getMessage() { @@ -151,4 +235,20 @@ public String getMessage() { } } -} + private final List getListOfCauses(Throwable ex) { + List list = new ArrayList(); + Throwable root = ex.getCause(); + if (root == null) { + return list; + } else { + while(true) { + list.add(root); + if (root.getCause() == null) { + return list; + } else { + root = root.getCause(); + } + } + } + } +} \ No newline at end of file diff --git a/rxjava/src/test/java/rx/exceptions/CompositeExceptionTest.java b/rxjava/src/test/java/rx/exceptions/CompositeExceptionTest.java index 9c14dd8014..5f5e4c8d11 100644 --- a/rxjava/src/test/java/rx/exceptions/CompositeExceptionTest.java +++ b/rxjava/src/test/java/rx/exceptions/CompositeExceptionTest.java @@ -16,7 +16,12 @@ package rx.exceptions; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.io.StringWriter; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -30,7 +35,6 @@ public class CompositeExceptionTest { private final Throwable ex3 = new Throwable("Ex3", ex2); public CompositeExceptionTest() { - ex1.initCause(ex2); } private CompositeException getNewCompositeExceptionWithEx123() { @@ -48,58 +52,119 @@ public void testMultipleWithSameCause() { Throwable e2 = new Throwable("2", rootCause); Throwable e3 = new Throwable("3", rootCause); CompositeException ce = new CompositeException("3 failures with same root cause", Arrays.asList(e1, e2, e3)); - + + System.err.println("----------------------------- print composite stacktrace"); + ce.printStackTrace(); assertEquals(3, ce.getExceptions().size()); + assertNoCircularReferences(ce); + assertNotNull(getRootCause(ce)); + System.err.println("----------------------------- print cause stacktrace"); + ce.getCause().printStackTrace(); } @Test(timeout = 1000) - public void testOneIsCauseOfAnother() { - Throwable rootCause = new Throwable("RootCause"); - Throwable throwable = new Throwable("1", rootCause); - CompositeException ce = new CompositeException("One is the cause of another", - Arrays.asList(rootCause, throwable)); - - assertEquals(1, ce.getExceptions().size()); + public void testCompositeExceptionFromParentThenChild() { + CompositeException cex = new CompositeException(Arrays.asList(ex1, ex2)); + + System.err.println("----------------------------- print composite stacktrace"); + cex.printStackTrace(); + assertEquals(2, cex.getExceptions().size()); + + assertNoCircularReferences(cex); + assertNotNull(getRootCause(cex)); + + System.err.println("----------------------------- print cause stacktrace"); + cex.getCause().printStackTrace(); } @Test(timeout = 1000) - public void testAttachCallingThreadStackParentThenChild() { - CompositeException.attachCallingThreadStack(ex1, ex2); - assertEquals("Ex2", ex1.getCause().getMessage()); + public void testCompositeExceptionFromChildThenParent() { + CompositeException cex = new CompositeException(Arrays.asList(ex2, ex1)); + + System.err.println("----------------------------- print composite stacktrace"); + cex.printStackTrace(); + assertEquals(2, cex.getExceptions().size()); + + assertNoCircularReferences(cex); + assertNotNull(getRootCause(cex)); + + System.err.println("----------------------------- print cause stacktrace"); + cex.getCause().printStackTrace(); } @Test(timeout = 1000) - public void testAttachCallingThreadStackChildThenParent() { - CompositeException.attachCallingThreadStack(ex2, ex1); - assertEquals("Ex1", ex2.getCause().getMessage()); + public void testCompositeExceptionFromChildAndComposite() { + CompositeException cex = new CompositeException(Arrays.asList(ex1, getNewCompositeExceptionWithEx123())); + + System.err.println("----------------------------- print composite stacktrace"); + cex.printStackTrace(); + assertEquals(3, cex.getExceptions().size()); + + assertNoCircularReferences(cex); + assertNotNull(getRootCause(cex)); + + System.err.println("----------------------------- print cause stacktrace"); + cex.getCause().printStackTrace(); } @Test(timeout = 1000) - public void testAttachCallingThreadStackAddComposite() { - CompositeException.attachCallingThreadStack(ex1, getNewCompositeExceptionWithEx123()); - assertEquals("Ex2", ex1.getCause().getMessage()); + public void testCompositeExceptionFromCompositeAndChild() { + CompositeException cex = new CompositeException(Arrays.asList(getNewCompositeExceptionWithEx123(), ex1)); + + System.err.println("----------------------------- print composite stacktrace"); + cex.printStackTrace(); + assertEquals(3, cex.getExceptions().size()); + + assertNoCircularReferences(cex); + assertNotNull(getRootCause(cex)); + + System.err.println("----------------------------- print cause stacktrace"); + cex.getCause().printStackTrace(); } @Test(timeout = 1000) - public void testAttachCallingThreadStackAddToComposite() { - CompositeException compositeEx = getNewCompositeExceptionWithEx123(); - CompositeException.attachCallingThreadStack(compositeEx, ex1); - assertEquals(CompositeException.CompositeExceptionCausalChain.MESSAGE, compositeEx.getCause().getMessage()); + public void testCompositeExceptionFromTwoDuplicateComposites() { + List exs = new ArrayList(); + exs.add(getNewCompositeExceptionWithEx123()); + exs.add(getNewCompositeExceptionWithEx123()); + CompositeException cex = new CompositeException(exs); + + System.err.println("----------------------------- print composite stacktrace"); + cex.printStackTrace(); + assertEquals(3, cex.getExceptions().size()); + + assertNoCircularReferences(cex); + assertNotNull(getRootCause(cex)); + + System.err.println("----------------------------- print cause stacktrace"); + cex.getCause().printStackTrace(); } - @Test(timeout = 1000) - 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(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()); + private static Throwable getRootCause(Throwable ex) { + Throwable root = ex.getCause(); + if (root == null) { + return null; + } else { + while(true) { + if (root.getCause() == null) { + return root; + } else { + root = root.getCause(); + } + } + } } } \ No newline at end of file