diff --git a/src/org/openstreetmap/josm/tools/StreamUtils.java b/src/org/openstreetmap/josm/tools/StreamUtils.java new file mode 100644 index 0000000..ead9c1c --- /dev/null +++ b/src/org/openstreetmap/josm/tools/StreamUtils.java @@ -0,0 +1,32 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.tools; + +import java.util.Iterator; +import java.util.Spliterator; +import java.util.Spliterators; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + +/** + * Utility methods for streams. + * @author Michael Zangl + * @since xxx + */ +public final class StreamUtils { + + /** + * Untility class + */ + private StreamUtils() {} + + /** + * Convert an iterator to a stream. + * @param The element type to iterate over + * @param iterator The iterator + * @return The stream of for that iterator. + */ + public static Stream toStream(Iterator iterator) { + Spliterator spliterator = Spliterators.spliteratorUnknownSize(iterator, Spliterator.ORDERED); + return StreamSupport.stream(spliterator, false); + } +} diff --git a/src/org/openstreetmap/josm/tools/bugreport/BugReport.java b/src/org/openstreetmap/josm/tools/bugreport/BugReport.java index 16758bf..a2f2287 100644 --- a/src/org/openstreetmap/josm/tools/bugreport/BugReport.java +++ b/src/org/openstreetmap/josm/tools/bugreport/BugReport.java @@ -1,6 +1,12 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.tools.bugreport; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.openstreetmap.josm.actions.ShowStatusReportAction; + /** * This class contains utility methods to create and handle a bug report. *

@@ -19,7 +25,7 @@ package org.openstreetmap.josm.tools.bugreport; * String tag = "..."; * try { * ... your code ... - * } catch (Throwable t) { + * } catch (RuntimeException t) { * throw BugReport.intercept(t).put("id", id).put("tag", tag); * } * @@ -31,12 +37,119 @@ package org.openstreetmap.josm.tools.bugreport; * @since 10285 */ public class BugReport { + private boolean includeStatusReport = true; + private boolean includeData = true; + private boolean includeAllStackTraces; + private ReportedException exception; + private final CopyOnWriteArrayList listeners = new CopyOnWriteArrayList<>(); + /** * Create a new bug report * @param e The {@link ReportedException} to use. No more data should be added after creating the report. */ public BugReport(ReportedException e) { - // TODO: Use this class to create the bug report. + this.exception = e; + includeAllStackTraces = e.mayHaveConcurrentSource(); + } + + /** + * Get if this report should include a system status report + * @return true to include it. + * @since xxx + */ + public boolean getIncludeStatusReport() { + return includeStatusReport; + } + + /** + * Set if this report should include a system status report + * @param includeStatusReport if the status report should be included + * @since xxx + */ + public void setIncludeStatusReport(boolean includeStatusReport) { + this.includeStatusReport = includeStatusReport; + fireChange(); + } + + /** + * Get if this report should include the data that was traced. + * @return true to include it. + * @since xxx + */ + public boolean getIncludeData() { + return includeData; + } + + /** + * Set if this report should include the data that was traced. + * @param includeData if data should be included + * @since xxx + */ + public void setIncludeData(boolean includeData) { + this.includeData = includeData; + fireChange(); + } + + /** + * Get if this report should include the stack traces for all other threads. + * @return true to include it. + * @since xxx + */ + public boolean getIncludeAllStackTraces() { + return includeAllStackTraces; + } + + /** + * Sets if this report should include the stack traces for all other threads. + * @param includeAllStackTraces if all stack traces should be included + * @since xxx + */ + public void setIncludeAllStackTraces(boolean includeAllStackTraces) { + this.includeAllStackTraces = includeAllStackTraces; + fireChange(); + } + + /** + * Gets the full string that should be send as error report. + * @return The string. + * @since xxx + */ + public String getReportText() { + StringWriter stringWriter = new StringWriter(); + PrintWriter out = new PrintWriter(stringWriter); + if (getIncludeStatusReport()) { + out.println(ShowStatusReportAction.getReportHeader()); + } + if (getIncludeData()) { + exception.printReportDataTo(out); + } + exception.printReportStackTo(out); + if (getIncludeAllStackTraces()) { + exception.printReportThreadsTo(out); + } + return stringWriter.toString().replaceAll("\r", ""); + } + + /** + * Add a new change listener. + * @param listener The listener + * @since xxx + */ + public void addChangeListener(BugReportListener listener) { + listeners.add(listener); + } + + /** + * Remove a change listener. + * @param listener The listener + * @since xxx + */ + public void removeChangeListener(BugReportListener listener) { + listeners.remove(listener); + } + + private void fireChange() { + listeners.stream().forEach(l -> l.bugReportChanged(this)); } /** @@ -74,4 +187,18 @@ public class BugReport { } return "?"; } + + /** + * A listener that listens to changes to this report. + * @author Michael Zangl + * @since xxx + */ + @FunctionalInterface + public interface BugReportListener { + /** + * Called whenever this bug report was changed, e.g. the data to be included in it. + * @param report The report that was changed. + */ + void bugReportChanged(BugReport report); + } } diff --git a/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java b/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java index 054eca0..c31c15c 100644 --- a/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java +++ b/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java @@ -102,7 +102,7 @@ public final class BugReportExceptionHandler implements Thread.UncaughtException String.valueOf(josmVersion), String.valueOf(latestVersion)); } } catch (IOException | NumberFormatException ex) { - Main.warn("Unable to detect latest version of JOSM: "+ex.getMessage()); + Main.warn("Unable to detect latest version of JOSM: "+ex.getMessage(), ex); } } // Build panel @@ -136,7 +136,7 @@ public final class BugReportExceptionHandler implements Thread.UncaughtException try { Main.platform.openUrl(Main.getJOSMWebsite()); } catch (IOException ex) { - Main.warn("Unable to access JOSM website: "+ex.getMessage()); + Main.warn("Unable to access JOSM website: "+ex.getMessage(), ex); } } else { // "Report bug" @@ -196,19 +196,18 @@ public final class BugReportExceptionHandler implements Thread.UncaughtException } static JPanel buildPanel(final Throwable e) { - StringWriter stack = new StringWriter(); - PrintWriter writer = new PrintWriter(stack); + DebugTextDisplay textarea; if (e instanceof ReportedException) { // Temporary! - ((ReportedException) e).printReportDataTo(writer); - ((ReportedException) e).printReportStackTo(writer); + textarea = new DebugTextDisplay(new BugReport((ReportedException) e)); } else { + StringWriter stack = new StringWriter(); + PrintWriter writer = new PrintWriter(stack); e.printStackTrace(writer); + String text = ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString(); + textarea = new DebugTextDisplay(text); } - String text = ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString(); - text = text.replaceAll("\r", ""); - JPanel p = new JPanel(new GridBagLayout()); p.add(new JMultilineLabel( tr("You have encountered an error in JOSM. Before you file a bug report " + @@ -219,7 +218,7 @@ public final class BugReportExceptionHandler implements Thread.UncaughtException tr("You should also update your plugins. If neither of those help please " + "file a bug report in our bugtracker using this link:")), GBC.eol().fill(GridBagConstraints.HORIZONTAL)); - p.add(new JButton(new ReportBugAction(text)), GBC.eop().insets(8, 0, 0, 0)); + p.add(new JButton(new ReportBugAction(textarea.getCodeText())), GBC.eop().insets(8, 0, 0, 0)); p.add(new JMultilineLabel( tr("There the error information provided below should already be " + "filled in for you. Please include information on how to reproduce " + @@ -230,9 +229,6 @@ public final class BugReportExceptionHandler implements Thread.UncaughtException "below at this URL:")), GBC.eol().fill(GridBagConstraints.HORIZONTAL)); p.add(new UrlLabel(Main.getJOSMWebsite()+"/newticket", 2), GBC.eop().insets(8, 0, 0, 0)); - // Wiki formatting for manual copy-paste - DebugTextDisplay textarea = new DebugTextDisplay(text); - if (textarea.copyToClippboard()) { p.add(new JLabel(tr("(The text has already been copied to your clipboard.)")), GBC.eop().fill(GridBagConstraints.HORIZONTAL)); diff --git a/src/org/openstreetmap/josm/tools/bugreport/BugReportSettingsPanel.java b/src/org/openstreetmap/josm/tools/bugreport/BugReportSettingsPanel.java new file mode 100644 index 0000000..4881906 --- /dev/null +++ b/src/org/openstreetmap/josm/tools/bugreport/BugReportSettingsPanel.java @@ -0,0 +1,38 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.tools.bugreport; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import javax.swing.BoxLayout; +import javax.swing.JCheckBox; +import javax.swing.JPanel; + +/** + * This panel displays the settings that can be changed before submitting a bug report to the web page. + * @author Michael Zangl + * @since xxx + */ +public class BugReportSettingsPanel extends JPanel { + /** + * Creates the new settings panel. + * @param report The report this panel should influence. + */ + public BugReportSettingsPanel(BugReport report) { + setLayout(new BoxLayout(this, BoxLayout.Y_AXIS)); + + JCheckBox statusReport = new JCheckBox(tr("Include the system status report.")); + statusReport.setSelected(report.getIncludeStatusReport()); + statusReport.addChangeListener(e -> report.setIncludeStatusReport(statusReport.isSelected())); + add(statusReport); + + JCheckBox data = new JCheckBox(tr("Include information about the data that was worked on.")); + data.setSelected(report.getIncludeData()); + data.addChangeListener(e -> report.setIncludeData(data.isSelected())); + add(data); + + JCheckBox allStackTraces = new JCheckBox(tr("Include all stack traces.")); + allStackTraces.setSelected(report.getIncludeAllStackTraces()); + allStackTraces.addChangeListener(e -> report.setIncludeAllStackTraces(allStackTraces.isSelected())); + add(allStackTraces); + } +} diff --git a/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java b/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java index a0e2841..dffe029 100644 --- a/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java +++ b/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java @@ -14,15 +14,16 @@ import org.openstreetmap.josm.tools.Utils; * @since 10055 */ public class DebugTextDisplay extends JScrollPane { + private static final String CODE_PATTERN = "{{{%n%s%n}}}"; private String text; + private JosmTextArea textArea; /** - * Creates a new text are with the fixed text - * @param textToDisplay The text to display. + * Creates a new text area. + * @since xxx */ - public DebugTextDisplay(String textToDisplay) { - text = "{{{\n" + Utils.strip(textToDisplay) + "\n}}}"; - JosmTextArea textArea = new JosmTextArea(text); + private DebugTextDisplay() { + textArea = new JosmTextArea(); textArea.setCaretPosition(0); textArea.setEditable(false); setViewportView(textArea); @@ -30,10 +31,49 @@ public class DebugTextDisplay extends JScrollPane { } /** - * Copies the debug text to the clippboard. + * Creates a new text area with an inital text to display + * @param textToDisplay The text to display. + */ + public DebugTextDisplay(String textToDisplay) { + this(); + setCodeText(textToDisplay); + } + + /** + * Creates a new text area that displays the bug report data + * @param report The bug report data to display. + */ + public DebugTextDisplay(BugReport report) { + this(); + setCodeText(report.getReportText()); + report.addChangeListener(e -> setCodeText(report.getReportText())); + } + + /** + * Sets the text that should be displayed in this view. + * @param textToDisplay The text + * @since xxx + */ + private void setCodeText(String textToDisplay) { + text = Utils.strip(textToDisplay).replaceAll("\r", ""); + textArea.setText(String.format(CODE_PATTERN, text)); + } + + /** + * Copies the debug text to the clippboard. This includes the code tags for trac. * @return true if copy was successful + * @since 10055 */ public boolean copyToClippboard() { - return Utils.copyToClipboard(text); + return Utils.copyToClipboard(String.format(CODE_PATTERN, text)); + } + + /** + * Gets the text this are displays, without the code tag. + * @return The stripped text set by {@link #setCodeText(String)} + * @since xxx + */ + public String getCodeText() { + return text; } } diff --git a/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java b/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java index 41bf1ba..054e666 100644 --- a/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java +++ b/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java @@ -2,17 +2,23 @@ package org.openstreetmap.josm.tools.bugreport; import java.io.PrintWriter; +import java.io.Serializable; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.ConcurrentModificationException; import java.util.IdentityHashMap; +import java.util.Iterator; import java.util.LinkedList; import java.util.Map; import java.util.Map.Entry; +import java.util.NoSuchElementException; import java.util.Set; import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.tools.StreamUtils; /** * This is a special exception that cannot be directly thrown. @@ -24,17 +30,19 @@ import org.openstreetmap.josm.Main; * @since 10285 */ public class ReportedException extends RuntimeException { - private static final int MAX_COLLECTION_ENTRIES = 30; /** - * + * How many entries of a collection to include in the bug report. */ + private static final int MAX_COLLECTION_ENTRIES = 30; + private static final long serialVersionUID = 737333873766201033L; + /** * We capture all stack traces on exception creation. This allows us to trace synchonization problems better. We cannot be really sure what * happened but we at least see which threads */ private final transient Map allStackTraces; - private final transient LinkedList

sections = new LinkedList<>(); + private final LinkedList
sections = new LinkedList<>(); private final transient Thread caughtOnThread; private final Throwable exception; private String methodWarningFrom; @@ -143,17 +151,15 @@ public class ReportedException extends RuntimeException { return false; } - Set dejaVu = Collections.newSetFromMap(new IdentityHashMap()); - return hasSameStackTrace(dejaVu, this.exception, e.exception); + return hasSameStackTrace(new CauseTraceIterator(), e.exception); } - private static boolean hasSameStackTrace(Set dejaVu, Throwable e1, Throwable e2) { - if (dejaVu.contains(e1)) { - // cycle. If it was the same until here, we assume both have that cycle. + private static boolean hasSameStackTrace(CauseTraceIterator causeTraceIterator, Throwable e2) { + if (!causeTraceIterator.hasNext()) { + // all done. return true; } - dejaVu.add(e1); - + Throwable e1 = causeTraceIterator.next(); StackTraceElement[] t1 = e1.getStackTrace(); StackTraceElement[] t2 = e2.getStackTrace(); @@ -166,7 +172,7 @@ public class ReportedException extends RuntimeException { if ((c1 == null) != (c2 == null)) { return false; } else if (c1 != null) { - return hasSameStackTrace(dejaVu, c1, c2); + return hasSameStackTrace(causeTraceIterator, c2); } else { return true; } @@ -227,7 +233,54 @@ public class ReportedException extends RuntimeException { .toString(); } - private static class SectionEntry { + + /** + * Check if this exception may be caused by a threading issue. + * @return true if it is. + * @since xxx + */ + public boolean mayHaveConcurrentSource() { + return StreamUtils.toStream(new CauseTraceIterator()) + .anyMatch(t -> t instanceof ConcurrentModificationException || t instanceof InvocationTargetException); + } + + /** + * Iterates over the causes for this exception. Ignores cycles and aborts iteration then. + * @author Michal Zangl + * @since xxx + */ + private final class CauseTraceIterator implements Iterator { + private Throwable current = exception; + private final Set dejaVu = Collections.newSetFromMap(new IdentityHashMap()); + + @Override + public boolean hasNext() { + return current != null; + } + + @Override + public Throwable next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + Throwable toReturn = current; + advance(); + return toReturn; + } + + private void advance() { + dejaVu.add(current); + current = current.getCause(); + if (current != null && dejaVu.contains(current)) { + current = null; + } + } + } + + private static class SectionEntry implements Serializable { + + private static final long serialVersionUID = 1L; + private final String key; private final String value; @@ -248,7 +301,9 @@ public class ReportedException extends RuntimeException { } } - private static class Section { + private static class Section implements Serializable { + + private static final long serialVersionUID = 1L; private String sectionName; private ArrayList entries = new ArrayList<>();