Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2016-03-17 15:34:07 -0400
committerMatthias Sohn2016-03-21 20:48:04 -0400
commit5ceb6c1e729be7dffd252acad4949944d111f009 (patch)
tree89625685ad90922d0101baa36c5e6e54b8fac0a5
parentd4caff79c066adb726f89a5f92ce6329d82a6176 (diff)
downloadegit-5ceb6c1e729be7dffd252acad4949944d111f009.tar.gz
egit-5ceb6c1e729be7dffd252acad4949944d111f009.tar.xz
egit-5ceb6c1e729be7dffd252acad4949944d111f009.zip
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 <thomas.wolf@paranor.ch>
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java125
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java2
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 6ccb9a48b..1d37155c7 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;
@@ -112,10 +113,46 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener
}
/**
+ * 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 <code>show</code> is
* <code>true</code> 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);
}
@@ -171,6 +206,46 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener
}
/**
+ * @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.
*
* @return our theme.
@@ -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 35efab347..557d25465 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;

Back to the top