3.x: Change how the cause of CompositeException is generated (#6748)

akarnokd · web-flow · commit 0f0e219ac79a · 2019-12-07T22:36:21.000+01:00
* 3.x: Change how the cause of CompositeException is generated

* Fix reoccurrence formatting.

* Fix a mistake in the unit test
diff --git a/src/main/java/io/reactivex/rxjava3/exceptions/CompositeException.java b/src/main/java/io/reactivex/rxjava3/exceptions/CompositeException.java
@@ -106,40 +106,69 @@ public String getMessage() {
     @NonNull
     public synchronized Throwable getCause() { // NOPMD
         if (cause == null) {
-            // we lazily generate this causal chain if this is called
-            CompositeExceptionCausalChain localCause = new CompositeExceptionCausalChain();
-            Set<Throwable> seenCauses = new HashSet<Throwable>();
-
-            Throwable chain = localCause;
-            for (Throwable e : exceptions) {
-                if (seenCauses.contains(e)) {
-                    // already seen this outer Throwable so skip
-                    continue;
-                }
-                seenCauses.add(e);
-
-                List<Throwable> 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;
+            String separator = System.getProperty("line.separator");
+            if (exceptions.size() > 1) {
+                Map<Throwable, Boolean> seenCauses = new IdentityHashMap<Throwable, Boolean>();
+
+                StringBuilder aggregateMessage = new StringBuilder();
+                aggregateMessage.append("Multiple exceptions (").append(exceptions.size()).append(")").append(separator);
+
+                for (Throwable inner : exceptions) {
+                    int depth = 0;
+                    while (inner != null) {
+                        for (int i = 0; i < depth; i++) {
+                            aggregateMessage.append("  ");
+                        }
+                        aggregateMessage.append("|-- ");
+                        aggregateMessage.append(inner.getClass().getCanonicalName()).append(": ");
+                        String innerMessage = inner.getMessage();
+                        if (innerMessage != null && innerMessage.contains(separator)) {
+                            aggregateMessage.append(separator);
+                            for (String line : innerMessage.split(separator)) {
+                                for (int i = 0; i < depth + 2; i++) {
+                                    aggregateMessage.append("  ");
+                                }
+                                aggregateMessage.append(line).append(separator);
+                            }
+                        } else {
+                            aggregateMessage.append(innerMessage);
+                            aggregateMessage.append(separator);
+                        }
+
+                        for (int i = 0; i < depth + 2; i++) {
+                            aggregateMessage.append("  ");
+                        }
+                        StackTraceElement[] st = inner.getStackTrace();
+                        if (st.length > 0) {
+                            aggregateMessage.append("at ").append(st[0]).append(separator);
+                        }
+
+                        if (!seenCauses.containsKey(inner)) {
+                            seenCauses.put(inner, true);
+
+                            inner = inner.getCause();
+                            depth++;
+                        } else {
+                            inner = inner.getCause();
+                            if (inner != null) {
+                                for (int i = 0; i < depth + 2; i++) {
+                                    aggregateMessage.append("  ");
+                                }
+                                aggregateMessage.append("|-- ");
+                                aggregateMessage.append("(cause not expanded again) ");
+                                aggregateMessage.append(inner.getClass().getCanonicalName()).append(": ");
+                                aggregateMessage.append(inner.getMessage());
+                                aggregateMessage.append(separator);
+                            }
+                            break;
+                        }
                     }
-                    seenCauses.add(child);
                 }
 
-                // we now have 'e' as the last in the chain
-                try {
-                    chain.initCause(e);
-                } catch (Throwable t) { // NOPMD
-                    // 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 = getRootCause(chain);
+                cause = new ExceptionOverview(aggregateMessage.toString().trim());
+            } else {
+                cause = exceptions.get(0);
             }
-            cause = localCause;
         }
         return cause;
     }
@@ -236,31 +265,21 @@ void println(Object o) {
         }
     }
 
-    static final class CompositeExceptionCausalChain extends RuntimeException {
+    /**
+     * Contains a formatted message with a simplified representation of the exception graph
+     * contained within the CompositeException.
+     */
+    static final class ExceptionOverview extends RuntimeException {
+
         private static final long serialVersionUID = 3875212506787802066L;
-        /* package-private */static final String MESSAGE = "Chain of Causes for CompositeException In Order Received =>";
 
-        @Override
-        public String getMessage() {
-            return MESSAGE;
+        ExceptionOverview(String message) {
+            super(message);
         }
-    }
 
-    private List<Throwable> getListOfCauses(Throwable ex) {
-        List<Throwable> list = new ArrayList<Throwable>();
-        Throwable root = ex.getCause();
-        if (root == null || root == ex) {
-            return list;
-        } else {
-            while (true) {
-                list.add(root);
-                Throwable cause = root.getCause();
-                if (cause == null || cause == root) {
-                    return list;
-                } else {
-                    root = cause;
-                }
-            }
+        @Override
+        public synchronized Throwable fillInStackTrace() {
+            return this;
         }
     }
 
@@ -271,24 +290,4 @@ private List<Throwable> getListOfCauses(Throwable ex) {
     public int size() {
         return exceptions.size();
     }
-
-    /**
-     * Returns the root cause of {@code e}. If {@code e.getCause()} returns {@code null} or {@code e}, just return {@code e} itself.
-     *
-     * @param e the {@link Throwable} {@code e}.
-     * @return The root cause of {@code e}. If {@code e.getCause()} returns {@code null} or {@code e}, just return {@code e} itself.
-     */
-    /*private */Throwable getRootCause(Throwable e) {
-        Throwable root = e.getCause();
-        if (root == null || e == root) {
-            return e;
-        }
-        while (true) {
-            Throwable cause = root.getCause();
-            if (cause == null || cause == root) {
-                return root;
-            }
-            root = cause;
-        }
-    }
 }
diff --git a/src/test/java/io/reactivex/rxjava3/exceptions/CompositeExceptionTest.java b/src/test/java/io/reactivex/rxjava3/exceptions/CompositeExceptionTest.java
@@ -23,7 +23,6 @@
 import org.junit.Test;
 
 import io.reactivex.rxjava3.core.RxJavaTest;
-import io.reactivex.rxjava3.exceptions.CompositeException.CompositeExceptionCausalChain;
 
 public class CompositeExceptionTest extends RxJavaTest {
 
@@ -251,41 +250,6 @@ public void messageVarargs() {
         assertEquals("3 exceptions occurred. ", compositeException.getMessage());
     }
 
-    @Test
-    public void complexCauses() {
-        Throwable e1 = new Throwable("1");
-        Throwable e2 = new Throwable("2");
-        e1.initCause(e2);
-
-        Throwable e3 = new Throwable("3");
-        Throwable e4 = new Throwable("4");
-        e3.initCause(e4);
-
-        Throwable e5 = new Throwable("5");
-        Throwable e6 = new Throwable("6");
-        e5.initCause(e6);
-
-        CompositeException compositeException = new CompositeException(e1, e3, e5);
-        assertTrue(compositeException.getCause() instanceof CompositeExceptionCausalChain);
-
-        List<Throwable> causeChain = new ArrayList<Throwable>();
-        Throwable cause = compositeException.getCause().getCause();
-        while (cause != null) {
-            causeChain.add(cause);
-            cause = cause.getCause();
-        }
-        // The original relations
-        //
-        // e1 -> e2
-        // e3 -> e4
-        // e5 -> e6
-        //
-        // will be set to
-        //
-        // e1 -> e2 -> e3 -> e4 -> e5 -> e6
-        assertEquals(Arrays.asList(e1, e2, e3, e4, e5, e6), causeChain);
-    }
-
     @Test
     public void constructorWithNull() {
         assertTrue(new CompositeException((Throwable[])null).getExceptions().get(0) instanceof NullPointerException);
@@ -306,81 +270,96 @@ public void printStackTrace() {
     }
 
     @Test
-    public void cyclicRootCause() {
-        RuntimeException te = new RuntimeException() {
-
-            private static final long serialVersionUID = -8492568224555229753L;
-            Throwable cause;
-
-            @Override
-            public Throwable initCause(Throwable c) {
-                return this;
-            }
-
-            @Override
-            public Throwable getCause() {
-                return cause;
-            }
-        };
-
-        assertSame(te, new CompositeException(te).getCause().getCause());
-
-        assertSame(te, new CompositeException(new RuntimeException(te)).getCause().getCause().getCause());
+    public void badException() {
+        Throwable e = new BadException();
+        assertSame(e, new CompositeException(e).getCause().getCause());
+        assertSame(e, new CompositeException(new RuntimeException(e)).getCause().getCause().getCause());
     }
 
     @Test
-    public void nullRootCause() {
-        RuntimeException te = new RuntimeException() {
+    public void exceptionOverview() {
+        CompositeException composite = new CompositeException(
+                new TestException("ex1"),
+                new TestException("ex2"),
+                new TestException("ex3", new TestException("ex4"))
+        );
+
+        String overview = composite.getCause().getMessage();
+
+        assertTrue(overview, overview.contains("Multiple exceptions (3)"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex1"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex2"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex3"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex4"));
+        assertTrue(overview, overview.contains("at io.reactivex.rxjava3.exceptions.CompositeExceptionTest.exceptionOverview"));
+    }
 
-            private static final long serialVersionUID = -8492568224555229753L;
+    @Test
+    public void causeWithExceptionWithoutStacktrace() {
+        CompositeException composite = new CompositeException(
+                new TestException("ex1"),
+                new CompositeException.ExceptionOverview("example")
+        );
 
-            @Override
-            public Throwable getCause() {
-                return null;
-            }
-        };
+        String overview = composite.getCause().getMessage();
 
-        assertSame(te, new CompositeException(te).getCause().getCause());
+        assertTrue(overview, overview.contains("Multiple exceptions (2)"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex1"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.CompositeException.ExceptionOverview: example"));
 
-        assertSame(te, new CompositeException(new RuntimeException(te)).getCause().getCause().getCause());
+        assertEquals(overview, 2, overview.split("at\\s").length);
     }
 
     @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());
+    public void reoccurringException() {
+        TestException ex0 = new TestException("ex0");
+        TestException ex1 = new TestException("ex1", ex0);
+        CompositeException composite = new CompositeException(
+                ex1,
+                new TestException("ex2", ex1)
+        );
+
+        String overview = composite.getCause().getMessage();
+        System.err.println(overview);
+
+        assertTrue(overview, overview.contains("Multiple exceptions (2)"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex0"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex1"));
+        assertTrue(overview, overview.contains("io.reactivex.rxjava3.exceptions.TestException: ex2"));
+        assertTrue(overview, overview.contains("(cause not expanded again) io.reactivex.rxjava3.exceptions.TestException: ex0"));
+        assertEquals(overview, 5, overview.split("at\\s").length);
     }
 
     @Test
-    public void rootCauseEval() {
-        final TestException ex0 = new TestException();
-        Throwable throwable = new Throwable() {
-
-            private static final long serialVersionUID = 3597694032723032281L;
-
-            @Override
-            public synchronized Throwable getCause() {
-                return ex0;
-            }
-        };
-        CompositeException ex = new CompositeException(throwable);
-        assertSame(ex0, ex.getRootCause(ex));
+    public void nestedMultilineMessage() {
+        TestException ex1 = new TestException("ex1");
+        TestException ex2 = new TestException("ex2");
+        CompositeException composite1 = new CompositeException(
+                ex1,
+                ex2
+        );
+        TestException ex3 = new TestException("ex3");
+        TestException ex4 = new TestException("ex4", composite1);
+
+        CompositeException composite2 = new CompositeException(
+                ex3,
+                ex4
+        );
+
+        String overview = composite2.getCause().getMessage();
+        System.err.println(overview);
+
+        assertTrue(overview, overview.contains("        Multiple exceptions (2)"));
+        assertTrue(overview, overview.contains("        |-- io.reactivex.rxjava3.exceptions.TestException: ex1"));
+        assertTrue(overview, overview.contains("        |-- io.reactivex.rxjava3.exceptions.TestException: ex2"));
     }
 
     @Test
-    public void rootCauseSelf() {
-        Throwable throwable = new Throwable() {
+    public void singleExceptionIsTheCause() {
+        TestException ex = new TestException("ex1");
+        CompositeException composite = new CompositeException(ex);
 
-            private static final long serialVersionUID = -4398003222998914415L;
-
-            @Override
-            public synchronized Throwable getCause() {
-                return this;
-            }
-        };
-        CompositeException tmp = new CompositeException(new TestException());
-        assertSame(throwable, tmp.getRootCause(throwable));
+        assertSame(composite.getCause(), ex);
     }
 }
 
