diff --git a/src/org/openstreetmap/josm/gui/MapView.java b/src/org/openstreetmap/josm/gui/MapView.java
index 63e4bb4..3a4b1ed 100644
--- a/src/org/openstreetmap/josm/gui/MapView.java
+++ b/src/org/openstreetmap/josm/gui/MapView.java
@@ -77,7 +77,6 @@ import org.openstreetmap.josm.tools.AudioPlayer;
import org.openstreetmap.josm.tools.Shortcut;
import org.openstreetmap.josm.tools.Utils;
import org.openstreetmap.josm.tools.bugreport.BugReport;
-import org.openstreetmap.josm.tools.bugreport.BugReportExceptionHandler;
/**
* This is a component used in the {@link MapFrame} for browsing the map. It use is to
@@ -826,8 +825,7 @@ LayerManager.LayerChangeListener, MainLayerManager.ActiveLayerChangeListener {
painter.paint(paintGraphics);
g.setPaintMode();
} catch (RuntimeException t) {
- //TODO: only display.
- throw BugReport.intercept(t).put("layer", layer).put("bounds", box);
+ BugReport.intercept(t).put("layer", layer).put("bounds", box).warn();
}
}
@@ -836,7 +834,12 @@ LayerManager.LayerChangeListener, MainLayerManager.ActiveLayerChangeListener {
*/
@Override
public void paint(Graphics g) {
- if (!prepareToDraw()) {
+ try {
+ if (!prepareToDraw()) {
+ return;
+ }
+ } catch (RuntimeException e) {
+ BugReport.intercept(e).put("center", () -> getCenter()).warn();
return;
}
@@ -914,21 +917,18 @@ LayerManager.LayerChangeListener, MainLayerManager.ActiveLayerChangeListener {
paintLayer(visibleLayers.get(i), tempG, box);
}
- synchronized (temporaryLayers) {
- for (MapViewPaintable mvp : temporaryLayers) {
- try {
- mvp.paint(tempG, this, box);
- } catch (RuntimeException e) {
- throw BugReport.intercept(e).put("mvp", mvp);
- }
- }
+ try {
+ drawTemporaryLayers(tempG, box);
+ } catch (RuntimeException e) {
+ BugReport.intercept(e).put("temporaryLayers", temporaryLayers).warn();
}
// draw world borders
try {
drawWorldBorders(tempG);
} catch (RuntimeException e) {
- throw BugReport.intercept(e).put("bounds", getProjection()::getWorldBoundsLatLon);
+ // getProjection() needs to be inside lambda to catch errors.
+ BugReport.intercept(e).put("bounds", () -> getProjection().getWorldBoundsLatLon()).warn();
}
if (Main.isDisplayingMapView() && Main.map.filterDialog != null) {
@@ -970,6 +970,18 @@ LayerManager.LayerChangeListener, MainLayerManager.ActiveLayerChangeListener {
super.paint(g);
}
+ private void drawTemporaryLayers(Graphics2D tempG, Bounds box) {
+ synchronized (temporaryLayers) {
+ for (MapViewPaintable mvp : temporaryLayers) {
+ try {
+ mvp.paint(tempG, this, box);
+ } catch (RuntimeException e) {
+ throw BugReport.intercept(e).put("mvp", mvp);
+ }
+ }
+ }
+ }
+
private void drawWorldBorders(Graphics2D tempG) {
tempG.setColor(Color.WHITE);
Bounds b = getProjection().getWorldBoundsLatLon();
@@ -1025,8 +1037,6 @@ LayerManager.LayerChangeListener, MainLayerManager.ActiveLayerChangeListener {
zoomTo(initialViewport);
initialViewport = null;
}
- if (BugReportExceptionHandler.exceptionHandlingInProgress())
- return false;
if (getCenter() == null)
return false; // no data loaded yet.
diff --git a/src/org/openstreetmap/josm/tools/bugreport/BugReportDialog.java b/src/org/openstreetmap/josm/tools/bugreport/BugReportDialog.java
index f4dec4c..e928e1b 100644
--- a/src/org/openstreetmap/josm/tools/bugreport/BugReportDialog.java
+++ b/src/org/openstreetmap/josm/tools/bugreport/BugReportDialog.java
@@ -18,16 +18,22 @@ import javax.swing.JCheckBox;
import javax.swing.JComponent;
import javax.swing.JDialog;
import javax.swing.JLabel;
+import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.KeyStroke;
import javax.swing.UIManager;
import org.openstreetmap.josm.Main;
import org.openstreetmap.josm.actions.ExpertToggleAction;
+import org.openstreetmap.josm.gui.preferences.plugin.PluginPreference;
+import org.openstreetmap.josm.gui.util.GuiHelper;
import org.openstreetmap.josm.gui.widgets.JMultilineLabel;
import org.openstreetmap.josm.gui.widgets.UrlLabel;
+import org.openstreetmap.josm.plugins.PluginDownloadTask;
+import org.openstreetmap.josm.plugins.PluginHandler;
import org.openstreetmap.josm.tools.GBC;
import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.bugreport.BugReportQueue.SuppressionMode;
/**
* This is a dialog that can be used to display a bug report.
@@ -42,7 +48,8 @@ public class BugReportDialog extends JDialog {
private final JPanel content = new JPanel(new GridBagLayout());
private final BugReport report;
private final DebugTextDisplay textPanel;
- private JCheckBox cbSuppress;
+ private JCheckBox cbSuppressSingle;
+ private JCheckBox cbSuppressAll;
/**
* Create a new dialog.
@@ -148,12 +155,15 @@ public class BugReportDialog extends JDialog {
private void addIgnoreButton() {
JPanel panel = new JPanel(new GridBagLayout());
- cbSuppress = new JCheckBox(tr("Suppress further error dialogs for this session."));
- cbSuppress.setVisible(false);
- panel.add(cbSuppress, GBC.std().fill(GBC.HORIZONTAL));
+ cbSuppressSingle = new JCheckBox(tr("Suppress this error for this session."));
+ cbSuppressSingle.setVisible(false);
+ panel.add(cbSuppressSingle, GBC.std(0, 0).fill(GBC.HORIZONTAL));
+ cbSuppressAll = new JCheckBox(tr("Suppress further error dialogs for this session."));
+ cbSuppressAll.setVisible(false);
+ panel.add(cbSuppressAll, GBC.std(0, 1).fill(GBC.HORIZONTAL));
JButton ignore = new JButton(tr("Ignore this error."));
ignore.addActionListener(e -> closeDialog());
- panel.add(ignore, GBC.eol());
+ panel.add(ignore, GBC.std(1, 0).span(1, 2).anchor(GBC.CENTER));
content.add(panel, GBC.eol().fill(GBC.HORIZONTAL).insets(20));
}
@@ -162,7 +172,16 @@ public class BugReportDialog extends JDialog {
* @param showSuppress true to show the suppress errors checkbox.
*/
public void setShowSuppress(boolean showSuppress) {
- cbSuppress.setVisible(showSuppress);
+ cbSuppressSingle.setVisible(showSuppress);
+ pack();
+ }
+
+ /**
+ * Shows or hides the suppress all errors button
+ * @param showSuppress true to show the suppress errors checkbox.
+ */
+ public void setShowSuppressAll(boolean showSuppress) {
+ cbSuppressAll.setVisible(showSuppress);
pack();
}
@@ -170,8 +189,14 @@ public class BugReportDialog extends JDialog {
* Check if the checkbox to suppress further errors was selected
* @return true if the user wishes to suppress errors.
*/
- public boolean shouldSuppressFurtherErrors() {
- return cbSuppress.isSelected();
+ public SuppressionMode shouldSuppressFurtherErrors() {
+ if (cbSuppressAll.isSelected()) {
+ return SuppressionMode.ALL;
+ } else if (cbSuppressSingle.isSelected()) {
+ return SuppressionMode.SAME;
+ } else {
+ return SuppressionMode.NONE;
+ }
}
private void closeDialog() {
@@ -201,4 +226,41 @@ public class BugReportDialog extends JDialog {
}
return null;
}
+
+ /**
+ * Show the bug report for a given exception
+ * @param e The exception to display
+ * @param exceptionCounter A counter of how many exceptions have already been worked on
+ * @return The new suppression status
+ */
+ public static SuppressionMode showFor(ReportedException e, int exceptionCounter) {
+ if (e.isOutOfMemory()) {
+ // do not translate the string, as translation may raise an exception
+ JOptionPane.showMessageDialog(Main.parent, "JOSM is out of memory. " +
+ "Strange things may happen.\nPlease restart JOSM with the -Xmx###M option,\n" +
+ "where ### is the number of MB assigned to JOSM (e.g. 256).\n" +
+ "Currently, " + Runtime.getRuntime().maxMemory()/1024/1024 + " MB are available to JOSM.",
+ "Error",
+ JOptionPane.ERROR_MESSAGE
+ );
+ return SuppressionMode.NONE;
+ } else {
+ return GuiHelper.runInEDTAndWaitAndReturn(() -> {
+ PluginDownloadTask downloadTask = PluginHandler.updateOrdisablePluginAfterException(e);
+ if (downloadTask != null) {
+ // Ask for restart to install new plugin
+ PluginPreference.notifyDownloadResults(
+ Main.parent, downloadTask, !downloadTask.getDownloadedPlugins().isEmpty());
+ return SuppressionMode.NONE;
+ }
+
+ BugReport report = new BugReport(e);
+ BugReportDialog dialog = new BugReportDialog(report);
+ dialog.setShowSuppress(exceptionCounter > 0);
+ dialog.setShowSuppressAll(exceptionCounter > 1);
+ dialog.setVisible(true);
+ return dialog.shouldSuppressFurtherErrors();
+ });
+ }
+ }
}
diff --git a/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java b/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java
index 461b73f..1decdd1 100644
--- a/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java
+++ b/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java
@@ -1,16 +1,6 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.tools.bugreport;
-import java.awt.GraphicsEnvironment;
-
-import javax.swing.JOptionPane;
-import javax.swing.SwingUtilities;
-
-import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.gui.preferences.plugin.PluginPreference;
-import org.openstreetmap.josm.plugins.PluginDownloadTask;
-import org.openstreetmap.josm.plugins.PluginHandler;
-
/**
* An exception handler that asks the user to send a bug report.
*
@@ -19,62 +9,6 @@ import org.openstreetmap.josm.plugins.PluginHandler;
*/
public final class BugReportExceptionHandler implements Thread.UncaughtExceptionHandler {
- private static boolean handlingInProgress;
- private static volatile BugReporterThread bugReporterThread;
- private static int exceptionCounter;
- private static boolean suppressExceptionDialogs;
-
- static final class BugReporterThread extends Thread {
-
- private final class BugReporterWorker implements Runnable {
- private final PluginDownloadTask pluginDownloadTask;
-
- private BugReporterWorker(PluginDownloadTask pluginDownloadTask) {
- this.pluginDownloadTask = pluginDownloadTask;
- }
-
- @Override
- public void run() {
- // Then ask for submitting a bug report, for exceptions thrown from a plugin too, unless updated to a new version
- if (pluginDownloadTask == null) {
- askForBugReport(e);
- } else {
- // Ask for restart to install new plugin
- PluginPreference.notifyDownloadResults(
- Main.parent, pluginDownloadTask, !pluginDownloadTask.getDownloadedPlugins().isEmpty());
- }
- }
- }
-
- private final Throwable e;
-
- /**
- * Constructs a new {@code BugReporterThread}.
- * @param t the exception
- */
- private BugReporterThread(Throwable t) {
- super("Bug Reporter");
- this.e = t;
- }
-
- static void askForBugReport(final Throwable e) {
- if (GraphicsEnvironment.isHeadless()) {
- return;
- }
- BugReport report = new BugReport(BugReport.intercept(e));
- BugReportDialog dialog = new BugReportDialog(report);
- dialog.setShowSuppress(exceptionCounter > 1);
- dialog.setVisible(true);
- suppressExceptionDialogs = dialog.shouldSuppressFurtherErrors();
- }
-
- @Override
- public void run() {
- // Give the user a chance to deactivate the plugin which threw the exception (if it was thrown from a plugin)
- SwingUtilities.invokeLater(new BugReporterWorker(PluginHandler.updateOrdisablePluginAfterException(e)));
- }
- }
-
@Override
public void uncaughtException(Thread t, Throwable e) {
handleException(e);
@@ -85,33 +19,7 @@ public final class BugReportExceptionHandler implements Thread.UncaughtException
* @param e the exception
*/
public static synchronized void handleException(final Throwable e) {
- if (handlingInProgress || suppressExceptionDialogs)
- return; // we do not handle secondary exceptions, this gets too messy
- if (bugReporterThread != null && bugReporterThread.isAlive())
- return;
- handlingInProgress = true;
- exceptionCounter++;
- try {
- Main.error(e);
- if (Main.parent != null) {
- if (e instanceof OutOfMemoryError) {
- // do not translate the string, as translation may raise an exception
- JOptionPane.showMessageDialog(Main.parent, "JOSM is out of memory. " +
- "Strange things may happen.\nPlease restart JOSM with the -Xmx###M option,\n" +
- "where ### is the number of MB assigned to JOSM (e.g. 256).\n" +
- "Currently, " + Runtime.getRuntime().maxMemory()/1024/1024 + " MB are available to JOSM.",
- "Error",
- JOptionPane.ERROR_MESSAGE
- );
- return;
- }
-
- bugReporterThread = new BugReporterThread(e);
- bugReporterThread.start();
- }
- } finally {
- handlingInProgress = false;
- }
+ BugReport.intercept(e).warn();
}
/**
@@ -119,6 +27,6 @@ public final class BugReportExceptionHandler implements Thread.UncaughtException
* @return {@code true} if an exception is currently being handled, {@code false} otherwise
*/
public static boolean exceptionHandlingInProgress() {
- return handlingInProgress;
+ return BugReportQueue.getInstance().exceptionHandlingInProgress();
}
}
diff --git a/src/org/openstreetmap/josm/tools/bugreport/BugReportQueue.java b/src/org/openstreetmap/josm/tools/bugreport/BugReportQueue.java
new file mode 100644
index 0000000..69133c6
--- /dev/null
+++ b/src/org/openstreetmap/josm/tools/bugreport/BugReportQueue.java
@@ -0,0 +1,126 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.tools.bugreport;
+
+import java.awt.GraphicsEnvironment;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.function.BiFunction;
+
+import org.openstreetmap.josm.Main;
+
+/**
+ * This class handles the display of the bug report dialog.
+ * @author Michael Zangl
+ * @since xxx
+ */
+public class BugReportQueue {
+
+ private static final BugReportQueue INSTANCE = new BugReportQueue();
+
+ private LinkedList reportsToDisplay = new LinkedList<>();
+ private boolean suppressAllMessages;
+ private ArrayList suppressFor = new ArrayList<>();
+ private Thread displayThread;
+ private final BiFunction bugReportHandler = getBestHandler();
+ private int displayedErrors;
+
+ private boolean inReportDialog;
+
+
+ /**
+ * The suppression mode that should be used after the dialog was closed.
+ */
+ public enum SuppressionMode {
+ /**
+ * Suppress no dialogs.
+ */
+ NONE,
+ /**
+ * Suppress only the ones that are for the same error
+ */
+ SAME,
+ /**
+ * Suppress all report dialogs
+ */
+ ALL
+ }
+
+ /**
+ * Submit a new error to be displayed
+ * @param report The error to display
+ */
+ public synchronized void submit(ReportedException report) {
+ if (suppressAllMessages || suppressFor.stream().anyMatch(report::isSame)) {
+ Main.info("User requested to skip error " + report);
+ } else if (reportsToDisplay.size() > 100 || reportsToDisplay.stream().filter(report::isSame).count() >= 10) {
+ Main.warn("Too many errors. Dropping " + report);
+ } else {
+ reportsToDisplay.add(report);
+ if (displayThread == null) {
+ displayThread = new Thread(this::displayAll, "bug-report-display");
+ displayThread.start();
+ }
+ notifyAll();
+ }
+ }
+
+ private void displayAll() {
+ try {
+ while (true) {
+ ReportedException e = getNext();
+ SuppressionMode suppress = displayFor(e);
+ handleDialogResult(e, suppress);
+ }
+ } catch (InterruptedException e) {
+ displayFor(BugReport.intercept(e));
+ }
+ }
+
+ private synchronized void handleDialogResult(ReportedException e, SuppressionMode suppress) {
+ if (suppress == SuppressionMode.ALL) {
+ suppressAllMessages = true;
+ reportsToDisplay.clear();
+ } else if (suppress == SuppressionMode.SAME) {
+ suppressFor.add(e);
+ reportsToDisplay.removeIf(e::isSame);
+ }
+ displayedErrors++;
+ inReportDialog = false;
+ }
+
+ private synchronized ReportedException getNext() throws InterruptedException {
+ while (reportsToDisplay.isEmpty()) {
+ wait();
+ }
+ inReportDialog = true;
+ return reportsToDisplay.removeFirst();
+ }
+
+ private SuppressionMode displayFor(ReportedException e) {
+ return bugReportHandler.apply(e, getDisplayedErrors());
+ }
+
+ private synchronized int getDisplayedErrors() {
+ return displayedErrors;
+ }
+
+ /**
+ * Check if the dialog is shown. Should only be used for e.g. debugging.
+ * @return true if the exception handler is still showing the exception to the user.
+ */
+ public synchronized boolean exceptionHandlingInProgress() {
+ return !reportsToDisplay.isEmpty() || inReportDialog;
+ }
+
+ private static BiFunction getBestHandler() {
+ if (GraphicsEnvironment.isHeadless()) {
+ return (e, index) -> { e.printStackTrace(); return SuppressionMode.NONE; };
+ } else {
+ return BugReportDialog::showFor;
+ }
+ }
+
+ public static BugReportQueue getInstance() {
+ return INSTANCE;
+ }
+}
diff --git a/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java b/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java
index 22efa08..f7c4d36 100644
--- a/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java
+++ b/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java
@@ -65,7 +65,16 @@ public class ReportedException extends RuntimeException {
*/
public void warn() {
methodWarningFrom = BugReport.getCallingMethod(2);
- // TODO: Open the dialog.
+ try {
+ BugReportQueue.getInstance().submit(this);
+ } catch (RuntimeException e) {
+ try {
+ e.printStackTrace();
+ } catch (RuntimeException e2) {
+ // we cannot do anything more...
+ // re-throwing this causes an infinite loop.
+ }
+ }
}
/**
@@ -242,14 +251,10 @@ public class ReportedException extends RuntimeException {
@Override
public String toString() {
- return new StringBuilder(48)
- .append("CrashReportedException [on thread ")
- .append(caughtOnThread)
- .append(']')
- .toString();
+ return "ReportedException [thread=" + caughtOnThread + ", exception=" + exception
+ + ", methodWarningFrom=" + methodWarningFrom + "]";
}
-
/**
* Check if this exception may be caused by a threading issue.
* @return true if it is.
@@ -261,6 +266,14 @@ public class ReportedException extends RuntimeException {
}
/**
+ * Check if this is caused by an out of memory situaition
+ * @return true if it is.
+ */
+ public boolean isOutOfMemory() {
+ return StreamUtils.toStream(CauseTraceIterator::new).anyMatch(t -> t instanceof OutOfMemoryError);
+ }
+
+ /**
* Iterates over the causes for this exception. Ignores cycles and aborts iteration then.
* @author Michal Zangl
* @since 10585
diff --git a/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandlerTest.java b/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandlerTest.java
index f479d0b..a030600 100644
--- a/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandlerTest.java
+++ b/test/unit/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandlerTest.java
@@ -21,14 +21,6 @@ public class BugReportExceptionHandlerTest {
}
/**
- * Unit test for {@link BugReportExceptionHandler.BugReporterThread#askForBugReport} method.
- */
- @Test
- public void testAskForBugReport() {
- BugReportExceptionHandler.BugReporterThread.askForBugReport(new Exception("testAskForBugReport"));
- }
-
- /**
* Unit test for {@link BugReportExceptionHandler#handleException} method.
*/
@Test