From 179fe79168908c6bd9b0df1220d7b226a892291d Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 19 Jun 2023 23:31:02 +0200 Subject: [PATCH 1/3] Better crash handler --- interface/src/CrashRecoveryHandler.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/interface/src/CrashRecoveryHandler.cpp b/interface/src/CrashRecoveryHandler.cpp index 94523333d4..0efbbe5fa0 100644 --- a/interface/src/CrashRecoveryHandler.cpp +++ b/interface/src/CrashRecoveryHandler.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -31,6 +32,7 @@ #include #include #include +#include bool CrashRecoveryHandler::checkForResetSettings(bool wasLikelyCrash, bool suppressPrompt) { @@ -79,11 +81,27 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show QRadioButton* option1 = new QRadioButton("Reset all my settings"); QRadioButton* option2 = new QRadioButton("Reset my settings but keep essential info"); QRadioButton* option3 = new QRadioButton("Continue with my current settings"); + + QLabel* crashReportLabel = new QLabel("To help us with debugging, you can enable automatic crash reports.\n" + "They'll only be seen by members of the Overte e.V. organization and\n" + "trusted developers, and will only be used for improving the code."); + + QCheckBox* crashReportCheckbox = new QCheckBox("Enable automatic crash reporting"); + + auto &ual = UserActivityLogger::getInstance(); + crashReportCheckbox->setChecked(ual.isCrashMonitorEnabled()); + + + option3->setChecked(true); layout->addWidget(option1); layout->addWidget(option2); layout->addWidget(option3); layout->addSpacing(12); + + layout->addWidget(crashReportLabel); + layout->addWidget(crashReportCheckbox); + layout->addSpacing(12); layout->addStretch(); QDialogButtonBox* buttons = new QDialogButtonBox(QDialogButtonBox::Ok); @@ -103,6 +121,8 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show } } + ual.crashMonitorDisable(!crashReportCheckbox->isChecked()); + // Dialog cancelled or "do nothing" option chosen return CrashRecoveryHandler::DO_NOTHING; } From de706ab4581ea9d8d85e4babad4d3b64ef887fca Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 25 Jun 2023 20:14:13 +0200 Subject: [PATCH 2/3] Rework crash reporting UI * Encourage users running dev code to submit reports * Change from "Disable" to "Enable" * Always initialize Crashpad and just prevent uploads if disabled * Fix menu not being in sync * Add documentation * Keep track of whether Crashpad initialized successfully --- interface/src/CrashHandler.h | 2 + interface/src/CrashHandler_Breakpad.cpp | 5 + interface/src/CrashHandler_Crashpad.cpp | 47 +++++++- interface/src/CrashHandler_None.cpp | 3 + interface/src/CrashRecoveryHandler.cpp | 114 ++++++++++++++++-- interface/src/CrashRecoveryHandler.h | 7 ++ interface/src/Menu.cpp | 9 +- interface/src/Menu.h | 4 +- interface/src/main.cpp | 14 ++- interface/src/ui/PreferencesDialog.cpp | 10 +- .../networking/src/UserActivityLogger.cpp | 13 +- libraries/networking/src/UserActivityLogger.h | 73 +++++++++-- 12 files changed, 262 insertions(+), 39 deletions(-) diff --git a/interface/src/CrashHandler.h b/interface/src/CrashHandler.h index 27c5d76e1c..a22a614fc5 100644 --- a/interface/src/CrashHandler.h +++ b/interface/src/CrashHandler.h @@ -21,5 +21,7 @@ Q_DECLARE_LOGGING_CATEGORY(crash_handler) bool startCrashHandler(std::string appPath); void setCrashAnnotation(std::string name, std::string value); void startCrashHookMonitor(QCoreApplication* app); +void setCrashReportingEnabled(bool value); + #endif // hifi_CrashHandler_h diff --git a/interface/src/CrashHandler_Breakpad.cpp b/interface/src/CrashHandler_Breakpad.cpp index 839e99ad3d..fa24b43107 100644 --- a/interface/src/CrashHandler_Breakpad.cpp +++ b/interface/src/CrashHandler_Breakpad.cpp @@ -81,6 +81,11 @@ void setCrashAnnotation(std::string name, std::string value) { flushAnnotations(); } +void setCrashReportingEnabled(bool value) { + qCritical() << "Can't set crash reporting status on Breakpad."; + qCritical() << "Breakpad is deprecated and needs replacing!"; +} + void startCrashHookMonitor(QCoreApplication* app) { } diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index a9efdea98e..5b39a5f88a 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -43,6 +43,8 @@ Q_LOGGING_CATEGORY(crash_handler, "overte.crash_handler") #include #include #include +#include + static const std::string BACKTRACE_URL{ CMAKE_BACKTRACE_URL }; static const std::string BACKTRACE_TOKEN{ CMAKE_BACKTRACE_TOKEN }; @@ -134,6 +136,9 @@ bool SpinLockLocker::relock(int msecs /* = -1 */ ) { // ------------------------------------------------------------------------------------------------ crashpad::CrashpadClient* client{ nullptr }; +std::unique_ptr< crashpad::CrashReportDatabase > crashpadDatabase; + + SpinLock crashpadAnnotationsProtect; crashpad::SimpleStringDictionary* crashpadAnnotations{ nullptr }; @@ -409,14 +414,20 @@ bool startCrashHandler(std::string appPath) { base::FilePath handler(handlerPath); qCDebug(crash_handler) << "Opening crashpad database" << QString::fromStdString(crashpadDbPath); - auto database = crashpad::CrashReportDatabase::Initialize(db); - if (database == nullptr || database->GetSettings() == nullptr) { + crashpadDatabase = crashpad::CrashReportDatabase::Initialize(db); + if (crashpadDatabase == nullptr || crashpadDatabase->GetSettings() == nullptr) { qCCritical(crash_handler) << "Failed to open crashpad database" << QString::fromStdString(crashpadDbPath); return false; } // Enable automated uploads. - database->GetSettings()->SetUploadsEnabled(true); + QObject::connect(&UserActivityLogger::getInstance(), &UserActivityLogger::crashReportingEnabledChanged, []() { + auto &ual = UserActivityLogger::getInstance(); + setCrashReportingEnabled(ual.isCrashReportingEnabled()); + }); + + crashpadDatabase->GetSettings()->SetUploadsEnabled(UserActivityLogger::getInstance().isCrashReportingEnabled()); + if (!client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true)) { qCCritical(crash_handler) << "Failed to start crashpad handler"; @@ -432,6 +443,36 @@ bool startCrashHandler(std::string appPath) { return true; } +void setCrashReportingEnabled(bool enabled) { + auto settings = crashpadDatabase->GetSettings(); + settings->SetUploadsEnabled(enabled); + + if (enabled) { + // Enabled now, was disabled before + // + // Reports are generated while uploads are disabled, so if we didn't do this we could + // send something the user doesn't want sent. So if reporting was disabled then enabled, + // remove any pending reports. + qCDebug(crash_handler) << "Removing any pending crash reports"; + + std::vector pendingReports; + crashpad::CrashReportDatabase::OperationStatus status; + + status = crashpadDatabase->GetPendingReports(&pendingReports); + + if (status != crashpad::CrashReportDatabase::kNoError) { + qCWarning(crash_handler) << "Failed to get pending reports"; + } else { + for (const auto& report : pendingReports) { + qCDebug(crash_handler) << "Deleted crash report" << QString::fromStdString(report.uuid.ToString()); + crashpadDatabase->DeleteReport(report.uuid); + } + } + } + + qCInfo(crash_handler) << "Crashpad uploads " << (enabled ? QString("enabled") : QString("disabled")); +} + void setCrashAnnotation(std::string name, std::string value) { if (client) { SpinLockLocker guard(crashpadAnnotationsProtect); diff --git a/interface/src/CrashHandler_None.cpp b/interface/src/CrashHandler_None.cpp index 718a955e81..53ff5b8286 100644 --- a/interface/src/CrashHandler_None.cpp +++ b/interface/src/CrashHandler_None.cpp @@ -31,4 +31,7 @@ void setCrashAnnotation(std::string name, std::string value) { void startCrashHookMonitor(QCoreApplication* app) { } +void setCrashReportingEnabled(bool value) { + +} #endif diff --git a/interface/src/CrashRecoveryHandler.cpp b/interface/src/CrashRecoveryHandler.cpp index 0efbbe5fa0..df0e490895 100644 --- a/interface/src/CrashRecoveryHandler.cpp +++ b/interface/src/CrashRecoveryHandler.cpp @@ -27,15 +27,22 @@ #include "Menu.h" + #include #include #include #include #include #include +#include + + static const QString BACKTRACE_URL{ CMAKE_BACKTRACE_URL }; bool CrashRecoveryHandler::checkForResetSettings(bool wasLikelyCrash, bool suppressPrompt) { + Setting::Handle crashReportingAsked { "CrashReportingAsked", false }; + + Settings settings; settings.beginGroup("Developer"); QVariant displayCrashOptions = settings.value(MenuOption::DisplayCrashOptions); @@ -47,6 +54,7 @@ bool CrashRecoveryHandler::checkForResetSettings(bool wasLikelyCrash, bool suppr // If option does not exist in Interface.ini so assume default behavior. bool displaySettingsResetOnCrash = !displayCrashOptions.isValid() || displayCrashOptions.toBool(); + bool userPrompted = false; if (suppressPrompt) { return wasLikelyCrash; @@ -54,18 +62,106 @@ bool CrashRecoveryHandler::checkForResetSettings(bool wasLikelyCrash, bool suppr if (wasLikelyCrash || askToResetSettings) { if (displaySettingsResetOnCrash || askToResetSettings) { + userPrompted = true; Action action = promptUserForAction(wasLikelyCrash); if (action != DO_NOTHING) { handleCrash(action); } } } + + if (!userPrompted) { + // Both dialogs share a purpose -- if we already showed the full dialog, no need to show this one. + if (BuildInfo::BUILD_TYPE != BuildInfo::BuildType::Stable) { + // We didn't crash but are running a development build -- we'd like reports if possible. + if (suggestCrashReporting()) { + // If suggestCrashReporting returns false, we didn't ask the user. + crashReportingAsked.set(true); + } + } + + } + return wasLikelyCrash; } + +bool CrashRecoveryHandler::suggestCrashReporting() { + QDialog crashDialog; + + crashDialog.setWindowTitle("You are running an experimental version of Overte"); + + QVBoxLayout* layout = new QVBoxLayout; + + QString explainText; + auto &ual = UserActivityLogger::getInstance(); + + + + switch(BuildInfo::BUILD_TYPE) { + case BuildInfo::BuildType::Dev: + explainText = "You're running a pre-release version. This is an official release, but the code\n" + "is not yet considered to be fully stable. We'd highly appreciate it if you enabled\n" + "crash reporting to help us test the upcoming release."; + break; + case BuildInfo::BuildType::PR: + // TODO: It would be nice to have here the PR number, and who submitted it. This would require GHA support. + explainText = "You're running a PR version. This is experimental code contributed by a third party\n" + "and not yet part of the official code. We'd highly appreciate it if you enabled\n" + "crash reporting to help us test this potential addition."; + break; + case BuildInfo::BuildType::Master: + explainText = "You're running a pre-release version. This is an official release, but the code\n" + "is not yet considered to be fully stable. We'd highly appreciate it if you enabled\n" + "crash reporting to help us test the upcoming release."; + break; + case BuildInfo::BuildType::Stable: + explainText = "You're running a stable version. This is an official release, and should perform\n" + "correctly. Nevertheless, we'd highly appreciate it if you enabled crash reporting\n" + "to help us catch any remaining problems."; + break; + } + + if (!ual.isCrashMonitorStarted()) { + qWarning() << "Crash reporting not working, skipping suggestion to enable it."; + return false; + } + + QLabel* explainLabel = new QLabel(explainText); + + QLabel* crashReportLabel = new QLabel("Reports can only be seen by members of the Overte e.V. organization and\n" + "trusted developers, and will only be used for improving the code."); + + QCheckBox* crashReportCheckbox = new QCheckBox("Enable automatic crash reporting"); + + crashReportCheckbox->setChecked(ual.isCrashReportingEnabled()); + crashReportCheckbox->setEnabled(ual.isCrashMonitorStarted()); + + layout->addWidget(explainLabel); + layout->addSpacing(12); + layout->addWidget(crashReportLabel); + layout->addWidget(crashReportCheckbox); + layout->addSpacing(12); + layout->addStretch(); + + QDialogButtonBox* buttons = new QDialogButtonBox(QDialogButtonBox::Ok); + layout->addWidget(buttons); + crashDialog.connect(buttons, SIGNAL(accepted()), SLOT(accept())); + + crashDialog.setLayout(layout); + + crashDialog.exec(); + + ual.setCrashReportingEnabled(crashReportCheckbox->isChecked()); + + return true; +} + CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool showCrashMessage) { QDialog crashDialog; QLabel* label; + auto &ual = UserActivityLogger::getInstance(); + if (showCrashMessage) { crashDialog.setWindowTitle("Interface Crashed Last Run"); label = new QLabel("If you are having trouble starting would you like to reset your settings?"); @@ -81,17 +177,21 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show QRadioButton* option1 = new QRadioButton("Reset all my settings"); QRadioButton* option2 = new QRadioButton("Reset my settings but keep essential info"); QRadioButton* option3 = new QRadioButton("Continue with my current settings"); + QLabel* crashReportLabel = nullptr; - QLabel* crashReportLabel = new QLabel("To help us with debugging, you can enable automatic crash reports.\n" - "They'll only be seen by members of the Overte e.V. organization and\n" - "trusted developers, and will only be used for improving the code."); + if (ual.isCrashMonitorStarted()) { + crashReportLabel = new QLabel("To help us with debugging, you can enable automatic crash reports.\n" + "They'll only be seen by members of the Overte e.V. organization and\n" + "trusted developers, and will only be used for improving the code."); + } else { + crashReportLabel = new QLabel("Unfortunately, crash reporting isn't built into this release."); + } QCheckBox* crashReportCheckbox = new QCheckBox("Enable automatic crash reporting"); - auto &ual = UserActivityLogger::getInstance(); - crashReportCheckbox->setChecked(ual.isCrashMonitorEnabled()); - + crashReportCheckbox->setChecked(ual.isCrashReportingEnabled()); + crashReportCheckbox->setEnabled(ual.isCrashMonitorStarted()); option3->setChecked(true); layout->addWidget(option1); @@ -121,7 +221,7 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show } } - ual.crashMonitorDisable(!crashReportCheckbox->isChecked()); + ual.setCrashReportingEnabled(crashReportCheckbox->isChecked()); // Dialog cancelled or "do nothing" option chosen return CrashRecoveryHandler::DO_NOTHING; diff --git a/interface/src/CrashRecoveryHandler.h b/interface/src/CrashRecoveryHandler.h index 8df72eeb75..c0105e9045 100644 --- a/interface/src/CrashRecoveryHandler.h +++ b/interface/src/CrashRecoveryHandler.h @@ -14,6 +14,11 @@ #include + + + + + class CrashRecoveryHandler { public: @@ -26,8 +31,10 @@ private: DO_NOTHING }; + static bool suggestCrashReporting(); static Action promptUserForAction(bool showCrashMessage); static void handleCrash(Action action); + }; #endif // hifi_CrashRecoveryHandler_h diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index cec88827e9..9ff82b891b 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -368,7 +368,7 @@ Menu::Menu() { // Developer > Scripting > Verbose Logging addCheckableActionToQMenuAndActionHash(scriptingOptionsMenu, MenuOption::VerboseLogging, 0, false, qApp, SLOT(updateVerboseLogging())); - + // Developer > Scripting > Enable Cachebusting of Script.require addCheckableActionToQMenuAndActionHash(scriptingOptionsMenu, MenuOption::CachebustRequire, 0, false, qApp, SLOT(setCachebustRequire())); @@ -637,11 +637,12 @@ Menu::Menu() { &UserActivityLogger::getInstance(), SLOT(disable(bool))); addCheckableActionToQMenuAndActionHash(networkMenu, - MenuOption::DisableCrashLogger, + MenuOption::EnableCrashReporting, 0, - true, + UserActivityLogger::getInstance().isCrashReportingEnabled(), &UserActivityLogger::getInstance(), - SLOT(crashMonitorDisable(bool))); + SLOT(setCrashReportingEnabled(bool))); + addActionToQMenuAndActionHash(networkMenu, MenuOption::ShowDSConnectTable, 0, qApp, SLOT(loadDomainConnectionDialog())); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index c42345894c..2bb9f10e68 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -90,7 +90,7 @@ namespace MenuOption { const QString DeleteAvatarEntitiesBookmark = "Delete Avatar Entities Bookmark"; const QString DeleteBookmark = "Delete Bookmark..."; const QString DisableActivityLogger = "Disable Activity Logger"; - const QString DisableCrashLogger = "Disable Crash Reporter"; + const QString EnableCrashReporting = "Enable Crash Reporting"; const QString DisableEyelidAdjustment = "Disable Eyelid Adjustment"; const QString DisableLightEntities = "Disable Light Entities"; const QString DisplayCrashOptions = "Display Crash Options"; @@ -145,7 +145,7 @@ namespace MenuOption { const QString OctreeStats = "Entity Statistics"; const QString OnePointCalibration = "1 Point Calibration"; const QString OnlyDisplayTopTen = "Only Display Top Ten"; - const QString OpenVrThreadedSubmit = "OpenVR Threaded Submit"; + const QString OpenVrThreadedSubmit = "OpenVR Threaded Submit"; const QString OutputMenu = "Display"; const QString Overlays = "Show Overlays"; const QString PackageModel = "Package Avatar as .fst..."; diff --git a/interface/src/main.cpp b/interface/src/main.cpp index de08708e41..4d86c2c4d7 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -421,11 +421,17 @@ int main(int argc, const char* argv[]) { } qDebug() << "UserActivityLogger is enabled:" << ual.isEnabled(); - bool isCrashHandlerEnabled = ual.isCrashMonitorEnabled() || parser.isSet(forceCrashReportingOption); - qDebug() << "Crash handler logger is enabled:" << isCrashHandlerEnabled; - if (isCrashHandlerEnabled) { - auto crashHandlerStarted = startCrashHandler(argv[0]); + if (parser.isSet(forceCrashReportingOption)) { + qInfo() << "Crash reporting enabled on the command-line"; + ual.setCrashReportingEnabled(true); + } + + auto crashHandlerStarted = startCrashHandler(argv[0]); + if (crashHandlerStarted) { + ual.setCrashMonitorStarted(true); qDebug() << "Crash handler started:" << crashHandlerStarted; + } else { + qWarning() << "Crash handler failed to start"; } const QString& applicationName = getInterfaceSharedMemoryName(); diff --git a/interface/src/ui/PreferencesDialog.cpp b/interface/src/ui/PreferencesDialog.cpp index 6d13f586d4..e8c2166d26 100644 --- a/interface/src/ui/PreferencesDialog.cpp +++ b/interface/src/ui/PreferencesDialog.cpp @@ -238,9 +238,9 @@ void setupPreferences() { auto preference = new BrowsePreference(SNAPSHOTS, "Put my snapshots here", getter, setter); preferences->addPreference(preference); } - + { - auto getter = []()->bool { + auto getter = []()->bool { if (!DependencyManager::get()->_snapshotNotifications.isSet()){ DependencyManager::get()->_snapshotNotifications.set(true); } @@ -278,7 +278,7 @@ void setupPreferences() { preference->setItems(items); preferences->addPreference(preference); } - + { auto getter = []()->float { return SnapshotAnimated::snapshotAnimatedDuration.get(); }; auto setter = [](float value) { SnapshotAnimated::snapshotAnimatedDuration.set(value); }; @@ -299,8 +299,8 @@ void setupPreferences() { } { - auto getter = []()->bool { return !Menu::getInstance()->isOptionChecked(MenuOption::DisableCrashLogger); }; - auto setter = [](bool value) { Menu::getInstance()->setIsOptionChecked(MenuOption::DisableCrashLogger, !value); }; + auto getter = []()->bool { return Menu::getInstance()->isOptionChecked(MenuOption::EnableCrashReporting); }; + auto setter = [](bool value) { Menu::getInstance()->setIsOptionChecked(MenuOption::EnableCrashReporting, value); }; preferences->addPreference(new CheckPreference("Privacy", "Send crashes - Overte uses information provided by your " "client to improve the product through crash reports. By allowing Overte to collect " "this information you are helping to improve the product. ", getter, setter)); diff --git a/libraries/networking/src/UserActivityLogger.cpp b/libraries/networking/src/UserActivityLogger.cpp index 4454648564..96a1625dda 100644 --- a/libraries/networking/src/UserActivityLogger.cpp +++ b/libraries/networking/src/UserActivityLogger.cpp @@ -34,10 +34,6 @@ void UserActivityLogger::disable(bool disable) { _disabled.set(disable); } -void UserActivityLogger::crashMonitorDisable(bool disable) { - _crashMonitorDisabled.set(disable); -} - void UserActivityLogger::logAction(QString action, QJsonObject details, JSONCallbackParameters params) { // qCDebug(networking).nospace() << ">>> UserActivityLogger::logAction(" << action << "," << QJsonDocument(details).toJson(); // This logs what the UserActivityLogger would normally send to centralized servers. @@ -86,6 +82,15 @@ void UserActivityLogger::requestError(QNetworkReply* errorReply) { qCDebug(networking) << errorReply->error() << "-" << errorReply->errorString(); } +void UserActivityLogger::setCrashReportingEnabled(bool enabled) { + bool old = _crashReportingEnabled.get(); + _crashReportingEnabled.set(enabled); + + if (old != enabled) { + emit crashReportingEnabledChanged(); + } + } + void UserActivityLogger::launch(QString applicationVersion, bool previousSessionCrashed, int previousSessionRuntime) { const QString ACTION_NAME = "launch"; QJsonObject actionDetails; diff --git a/libraries/networking/src/UserActivityLogger.h b/libraries/networking/src/UserActivityLogger.h index 39efee03c4..251468b0d3 100644 --- a/libraries/networking/src/UserActivityLogger.h +++ b/libraries/networking/src/UserActivityLogger.h @@ -27,40 +27,93 @@ const QString USER_ACTIVITY_URL = "/api/v1/user_activities"; class UserActivityLogger : public QObject { Q_OBJECT - + public: static UserActivityLogger& getInstance(); - + public slots: bool isEnabled() { return !_disabled.get(); } bool isDisabledSettingSet() const { return _disabled.isSet(); } - bool isCrashMonitorEnabled() { return !_crashMonitorDisabled.get(); } - bool isCrashMonitorDisabledSettingSet() const { return _crashMonitorDisabled.isSet(); } + + /** + * @brief Whether the crash monitor has been successfully started + * + * Reasons for it failing to start include: + * + * * Not having a crash reporter for the platform + * * Crash reporter not being configured with reporting URLs (CMAKE_BACKTRACE_TOKEN and CMAKE_BACKTRACE_URL) + * * Crash reporter is present and configured, but failed to initialize for some reason + * + * @return true Crash reporter is present, configured and working. + * @return false Crash reporter has not been started for one of the above reasons. + */ + bool isCrashMonitorStarted() const { return _crashMonitorStarted; } + + + /** + * @brief Whether the crash monitor will report crashes if they occur + * + * This setting is independent of isCrashMonitorStarted() -- crash reporting may be enabled but fail to work + * due to the crash reporting component being missing or failing to initialize. + * + * @return true Crashes will be reported to CMAKE_BACKTRACE_URL + * @return false Crashes will not be reported + */ + bool isCrashReportingEnabled() { return _crashReportingEnabled.get(); } + + /** + * @brief Marks the crash monitor as started + * + * @warning Only to be used as part of the startup process + * + * @param started + */ + void setCrashMonitorStarted(bool started) { _crashMonitorStarted = started; } + + /** + * @brief Set whether we want to submit crash reports to the report server + * + * The report server is configured with CMAKE_BACKTRACE_URL. + * Emits crashReportingEnabledChanged signal. + * + * @param enabled Whether it's enabled. + */ + void setCrashReportingEnabled(bool enabled); void disable(bool disable); - void crashMonitorDisable(bool disable); void logAction(QString action, QJsonObject details = QJsonObject(), JSONCallbackParameters params = JSONCallbackParameters()); - + void launch(QString applicationVersion, bool previousSessionCrashed, int previousSessionRuntime); void insufficientGLVersion(const QJsonObject& glData); - + void changedDisplayName(QString displayName); void changedModel(QString typeOfModel, QString modelURL); void changedDomain(QString domainURL); void connectedDevice(QString typeOfDevice, QString deviceName); void loadedScript(QString scriptName); void wentTo(AddressManager::LookupTrigger trigger, QString destinationType, QString destinationName); - + +signals: + + /** + * @brief The crash reporting setting has been changed. + * + * This signal is used by the crash reporter to enable/disable itself. + * + */ + void crashReportingEnabledChanged(); + private slots: void requestError(QNetworkReply* errorReply); - + private: UserActivityLogger(); Setting::Handle _disabled { "UserActivityLoggerDisabled", true }; - Setting::Handle _crashMonitorDisabled { "CrashMonitorDisabled2", true }; + Setting::Handle _crashReportingEnabled { "CrashReportingEnabled", false }; + bool _crashMonitorStarted {false}; QElapsedTimer _timer; }; From b2960d5a19bae2cd841085b80f98118244aa5a62 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 25 Jun 2023 21:46:57 +0200 Subject: [PATCH 3/3] Review fixes --- interface/src/CrashRecoveryHandler.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/interface/src/CrashRecoveryHandler.cpp b/interface/src/CrashRecoveryHandler.cpp index df0e490895..6f17492db2 100644 --- a/interface/src/CrashRecoveryHandler.cpp +++ b/interface/src/CrashRecoveryHandler.cpp @@ -89,7 +89,7 @@ bool CrashRecoveryHandler::checkForResetSettings(bool wasLikelyCrash, bool suppr bool CrashRecoveryHandler::suggestCrashReporting() { QDialog crashDialog; - crashDialog.setWindowTitle("You are running an experimental version of Overte"); + crashDialog.setWindowTitle("Overte"); QVBoxLayout* layout = new QVBoxLayout; @@ -129,8 +129,8 @@ bool CrashRecoveryHandler::suggestCrashReporting() { QLabel* explainLabel = new QLabel(explainText); - QLabel* crashReportLabel = new QLabel("Reports can only be seen by members of the Overte e.V. organization and\n" - "trusted developers, and will only be used for improving the code."); + QLabel* crashReportLabel = new QLabel("Reports can only be seen by developers trusted by the Overte e.V.\n" + "organization, and will only be used for improving the code."); QCheckBox* crashReportCheckbox = new QCheckBox("Enable automatic crash reporting"); @@ -181,8 +181,8 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show if (ual.isCrashMonitorStarted()) { crashReportLabel = new QLabel("To help us with debugging, you can enable automatic crash reports.\n" - "They'll only be seen by members of the Overte e.V. organization and\n" - "trusted developers, and will only be used for improving the code."); + "They'll only be seen by developers trusted by the Overte e.V. organization,\n" + "and will only be used for improving the code."); } else { crashReportLabel = new QLabel("Unfortunately, crash reporting isn't built into this release."); }