Changeset 10585 in josm

Timestamp:

2016-07-22T22:21:19+02:00 (8 years ago)

Author:

Don-vip

Message:

see #11390, fix #12905 - Add more information to the bug report (patch by michael2402) - gsoc-core - requires java 8

Location:

trunk/src/org/openstreetmap/josm/tools

Files:

• StreamUtils.java (added)
• bugreport/BugReport.java (modified) (4 diffs)
• bugreport/BugReportExceptionHandler.java (modified) (3 diffs)
• bugreport/BugReportSettingsPanel.java (added)
• bugreport/DebugTextDisplay.java (modified) (2 diffs)
• bugreport/ReportedException.java (modified) (7 diffs)

trunk/src/org/openstreetmap/josm/tools/bugreport/BugReport.java

@@ -1 +1 @@
 // 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;
 
 /**
@@ -20 +26 @@
  * try {
  *   ... your code ...
- * } catch (Throwable t) {
+ * } catch (RuntimeException t) {
  *   throw BugReport.intercept(t).put("id", id).put("tag", tag);
  * }
@@ -32 +38 @@
  */
 public final class BugReport {
+    private boolean includeStatusReport = true;
+    private boolean includeData = true;
+    private boolean includeAllStackTraces;
+    private ReportedException exception;
+    private final CopyOnWriteArrayList<BugReportListener> 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.
      */
-    private BugReport(ReportedException e) {
-        // TODO: Use this class to create the bug report.
+    BugReport(ReportedException e) {
+        this.exception = e;
+        includeAllStackTraces = e.mayHaveConcurrentSource();
+    }
+
+    /**
+     * Get if this report should include a system status report
+     * @return <code>true</code> to include it.
+     * @since 10585
+     */
+    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 10585
+     */
+    public void setIncludeStatusReport(boolean includeStatusReport) {
+        this.includeStatusReport = includeStatusReport;
+        fireChange();
+    }
+
+    /**
+     * Get if this report should include the data that was traced.
+     * @return <code>true</code> to include it.
+     * @since 10585
+     */
+    public boolean getIncludeData() {
+        return includeData;
+    }
+
+    /**
+     * Set if this report should include the data that was traced.
+     * @param includeData if data should be included
+     * @since 10585
+     */
+    public void setIncludeData(boolean includeData) {
+        this.includeData = includeData;
+        fireChange();
+    }
+
+    /**
+     * Get if this report should include the stack traces for all other threads.
+     * @return <code>true</code> to include it.
+     * @since 10585
+     */
+    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 10585
+     */
+    public void setIncludeAllStackTraces(boolean includeAllStackTraces) {
+        this.includeAllStackTraces = includeAllStackTraces;
+        fireChange();
+    }
+
+    /**
+     * Gets the full string that should be send as error report.
+     * @return The string.
+     * @since 10585
+     */
+    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 10585
+     */
+    public void addChangeListener(BugReportListener listener) {
+        listeners.add(listener);
+    }
+
+    /**
+     * Remove a change listener.
+     * @param listener The listener
+     * @since 10585
+     */
+    public void removeChangeListener(BugReportListener listener) {
+        listeners.remove(listener);
+    }
+
+    private void fireChange() {
+        listeners.stream().forEach(l -> l.bugReportChanged(this));
     }
 
@@ -75 +188 @@
         return "?";
     }
+
+    /**
+     * A listener that listens to changes to this report.
+     * @author Michael Zangl
+     * @since 10585
+     */
+    @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);
+    }
 }

trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java

@@ -197 +197 @@
 
     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 {
-            e.printStackTrace(writer);
-        }
-
-        String text = ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString();
-        text = text.replaceAll("\r", "");
+            StringWriter stack = new StringWriter();
+            e.printStackTrace(new PrintWriter(stack));
+            textarea = new DebugTextDisplay(ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString());
+        }
 
         JPanel p = new JPanel(new GridBagLayout());
@@ -220 +217 @@
                         "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 " +
@@ -230 +227 @@
                         "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()) {

trunk/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java

@@ -15 +15 @@
  */
 public class DebugTextDisplay extends JScrollPane {
-    private final String text;
+    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 10585
      */
-    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);
@@ -31 +32 @@
 
     /**
-     * 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.
+     * @since 10585
+     */
+    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
+     */
+    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 <code>true</code> if copy was successful
      */
     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 10585
+     */
+    public String getCodeText() {
+        return text;
     }
 }

trunk/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java

@@ -3 +3 @@
 
 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;
 
 /**
@@ -25 +31 @@
  */
 public class ReportedException extends RuntimeException {
+    /**
+     * 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
@@ -35 +43 @@
      */
     private final transient Map<Thread, StackTraceElement[]> allStackTraces;
-    private final transient LinkedList<Section> sections = new LinkedList<>();
+    private final LinkedList<Section> sections = new LinkedList<>();
     private final transient Thread caughtOnThread;
     private final Throwable exception;
@@ -144 +152 @@
         }
 
-        Set<Throwable> dejaVu = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>());
-        return hasSameStackTrace(dejaVu, this.exception, e.exception);
-    }
-
-    private static boolean hasSameStackTrace(Set<Throwable> dejaVu, Throwable e1, Throwable e2) {
-        if (dejaVu.contains(e1)) {
-            // cycle. If it was the same until here, we assume both have that cycle.
+        return hasSameStackTrace(new CauseTraceIterator(), e.exception);
+    }
+
+    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();
@@ -167 +173 @@
             return false;
         } else if (c1 != null) {
-            return hasSameStackTrace(dejaVu, c1, c2);
+            return hasSameStackTrace(causeTraceIterator, c2);
         } else {
             return true;
@@ -228 +234 @@
     }
 
-    private static class SectionEntry {
+
+    /**
+     * Check if this exception may be caused by a threading issue.
+     * @return <code>true</code> if it is.
+     * @since 10585
+     */
+    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 10585
+     */
+    private final class CauseTraceIterator implements Iterator<Throwable> {
+        private Throwable current = exception;
+        private final Set<Throwable> dejaVu = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>());
+
+        @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;
@@ -249 +302 @@
     }
 
-    private static class Section {
+    private static class Section implements Serializable {
+
+        private static final long serialVersionUID = 1L;
 
         private final String sectionName;