obsproject
frontend: Add automatic crash log upload after crashes for Windows and macOS
Description
This PR refactors the current safe mode implementation to allow automatic uploading of the most recent crash report upon a detected crash while avoiding duplicate uploads and also enables the feature on macOS.
Motivation and Context
The primary ask for this work was to enable the automatic upload of crash reports after OBS Studio has crashed and also enable this functionality to more platforms than just Windows.
[!NOTE] An even better solution towards this goal would be to use separate minimal process that uses platform-specific methods to "attach" to the main process and be able to detect and react to a crash and thus offer to relaunch the application and uploading a crash log from within it itself. That kind of solution was out of scope for this first iterative step.
To that end the following changes have been implemented:
- Crash mode detection and crash log uploading have been moved out of the application code base into their own separate
CrashHandlerclass. - Direct interaction between
CrashHandler,OBSApp, andOBSBasichas been decoupled and has been replaced by Qt signals which code can connect to and react accordingly - Each crash sentinel now represents a unique application launch, which potentially allows the
CrashHandlerclass to distinguish between a "live" sentinel for the currently running instance versus an "old" sentinel, which would represent an unclean prior shutdown - The most recent crash log file and corresponding upload URL are retained in the application config to avoid multiple uploads of the same crash log.
- When the most recent crash log file has not changed, the "Upload" menu action will just yield the prior URL.
- This also allows users to yield the URL of an automatic crash log upload
- When an unclean shutdown (or "crash") is detected, the user is still provided with a choice to run OBS Studio in "Safe Mode". An additional checkbox to automatically upload the most recent crash is also provided. The automatic upload is opt-in by default.
Platform differences
On Windows the existing custom crash log files are checked and used for crash log upload.
Changes: The CrashHandler class will look for the most recent file by actual creation date and not simply by the time code in its file name anymore.
On macOS the diagnostics reports generated by the operating system are used. No custom crash files are involved. As macOS "retires" active crash reports after some time, the subdirectory for retired reports is also taken into account when looking for a "most recent" crash, with the file creation date used for sorting the results.
On Linux and FreeBSD no searches for crash reports take place, the class methods are implemented as stubs to ensure the class implementation is complete on these platforms.
How Has This Been Tested?
Created custom crash logs in the locations used for macOS and Windows and checked that most recent crashes are found and uploaded from these locations accordingly.
- Checked behaviour when a new crash file was found (file was uploaded, new URL yielded)
- Checked behaviour when no new crash file was found but URL was empty (file was uploaded, new URL was displayed to user)
- Checked behaviour when file upload was unsuccessful (error message was displayed)
- Checked behaviour when file upload was successful but response JSON does not contain
urlfield (error message was displayed) - Checked behaviour when no new crash file was found with available URL (no upload took place, old URL was displayed to user)
- Checked behaviour when crash sentinel with old UUID was placed in OBS Studio settings directory (prior crash was "detected", safe mode choice was presented)
- Checked behaviour when automatic crash upload was selected in safe mode dialog (crash log was uploaded, no URL dialog was displayed)
- Checked behaviour when crash upload was selected after automatic crash upload (existing crash log URL was displayed to user)
- Checked behaviour when application was compiled in Debug configuration (no safe mode dialogs appeared, crash log behaviour was unchanged)
Types of changes
- New feature (non-breaking change which adds functionality)
- Code cleanup (non-breaking change which makes code smaller or more readable)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
Just things noticed after a quick test:
I think this text on the bottom could just say "Upload most recent OBS Studio crash report" rather than phrasing it as a question.
After checking this box, I also do not seem to get a popup showing the URL of the uploaded report like I do if I manually select in the menu "Help->Crash Reports->Upload Previous Crash Report". Is this expected or a bug?
After checking this box, I also do not seem to get a popup showing the URL of the uploaded report like I do if I manually select in the menu "Help->Crash Reports->Upload Previous Crash Report". Is this expected or a bug?
No that's intended - the requirement was that no further popups should be displayed to the user as we already have too many of those. Using the menu entry to get at that same URL was the proposed solution.
~~So, if I check that box, and then I navigate to the aforementioned help menu item (Help->Crash Reports->Upload Previous Crash Report), does that not upload the same report twice? Or do we detect that the report was already uploaded and just show that URL?~~
Edit: I see this is covered in the original description; duplicate uploads don't happen if the 'most recent detected crash log' has not changed.
I do think it's a bit weird to have the redundancy of the checkbox if, no matter what, the Help menu is still required to yield the URL of the uploaded log.
I do think it's a bit weird to have the redundancy of the checkbox if, no matter what, the Help menu is still required to yield the URL of the uploaded log.
As I said, that's by design, maybe @Warchamp7 can chime in here as that was the solution we ended up with here.
The basic idea is to lower the barrier for crash reports to be uploaded, particularly as this would allow automatic analysis of "current crashes" server-side without the manual input by users.
Got it; yeah, if part of the idea here is 'eventual server-side processing' then I think that makes sense enough to me.
Removed the intro text for every upload on macOS to upload the unchanged JSON contents of the crash log to the server. That way the file can be downloaded and parsed/opened directly (albeit with a possible change of file suffix) in Xcode or Console.
Added an experimental change for testers:
Instead of relying on the class destructors, both OBSApp's and CrashHandler's clean up code is now triggered by the aboutToQuit signal available to all sub-classes of QCoreApplication.
Per Qt, slots connected to signals are executed in the order they are added, so adding OBSApp's own handler first (and before constructing the CrashHandler) ensures that the application cleanup and libobos shutdown happen first, followed by the crash handler's cleanup and any crashes introduced by plugins will potentially leave the sentinel be.
I haven't had a chance to review this, but I might eventually prefer the migration to aboutToQuit in a separate PR, unless there is merit/value in keeping them together here.
I haven't had a chance to review this, but I might eventually prefer the migration to
aboutToQuitin a separate PR, unless there is merit/value in keeping them together here.
Both have to be added at the same time. Otherwise plugin shutdown happens after the CrashHandler has been shut down and thus any 3rd party plugin could introduce an unclean shutdown and thus we could not provide the "safe mode" as a fix against that misbehaving 3rd party plugin.
So it's either both or none of the changes.
Made the following substantial changes based on feedback:
- Log uploads (application logs as well as crash logs) now present a confirmation dialog with a link to the privacy policy first
- Application log uploads are now hooked-up using the same events-based system as crash log uploads
- Each commit is atomic, the project can be compiled without issue even with only the intermediate commits checked out. Only the final commit implements the actual changes.
The link to the Privacy Policy does not appear to be clickable on Windows. I'm not quite sure what needs to be done to solve that.
Can we add a QTimer to the upload process to make the uploading message visible for a minimum amount of time and avoid the flicker?
The link to the Privacy Policy does not appear to be clickable on Windows. I'm not quite sure what needs to be done to solve that.
There's no special handling for this on my end - I just use HTML in the string which is usually automatically translated/rendered by Qt. That pattern is used in other places in the UI as well.
Can we add a QTimer to the upload process to make the uploading message visible for a minimum amount of time and avoid the flicker?
One could add a timer for displaying the message that is canceled if a result event is emitted (because the timer should be associated with the visual behaviour of the dialog, not the actual upload). That would require "sharing" the timer reference somehow, but should be doable if Qt is reasonable enough about this.
Adapted changes from @Warchamp7 and addressed most open review comments via force-push.
For posterity, in last minute testing we discovered a long-standing issue that predates this PR where on First Run, OBS Studio will not create a sentinel file (used for checking if OBS shutdown cleanly). If a user crashes during their First Run of OBS Studio, the following run will not trigger the Unclean Shutdown dialog or the automatic crash reporting option.