Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CompositeException stops mutating nested Exceptions #1388

Merged
merged 3 commits into from
Jun 25, 2014
Merged

CompositeException stops mutating nested Exceptions #1388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f4ae92a
CompositeException does not mutate nested exceptions by attaching sta…
mattrjacobs Jun 25, 2014
3b95e2f
Removed unused import
mattrjacobs Jun 25, 2014
8e27cdc
Fixed up SafeObserverTest w.r.t. CompositeException changes
mattrjacobs Jun 25, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 107 additions & 52 deletions rxjava-core/src/main/java/rx/exceptions/CompositeException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,38 +15,44 @@
*/
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.
* <p>
* Use <code>getMessage()</code> 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()}
*/
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>();
CompositeExceptionCausalChain _cause = new CompositeExceptionCausalChain();
int count = 0;
for (Throwable e : errors) {
count++;
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);
this.message = count + " exceptions occurred. See them in causal chain below.";
this.cause = _cause;
this.message = exceptions.size() + " exceptions occurred. See them in causal chain below.";
}

public CompositeException(Collection<Throwable> errors) {
Expand All @@ -70,57 +76,106 @@ public String getMessage() {

@Override
public synchronized Throwable getCause() {
return cause;
return null;
}

@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();
/**
* 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);
}

/* package-private */ static void attachCallingThreadStack(Throwable e, Throwable cause) {
Set<Throwable> seenCauses = new HashSet<Throwable>();
@Override
public void printStackTrace(PrintStream s) {
printStackTrace(new WrappedPrintStream(s));
}

while (e.getCause() != null) {
e = e.getCause();
if (seenCauses.contains(e.getCause())) {
break;
} else {
seenCauses.add(e.getCause());
}
@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++;
}
// 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
synchronized (s.lock()) {
s.println(bldr.toString());
}
}

/* 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 =>";
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(), "");
}
}

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;
}

Object lock() {
return printStream;
}

@Override
public String getMessage() {
return MESSAGE;
void println(Object o) {
printStream.println(o);
}
}

private static class WrappedPrintWriter extends PrintStreamOrWriter {
private final PrintWriter printWriter;

WrappedPrintWriter(PrintWriter printWriter) {
this.printWriter = printWriter;
}

Object lock() {
return printWriter;
}

void println(Object o) {
printWriter.println(o);
}
}
}
64 changes: 39 additions & 25 deletions rxjava-core/src/test/java/rx/exceptions/CompositeExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

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;
Expand All @@ -30,7 +33,6 @@ public class CompositeExceptionTest {
private final Throwable ex3 = new Throwable("Ex3", ex2);

public CompositeExceptionTest() {
ex1.initCause(ex2);
}

private CompositeException getNewCompositeExceptionWithEx123() {
Expand All @@ -50,46 +52,58 @@ 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 testAttachCallingThreadStackParentThenChild() {
CompositeException.attachCallingThreadStack(ex1, ex2);
assertEquals("Ex2", ex1.getCause().getMessage());
public void testCompositeExceptionFromParentThenChild() {
CompositeException cex = new CompositeException(Arrays.asList(ex1, ex2));
assertNoCircularReferences(cex);
assertEquals(2, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackChildThenParent() {
CompositeException.attachCallingThreadStack(ex2, ex1);
assertEquals("Ex1", ex2.getCause().getMessage());
public void testCompositeExceptionFromChildThenParent() {
CompositeException cex = new CompositeException(Arrays.asList(ex2, ex1));
assertNoCircularReferences(cex);
assertEquals(2, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddComposite() {
CompositeException.attachCallingThreadStack(ex1, getNewCompositeExceptionWithEx123());
assertEquals("Ex2", ex1.getCause().getMessage());
public void testCompositeExceptionFromChildAndComposite() {
CompositeException cex = new CompositeException(Arrays.asList(ex1, getNewCompositeExceptionWithEx123()));
assertNoCircularReferences(cex);
assertEquals(3, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddToComposite() {
CompositeException compositeEx = getNewCompositeExceptionWithEx123();
CompositeException.attachCallingThreadStack(compositeEx, ex1);
assertEquals(CompositeException.CompositeExceptionCausalChain.MESSAGE, compositeEx.getCause().getMessage());
public void testCompositeExceptionFromCompositeAndChild() {
CompositeException cex = new CompositeException(Arrays.asList(getNewCompositeExceptionWithEx123(), ex1));
assertNoCircularReferences(cex);
assertEquals(3, cex.getExceptions().size());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddCompositeToItself() {
CompositeException compositeEx = getNewCompositeExceptionWithEx123();
CompositeException.attachCallingThreadStack(compositeEx, compositeEx);
assertEquals(CompositeException.CompositeExceptionCausalChain.MESSAGE, compositeEx.getCause().getMessage());
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());
}

@Test(timeout = 1000)
public void testAttachCallingThreadStackAddExceptionsToEachOther() {
CompositeException.attachCallingThreadStack(ex1, ex2);
CompositeException.attachCallingThreadStack(ex2, ex1);
assertEquals("Ex2", ex1.getCause().getMessage());
assertEquals("Ex1", ex2.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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion is commented out. Does this actually test anything?

}
}
Loading