TrenchBroom / TrenchBroom

Cross-Platform Level Editor
kristianduske.com/trenchbroom
GNU General Public License v3.0
1.88k stars 224 forks source link

Add crash reporting #1130

Closed kduske closed 7 years ago

kduske commented 8 years ago

When a crash occurs:

  1. Save the map file to -crash-.map
  2. Show a dialog informing the user of the crash and the saved file. The dialog should also contain a button to send a crash report, which is an email containing the following information:
    • The exception message.
    • The stack trace where the crash occurred.
    • The map file as an attachment.
kduske commented 8 years ago

@ericwa there's this: http://docs.wxwidgets.org/trunk/classwx_debug_report.html

I knew about it and have dismissed using it for some reason that I don't remember. Maybe I was wrong?

ericwa commented 8 years ago

I just experimented with OnFatalException and wxStackWalker a bit, and was able to get it working on OS X.

I tried triggering a crash with a null pointer dereference, and the OnFatalException handler gets called and I can print the stack trace of where the null pointer dereference happened. Haven't tried wxDebugReport yet, but it sounds good.

ericwa commented 8 years ago

Here's what wxDebugReport looks like: screen shot 2016-03-14 at 2 39 07 pm Looks like you can add arbitrary information to the xml file - GL driver version would be useful.

I'm not sure if it's necessarily useful, the dialog is kind of verbose, and I think it would be better to save the crash report alongside the map file, rather than in a temporary directory. The other concern I have is, sending the report by email doesn't sound great to me; I don't have a desktop email client set up and wxWidgets doesn't seem to have a way to compose an email with a given attachment.

What if TB did the following instead?

  1. Save a report to mapname-crash.txt (in the same directory as the map) with:
    • stack trace
    • GL driver info, OS version, etc.
  2. Save the map to mapname-crash.map (note: depending on how TB failed, I expect this will crash sometimes).
  3. Show a dialog saying "TB crashed. A log was saved to mapname-crash.txt and the current state of the map was saved to mapname-crash.map. Please create an issue at https://github.com/kduske/TrenchBroom/ and upload both files."
kduske commented 8 years ago

Yes, that sounds better than the email option. One thing to consider is where to save the map and error file if the map file was currently not yet saved. The file should contain all necessary info, which I consider to be

Also, it doesn't have to be XML at all. Actually, I'd much rather it just be text. So, since you have this half working already, would you consider implementing this?

ericwa commented 8 years ago

Sounds good - I'll continue implementing this. Yeah, I was leaning towards plain text as well.

Looks like http://docs.wxwidgets.org/trunk/classwx_standard_paths.html offers access to places like the user's Documents or Desktop directory, so maybe putting crash reports for unsaved documents in Documents would work?

Also need to handle the case when mapname-crash.{map/txt} already exist - maybe appending a number suffix like mapname-crash-2.txt?

kduske commented 8 years ago

Yes to both questions.