From a96e91919cb16b47706e51a7ad070c2c34a53efb Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Mon, 12 Jan 2015 22:22:32 +0000 Subject: [PATCH 1/2] Tidy up some error handling. --- .../dmdirc/logger/ProgramErrorManager.java | 43 ++++++------------- .../logger/SentryLoggingErrorManager.java | 1 - .../ui/core/errors/CoreErrorsDialogModel.java | 9 +++- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/com/dmdirc/logger/ProgramErrorManager.java b/src/com/dmdirc/logger/ProgramErrorManager.java index 9469b0de5..caebd1129 100644 --- a/src/com/dmdirc/logger/ProgramErrorManager.java +++ b/src/com/dmdirc/logger/ProgramErrorManager.java @@ -23,11 +23,13 @@ package com.dmdirc.logger; import com.dmdirc.DMDircMBassador; +import com.dmdirc.events.AppErrorEvent; import com.dmdirc.events.ErrorEvent; import com.dmdirc.events.FatalProgramErrorEvent; import com.dmdirc.events.NonFatalProgramErrorEvent; import com.dmdirc.events.ProgramErrorDeletedEvent; import com.dmdirc.events.ProgramErrorEvent; +import com.dmdirc.events.UserErrorEvent; import com.dmdirc.util.EventUtils; import com.google.common.base.Throwables; @@ -52,12 +54,6 @@ @Singleton public class ProgramErrorManager { - /** A list of exceptions which we don't consider bugs and thus don't report. */ - private static final Class[] BANNED_EXCEPTIONS = new Class[]{ - NoSuchMethodError.class, NoClassDefFoundError.class, - UnsatisfiedLinkError.class, AbstractMethodError.class, - IllegalAccessError.class, OutOfMemoryError.class, - NoSuchFieldError.class,}; /** The event bus to listen for errors on. */ private final DMDircMBassador eventBus; /** The current list of errors. */ @@ -81,9 +77,18 @@ public void initialise() { } @Handler - void handleErrorEvent(final ErrorEvent event) { + void handleAppError(final AppErrorEvent event) { + handleErrorEvent(event, true); + } + + @Handler + void handleUserError(final UserErrorEvent event) { + handleErrorEvent(event, false); + } + + private void handleErrorEvent(final ErrorEvent event, final boolean appError) { final ProgramError error = addError(event.getLevel(), event.getMessage(), - event.getThrowable(), event.getDetails(), isValidError(event.getThrowable())); + event.getThrowable(), event.getDetails(), appError); if (error.getLevel() == ErrorLevel.FATAL) { eventBus.publish(new FatalProgramErrorEvent(error)); } else { @@ -160,26 +165,4 @@ public void deleteAll() { public Set getErrors() { return Collections.unmodifiableSet(errors); } - - /** - * Determines whether or not the specified exception is one that we are willing to report. - * - * @param exception The exception to test - * - * @since 0.6.3m1 - * @return True if the exception may be reported, false otherwise - */ - private boolean isValidError(final Throwable exception) { - // TODO: Dedupe this from here and SentryLoggingErrorManager - Throwable target = exception; - while (target != null) { - for (Class bad : BANNED_EXCEPTIONS) { - if (bad.equals(target.getClass())) { - return false; - } - } - target = target.getCause(); - } - return true; - } } diff --git a/src/com/dmdirc/logger/SentryLoggingErrorManager.java b/src/com/dmdirc/logger/SentryLoggingErrorManager.java index 5b5db25af..fd221c311 100644 --- a/src/com/dmdirc/logger/SentryLoggingErrorManager.java +++ b/src/com/dmdirc/logger/SentryLoggingErrorManager.java @@ -149,7 +149,6 @@ private Optional getSourceLine(final Collection trace) { * @return True if the exception may be reported, false otherwise */ private boolean isValidError(final Throwable exception) { - // TODO: Dedupe this from here and ProgramErrorManager Throwable target = exception; while (target != null) { for (Class bad : BANNED_EXCEPTIONS) { diff --git a/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java b/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java index 9ae22ad20..6e6713a89 100644 --- a/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java +++ b/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java @@ -29,6 +29,7 @@ import com.dmdirc.interfaces.ui.ErrorsDialogModel; import com.dmdirc.interfaces.ui.ErrorsDialogModelListener; import com.dmdirc.logger.ErrorManager; +import com.dmdirc.logger.ErrorReportStatus; import com.dmdirc.logger.ProgramError; import com.dmdirc.util.collections.ListenerList; @@ -122,7 +123,13 @@ public boolean isDeleteAllAllowed() { @Override public boolean isSendAllowed() { - return selectedError.isPresent(); + if (selectedError.isPresent()) { + final ErrorReportStatus status = selectedError.map(DisplayableError::getReportStatus) + .orElse(ErrorReportStatus.NOT_APPLICABLE); + return status == ErrorReportStatus.WAITING || status == ErrorReportStatus.ERROR; + } else { + return false; + } } @Override From bad2813641a4ff24d7c09eb1e388eb9edf3fbc4e Mon Sep 17 00:00:00 2001 From: Greg Holmes Date: Mon, 12 Jan 2015 22:26:33 +0000 Subject: [PATCH 2/2] Simplify some logic. --- .../dmdirc/ui/core/errors/CoreErrorsDialogModel.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java b/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java index 6e6713a89..1a5a6ea36 100644 --- a/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java +++ b/src/com/dmdirc/ui/core/errors/CoreErrorsDialogModel.java @@ -123,13 +123,9 @@ public boolean isDeleteAllAllowed() { @Override public boolean isSendAllowed() { - if (selectedError.isPresent()) { - final ErrorReportStatus status = selectedError.map(DisplayableError::getReportStatus) - .orElse(ErrorReportStatus.NOT_APPLICABLE); - return status == ErrorReportStatus.WAITING || status == ErrorReportStatus.ERROR; - } else { - return false; - } + final ErrorReportStatus status = selectedError.map(DisplayableError::getReportStatus) + .orElse(ErrorReportStatus.NOT_APPLICABLE); + return status == ErrorReportStatus.WAITING || status == ErrorReportStatus.ERROR; } @Override