From febc994cecb16bf3c650a18afe0c176888719f68 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Tue, 7 Mar 2023 10:02:09 +0100 Subject: [PATCH] feat(GODT-2446): Attach logs to sentry reports for relevant bridge-gui exceptions. Cherry picked from release/perth_narrows (2aa4e7c) # Conflicts: # internal/frontend/bridge-gui/bridge-gui/AppController.cpp # internal/frontend/bridge-gui/bridge-gui/AppController.h # internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp # internal/frontend/bridge-gui/bridge-gui/LogUtils.h # internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp # internal/frontend/bridge-gui/bridge-gui/SentryUtils.cpp # internal/frontend/bridge-gui/bridge-gui/SentryUtils.h # internal/frontend/bridge-gui/bridge-gui/main.cpp --- .../bridge-gui/bridge-gui/AppController.cpp | 18 +++---- .../bridge-gui/bridge-gui/AppController.h | 3 +- .../bridge-gui/EventStreamWorker.cpp | 2 +- .../bridge-gui/bridge-gui/LogUtils.cpp | 38 +++++++++++++++ .../frontend/bridge-gui/bridge-gui/LogUtils.h | 1 + .../bridge-gui/bridge-gui/QMLBackend.cpp | 9 ++-- .../bridge-gui/bridge-gui/QMLBackend.h | 2 +- .../bridge-gui/bridge-gui/SentryUtils.cpp | 47 +++++++++++++++++++ .../bridge-gui/bridge-gui/SentryUtils.h | 3 +- .../frontend/bridge-gui/bridge-gui/main.cpp | 20 +++----- .../bridgepp/bridgepp/Exception/Exception.cpp | 37 +++++++++++++-- .../bridgepp/bridgepp/Exception/Exception.h | 7 ++- 12 files changed, 149 insertions(+), 38 deletions(-) diff --git a/internal/frontend/bridge-gui/bridge-gui/AppController.cpp b/internal/frontend/bridge-gui/bridge-gui/AppController.cpp index e0df2a5a..0f2ad890 100644 --- a/internal/frontend/bridge-gui/bridge-gui/AppController.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/AppController.cpp @@ -90,20 +90,14 @@ Settings &AppController::settings() { //**************************************************************************************************************************************************** -/// \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); } diff --git a/internal/frontend/bridge-gui/bridge-gui/AppController.h b/internal/frontend/bridge-gui/bridge-gui/AppController.h index f8955dd5..d55e1f11 100644 --- a/internal/frontend/bridge-gui/bridge-gui/AppController.h +++ b/internal/frontend/bridge-gui/bridge-gui/AppController.h @@ -28,6 +28,7 @@ class Log; class Overseer; class GRPCClient; class ProcessMonitor; +class Exception; } //@formatter:on @@ -54,7 +55,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/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 index 35c1c150..4964039b 100644 --- a/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp @@ -24,6 +24,11 @@ using namespace bridgepp; +namespace { +qsizetype const logFileTailMaxLength = 25 * 1024; ///< The maximum length of the portion of log returned by tailOfLatestBridgeLog() +} + + //**************************************************************************************************************************************************** /// \return user logs directory used by bridge. //**************************************************************************************************************************************************** @@ -64,3 +69,36 @@ Log &initLog() { return log; } + + +//**************************************************************************************************************************************************** +/// \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 index b324301e..60e1896d 100644 --- a/internal/frontend/bridge-gui/bridge-gui/LogUtils.h +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.h @@ -24,6 +24,7 @@ bridgepp::Log &initLog(); ///< Initialize the application log. +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 c959a3a4..66a62488 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp @@ -19,15 +19,16 @@ #include "QMLBackend.h" #include "BuildConfig.h" #include "EventStreamWorker.h" +#include "LogUtils.h" #include -#include #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; @@ -596,7 +597,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 d5ff668b..880b7a13 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h @@ -237,7 +237,7 @@ signals: // Signals received from the Go backend, to be forwarded to QML void imapLoginWhileSignedOut(QString const& username); ///< Signal for the notification of IMAP login attempt on a signed out account. // 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 c940010c..b7d80720 100644 --- a/internal/frontend/bridge-gui/bridge-gui/SentryUtils.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/SentryUtils.cpp @@ -18,6 +18,7 @@ #include "SentryUtils.h" #include "BuildConfig.h" #include +#include using namespace bridgepp; @@ -26,6 +27,23 @@ using namespace bridgepp; 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; + } + } +} + + //**************************************************************************************************************************************************** /// \brief Get a hash of the computer's host name //**************************************************************************************************************************************************** @@ -96,6 +114,8 @@ 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); @@ -132,3 +152,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 28e6aeb1..8d1d966e 100644 --- a/internal/frontend/bridge-gui/bridge-gui/SentryUtils.h +++ b/internal/frontend/bridge-gui/bridge-gui/SentryUtils.h @@ -25,6 +25,7 @@ void initSentry(); 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 35d0e14e..cc0978aa 100644 --- a/internal/frontend/bridge-gui/bridge-gui/main.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/main.cpp @@ -219,8 +219,8 @@ void focusOtherInstance() { } catch (Exception const &e) { app().log().error(e.qwhat()); - auto uuid = reportSentryException(SENTRY_LEVEL_ERROR, "Exception occurred during focusOtherInstance()", "Exception", e.what()); - app().log().fatal(QString("reportID: %1 Captured exception: %2").arg(QByteArray(uuid.bytes, 16).toHex(), e.qwhat())); + 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())); } } @@ -308,7 +308,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. @@ -337,6 +338,7 @@ int main(int argc, char *argv[]) { QQuickWindow::setSceneGraphBackend((app().settings().useSoftwareRenderer() || 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())); @@ -398,17 +400,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"; - closeBridgeApp(); + 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. };