From 2aa4e7c9da24d82bf7687f3005340d9a772e58e4 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Mon, 6 Mar 2023 13:02:57 +0100 Subject: [PATCH] feat(GODT-2446): Attach logs to sentry reports for relevant bridge-gui exceptions. --- .../bridge-gui/bridge-gui/AppController.cpp | 27 ++++---- .../bridge-gui/bridge-gui/AppController.h | 13 ++-- .../bridge-gui/bridge-gui/CMakeLists.txt | 1 + .../bridge-gui/EventStreamWorker.cpp | 2 +- .../bridge-gui/bridge-gui/LogUtils.cpp | 62 +++++++++++++++++++ .../frontend/bridge-gui/bridge-gui/LogUtils.h | 26 ++++++++ .../bridge-gui/bridge-gui/QMLBackend.cpp | 7 ++- .../bridge-gui/bridge-gui/QMLBackend.h | 2 +- .../bridge-gui/bridge-gui/SentryUtils.cpp | 50 +++++++++++++++ .../bridge-gui/bridge-gui/SentryUtils.h | 4 +- .../frontend/bridge-gui/bridge-gui/main.cpp | 18 +++--- .../bridgepp/bridgepp/Exception/Exception.cpp | 37 +++++++++-- .../bridgepp/bridgepp/Exception/Exception.h | 7 ++- 13 files changed, 211 insertions(+), 45 deletions(-) create mode 100644 internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp create mode 100644 internal/frontend/bridge-gui/bridge-gui/LogUtils.h diff --git a/internal/frontend/bridge-gui/bridge-gui/AppController.cpp b/internal/frontend/bridge-gui/bridge-gui/AppController.cpp index 4152d7d9..25d8e1f4 100644 --- a/internal/frontend/bridge-gui/bridge-gui/AppController.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/AppController.cpp @@ -32,6 +32,7 @@ namespace { QString const noWindowFlag = "--no-window"; ///< The no-window command-line flag. } + //**************************************************************************************************************************************************** /// \return The AppController instance. //**************************************************************************************************************************************************** @@ -71,36 +72,34 @@ ProcessMonitor *AppController::bridgeMonitor() const { //**************************************************************************************************************************************************** -/// \param[in] function The function that caught the exception. -/// \param[in] message The error message. -/// \param[in] details The details for the error. +/// \param[in] exception The exception that triggered the fatal error. //**************************************************************************************************************************************************** -void AppController::onFatalError(QString const &function, QString const &message, QString const& details) { - QString fullMessage = QString("%1(): %2").arg(function, message); - if (!details.isEmpty()) - fullMessage += "\n\nDetails:\n" + details; - sentry_uuid_s const uuid = reportSentryException(SENTRY_LEVEL_ERROR, "AppController got notified of a fatal error", "Exception", - fullMessage.toLocal8Bit()); - QMessageBox::critical(nullptr, tr("Error"), message); +void AppController::onFatalError(Exception const &exception) { + sentry_uuid_t uuid = reportSentryException("AppController got notified of a fatal error", exception); + + QMessageBox::critical(nullptr, tr("Error"), exception.what()); restart(true); - log().fatal(QString("reportID: %1 Captured exception: %2").arg(QByteArray(uuid.bytes, 16).toHex(), fullMessage)); + log().fatal(QString("reportID: %1 Captured exception: %2").arg(QByteArray(uuid.bytes, 16).toHex(), exception.detailedWhat())); qApp->exit(EXIT_FAILURE); } + void AppController::restart(bool isCrashing) { if (!launcher_.isEmpty()) { QProcess p; - log_->info(QString("Restarting - App : %1 - Args : %2").arg(launcher_,launcherArgs_.join(" "))); + log_->info(QString("Restarting - App : %1 - Args : %2").arg(launcher_, launcherArgs_.join(" "))); QStringList args = launcherArgs_; - if (isCrashing) + if (isCrashing) { args.append(noWindowFlag); + } p.startDetached(launcher_, args); p.waitForStarted(); } } -void AppController::setLauncherArgs(const QString& launcher, const QStringList& args){ + +void AppController::setLauncherArgs(const QString &launcher, const QStringList &args) { launcher_ = launcher; launcherArgs_ = args; } \ No newline at end of file diff --git a/internal/frontend/bridge-gui/bridge-gui/AppController.h b/internal/frontend/bridge-gui/bridge-gui/AppController.h index cba3bebd..9821384c 100644 --- a/internal/frontend/bridge-gui/bridge-gui/AppController.h +++ b/internal/frontend/bridge-gui/bridge-gui/AppController.h @@ -20,21 +20,16 @@ #define BRIDGE_GUI_APP_CONTROLLER_H +// @formatter:off class QMLBackend; - - namespace bridgepp { class Log; - - class Overseer; - - class GRPCClient; - - class ProcessMonitor; +class Exception; } +// @formatter:off //**************************************************************************************************************************************************** @@ -58,7 +53,7 @@ public: // member functions. void setLauncherArgs(const QString& launcher, const QStringList& args); public slots: - void onFatalError(QString const &function, QString const &message, QString const& details); ///< Handle fatal errors. + void onFatalError(bridgepp::Exception const& e); ///< Handle fatal errors. private: // member functions AppController(); ///< Default constructor. diff --git a/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt b/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt index cf40a5e8..4d4d6d89 100644 --- a/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt +++ b/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt @@ -112,6 +112,7 @@ add_executable(bridge-gui BridgeApp.cpp BridgeApp.h CommandLine.cpp CommandLine.h EventStreamWorker.cpp EventStreamWorker.h + LogUtils.cpp LogUtils.h main.cpp Pch.h BuildConfig.h diff --git a/internal/frontend/bridge-gui/bridge-gui/EventStreamWorker.cpp b/internal/frontend/bridge-gui/bridge-gui/EventStreamWorker.cpp index 10d6ba90..ec38d853 100644 --- a/internal/frontend/bridge-gui/bridge-gui/EventStreamWorker.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/EventStreamWorker.cpp @@ -52,7 +52,7 @@ void EventStreamReader::run() { emit finished(); } catch (Exception const &e) { - reportSentryException(SENTRY_LEVEL_ERROR, "Error during event stream read", "Exception", e.what()); + reportSentryException("Error during event stream read", e); emit error(e.qwhat()); } } diff --git a/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp b/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp new file mode 100644 index 00000000..de179dfc --- /dev/null +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp @@ -0,0 +1,62 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + + +#include "LogUtils.h" +#include + + +using namespace bridgepp; + + +namespace { + qsizetype const logFileTailMaxLength = 25 * 1024; ///< The maximum length of the portion of log returned by tailOfLatestBridgeLog() +} + +//**************************************************************************************************************************************************** +/// \brief Return the path of the latest bridge log. +/// \return The path of the latest bridge log file. +/// \return An empty string if no bridge log file was found. +//**************************************************************************************************************************************************** +QString latestBridgeLogPath() { + QDir const logsDir(userLogsDir()); + if (logsDir.isEmpty()) { + return QString(); + } + QFileInfoList files = logsDir.entryInfoList({ "v*.log" }, QDir::Files); // could do sorting, but only by last modification time. we want to sort by creation time. + std::sort(files.begin(), files.end(), [](QFileInfo const &lhs, QFileInfo const &rhs) -> bool { + return lhs.birthTime() < rhs.birthTime(); + }); + return files.back().absoluteFilePath(); +} + + +//**************************************************************************************************************************************************** +/// Return the maxSize last bytes of the latest bridge log. +//**************************************************************************************************************************************************** +QByteArray tailOfLatestBridgeLog() { + QString path = latestBridgeLogPath(); + if (path.isEmpty()) { + return QByteArray(); + } + + QFile file(path); + return file.open(QIODevice::Text | QIODevice::ReadOnly) ? file.readAll().right(logFileTailMaxLength) : QByteArray(); +} + + + diff --git a/internal/frontend/bridge-gui/bridge-gui/LogUtils.h b/internal/frontend/bridge-gui/bridge-gui/LogUtils.h new file mode 100644 index 00000000..53320380 --- /dev/null +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.h @@ -0,0 +1,26 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + + +#ifndef BRIDGE_GUI_LOG_UTILS_H +#define BRIDGE_GUI_LOG_UTILS_H + + +QByteArray tailOfLatestBridgeLog(); ///< Return the last bytes of the last bridge log. + + +#endif //BRIDGE_GUI_LOG_UTILS_H diff --git a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp index 01a881c9..66edd2c5 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp @@ -19,14 +19,15 @@ #include "QMLBackend.h" #include "EventStreamWorker.h" #include "BuildConfig.h" +#include "LogUtils.h" #include #include #include #define HANDLE_EXCEPTION(x) try { x } \ - catch (Exception const &e) { emit fatalError(__func__, e.qwhat(), e.details()); } \ - catch (...) { emit fatalError(__func__, QString("An unknown exception occurred"), QString()); } + catch (Exception const &e) { emit fatalError(e); } \ + catch (...) { emit fatalError(Exception("An unknown exception occurred", QString(), __func__)); } #define HANDLE_EXCEPTION_RETURN_BOOL(x) HANDLE_EXCEPTION(x) return false; #define HANDLE_EXCEPTION_RETURN_QSTRING(x) HANDLE_EXCEPTION(x) return QString(); #define HANDLE_EXCEPTION_RETURN_ZERO(x) HANDLE_EXCEPTION(x) return 0; @@ -594,7 +595,7 @@ void QMLBackend::login(QString const &username, QString const &password) const { HANDLE_EXCEPTION( if (username.compare("coco@bandicoot", Qt::CaseInsensitive) == 0) { throw Exception("User requested bridge-gui to crash by trying to log as coco@bandicoot", - "This error exists for test purposes and should be ignored."); + "This error exists for test purposes and should be ignored.", __func__, tailOfLatestBridgeLog()); } app().grpc().login(username, password); ) diff --git a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h index e9b42c1d..d02ba37b 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h @@ -235,7 +235,7 @@ signals: // Signals received from the Go backend, to be forwarded to QML void selectUser(QString const); ///< Signal that request the given user account to be displayed. // This signal is emitted when an exception is intercepted is calls triggered by QML. QML engine would intercept the exception otherwise. - void fatalError(QString const &function, QString const &message, QString const &details) const; ///< Signal emitted when an fatal error occurs. + void fatalError(bridgepp::Exception const& e) const; ///< Signal emitted when an fatal error occurs. private: // member functions void retrieveUserList(); ///< Retrieve the list of users via gRPC. diff --git a/internal/frontend/bridge-gui/bridge-gui/SentryUtils.cpp b/internal/frontend/bridge-gui/bridge-gui/SentryUtils.cpp index 73b6ee75..74aadafc 100644 --- a/internal/frontend/bridge-gui/bridge-gui/SentryUtils.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/SentryUtils.cpp @@ -18,14 +18,34 @@ #include "SentryUtils.h" #include "BuildConfig.h" #include +#include #include #include #include #include + static constexpr const char *LoggerName = "bridge-gui"; + +//**************************************************************************************************************************************************** +/// \return The temporary file used for sentry attachment. +//**************************************************************************************************************************************************** +QString sentryAttachmentFilePath() { + static QString path; + if (!path.isEmpty()) { + return path; + } + while (true) { + path = QDir::temp().absoluteFilePath(QUuid::createUuid().toString(QUuid::WithoutBraces) + ".txt"); // Sentry does not offer preview for .log files. + if (!QFileInfo::exists(path)) { + return path; + } + } +} + + QByteArray getProtectedHostname() { QByteArray hostname = QCryptographicHash::hash(QSysInfo::machineHostName().toUtf8(), QCryptographicHash::Sha256); return hostname.toHex(); @@ -63,12 +83,15 @@ sentry_options_t* newSentryOptions(const char *sentryDNS, const char *cacheDir) sentry_options_set_release(sentryOptions, appVersion(PROJECT_VER).toUtf8()); sentry_options_set_max_breadcrumbs(sentryOptions, 50); sentry_options_set_environment(sentryOptions, PROJECT_BUILD_ENV); + QByteArray const array = sentryAttachmentFilePath().toLocal8Bit(); + sentry_options_add_attachment(sentryOptions, array.constData()); // Enable this for debugging sentry. // sentry_options_set_debug(sentryOptions, 1); return sentryOptions; } + sentry_uuid_t reportSentryEvent(sentry_level_t level, const char *message) { auto event = sentry_value_new_message_event(level, LoggerName, message); return sentry_capture_event(event); @@ -92,3 +115,30 @@ sentry_uuid_t reportSentryException(sentry_level_t level, const char *message, c } +//**************************************************************************************************************************************************** +/// \param[in] message The message for the exception. +/// \param[in] function The name of the function that triggered the exception. +/// \param[in] exception The exception. +/// \return The Sentry exception UUID. +//**************************************************************************************************************************************************** +sentry_uuid_t reportSentryException(QString const &message, bridgepp::Exception const exception) { + QByteArray const attachment = exception.attachment(); + QFile file(sentryAttachmentFilePath()); + bool const hasAttachment = !attachment.isEmpty(); + if (hasAttachment) { + if (file.open(QIODevice::Text | QIODevice::WriteOnly)) { + file.write(attachment); + file.close(); + } + } + + sentry_uuid_t const uuid = reportSentryException(SENTRY_LEVEL_ERROR, message.toLocal8Bit(), "Exception", + exception.detailedWhat().toLocal8Bit()); + + if (hasAttachment) { + file.remove(); + } + + return uuid; +} + diff --git a/internal/frontend/bridge-gui/bridge-gui/SentryUtils.h b/internal/frontend/bridge-gui/bridge-gui/SentryUtils.h index 7b721066..bb30e34e 100644 --- a/internal/frontend/bridge-gui/bridge-gui/SentryUtils.h +++ b/internal/frontend/bridge-gui/bridge-gui/SentryUtils.h @@ -21,9 +21,11 @@ #include + void setSentryReportScope(); sentry_options_t* newSentryOptions(const char * sentryDNS, const char * cacheDir); sentry_uuid_t reportSentryEvent(sentry_level_t level, const char *message); -sentry_uuid_t reportSentryException(sentry_level_t level, const char *message, const char *exceptionType, const char *exception); +sentry_uuid_t reportSentryException(QString const& message, bridgepp::Exception const exception); + #endif //BRIDGE_GUI_SENTRYUTILS_H diff --git a/internal/frontend/bridge-gui/bridge-gui/main.cpp b/internal/frontend/bridge-gui/bridge-gui/main.cpp index 0064bde5..c4f69a6a 100644 --- a/internal/frontend/bridge-gui/bridge-gui/main.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/main.cpp @@ -22,6 +22,7 @@ #include "QMLBackend.h" #include "SentryUtils.h" #include "BuildConfig.h" +#include "LogUtils.h" #include #include #include @@ -238,7 +239,7 @@ void focusOtherInstance() { } catch (Exception const &e) { app().log().error(e.qwhat()); - auto uuid = reportSentryException(SENTRY_LEVEL_ERROR, "Exception occurred during focusOtherInstance()", "Exception", e.what()); + auto uuid = reportSentryException("Exception occurred during focusOtherInstance()", e); app().log().fatal(QString("reportID: %1 Captured exception: %2").arg(QByteArray(uuid.bytes, 16).toHex()).arg(e.qwhat())); } } @@ -332,7 +333,8 @@ int main(int argc, char *argv[]) { if (!cliOptions.attach) { if (isBridgeRunning()) { - throw Exception("An orphan instance of bridge is already running. Please terminate it and relaunch the application."); + throw Exception("An orphan instance of bridge is already running. Please terminate it and relaunch the application.", + QString(), QString(), tailOfLatestBridgeLog()); } // before launching bridge, we remove any trailing service config file, because we need to make sure we get a newly generated one. @@ -359,6 +361,7 @@ int main(int argc, char *argv[]) { QQuickWindow::setSceneGraphBackend(cliOptions.useSoftwareRenderer ? "software" : "rhi"); log.info(QString("Qt Quick renderer: %1").arg(QQuickWindow::sceneGraphBackend())); + QQmlApplicationEngine engine; std::unique_ptr rootComponent(createRootQmlComponent(engine)); std::unique_ptr rootObject(rootComponent->create(engine.rootContext())); @@ -420,16 +423,9 @@ int main(int argc, char *argv[]) { return result; } catch (Exception const &e) { - QString fullMessage = e.qwhat(); - bool const hasDetails = !e.details().isEmpty(); - if (hasDetails) - fullMessage += "\n\nDetails:\n" + e.details(); - sentry_uuid_s const uuid = reportSentryException(SENTRY_LEVEL_ERROR, "Exception occurred during main", "Exception", fullMessage.toLocal8Bit()); + sentry_uuid_s const uuid = reportSentryException("Exception occurred during main", e); QMessageBox::critical(nullptr, "Error", e.qwhat()); - QTextStream errStream(stderr); - errStream << "reportID: " << QByteArray(uuid.bytes, 16).toHex() << " Captured exception :" << e.qwhat() << "\n"; - if (hasDetails) - errStream << "\nDetails:\n" << e.details() << "\n"; + QTextStream(stderr) << "reportID: " << QByteArray(uuid.bytes, 16).toHex() << " Captured exception :" << e.detailedWhat() << "\n"; return EXIT_FAILURE; } } diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp index eeda30d6..dc614b88 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp @@ -25,12 +25,15 @@ namespace bridgepp { //**************************************************************************************************************************************************** /// \param[in] what A description of the exception. /// \param[in] details The optional details for the exception. +/// \param[in] function The name of the calling function. //**************************************************************************************************************************************************** -Exception::Exception(QString qwhat, QString details) noexcept +Exception::Exception(QString qwhat, QString details, QString function, QByteArray attachment) noexcept : std::exception() , qwhat_(std::move(qwhat)) , what_(qwhat_.toLocal8Bit()) - , details_(std::move(details)) { + , details_(std::move(details)) + , function_(std::move(function)) + , attachment_(std::move(attachment)) { } @@ -41,7 +44,9 @@ Exception::Exception(Exception const &ref) noexcept : std::exception(ref) , qwhat_(ref.qwhat_) , what_(ref.what_) - , details_(ref.details_) { + , details_(ref.details_) + , function_(ref.function_) + , attachment_(ref.attachment_) { } @@ -52,7 +57,9 @@ Exception::Exception(Exception &&ref) noexcept : std::exception(ref) , qwhat_(ref.qwhat_) , what_(ref.what_) - , details_(ref.details_) { + , details_(ref.details_) + , function_(ref.function_) + , attachment_(ref.attachment_) { } @@ -80,4 +87,26 @@ QString Exception::details() const noexcept { } +//**************************************************************************************************************************************************** +/// \return The attachment for the exception. +//**************************************************************************************************************************************************** +QByteArray Exception::attachment() const noexcept { + return attachment_; +} + + +//**************************************************************************************************************************************************** +/// \return The details exception. +//**************************************************************************************************************************************************** +QString Exception::detailedWhat() const { + QString result = qwhat_; + if (!function_.isEmpty()) { + result = QString("%1(): %2").arg(function_, result); + } + if (!details_.isEmpty()) { + result += "\n\nDetails:\n" + details_; + } + return result; +} + } // namespace bridgepp diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h index ff565d23..dad9a867 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h @@ -31,7 +31,8 @@ namespace bridgepp { //**************************************************************************************************************************************************** class Exception : public std::exception { public: // member functions - explicit Exception(QString qwhat = QString(), QString details = QString()) noexcept; ///< Constructor + explicit Exception(QString qwhat = QString(), QString details = QString(), QString function = QString(), + QByteArray attachment = QByteArray()) noexcept; ///< Constructor Exception(Exception const &ref) noexcept; ///< copy constructor Exception(Exception &&ref) noexcept; ///< copy constructor Exception &operator=(Exception const &) = delete; ///< Disabled assignment operator @@ -40,11 +41,15 @@ public: // member functions QString qwhat() const noexcept; ///< Return the description of the exception as a QString const char *what() const noexcept override; ///< Return the description of the exception as C style string QString details() const noexcept; ///< Return the details for the exception + QByteArray attachment() const noexcept; ///< Return the attachment for the exception. + QString detailedWhat() const; ///< Return the detailed description of the message (i.e. including the function name and the details). private: // data members QString const qwhat_; ///< The description of the exception. QByteArray const what_; ///< The c-string version of the qwhat message. Stored as a QByteArray for automatic lifetime management. QString const details_; ///< The optional details for the exception. + QString const function_; ///< The name of the function that created the exception. + QByteArray const attachment_; ///< The attachment to add to the exception. };