From 632ce294e5af5a6cbba8a35181e189ffd55665d2 Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Thu, 3 Dec 2015 21:07:16 -0500 Subject: [PATCH 1/2] Improves unexpected throwable handling --- .../wpi/grip/core/events/FatalErrorEvent.java | 19 -------- .../core/events/UnexpectedThrowableEvent.java | 44 +++++++++++++++++++ .../wpi/grip/core/sources/CameraSource.java | 11 ++--- .../java/edu/wpi/grip/ui/ExceptionAlert.java | 41 ++++++++--------- ui/src/main/java/edu/wpi/grip/ui/Main.java | 25 +++++++---- .../wpi/grip/ui/pipeline/AddSourceView.java | 10 ++--- 6 files changed, 86 insertions(+), 64 deletions(-) delete mode 100644 core/src/main/java/edu/wpi/grip/core/events/FatalErrorEvent.java create mode 100644 core/src/main/java/edu/wpi/grip/core/events/UnexpectedThrowableEvent.java 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 deleted file mode 100644 index 75ebad8955..0000000000 --- a/core/src/main/java/edu/wpi/grip/core/events/FatalErrorEvent.java +++ /dev/null @@ -1,19 +0,0 @@ -package edu.wpi.grip.core.events; - -/** - * Event should be thrown when a fatal event occurs. - */ -public class FatalErrorEvent { - private Throwable throwable; - - /** - * @param throwable The exception that was caught - */ - public FatalErrorEvent(Throwable throwable) { - this.throwable = throwable; - } - - public Throwable getThrowable() { - return throwable; - } -} 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 new file mode 100644 index 0000000000..7e4539ae24 --- /dev/null +++ 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 index e8aa2ce592..05046e4408 100644 --- 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/ui/src/main/java/edu/wpi/grip/ui/ExceptionAlert.java b/ui/src/main/java/edu/wpi/grip/ui/ExceptionAlert.java index 146e551095..3858e70fad 100644 --- 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,11 @@ public final class ExceptionAlert extends Alert { private final String exceptionMessage; private final String systemInfoMessage; + 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 +60,21 @@ public final class ExceptionAlert extends Alert { * the issue website. * @see Inspiration */ - 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.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 +93,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 +154,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 getAdditionalInfoMessage() { + return "Message: " + message + "\n"; } /** @@ -177,7 +173,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 +199,9 @@ 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(); + final StringBuilder builder = new StringBuilder(ISSUE_PROMPT_QUESTION).append("\n\n\n\n") + .append(getAdditionalInfoMessage()).append("\n").append(systemInfoMessage).append(exceptionMessage); + return builder.toString(); } diff --git a/ui/src/main/java/edu/wpi/grip/ui/Main.java b/ui/src/main/java/edu/wpi/grip/ui/Main.java index aec3661dc5..94e6c32048 100644 --- 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,18 +42,22 @@ 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(); @@ -62,5 +65,11 @@ public final void onFatalErrorEvent(FatalErrorEvent error) { } } }); + + 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 index b5899f9f57..293932b349 100644 --- 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,7 +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)); + eventBus.post(new UnexpectedThrowableEvent(e, "Tried to create an invalid source")); e.printStackTrace(); } }); @@ -120,8 +119,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")); } }); }); From f97b9fa9b7ed49f9e2b9b6d83464e8b4e4a76c63 Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Mon, 7 Dec 2015 17:03:41 -0500 Subject: [PATCH 2/2] Remove StackTrace print from AddSourcesView --- .../grip/core/events/UnexpectedThrowableEvent.java | 2 +- gradle/wrapper/gradle-wrapper.properties | 2 +- .../main/java/edu/wpi/grip/ui/ExceptionAlert.java | 13 +++++++++---- .../edu/wpi/grip/ui/pipeline/AddSourceView.java | 1 - 4 files changed, 11 insertions(+), 7 deletions(-) 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 index 7e4539ae24..ce48aadb05 100644 --- a/core/src/main/java/edu/wpi/grip/core/events/UnexpectedThrowableEvent.java +++ b/core/src/main/java/edu/wpi/grip/core/events/UnexpectedThrowableEvent.java @@ -31,7 +31,7 @@ public Throwable getThrowable() { return throwable; } - public String getMessage(){ + public String getMessage() { return message; } diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index ba79ec704b..4879791ba2 100644 --- 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 index 3858e70fad..3a3fa5b556 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/ExceptionAlert.java +++ b/ui/src/main/java/edu/wpi/grip/ui/ExceptionAlert.java @@ -47,6 +47,7 @@ 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; @@ -71,6 +72,7 @@ public ExceptionAlert(final Parent root, final Throwable throwable, final String this.exceptionMessage = generateExceptionMessage(throwable); this.systemInfoMessage = generateSystemInfoMessage(); + this.additionalInfoMessage = generateAdditionalInfoMessage(); this.initialCause = getInitialCause(throwable); this.setTitle(initialCause.getClass().getSimpleName()); @@ -162,7 +164,7 @@ private Throwable getInitialCause(Throwable throwable) { } } - private String getAdditionalInfoMessage() { + private String generateAdditionalInfoMessage() { return "Message: " + message + "\n"; } @@ -199,9 +201,12 @@ private String generateSystemInfoMessage() { * @return The fully constructed issue text. */ private String issueText() { - final StringBuilder builder = new StringBuilder(ISSUE_PROMPT_QUESTION).append("\n\n\n\n") - .append(getAdditionalInfoMessage()).append("\n").append(systemInfoMessage).append(exceptionMessage); - return builder.toString(); + return ISSUE_PROMPT_QUESTION + + "\n\n\n\n" + + additionalInfoMessage + + "\n" + + systemInfoMessage + + exceptionMessage; } 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 index 293932b349..bdb045d154 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/pipeline/AddSourceView.java +++ b/ui/src/main/java/edu/wpi/grip/ui/pipeline/AddSourceView.java @@ -78,7 +78,6 @@ public AddSourceView(EventBus eventBus) { eventBus.post(new SourceAddedEvent(source)); } catch (IOException e) { eventBus.post(new UnexpectedThrowableEvent(e, "Tried to create an invalid source")); - e.printStackTrace(); } }); });