Merge pull request #179 from JLLeitschuh/chore/betterErrorAndExceptionHandling

ThomasJClark · ThomasJClark · commit 8663b73c6f27 · 2015-12-08T15:11:09.000-05:00
Improves unexpected throwable handling
diff --git a/core/src/main/java/edu/wpi/grip/core/events/FatalErrorEvent.java b/core/src/main/java/edu/wpi/grip/core/events/FatalErrorEvent.java
diff --git a/core/src/main/java/edu/wpi/grip/core/events/UnexpectedThrowableEvent.java b/core/src/main/java/edu/wpi/grip/core/events/UnexpectedThrowableEvent.java
@@ -0,0 +1,44 @@
+package edu.wpi.grip.core.events;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * Event should be thrown when Unexpected Throwable ends up in an {@link Thread#uncaughtExceptionHandler}.
+ */
+public final class UnexpectedThrowableEvent {
+    private final Throwable throwable;
+    private final boolean fatal;
+    private final String message;
+
+    /**
+     * @param throwable The throwable that was caught.
+     * @param message   Any additional information that should be displayed to the user. Nullable.
+     * @param fatal     True if this cause the application to quit forcibly.
+     *                  If the throwable is an {@link Error} than this is automatically true. Defaults to false.
+     */
+    public UnexpectedThrowableEvent(Throwable throwable, String message, boolean fatal) {
+        this.throwable = checkNotNull(throwable, "Throwable can not be null");
+        this.message = checkNotNull(message, "Message can not be null");
+        this.fatal = (throwable instanceof Error) || fatal;
+    }
+
+    public UnexpectedThrowableEvent(Throwable throwable, String message) {
+        this(throwable, message, false);
+    }
+
+
+    public Throwable getThrowable() {
+        return throwable;
+    }
+
+    public String getMessage() {
+        return message;
+    }
+
+    /**
+     * @return True if this should cause the program to shutdown after it is handled
+     */
+    public boolean isFatal() {
+        return fatal;
+    }
+}
diff --git a/core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java b/core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java
@@ -9,7 +9,7 @@
 import edu.wpi.grip.core.SocketHint;
 import edu.wpi.grip.core.SocketHints;
 import edu.wpi.grip.core.Source;
-import edu.wpi.grip.core.events.FatalErrorEvent;
+import edu.wpi.grip.core.events.UnexpectedThrowableEvent;
 import edu.wpi.grip.core.events.SourceRemovedEvent;
 import org.bytedeco.javacpp.opencv_core.Mat;
 import org.bytedeco.javacv.*;
@@ -172,16 +172,11 @@ private synchronized void startVideo(FrameGrabber grabber) throws IOException {
         }, "Camera");
         frameExecutor.setUncaughtExceptionHandler(
                 (thread, exception) -> {
-                    // TODO Pass Exception to the UI.
-                    System.err.println("Webcam Frame Grabber Thread crashed with uncaught exception:");
-                    exception.printStackTrace();
-                    eventBus.post(new FatalErrorEvent(exception));
+                    eventBus.post(new UnexpectedThrowableEvent(exception, "Webcam Frame Grabber Thread crashed with uncaught exception"));
                     try {
                         stopVideo();
                     } catch (TimeoutException e) {
-                        System.err.println("Webcam Frame Grabber could not be stopped!");
-                        e.printStackTrace();
-                        eventBus.post(new FatalErrorEvent(e));
+                        eventBus.post(new UnexpectedThrowableEvent(e, "Webcam Frame Grabber could not be stopped!"));
                     }
                 }
         );
diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
@@ -1,4 +1,4 @@
-#Fri Nov 20 19:04:57 EST 2015
+#Tue Dec 08 15:02:34 EST 2015
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME
diff --git a/ui/src/main/java/edu/wpi/grip/ui/ExceptionAlert.java b/ui/src/main/java/edu/wpi/grip/ui/ExceptionAlert.java
@@ -1,5 +1,6 @@
 package edu.wpi.grip.ui;
 
+import com.google.common.base.Throwables;
 import com.google.common.net.UrlEscapers;
 import javafx.application.HostServices;
 import javafx.application.Platform;
@@ -11,9 +12,6 @@
 import javafx.scene.input.ClipboardContent;
 import javafx.scene.layout.GridPane;
 
-import java.io.PrintWriter;
-import java.io.StringWriter;
-
 import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
@@ -49,11 +47,12 @@ public final class ExceptionAlert extends Alert {
 
     private final String exceptionMessage;
     private final String systemInfoMessage;
+    private final String additionalInfoMessage;
+    private final String message;
     private final Throwable initialCause;
 
     private final ButtonType openGitHubIssuesBtnType = new ButtonType("Open GitHub Issues");
     private final ButtonType copyToClipboardBtnType = new ButtonType("Copy To Clipboard");
-    private final ButtonType closeBtnType = new ButtonType("Cancel", ButtonBar.ButtonData.CANCEL_CLOSE);
     private final Node initialFocusElement;
 
     /**
@@ -62,17 +61,22 @@ public final class ExceptionAlert extends Alert {
      *                  the issue website.
      * @see <a href="http://code.makery.ch/blog/javafx-dialogs-official/">Inspiration</a>
      */
-    public ExceptionAlert(final Parent root, final Throwable throwable, final HostServices services) {
+    public ExceptionAlert(final Parent root, final Throwable throwable, final String message, boolean isFatal, final HostServices services) {
         super(AlertType.ERROR);
+        checkNotNull(root, "The parent can not be null");
         checkNotNull(throwable, "The Throwable can not be null");
+        this.message = checkNotNull(message, "The message can not be null");
         checkNotNull(services, "HostServices can not be null");
 
+        final ButtonType closeBtnType = new ButtonType(isFatal ? "Quit" : "Cancel", ButtonBar.ButtonData.CANCEL_CLOSE);
+
         this.exceptionMessage = generateExceptionMessage(throwable);
         this.systemInfoMessage = generateSystemInfoMessage();
-        this.initialCause = generateInitialCause(throwable);
+        this.additionalInfoMessage = generateAdditionalInfoMessage();
+        this.initialCause = getInitialCause(throwable);
 
         this.setTitle(initialCause.getClass().getSimpleName());
-        this.setHeaderText(initialCause.getMessage());
+        this.setHeaderText((isFatal ? "FATAL: " : "") + message);
 
         // Set stylesheet
         this.getDialogPane().styleProperty().bind(root.styleProperty());
@@ -91,7 +95,7 @@ public ExceptionAlert(final Parent root, final Throwable throwable, final HostSe
         final Label issuePasteLabel = new Label(ISSUE_PROMPT_TEXT);
         issuePasteLabel.setWrapText(true);
 
-        final TextArea issueText = new TextArea(stackTrace(throwable));
+        final TextArea issueText = new TextArea(Throwables.getStackTraceAsString(throwable));
         issuePasteLabel.setLabelFor(issueText);
         issueText.setEditable(false);
         issueText.setWrapText(true);
@@ -152,22 +156,16 @@ public final void setInitialFocus() {
      * @param throwable The throwable to iterate through.
      * @return The initial throwable
      */
-    private Throwable generateInitialCause(Throwable throwable) {
+    private Throwable getInitialCause(Throwable throwable) {
         if (throwable.getCause() == null) {
             return throwable;
         } else {
-            return generateInitialCause(throwable.getCause());
+            return getInitialCause(throwable.getCause());
         }
     }
 
-    /**
-     * Generates the Throwable's stack trace as a string.
-     */
-    private String stackTrace(Throwable throwable) {
-        final StringWriter sw = new StringWriter();
-        final PrintWriter pw = new PrintWriter(sw);
-        throwable.printStackTrace(pw);
-        return sw.toString();
+    private String generateAdditionalInfoMessage() {
+        return "Message: " + message + "\n";
     }
 
     /**
@@ -177,7 +175,7 @@ private String stackTrace(Throwable throwable) {
      * @return The markdown for the exception.
      */
     private String generateExceptionMessage(Throwable throwable) {
-        return new StringBuilder(stackTrace(throwable)
+        return new StringBuilder(Throwables.getStackTraceAsString(throwable)
                 /* Allow users to maintain anonymity */
                 .replace(System.getProperty("user.home"), "$HOME").replace(System.getProperty("user.name"), "$USER"))
                 .insert(0, "## Stack Trace:\n```java\n").append("\n```").toString();
@@ -203,10 +201,12 @@ private String generateSystemInfoMessage() {
      * @return The fully constructed issue text.
      */
     private String issueText() {
-        return new StringBuilder(ISSUE_PROMPT_QUESTION)
-                .append("\n\n\n\n")
-                .append(systemInfoMessage)
-                .append(exceptionMessage).toString();
+        return ISSUE_PROMPT_QUESTION
+                + "\n\n\n\n"
+                + additionalInfoMessage
+                + "\n"
+                + systemInfoMessage
+                + exceptionMessage;
     }
 
 
diff --git a/ui/src/main/java/edu/wpi/grip/ui/Main.java b/ui/src/main/java/edu/wpi/grip/ui/Main.java
@@ -3,7 +3,7 @@
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.Subscribe;
 import com.sun.javafx.application.PlatformImpl;
-import edu.wpi.grip.core.events.FatalErrorEvent;
+import edu.wpi.grip.core.events.UnexpectedThrowableEvent;
 import edu.wpi.grip.ui.util.DPIUtility;
 import javafx.application.Application;
 import javafx.scene.Parent;
@@ -13,7 +13,7 @@
 
 public class Main extends Application {
     private final EventBus eventBus = new EventBus((exception, context) -> {
-        this.onFatalErrorEvent(new FatalErrorEvent(exception));
+        this.triggerUnexpectedThrowableEvent(new UnexpectedThrowableEvent(exception, "An Event Bus subscriber threw an uncaught exception"));
     });
 
     private final Object dialogLock = new Object();
@@ -31,8 +31,7 @@ public void start(Stage stage) {
          * Any exceptions thrown by the UI will be caught here and an exception dialog will be displayed
          */
         Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> {
-            this.eventBus.post(new FatalErrorEvent(throwable));
-
+            this.eventBus.post(new UnexpectedThrowableEvent(throwable, "The UI Thread threw an uncaught exception"));
         });
 
         root.setStyle("-fx-font-size: " + DPIUtility.FONT_SIZE + "px");
@@ -43,24 +42,34 @@ public void start(Stage stage) {
         stage.show();
     }
 
+    private void triggerUnexpectedThrowableEvent(UnexpectedThrowableEvent event) {
+        eventBus.post(event);
+    }
+
     @Subscribe
-    public final void onFatalErrorEvent(FatalErrorEvent error) {
+    public final void onUnexpectedThrowableEvent(UnexpectedThrowableEvent event) {
         // Print throwable before showing the exception so that errors are in order in the console
-        error.getThrowable().printStackTrace();
+        event.getThrowable().printStackTrace();
         PlatformImpl.runAndWait(() -> {
             synchronized (this.dialogLock) {
                 try {
                     // Don't create more than one exception dialog at the same time
-                    final ExceptionAlert exceptionAlert = new ExceptionAlert(root, error.getThrowable(), getHostServices());
+                    final ExceptionAlert exceptionAlert = new ExceptionAlert(root, event.getThrowable(), event.getMessage(), event.isFatal(), getHostServices());
                     exceptionAlert.setInitialFocus();
                     exceptionAlert.showAndWait();
-                } catch (Exception e) {
+                } catch (Throwable e) {
                     // Well in this case something has gone very, very wrong
                     // We don't want to create a feedback loop either.
                     e.printStackTrace();
                     System.exit(1); // Ensure we shut down the application if we get an exception
                 }
             }
         });
+
+        if(event.isFatal()) {
+            System.err.println("Original fatal exception");
+            event.getThrowable().printStackTrace();
+            System.exit(1);
+        }
     }
 }
diff --git a/ui/src/main/java/edu/wpi/grip/ui/pipeline/AddSourceView.java b/ui/src/main/java/edu/wpi/grip/ui/pipeline/AddSourceView.java
@@ -1,7 +1,7 @@
 package edu.wpi.grip.ui.pipeline;
 
 import com.google.common.eventbus.EventBus;
-import edu.wpi.grip.core.events.FatalErrorEvent;
+import edu.wpi.grip.core.events.UnexpectedThrowableEvent;
 import edu.wpi.grip.core.events.SourceAddedEvent;
 import edu.wpi.grip.core.sources.CameraSource;
 import edu.wpi.grip.core.sources.ImageFileSource;
@@ -51,8 +51,7 @@ public AddSourceView(EventBus eventBus) {
                 try {
                     eventBus.post(new SourceAddedEvent(new ImageFileSource(eventBus, file)));
                 } catch (IOException e) {
-                    e.printStackTrace();
-                    eventBus.post(new FatalErrorEvent(e));
+                    eventBus.post(new UnexpectedThrowableEvent(e, "Tried to create an invalid source"));
                 }
             });
         });
@@ -78,8 +77,7 @@ public AddSourceView(EventBus eventBus) {
                     final CameraSource source = new CameraSource(eventBus, cameraIndex.getValue());
                     eventBus.post(new SourceAddedEvent(source));
                 } catch (IOException e) {
-                    eventBus.post(new FatalErrorEvent(e));
-                    e.printStackTrace();
+                    eventBus.post(new UnexpectedThrowableEvent(e, "Tried to create an invalid source"));
                 }
             });
         });
@@ -120,8 +118,7 @@ public AddSourceView(EventBus eventBus) {
                     final CameraSource source = new CameraSource(eventBus, cameraAddress.getText());
                     eventBus.post(new SourceAddedEvent(source));
                 } catch (IOException e) {
-                    eventBus.post(new FatalErrorEvent(e));
-                    e.printStackTrace();
+                    eventBus.post(new UnexpectedThrowableEvent(e, "Tried to create an invalid source"));
                 }
             });
         });

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-#Fri Nov 20 19:04:57 EST 2015`
	`1`	`+#Tue Dec 08 15:02:34 EST 2015`
`2`	`2`	`distributionBase=GRADLE_USER_HOME`
`3`	`3`	`distributionPath=wrapper/dists`
`4`	`4`	`zipStoreBase=GRADLE_USER_HOME`