Modify

Opened 9 years ago

Closed 8 years ago

#12805 closed enhancement (fixed)

More detailed bug reports

Reported by: michael2402 Owned by: michael2402
Priority: normal Milestone: 16.08
Component: Core bugreport Version:
Keywords: gsoc-core Cc: Don-vip, bastiK, stoecker

Description

Currently, bug reports only contain the stack trace of the method that threw the exception. We know which part of code threw the exception but we do not know which part of the data.

I want to allow custom variables to be added the the bug report. We can add catch blocks to the code that add information about the data that caused the exception. This should not contain really sensitive information but I think that adding e.g. the OSM ID is OK.

A report cold look like this:

== Stacktrace ==
<Full Java Stacktace>

== StyledMapRenderer#render ==
primitive: {Node id=...}

== OsmDataLayer#paint ==
virtualNodes: true

== MapView#paintLayer ==
layer: OsmDataLayer@...
bounds: Bounds[...]

For adding this, I suggest to use:

try {
    ... do some things that might crash here ...
} catch (Throwable t) {
    throw BugReports.create(t).put("name", value);
}

The mechanics behind this:

  • BugReports.create wraps t with a new BugReportingException.
  • The method calling this is automatically added.
  • This exception has a put(key, value) method that allows us to add any String values to it. toString() is called in a catch block to avoid more exceptions.
  • put returns the same exception for easy chaining.
  • You can simply throw that BugReportingException - it will be caught by the global exception handler.

If you want to not throw the exception, you can use BugReportingException#display(). This displays the error dialog to the user and lets JOSM continue. This is useful e.g. for map painting: we do not want to freeze JOSM just because there is an error in the render engine - the rest of it should still work and allow you to save your changes.

There should be no performance impact - try blocks do not generate any java bytecode and we don't care about performance after an exception occurred.

While doing this, I want to improve the error report dialog to include more information. There should be only 2 dialogs:
(1) A status report that informs the user of what happend and what can be done (update, ...)

(2) A dialog allowing the user to prepare the bug report.

The steps to implement this are:
(1) Add the bug report code for rethrow only. Timeframe: May 23 to May 27.
(2) Create the new Bug report dialog and replace the old code (big patch) Timeframe: May 30 to June 10.
(4) Add catch blocks to the code on strategic places, replace Main.error(t) using the new dialog where appropriate. (Timeframe: -)

While doing this, I want to add all bug report related code to the package ...josm.tools.bugreport

Attachments (2)

josm-errors.png (135.0 KB ) - added by michael2402 9 years ago.
josm-errors2.png (71.0 KB ) - added by michael2402 9 years ago.

Download all attachments as: .zip

Change History (14)

by michael2402, 9 years ago

Attachment: josm-errors.png added

by michael2402, 9 years ago

Attachment: josm-errors2.png added

comment:1 by Don-vip, 9 years ago

This looks very good!
One general advice in new UI: avoid negative sentences: "I think..." is easier to understand that "I don't think..."

Although most of the time users don't know if it's a core, plugin, or java problem: that's up to us. Maybe these checkboxes are not needed.

comment:2 by Don-vip, 9 years ago

Milestone: 16.05

comment:3 by stoecker, 9 years ago

I miss a handling of errors in passing your optional arguments. E.g. assume access to osmid causes a bug and the bugreport wants to send exactly that id. You don't get the original error, but the unhelpful error accessing the id.

in reply to:  3 comment:4 by michael2402, 9 years ago

Replying to stoecker:

I miss a handling of errors in passing your optional arguments. E.g. assume access to osmid causes a bug and the bugreport wants to send exactly that id. You don't get the original error, but the unhelpful error accessing the id.

toString() is called in a catch block to avoid more exceptions.

This should allow us to catch most errors. For now, it should be enough to simply pass the objects and call toString(). We can add more converters in the future (Arrays, ...) but I think that toString() should be enough for now.

As long as all calls are simply put(...) calls with no other methods called there should be no problem.

in reply to:  1 comment:5 by michael2402, 9 years ago

Replying to Don-vip:

This looks very good!
One general advice in new UI: avoid negative sentences: "I think..." is easier to understand that "I don't think..."

Although most of the time users don't know if it's a core, plugin, or java problem: that's up to us. Maybe these checkboxes are not needed.

Thanks.

We can leave them away for now. If we get too many reports form people using old JOSM versions or crashed plugins, we can add them and disable the ticket button until they are checked - so that people do not just press it without reading.

comment:6 by stoecker, 9 years ago

Please keep in mind that e.g. Debian uses an own bug reporting. They are probably happy when the can rely on a better interface instead of patching JOSM code.

comment:7 by Don-vip, 9 years ago

Milestone: 16.0516.06

comment:8 by michael2402, 9 years ago

Milestone: 16.0616.07

comment:9 by Klumbumbus, 9 years ago

Component: CoreCore bugreport

comment:10 by michael2402, 9 years ago

Milestone: 16.0716.08

The plugin information can still be moved to this new report. I also wrote a new bug report queue to replace the handler thread. It will prevent multiple dialogs from opening at once and allow to suppress only that single error.

comment:11 by Don-vip, 8 years ago

still something to do, or could we close this ticket as fixed?

comment:12 by michael2402, 8 years ago

Resolution: fixed
Status: newclosed

The only thing that is missing from my initial design is integrating the plugin warning into the dialog. It is still shown before the dialog opens. But we can leave it the way it is now.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain michael2402.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.