From 5ceb6c1e729be7dffd252acad4949944d111f009 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Thu, 17 Mar 2016 20:34:07 +0100 Subject: Improve error reporting in FetchGerritChangePage If a connection failure occurs during the change number content assist, the user feedback was poor. Some quick flickering of the progress bar and that was it. The error log contained a stack trace starting with two InvocationTargetExceptions without exception messages, which isn't exactly helpful either. Improve this in two ways: * Generally skip InvocationTargetExceptions with empty messages when reporting or logging errors. * In the case of the content proposal in the FetchGerritChangePage, show the error to the user. * Move the error reporting methods in Activator together. I completely missed at first that there was a createErrorStatus() further below... Change-Id: I9fa6bb6207539cff1c98850f7abf5d127077115c Signed-off-by: Thomas Wolf --- .../src/org/eclipse/egit/ui/Activator.java | 125 +++++++++++++-------- .../ui/internal/fetch/FetchGerritChangePage.java | 2 +- 2 files changed, 81 insertions(+), 46 deletions(-) diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java index 6ccb9a48b2..1d37155c71 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java @@ -14,6 +14,7 @@ package org.eclipse.egit.ui; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.net.Authenticator; import java.net.ProxySelector; import java.util.ArrayList; @@ -111,11 +112,47 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener return getDefault().getBundle().getSymbolicName(); } + /** + * Creates an {@link IStatus} from the parameters. If the throwable is an + * {@link InvocationTargetException}, the status is created from the first + * exception that is either not an InvocationTargetException or that has a + * message. If the message passed is empty, tries to supply a message from + * that exception. + * + * @param severity + * of the {@link IStatus} + * @param message + * for the status + * @param throwable + * that caused the status, may be {@code null} + * @return the status + */ + private static IStatus toStatus(int severity, String message, + Throwable throwable) { + Throwable exc = throwable; + while (exc instanceof InvocationTargetException) { + String msg = exc.getLocalizedMessage(); + if (msg != null && !msg.isEmpty()) { + break; + } + Throwable cause = exc.getCause(); + if (cause == null) { + break; + } + exc = cause; + } + if (exc != null && (message == null || message.isEmpty())) { + message = exc.getLocalizedMessage(); + } + return new Status(severity, getPluginId(), message, exc); + } + /** * Handle an error. The error is logged. If show is * true the error is shown to the user. * - * @param message a localized message + * @param message + * a localized message * @param throwable * @param show */ @@ -138,8 +175,7 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener */ public static void handleIssue(int severity, String message, Throwable throwable, boolean show) { - IStatus status = new Status(severity, getPluginId(), message, - throwable); + IStatus status = toStatus(severity, message, throwable); int style = StatusManager.LOG; if (show) style |= StatusManager.SHOW; @@ -154,8 +190,7 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener * @param throwable */ public static void showError(String message, Throwable throwable) { - IStatus status = new Status(IStatus.ERROR, getPluginId(), message, - throwable); + IStatus status = toStatus(IStatus.ERROR, message, throwable); StatusManager.getManager().handle(status, StatusManager.SHOW); } @@ -170,6 +205,46 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener StatusManager.getManager().handle(status, StatusManager.SHOW); } + /** + * @param message + * @param e + */ + public static void logError(String message, Throwable e) { + handleError(message, e, false); + } + + /** + * @param message + * @param e + */ + public static void error(String message, Throwable e) { + handleError(message, e, false); + } + + /** + * Creates an error status + * + * @param message + * a localized message + * @param throwable + * @return a new Status object + */ + public static IStatus createErrorStatus(String message, + Throwable throwable) { + return toStatus(IStatus.ERROR, message, throwable); + } + + /** + * Creates an error status + * + * @param message + * a localized message + * @return a new Status object + */ + public static IStatus createErrorStatus(String message) { + return toStatus(IStatus.ERROR, message, null); + } + /** * Get the theme used by this plugin. * @@ -678,46 +753,6 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener KnownHosts.store(); super.saveDialogSettings(); } - - /** - * @param message - * @param e - */ - public static void logError(String message, Throwable e) { - handleError(message, e, false); - } - - /** - * @param message - * @param e - */ - public static void error(String message, Throwable e) { - handleError(message, e, false); - } - - /** - * Creates an error status - * - * @param message - * a localized message - * @param throwable - * @return a new Status object - */ - public static IStatus createErrorStatus(String message, Throwable throwable) { - return new Status(IStatus.ERROR, getPluginId(), message, throwable); - } - - /** - * Creates an error status - * - * @param message - * a localized message - * @return a new Status object - */ - public static IStatus createErrorStatus(String message) { - return new Status(IStatus.ERROR, getPluginId(), message); - } - /** * @return the {@link RepositoryUtil} instance */ diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java index 35efab3479..557d254659 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java @@ -889,7 +889,7 @@ public class FetchGerritChangePage extends WizardPage { try { proposals = getRefsForContentAssist(); } catch (InvocationTargetException e) { - Activator.handleError(e.getMessage(), e, false); + Activator.handleError(e.getMessage(), e, true); return null; } catch (InterruptedException e) { return null; -- cgit v1.2.3