Improve exception messages for failures in GlideFutures.

bumptech · Dec 27, 2017 · 7d35039 · 7d35039
1 parent ded8f77
commit 7d35039
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 41 deletions.
diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/test/ConcurrencyHelper.java b/instrumentation/src/androidTest/java/com/bumptech/glide/test/ConcurrencyHelper.java
@@ -43,7 +43,7 @@ public boolean await(long timeout, TimeUnit timeUnit) throws InterruptedExceptio
           reference.set(future.get(timeout, timeUnit));
           return true;
         } catch (ExecutionException e) {
-          throw new RuntimeException(e);
+          throw new RuntimeException(e.getCause());
         } catch (TimeoutException e) {
           return false;
         }

diff --git a/library/src/main/java/com/bumptech/glide/load/engine/GlideException.java b/library/src/main/java/com/bumptech/glide/load/engine/GlideException.java
@@ -26,6 +26,7 @@ public final class GlideException extends Exception {
   private Key key;
   private DataSource dataSource;
   private Class<?> dataClass;
+  private String detailMessage;
 
   public GlideException(String message) {
     this(message, Collections.<Throwable>emptyList());
@@ -36,7 +37,7 @@ public GlideException(String detailMessage, Throwable cause) {
   }
 
   public GlideException(String detailMessage, List<Throwable> causes) {
-    super(detailMessage);
+    this.detailMessage = detailMessage;
     setStackTrace(EMPTY_ELEMENTS);
     this.causes = causes;
   }
@@ -51,6 +52,8 @@ void setLoggingDetails(Key key, DataSource dataSource, Class<?> dataClass) {
     this.dataClass = dataClass;
   }
 
+
+
   // No need to synchronize when doing nothing whatsoever.
   @SuppressWarnings("UnsynchronizedOverridesSynchronized")
   @Override
@@ -131,12 +134,30 @@ private void printStackTrace(Appendable appendable) {
     appendCauses(getCauses(), new IndentedAppendable(appendable));
   }
 
+  // PMD doesn't seem to notice that we're allocating the builder with the suggested size.
+  @SuppressWarnings("PMD.InsufficientStringBufferDeclaration")
   @Override
   public String getMessage() {
-    return super.getMessage()
-        + (dataClass != null ? ", " + dataClass : "")
-        + (dataSource != null ? ", " + dataSource : "")
-        + (key != null ? ", " + key : "");
+    StringBuilder result = new StringBuilder(71)
+        .append(detailMessage)
+        .append(dataClass != null ? ", " + dataClass : "")
+        .append(dataSource != null ? ", " + dataSource : "")
+        .append(key != null ? ", " + key : "");
+
+    List<Throwable> rootCauses = getRootCauses();
+    if (rootCauses.isEmpty()) {
+      return result.toString();
+    } else if (rootCauses.size() == 1) {
+      result.append("\nThere was 1 cause:");
+    } else {
+      result.append("\nThere were ").append(rootCauses.size()).append(" causes:");
+    }
+    for (Throwable cause : rootCauses) {
+      result.append('\n')
+          .append(cause.getClass().getName()).append('(').append(cause.getMessage()).append(')');
+    }
+    result.append("\n call GlideException#logRootCauses(String) for more detail");
+    return result.toString();
   }
 
   // Appendable throws, PrintWriter, PrintStream, and IndentedAppendable do not, so this should

diff --git a/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java b/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java
@@ -11,8 +11,6 @@
 import com.bumptech.glide.request.target.Target;
 import com.bumptech.glide.request.transition.Transition;
 import com.bumptech.glide.util.Util;
-import java.io.PrintStream;
-import java.io.PrintWriter;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
@@ -211,7 +209,7 @@ private synchronized R doGet(Long timeoutMillis)
     if (Thread.interrupted()) {
       throw new InterruptedException();
     } else if (loadFailed) {
-      throw new GlideExecutionException(exception);
+      throw new ExecutionException(exception);
     } else if (isCancelled) {
       throw new CancellationException();
     } else if (!resultReceived) {
@@ -272,7 +270,6 @@ public synchronized boolean onResourceReady(
 
   @VisibleForTesting
   static class Waiter {
-
     // This is a simple wrapper class that is used to enable testing. The call to the wrapping class
     // is waited on appropriately.
     @SuppressWarnings("WaitNotInLoop")
@@ -284,35 +281,4 @@ void notifyAll(Object toNotify) {
       toNotify.notifyAll();
     }
   }
-
-  private static class GlideExecutionException extends ExecutionException {
-    private static final long serialVersionUID = 1L;
-
-
-    private final GlideException cause;
-
-    GlideExecutionException(GlideException cause) {
-      super();
-      this.cause = cause;
-    }
-
-    @Override
-    public void printStackTrace() {
-      printStackTrace(System.err);
-    }
-
-    @Override
-    public void printStackTrace(PrintStream s) {
-      super.printStackTrace(s);
-      s.print("Caused by: ");
-      cause.printStackTrace(s);
-    }
-
-    @Override
-    public void printStackTrace(PrintWriter s) {
-      super.printStackTrace(s);
-      s.print("Caused by: ");
-      cause.printStackTrace(s);
-    }
-  }
 }