[Patch] Add data to bug report

[michael2402]

See #12805

Adds a new error reporting class that allows you to add more information to a crash that happened.

I plan to extend the class BugReport to create the report text (status report, ...) in the next days.

I already added the catch code to MapView. Whenever an error happens during drawing the map, information about the layers that are displayed is printed. Those catch blocks don't cost any performance, so we can add as many as we want.

The report then looks like this:

=== REPORTED CRASH DATA ===
MapView#paintLayer:
 - layer: org.openstreetmap.josm.gui.layer.OsmDataLayer@6d4a427a
 - bounds: Bounds[49.4938256,8.0819993,49.4976837,8.0879016]

=== STACK TRACE ===
Thread: AWT-EventQueue-0 (16) of main
java.lang.RuntimeException: test
	at org.openstreetmap.josm.gui.layer.OsmDataLayer.paint(OsmDataLayer.java:408)
	at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:740)
	at org.openstreetmap.josm.gui.MapView.paint(MapView.java:811)
	at javax.swing.JComponent.paintChildren(JComponent.java:879)
...

[stoecker]

Why not shorten the lines instead of adding an exception?

[Don-vip]

It's because I think we need line numbers in the stacktrace. That's not the way to go indeed. Instead of adding an exception, it's better to add:

                // CHECKSTYLE.OFF: LineLength
                // @formatter:off
                // your long line
                // @formatter:on
                // CHECKSTYLE.ON: LineLength

this prevents Checkstyle to complain, and Eclipse formatter to mess with your code.

[michael2402]

Yes, I can change that. I'll update the patches.

The problem is that two stack traces should be considered the same if all frames are the same (same file, same line number, ...). We could improve this some day (only consider the last X frames, ...). I intend to use this to give the user the ability to only suppress this one type of error.

[michael2402]

I updated the patch. @formatter:on is disabled for the current formatter proflie, see #12877

[Don-vip]

Thanks! Some remarks:

• BugReport:
  • checkstyle: Utility classes should not have a public constructor
• ReportedException:
  • findbugs: caughtOnThread should be transient
  • sonar: methodWarningFrom should be final
  • sonar: "thread.getKey() == caughtOnThread" should use equals instead
  • sonar: niceThreadName should be static
  • sonar: hasSameStackTrace should be static
  • sonar: "catch (Throwable t)" should be replaced by "catch (RuntimeException e)"
  • sonar: makeCollectionNice should be static
  • sonar: makeCollectionNice should use a StringBuilder instead of string concatenation
  • sonar: SectionEntry.print should be documented
  • sonar: Section.put and Section.printSection should be documented

You should configure Eclipse to use the Checkstyle, Findbugs and SonarLint plugins :)

[michael2402]

Thanks for the hints. I installed the plugins and tested them.

I still have the following warnings:

• BugReport constructor - OK I'll fix this next week when working on that class.
• ReportedException methodWarningFrom / TODO - next week
• ReportedExceptionTest: no assert in test case - I have not found an easy way to convince sonar that I simply want the test to not throw any exception. And in one case it did not get the assertEquals inside the loop.

[michael2402]

Test cases

[Don-vip]

Replying to michael2402:

I still have the following warnings:

• ReportedExceptionTest: no assert in test case - I have not found an easy way to convince sonar that I simply want the test to not throw any exception. And in one case it did not get the assertEquals inside the loop.

Me neither, that's OK. I don't know if I'll keep this rule enabled. Sometimes it's useful...

[Don-vip]

In 10285/josm:

fix #12875 - Add data to bug report (patch by michael2402)