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)
Change History (14)
by , 9 years ago
Attachment: | josm-errors.png added |
---|
by , 9 years ago
Attachment: | josm-errors2.png added |
---|
follow-up: 5 comment:1 by , 9 years ago
comment:2 by , 9 years ago
Milestone: | → 16.05 |
---|
follow-up: 4 comment:3 by , 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.
comment:4 by , 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.
comment:5 by , 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 , 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 , 9 years ago
Milestone: | 16.05 → 16.06 |
---|
comment:8 by , 9 years ago
Milestone: | 16.06 → 16.07 |
---|
comment:9 by , 9 years ago
Component: | Core → Core bugreport |
---|
comment:10 by , 9 years ago
Milestone: | 16.07 → 16.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:12 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.
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.