diff --git a/src/main/java/io/reactivex/exceptions/CompositeException.java b/src/main/java/io/reactivex/exceptions/CompositeException.java index 07353e4738..0187464d27 100644 --- a/src/main/java/io/reactivex/exceptions/CompositeException.java +++ b/src/main/java/io/reactivex/exceptions/CompositeException.java @@ -40,37 +40,24 @@ public final class CompositeException extends RuntimeException { private final String message; private Throwable cause; - /** - * Constructs an empty CompositeException. - */ - public CompositeException() { - this.exceptions = new ArrayList(); - this.message = null; - } - /** * Constructs a CompositeException with the given array of Throwables as the * list of suppressed exceptions. * @param exceptions the Throwables to have as initially suppressed exceptions + * + * @throws IllegalArgumentException if exceptions is empty. */ public CompositeException(Throwable... exceptions) { - this.exceptions = new ArrayList(); - if (exceptions == null) { - this.message = "1 exception occurred. "; - this.exceptions.add(new NullPointerException("exceptions is null")); - } else { - this.message = exceptions.length + " exceptions occurred. "; - for (Throwable t : exceptions) { - this.exceptions.add(t != null ? t : new NullPointerException("One of the exceptions is null")); - } - } + this(exceptions == null ? + Arrays.asList(new NullPointerException("exceptions was null")) : Arrays.asList(exceptions)); } - /** * Constructs a CompositeException with the given array of Throwables as the * list of suppressed exceptions. * @param errors the Throwables to have as initially suppressed exceptions + * + * @throws IllegalArgumentException if errors is empty. */ public CompositeException(Iterable errors) { Set deDupedExceptions = new LinkedHashSet(); @@ -83,13 +70,15 @@ public CompositeException(Iterable errors) { if (ex != null) { deDupedExceptions.add(ex); } else { - deDupedExceptions.add(new NullPointerException()); + deDupedExceptions.add(new NullPointerException("Throwable was null!")); } } } else { - deDupedExceptions.add(new NullPointerException()); + deDupedExceptions.add(new NullPointerException("errors was null")); + } + if (deDupedExceptions.isEmpty()) { + throw new IllegalArgumentException("errors is empty"); } - localExceptions.addAll(deDupedExceptions); this.exceptions = Collections.unmodifiableList(localExceptions); this.message = exceptions.size() + " exceptions occurred. "; @@ -109,17 +98,6 @@ public String getMessage() { return message; } - /** - * Adds a suppressed exception to this composite. - *

The method is named this way to avoid conflicts with Java 7 environments - * and its addSuppressed() method. - * @param e the exception to suppress, nulls are converted to NullPointerExceptions - */ - public void suppress(Throwable e) { - exceptions.add(e != null ? e : new NullPointerException("null exception")); - } - - @Override public synchronized Throwable getCause() { // NOPMD if (cause == null) { @@ -266,15 +244,16 @@ public String getMessage() { private List getListOfCauses(Throwable ex) { List list = new ArrayList(); Throwable root = ex.getCause(); - if (root == null) { + if (root == null || root == ex) { return list; } else { while (true) { list.add(root); - if (root.getCause() == null) { + Throwable cause = root.getCause(); + if (cause == null || cause == root) { return list; } else { - root = root.getCause(); + root = cause; } } } @@ -288,16 +267,6 @@ public int size() { return exceptions.size(); } - /** - * Returns true if this CompositeException doesn't have a cause or - * any suppressed exceptions. - * @return true if this CompositeException doesn't have a cause or - * any suppressed exceptions. - */ - public boolean isEmpty() { - return exceptions.isEmpty(); - } - /** * Returns the root cause of {@code e}. If {@code e.getCause()} returns {@code null} or {@code e}, just return {@code e} itself. * @@ -306,15 +275,15 @@ public boolean isEmpty() { */ private Throwable getRootCause(Throwable e) { Throwable root = e.getCause(); - if (root == null /* || cause == root */) { // case might not be possible + if (root == null || cause == root) { return e; } while (true) { Throwable cause = root.getCause(); - if (cause == null /* || cause == root */) { // case might not be possible + if (cause == null || cause == root) { return root; } - root = root.getCause(); + root = cause; } } } \ No newline at end of file diff --git a/src/main/java/io/reactivex/internal/operators/flowable/FlowableFlatMap.java b/src/main/java/io/reactivex/internal/operators/flowable/FlowableFlatMap.java index d005faf6ec..2ff03470bb 100644 --- a/src/main/java/io/reactivex/internal/operators/flowable/FlowableFlatMap.java +++ b/src/main/java/io/reactivex/internal/operators/flowable/FlowableFlatMap.java @@ -14,6 +14,9 @@ package io.reactivex.internal.operators.flowable; import io.reactivex.plugins.RxJavaPlugins; + +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.atomic.*; @@ -550,39 +553,42 @@ boolean checkTerminate() { } void reportError(SimpleQueue q) { - CompositeException composite = null; + List composite = null; Throwable ex = null; - Throwable t; - int count = 0; for (;;) { + Throwable t; try { t = q.poll(); } catch (Throwable exc) { Exceptions.throwIfFatal(exc); - if (composite == null) { - composite = new CompositeException(ex); + if (ex == null) { + ex = exc; + } else { + if (composite == null) { + composite = new ArrayList(); + composite.add(ex); + } + composite.add(exc); } - composite.suppress(exc); break; } if (t == null) { break; } - if (count == 0) { + if (ex == null) { ex = t; } else { if (composite == null) { - composite = new CompositeException(ex); + composite = new ArrayList(); + composite.add(ex); } - composite.suppress(t); + composite.add(t); } - - count++; } if (composite != null) { - actual.onError(composite); + actual.onError(new CompositeException(composite)); } else { actual.onError(ex); } diff --git a/src/main/java/io/reactivex/internal/operators/observable/ObservableCombineLatest.java b/src/main/java/io/reactivex/internal/operators/observable/ObservableCombineLatest.java index 12c8a26215..12d9230d27 100644 --- a/src/main/java/io/reactivex/internal/operators/observable/ObservableCombineLatest.java +++ b/src/main/java/io/reactivex/internal/operators/observable/ObservableCombineLatest.java @@ -299,9 +299,8 @@ boolean checkTerminated(boolean d, boolean empty, Observer a, SpscLinkedArray void onError(Throwable e) { for (;;) { Throwable curr = error.get(); - if (curr instanceof CompositeException) { - CompositeException ce = new CompositeException((CompositeException)curr); - ce.suppress(e); + if (curr != null) { + CompositeException ce = new CompositeException(curr, e); e = ce; } Throwable next = e; diff --git a/src/main/java/io/reactivex/internal/operators/observable/ObservableFlatMap.java b/src/main/java/io/reactivex/internal/operators/observable/ObservableFlatMap.java index 2eb224bc51..3c344ff2d7 100644 --- a/src/main/java/io/reactivex/internal/operators/observable/ObservableFlatMap.java +++ b/src/main/java/io/reactivex/internal/operators/observable/ObservableFlatMap.java @@ -486,39 +486,42 @@ boolean checkTerminate() { } void reportError(SimpleQueue q) { - CompositeException composite = null; + List composite = null; Throwable ex = null; - Throwable t; - int count = 0; for (;;) { + Throwable t; try { t = q.poll(); } catch (Throwable exc) { Exceptions.throwIfFatal(exc); - if (composite == null) { - composite = new CompositeException(exc); + if (ex == null) { + ex = exc; + } else { + if (composite == null) { + composite = new ArrayList(); + composite.add(ex); + } + composite.add(exc); } - composite.suppress(exc); break; } if (t == null) { break; } - if (count == 0) { + if (ex == null) { ex = t; } else { if (composite == null) { - composite = new CompositeException(ex); + composite = new ArrayList(); + composite.add(ex); } - composite.suppress(t); + composite.add(t); } - - count++; } if (composite != null) { - actual.onError(composite); + actual.onError(new CompositeException(composite)); } else { actual.onError(ex); } diff --git a/src/main/java/io/reactivex/observers/BaseTestConsumer.java b/src/main/java/io/reactivex/observers/BaseTestConsumer.java index 354e3e017c..9d94b662cd 100644 --- a/src/main/java/io/reactivex/observers/BaseTestConsumer.java +++ b/src/main/java/io/reactivex/observers/BaseTestConsumer.java @@ -130,11 +130,8 @@ protected final AssertionError fail(String message) { ; AssertionError ae = new AssertionError(b.toString()); - CompositeException ce = new CompositeException(); - for (Throwable e : errors) { - ce.suppress(e); - } - if (!ce.isEmpty()) { + if (!errors.isEmpty()) { + CompositeException ce = new CompositeException(errors); ae.initCause(ce); } return ae; diff --git a/src/main/java/io/reactivex/observers/SafeObserver.java b/src/main/java/io/reactivex/observers/SafeObserver.java index 015cbe0c81..0e8b551c0b 100644 --- a/src/main/java/io/reactivex/observers/SafeObserver.java +++ b/src/main/java/io/reactivex/observers/SafeObserver.java @@ -142,26 +142,22 @@ public void onError(Throwable t) { done = true; if (s == null) { - CompositeException t2 = new CompositeException(t, new NullPointerException("Subscription not set!")); + Throwable npe = new NullPointerException("Subscription not set!"); try { actual.onSubscribe(EmptyDisposable.INSTANCE); } catch (Throwable e) { Exceptions.throwIfFatal(e); // can't call onError because the actual's state may be corrupt at this point - t2.suppress(e); - - RxJavaPlugins.onError(t2); + RxJavaPlugins.onError(new CompositeException(t, npe, e)); return; } try { - actual.onError(t2); + actual.onError(new CompositeException(t, npe)); } catch (Throwable e) { Exceptions.throwIfFatal(e); // if onError failed, all that's left is to report the error to plugins - t2.suppress(e); - - RxJavaPlugins.onError(t2); + RxJavaPlugins.onError(new CompositeException(t, npe, e)); } return; } diff --git a/src/main/java/io/reactivex/subscribers/SafeSubscriber.java b/src/main/java/io/reactivex/subscribers/SafeSubscriber.java index a7bcd64c0f..3f6a0ea533 100644 --- a/src/main/java/io/reactivex/subscribers/SafeSubscriber.java +++ b/src/main/java/io/reactivex/subscribers/SafeSubscriber.java @@ -130,26 +130,22 @@ public void onError(Throwable t) { done = true; if (s == null) { - CompositeException t2 = new CompositeException(t, new NullPointerException("Subscription not set!")); + Throwable npe = new NullPointerException("Subscription not set!"); try { actual.onSubscribe(EmptySubscription.INSTANCE); } catch (Throwable e) { Exceptions.throwIfFatal(e); // can't call onError because the actual's state may be corrupt at this point - t2.suppress(e); - - RxJavaPlugins.onError(t2); + RxJavaPlugins.onError(new CompositeException(t, npe, e)); return; } try { - actual.onError(t2); + actual.onError(new CompositeException(t, npe)); } catch (Throwable e) { Exceptions.throwIfFatal(e); // if onError failed, all that's left is to report the error to plugins - t2.suppress(e); - - RxJavaPlugins.onError(t2); + RxJavaPlugins.onError(new CompositeException(t, npe, e)); } return; } diff --git a/src/test/java/io/reactivex/exceptions/CompositeExceptionTest.java b/src/test/java/io/reactivex/exceptions/CompositeExceptionTest.java index b3926fd546..0c913a7ebb 100644 --- a/src/test/java/io/reactivex/exceptions/CompositeExceptionTest.java +++ b/src/test/java/io/reactivex/exceptions/CompositeExceptionTest.java @@ -44,7 +44,7 @@ public void testMultipleWithSameCause() { Throwable e1 = new Throwable("1", rootCause); Throwable e2 = new Throwable("2", rootCause); Throwable e3 = new Throwable("3", rootCause); - CompositeException ce = new CompositeException(Arrays.asList(e1, e2, e3)); + CompositeException ce = new CompositeException(e1, e2, e3); System.err.println("----------------------------- print composite stacktrace"); ce.printStackTrace(); @@ -56,9 +56,25 @@ public void testMultipleWithSameCause() { ce.getCause().printStackTrace(); } + @Test + public void testEmptyErrors() { + try { + new CompositeException(); + fail("CompositeException should fail if errors is empty"); + } catch(IllegalArgumentException e) { + assertEquals("errors is empty", e.getMessage()); + } + try { + new CompositeException(new ArrayList()); + fail("CompositeException should fail if errors is empty"); + } catch(IllegalArgumentException e) { + assertEquals("errors is empty", e.getMessage()); + } + } + @Test(timeout = 1000) public void testCompositeExceptionFromParentThenChild() { - CompositeException cex = new CompositeException(Arrays.asList(ex1, ex2)); + CompositeException cex = new CompositeException(ex1, ex2); System.err.println("----------------------------- print composite stacktrace"); cex.printStackTrace(); @@ -73,7 +89,7 @@ public void testCompositeExceptionFromParentThenChild() { @Test(timeout = 1000) public void testCompositeExceptionFromChildThenParent() { - CompositeException cex = new CompositeException(Arrays.asList(ex2, ex1)); + CompositeException cex = new CompositeException(ex2, ex1); System.err.println("----------------------------- print composite stacktrace"); cex.printStackTrace(); @@ -88,7 +104,7 @@ public void testCompositeExceptionFromChildThenParent() { @Test(timeout = 1000) public void testCompositeExceptionFromChildAndComposite() { - CompositeException cex = new CompositeException(Arrays.asList(ex1, getNewCompositeExceptionWithEx123())); + CompositeException cex = new CompositeException(ex1, getNewCompositeExceptionWithEx123()); System.err.println("----------------------------- print composite stacktrace"); cex.printStackTrace(); @@ -103,7 +119,7 @@ public void testCompositeExceptionFromChildAndComposite() { @Test(timeout = 1000) public void testCompositeExceptionFromCompositeAndChild() { - CompositeException cex = new CompositeException(Arrays.asList(getNewCompositeExceptionWithEx123(), ex1)); + CompositeException cex = new CompositeException(getNewCompositeExceptionWithEx123(), ex1); System.err.println("----------------------------- print composite stacktrace"); cex.printStackTrace(); @@ -184,7 +200,7 @@ public synchronized Throwable initCause(Throwable cause) { throw new UnsupportedOperationException(); } }; - CompositeException cex = new CompositeException(Arrays.asList(t, ex1)); + CompositeException cex = new CompositeException(t, ex1); System.err.println("----------------------------- print composite stacktrace"); cex.printStackTrace(); @@ -208,7 +224,7 @@ public synchronized Throwable initCause(Throwable cause) { return null; } }; - CompositeException cex = new CompositeException(Arrays.asList(t, ex1)); + CompositeException cex = new CompositeException(t, ex1); System.err.println("----------------------------- print composite stacktrace"); cex.printStackTrace(); @@ -223,7 +239,7 @@ public synchronized Throwable initCause(Throwable cause) { @Test public void messageCollection() { - CompositeException compositeException = new CompositeException(Arrays.asList(ex1, ex3)); + CompositeException compositeException = new CompositeException(ex1, ex3); assertEquals("2 exceptions occurred. ", compositeException.getMessage()); } @@ -275,27 +291,6 @@ public void constructorWithNull() { assertTrue(new CompositeException((Iterable)null).getExceptions().get(0) instanceof NullPointerException); assertTrue(new CompositeException(null, new TestException()).getExceptions().get(0) instanceof NullPointerException); - - CompositeException ce1 = new CompositeException(); - ce1.suppress(null); - - assertTrue(ce1.getExceptions().get(0) instanceof NullPointerException); - } - - @Test - public void isEmpty() { - assertTrue(new CompositeException().isEmpty()); - - assertFalse(new CompositeException(new TestException()).isEmpty()); - - CompositeException ce1 = new CompositeException(); - ce1.initCause(new TestException()); - - assertTrue(ce1.isEmpty()); - - ce1.suppress(new TestException()); - - assertEquals(1, ce1.size()); } @Test @@ -348,4 +343,17 @@ public Throwable getCause() { assertSame(te, new CompositeException(new RuntimeException(te)).getCause().getCause().getCause()); } -} \ No newline at end of file + @Test + public void badException() { + Throwable e = new BadException(); + assertSame(e, new CompositeException(e).getCause().getCause()); + assertSame(e, new CompositeException(new RuntimeException(e)).getCause().getCause().getCause()); + } +} + +class BadException extends Throwable { + @Override + public synchronized Throwable getCause() { + return this; + } +} diff --git a/src/test/java/io/reactivex/internal/operators/flowable/FlowableConcatDelayErrorTest.java b/src/test/java/io/reactivex/internal/operators/flowable/FlowableConcatDelayErrorTest.java index 7d1685104d..2ffb50b780 100644 --- a/src/test/java/io/reactivex/internal/operators/flowable/FlowableConcatDelayErrorTest.java +++ b/src/test/java/io/reactivex/internal/operators/flowable/FlowableConcatDelayErrorTest.java @@ -245,16 +245,11 @@ public void concatDelayErrorFlowableError() { CompositeException ce = (CompositeException)ts.errors().get(0); List cex = ce.getExceptions(); - assertEquals(2, cex.size()); - - assertTrue(cex.get(0).toString(), cex.get(0) instanceof CompositeException); - assertTrue(cex.get(1).toString(), cex.get(1) instanceof TestException); - - ce = (CompositeException)cex.get(0); - cex = ce.getExceptions(); + assertEquals(3, cex.size()); assertTrue(cex.get(0).toString(), cex.get(0) instanceof TestException); assertTrue(cex.get(1).toString(), cex.get(1) instanceof TestException); + assertTrue(cex.get(2).toString(), cex.get(2) instanceof TestException); } @SuppressWarnings("unchecked")