From 6385f3fab38c910917b5d03f23e25f69f285bbfd Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:57:03 +0200 Subject: [PATCH] [CrashReporting] Mock crash-report dialog and reporting preference (#7521) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [CrashReporting] Mock dialog, sentry skeleton and CI prerequisites Mock: DlgCrashReport preview dialog (no account needed, redacted log preview), CrashReportingMode Ask/Always/Never setting in DebugSettings, --simulate-crash flag and Help-menu preview entry. Send writes a local mock JSON only. Vendor sentry-native 0.17.3 via FetchContent behind WITH_CRASH_REPORTING (breakpad everywhere, backend none under sanitizers) plus a CrashReporter facade with early init and post-loop shutdown. DSN is a placeholder: empty CMake default with COCKATRICE_SENTRY_DSN env override, so SDK init is skipped and mock behavior is unchanged until a real DSN is configured. CI prerequisites riding here so every layer below builds green: libcurl dev plus CA bundles in the Linux images and CodeQL env, sentry Qt integration off (its own find_package cannot see the macOS thin Qt layout), and the StrFunc Using construction makensis requires. Tests: ctest green, format clean, code plus UI gates PASS. * [CrashReporting] Forward mode changes to SDK and fix Linux CI opt-in Connects PersonalSettings::crashReportingModeChanged to giveConsent()/revokeConsent() so switching from AlwaysSend to NeverSend (or Ask) mid-session reaches the SDK instead of leaving consent given until restart. Forwards WITH_CRASH_REPORTING into the docker container the way CCACHE_DIR is, so the Linux package leg actually builds with crash reporting ON instead of silently building OFF. * [CrashReporting] Give crash-report logging its own category per review * [CrashReporting] Drop unused DebugSettings constructor visibility per review * [CrashReporting] Document reporting modes on the enum per review * [CrashReporting] Match the dump scan to the writer's temp fallback Review nit: when the handler's SHGetKnownFolderPath lookup fails it writes to /../.., but the startup scan looked in QDir::tempPath(), so a dump produced through the fallback would never be found and its crash would never offer to be reported. Walk up the same two levels in the scan. The path is Windows-only (#ifdef Q_OS_WIN); the statements were compiled and run standalone against Qt 6 to confirm the API use. * [CrashReporting] Complete the dump-scan fallback path The previous commit walked the scan's fallback up two levels but dropped the CrashDumps/Cockatrice suffix that the writer appends unconditionally, so a dump written through the fallback was still invisible to the scan. Evaluate the writer's expression verbatim rather than mirroring it with QDir: cdUp() is existence-checked and silently leaves the path alone when the directory is missing, parent_path() is not, and QDir::absoluteFilePath() joins the shared suffix without risking a doubled separator when the walk lands on a root. Verified by compiling the block verbatim against Qt 6 with -Wextra -Werror and comparing it with the writer's join for both the normal and the fallback case. Also record that %LOCALAPPDATA% is the scan's stand-in for FOLDERID_LocalAppData and why the two normally agree. --------- Co-authored-by: Lukas Brübach --- .ci/Debian12/Dockerfile | 2 + .ci/Debian13/Dockerfile | 1 + .ci/Fedora43/Dockerfile | 2 + .ci/Fedora44/Dockerfile | 2 + .ci/Ubuntu24.04/Dockerfile | 2 + .ci/Ubuntu26.04/Dockerfile | 1 + .ci/compile.sh | 6 + .ci/docker.sh | 6 + .github/workflows/codeql.yml | 2 + .github/workflows/desktop-build.yml | 3 + CMakeLists.txt | 1 + cmake/CrashReporting.cmake | 124 ++++++++++++++ cockatrice/CMakeLists.txt | 4 + .../src/client/crash/crash_reporter.cpp | 119 ++++++++++++++ cockatrice/src/client/crash/crash_reporter.h | 66 ++++++++ cockatrice/src/interface/logger.cpp | 144 ++++++++++++++++- cockatrice/src/interface/logger.h | 12 +- .../widgets/dialogs/dlg_crash_report.cpp | 152 ++++++++++++++++++ .../widgets/dialogs/dlg_crash_report.h | 51 ++++++ .../settings_page/general_settings_page.cpp | 30 +++- .../settings_page/general_settings_page.h | 4 + cockatrice/src/main.cpp | 96 ++++++++++- .../libcockatrice/settings/debug_settings.cpp | 2 +- .../settings/personal_settings.cpp | 21 +++ .../settings/personal_settings.h | 11 ++ tests/CMakeLists.txt | 15 ++ tests/crash_reporter_test.cpp | 87 ++++++++++ tests/settings/settings_defaults_test.cpp | 27 ++++ 28 files changed, 988 insertions(+), 5 deletions(-) create mode 100644 cmake/CrashReporting.cmake create mode 100644 cockatrice/src/client/crash/crash_reporter.cpp create mode 100644 cockatrice/src/client/crash/crash_reporter.h create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_crash_report.cpp create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_crash_report.h create mode 100644 tests/crash_reporter_test.cpp diff --git a/.ci/Debian12/Dockerfile b/.ci/Debian12/Dockerfile index 14c6f018c..4da910dc0 100644 --- a/.ci/Debian12/Dockerfile +++ b/.ci/Debian12/Dockerfile @@ -3,11 +3,13 @@ FROM debian:12 RUN apt-get update && \ DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ build-essential \ + ca-certificates \ ccache \ cmake \ file \ g++ \ git \ + libcurl4-openssl-dev \ libgl-dev \ liblzma-dev \ libmariadb-dev-compat \ diff --git a/.ci/Debian13/Dockerfile b/.ci/Debian13/Dockerfile index b597307c5..94f711398 100644 --- a/.ci/Debian13/Dockerfile +++ b/.ci/Debian13/Dockerfile @@ -9,6 +9,7 @@ RUN apt-get update && \ file \ g++ \ git \ + libcurl4-openssl-dev \ libgl-dev \ liblzma-dev \ libmariadb-dev-compat \ diff --git a/.ci/Fedora43/Dockerfile b/.ci/Fedora43/Dockerfile index 463da5a51..8aa345651 100644 --- a/.ci/Fedora43/Dockerfile +++ b/.ci/Fedora43/Dockerfile @@ -1,10 +1,12 @@ FROM fedora:43 RUN dnf install -y \ + ca-certificates \ ccache \ cmake \ gcc-c++ \ git \ + libcurl-devel \ mariadb-devel \ ninja-build \ openssl-devel \ diff --git a/.ci/Fedora44/Dockerfile b/.ci/Fedora44/Dockerfile index 62238e760..15204fa1a 100644 --- a/.ci/Fedora44/Dockerfile +++ b/.ci/Fedora44/Dockerfile @@ -1,10 +1,12 @@ FROM fedora:44 RUN dnf install -y \ + ca-certificates \ ccache \ cmake \ gcc-c++ \ git \ + libcurl-devel \ mariadb-devel \ ninja-build \ openssl-devel \ diff --git a/.ci/Ubuntu24.04/Dockerfile b/.ci/Ubuntu24.04/Dockerfile index b9fb2b701..43497dd3b 100644 --- a/.ci/Ubuntu24.04/Dockerfile +++ b/.ci/Ubuntu24.04/Dockerfile @@ -3,11 +3,13 @@ FROM ubuntu:24.04 RUN apt-get update && \ DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ build-essential \ + ca-certificates \ ccache \ cmake \ file \ g++ \ git \ + libcurl4-openssl-dev \ libgl-dev \ liblzma-dev \ libmariadb-dev-compat \ diff --git a/.ci/Ubuntu26.04/Dockerfile b/.ci/Ubuntu26.04/Dockerfile index cc6528858..4dc861ae9 100644 --- a/.ci/Ubuntu26.04/Dockerfile +++ b/.ci/Ubuntu26.04/Dockerfile @@ -10,6 +10,7 @@ RUN apt-get update && \ g++ \ gcovr \ git \ + libcurl4-openssl-dev \ libgl-dev \ liblzma-dev \ libmariadb-dev-compat \ diff --git a/.ci/compile.sh b/.ci/compile.sh index 6789b9817..2aeeaa6ec 100755 --- a/.ci/compile.sh +++ b/.ci/compile.sh @@ -200,6 +200,12 @@ if [[ $USE_VCPKG ]]; then flags+=("-DUSE_VCPKG=1") flags+=("-DVCPKG_INSTALL_OPTIONS=--x-abi-tools-use-exact-versions") fi +if [[ ${WITH_CRASH_REPORTING:-} == "ON" ]]; then + # Release CI opts into the sentry-native SDK (default is OFF so from-source, distro and + # offline package builds never fetch it). Debug builds stay SDK-free so crashes from + # developer builds never reach Sentry. + flags+=("-DWITH_CRASH_REPORTING=ON") +fi # Add cmake --build flags buildflags=(--config "$BUILDTYPE") diff --git a/.ci/docker.sh b/.ci/docker.sh index 412ad6a1d..64ea6baa7 100644 --- a/.ci/docker.sh +++ b/.ci/docker.sh @@ -175,6 +175,12 @@ function RUN () args+=(--mount "type=bind,source=$GITHUB_OUTPUT,target=/gh_output") args+=(--env "GITHUB_OUTPUT=/gh_output") fi + # The container does not inherit host env vars: without this, WITH_CRASH_REPORTING=ON set in + # the workflow never reaches compile.sh inside the container, so the Linux packages silently + # build with crash reporting OFF while the libcurl packages in the Dockerfiles go unused. + if [[ ${WITH_CRASH_REPORTING:-} ]]; then + args+=(--env "WITH_CRASH_REPORTING=${WITH_CRASH_REPORTING}") + fi # shellcheck disable=2086 docker run "${args[@]}" $RUN_ARGS "$IMAGE_NAME" bash "$BUILD_SCRIPT" $RUN_OPTS "$@" return $? diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 75fbc59f1..75494dc3e 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -60,8 +60,10 @@ jobs: run: | sudo apt-get update sudo apt-get install -y --no-install-recommends \ + ca-certificates \ cmake \ g++ \ + libcurl4-openssl-dev \ libgl-dev \ liblzma-dev \ libmariadb-dev-compat \ diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index 0b9502fe3..2215ac2d2 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -239,6 +239,7 @@ jobs: SUFFIX: '-${{ matrix.distro }}${{ matrix.version }}' package: '${{ matrix.package }}' server_only: '${{ matrix.server_only }}' + WITH_CRASH_REPORTING: 'ON' run: | source .ci/docker.sh args=() @@ -558,6 +559,8 @@ jobs: VCPKG_DISABLE_METRICS: 1 VCPKG_FEATURE_FLAGS: dependencygraph VCPKG_TARGET_MACOS_VERSION: ${{ matrix.vcpkg_deployment_target }} + # Release legs compile the sentry-native SDK; Debug legs stay SDK-free. + WITH_CRASH_REPORTING: ${{ matrix.type == 'Release' && 'ON' || '' }} run: .ci/compile.sh --server --test --vcpkg # CPack reports a failed makensis run as "Problem running NSIS command" diff --git a/CMakeLists.txt b/CMakeLists.txt index 87f1c746a..b294f27ba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -220,6 +220,7 @@ endif() # when ENABLE_SANITIZERS or ENABLE_COVERAGE is on. include(Sanitizers) include(Coverage) +include(CrashReporting) # GNU systems need to define the Mersenne exponent for the RNG to compile w/o warning if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") diff --git a/cmake/CrashReporting.cmake b/cmake/CrashReporting.cmake new file mode 100644 index 000000000..a728e25c2 --- /dev/null +++ b/cmake/CrashReporting.cmake @@ -0,0 +1,124 @@ +# Crash reporting via sentry-native (Sentry protocol, MIT). +# Included once from the root CMakeLists.txt, right after the sanitizer/coverage helpers. +# +# Inputs: +# WITH_CRASH_REPORTING -- build the client with the sentry-native SDK (default OFF, our release +# CI passes ON; packagers and from-source builds stay SDK-free until this +# ships something real, so offline package builds never fetch the SDK) +# COCKATRICE_SENTRY_DSN -- Sentry DSN baked into the build (default empty = SDK init skipped at runtime) +# COCKATRICE_SENTRY_TAG -- sentry-native release tag to vendor via FetchContent +# ENABLE_SANITIZERS -- forces the sentry backend to "none" (crashpad vs ASan conflict) +# Defines: cockatrice_target_enable_crash_reporting() + +option(WITH_CRASH_REPORTING "Build with crash reporting via sentry-native" OFF) +set(COCKATRICE_SENTRY_DSN + "" + CACHE STRING "Sentry DSN baked into the client; empty disables crash-report upload at runtime" +) +# Pinned stable release; tag verified at https://github.com/getsentry/sentry-native/tags. +set(COCKATRICE_SENTRY_TAG + "0.17.3" + CACHE STRING "sentry-native release tag to vendor via FetchContent" +) + +# Wire the SDK (or the no-op fallback) into a client target. Always callable: with the +# option OFF the compile definitions below make crash_reporter.cpp compile to no-ops. +function(cockatrice_target_enable_crash_reporting _target) + if(WITH_CRASH_REPORTING AND TARGET sentry::sentry) + target_link_libraries(${_target} PRIVATE sentry::sentry) + target_compile_definitions(${_target} PRIVATE COCKATRICE_WITH_CRASH_REPORTING=1) + else() + target_compile_definitions(${_target} PRIVATE COCKATRICE_WITH_CRASH_REPORTING=0) + endif() + # Always baked in (possibly empty): main() prefers the COCKATRICE_SENTRY_DSN env var at runtime. + target_compile_definitions(${_target} PRIVATE "COCKATRICE_SENTRY_DSN=\"${COCKATRICE_SENTRY_DSN}\"") +endfunction() + +if(WITH_CRASH_REPORTING AND WITH_CLIENT) + include(FetchContent) + + if(ENABLE_SANITIZERS) + # The crash handler cannot run under ASan/UBSan, so there is no backend to capture with. + set(_sentry_backend "none") + elseif(WIN32 OR APPLE) + # Out-of-process handler; shipping the handler binary is a later increment, which unifies + # every OS on breakpad (in-process, no extra binary to ship). + set(_sentry_backend "crashpad") + else() + # In-process, no extra binary to ship. + set(_sentry_backend "breakpad") + endif() + # FORCE so toggling ENABLE_SANITIZERS on a reused build dir cannot leave a stale backend behind. + set(SENTRY_BACKEND + "${_sentry_backend}" + CACHE STRING "Crash handler backend for sentry-native" FORCE + ) + unset(_sentry_backend) + + # No Qt integration: it runs find_package(Qt6) inside the vendored build, which + # cannot see our thin/aqt Qt layouts (macOS CI) under foreign toolchains, and its + # breadcrumbs could carry unredacted log lines into native crash events that bypass + # before_send. Our attached redacted log preview already carries the log context. + set(SENTRY_INTEGRATION_QT + OFF + CACHE BOOL "Qt log-message breadcrumbs for sentry-native (off: discovery + secret surface)" FORCE + ) + # The transport stays at the SDK default (curl on Apple/Linux, winhttp on Windows). + set(SENTRY_BUILD_TESTS + OFF + CACHE BOOL "Build sentry-native tests" FORCE + ) + set(SENTRY_BUILD_EXAMPLES + OFF + CACHE BOOL "Build sentry-native examples" FORCE + ) + + fetchcontent_declare( + sentry + GIT_REPOSITORY https://github.com/getsentry/sentry-native.git + GIT_TAG ${COCKATRICE_SENTRY_TAG} + ) + fetchcontent_makeavailable(sentry) + + # Vendored sentry-native (and its breakpad/crashpad trees) warn under our -Wall -Wextra + # (e.g. breakpad's -Wmissing-field-initializers), and WARNING_AS_ERROR would fail the build + # on code we cannot fix. Those findings are not actionable for us, so opt the whole vendored + # tree out of warnings-as-errors while keeping the warnings themselves visible; our own + # targets keep -Werror. Interface libraries compile nothing, so they are skipped outright: + # an INTERFACE -Wno-error would propagate to our consumers and weaken their build. + function(_cockatrice_sentry_opt_out_of_werror _dir) + get_property( + _sentry_targets + DIRECTORY "${_dir}" + PROPERTY BUILDSYSTEM_TARGETS + ) + foreach(_sentry_target IN LISTS _sentry_targets) + get_target_property(_sentry_type ${_sentry_target} TYPE) + if(_sentry_type STREQUAL "INTERFACE_LIBRARY") + continue() + elseif(_sentry_type MATCHES "LIBRARY|EXECUTABLE") + get_target_property(_sentry_imported ${_sentry_target} IMPORTED) + if(NOT _sentry_imported) + target_compile_options(${_sentry_target} PRIVATE -Wno-error) + endif() + endif() + endforeach() + get_property( + _sentry_subdirs + DIRECTORY "${_dir}" + PROPERTY SUBDIRECTORIES + ) + foreach(_sentry_subdir IN LISTS _sentry_subdirs) + _cockatrice_sentry_opt_out_of_werror("${_sentry_subdir}") + endforeach() + endfunction() + if(NOT MSVC) + _cockatrice_sentry_opt_out_of_werror("${sentry_SOURCE_DIR}") + endif() + + message(STATUS "Crash reporting: sentry-native ${COCKATRICE_SENTRY_TAG} (backend: ${SENTRY_BACKEND})") +else() + message(STATUS "Crash reporting: SDK not vendored " + "(WITH_CRASH_REPORTING=${WITH_CRASH_REPORTING}, WITH_CLIENT=${WITH_CLIENT})" + ) +endif() diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 5baf10b1e..bec858dcb 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -15,6 +15,7 @@ set(cockatrice_SOURCES src/client/network/update/client/client_update_checker.cpp src/client/network/update/client/release_channel.cpp src/client/network/update/card_spoiler/spoiler_background_updater.cpp + src/client/crash/crash_reporter.cpp src/client/latency_graph_widget.cpp src/client/latency_status_widget.cpp src/client/lag_monitor.cpp @@ -30,6 +31,7 @@ set(cockatrice_SOURCES src/interface/widgets/dialogs/dlg_connect.cpp src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp src/interface/widgets/dialogs/dlg_create_game.cpp + src/interface/widgets/dialogs/dlg_crash_report.cpp src/interface/widgets/dialogs/dlg_default_tags_editor.cpp src/interface/widgets/dialogs/dlg_edit_avatar.cpp src/interface/widgets/dialogs/dlg_edit_password.cpp @@ -578,6 +580,8 @@ target_link_libraries( ${COCKATRICE_QT_MODULES} ) +cockatrice_target_enable_crash_reporting(cockatrice) + if(UNIX) if(APPLE) set(MACOSX_BUNDLE_INFO_STRING "${PROJECT_NAME}") diff --git a/cockatrice/src/client/crash/crash_reporter.cpp b/cockatrice/src/client/crash/crash_reporter.cpp new file mode 100644 index 000000000..fe1678b11 --- /dev/null +++ b/cockatrice/src/client/crash/crash_reporter.cpp @@ -0,0 +1,119 @@ +#include "crash_reporter.h" + +#if COCKATRICE_WITH_CRASH_REPORTING +#include +#endif + +#include +#include +#include +#include +#include +#include + +Q_LOGGING_CATEGORY(CrashReportLog, "crash_report"); + +bool CrashReporter::isReportingEnabled(const QString &dsn, PersonalSettings::CrashReportingMode consentMode) +{ + return !dsn.isEmpty() && consentMode != PersonalSettings::CrashReportingMode::NeverSend; +} + +void CrashReporter::init(const QString &dsn, + const QString &release, + const QString &databasePath, + PersonalSettings::CrashReportingMode consentMode) +{ + shutdown(); + sentryDatabasePath = databasePath; + if (!isReportingEnabled(dsn, consentMode)) { + return; + } + // Only created when the SDK will actually run: no-DSN launches stay side-effect free. +#if COCKATRICE_WITH_CRASH_REPORTING + QDir().mkpath(sentryDatabasePath); + if (consentMode != PersonalSettings::CrashReportingMode::AlwaysSend) { + // sentry-native persists consent to /user-consent and reloads it in sentry_init + // before old runs are processed, so a stale "given" from an earlier AlwaysSend run (or an + // earlier Send in Ask mode) would upload the next crash before the dialog asks. Our + // CrashReportingMode is the source of truth, so clear the persisted state first; asking + // afterwards re-establishes it via giveConsent()/revokeConsent(). + QFile::remove(sentryDatabasePath + QStringLiteral("/user-consent")); + } + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, dsn.toUtf8().constData()); + sentry_options_set_release(options, release.toUtf8().constData()); + sentry_options_set_database_path(options, sentryDatabasePath.toUtf8().constData()); + // Consent-gated uploads: crashes are captured into the local database but + // nothing leaves the machine until giveConsent(). + sentry_options_set_require_user_consent(options, 1); + ready = (sentry_init(options) == 0); +#else + Q_UNUSED(release); + ready = false; +#endif +} + +bool CrashReporter::isReady() const +{ + return ready; +} + +void CrashReporter::shutdown() +{ +#if COCKATRICE_WITH_CRASH_REPORTING + if (ready) { + sentry_close(); + } +#endif + ready = false; +} + +void CrashReporter::giveConsent() +{ +#if COCKATRICE_WITH_CRASH_REPORTING + if (ready) { + sentry_user_consent_give(); + } +#endif +} + +void CrashReporter::revokeConsent() +{ +#if COCKATRICE_WITH_CRASH_REPORTING + if (ready) { + sentry_user_consent_revoke(); + } +#endif +} + +QString CrashReporter::sanitizeLogLine(const QString &line) +{ + static const auto colorEscapeCodePattern = QRegularExpression(QStringLiteral("\033\\[[0-9;]*m")); + + QString sanitized = line; + sanitized.replace(colorEscapeCodePattern, ""); + return sanitized; +} + +QString CrashReporter::redactCockatriceUrls(const QString &line) +{ + static auto urlPattern = QRegularExpression(QStringLiteral("cockatrice://\\S+")); + + // Single pass over the original line: rescanning the replaced text would match a redacted + // secret-free URL again and loop forever. + QString redacted; + redacted.reserve(line.size()); + qsizetype pos = 0; + QRegularExpressionMatchIterator it = urlPattern.globalMatch(line); + while (it.hasNext()) { + QRegularExpressionMatch match = it.next(); + redacted.append(line.mid(pos, match.capturedStart() - pos)); + // The pattern only matches cockatrice:// URLs, so the check is folded into the parse: + // scheme://host, secrets dropped. + const QUrl parsed(match.captured(0)); + redacted.append(parsed.scheme() + QStringLiteral("://") + parsed.host()); + pos = match.capturedEnd(); + } + redacted.append(line.mid(pos)); + return redacted; +} diff --git a/cockatrice/src/client/crash/crash_reporter.h b/cockatrice/src/client/crash/crash_reporter.h new file mode 100644 index 000000000..f57ec6019 --- /dev/null +++ b/cockatrice/src/client/crash/crash_reporter.h @@ -0,0 +1,66 @@ +/** + * @file crash_reporter.h + * @ingroup Core + */ + +#ifndef CRASH_REPORTER_H +#define CRASH_REPORTER_H + +#include +#include +#include + +#ifndef COCKATRICE_WITH_CRASH_REPORTING +#define COCKATRICE_WITH_CRASH_REPORTING 0 +#endif + +#ifndef COCKATRICE_SENTRY_DSN +#define COCKATRICE_SENTRY_DSN "" +#endif + +Q_DECLARE_LOGGING_CATEGORY(CrashReportLog); + +/** + * @brief Sentry-backed crash-report facade, owned by main(). + * + * Single responsibility: init/shutdown the sentry-native SDK behind consent + * gating, plus the shared log redaction helpers. An empty DSN (or Never + * consent) skips the SDK entirely, so the mock dialog fallback behaves exactly + * as before. init() arms require-user-consent so cached crash envelopes never + * upload before the user is asked; giveConsent()/revokeConsent() (driven by + * the reporting-mode setting and the crash dialog) gate uploads, and any stale + * persisted consent from a previous AlwaysSend run is cleared before init when + * the mode is not AlwaysSend. The real send path is a later increment. All SDK + * calls are guarded so OFF builds compile to no-ops. + */ +class CrashReporter +{ +public: + CrashReporter() = default; + CrashReporter(const CrashReporter &) = delete; + CrashReporter &operator=(const CrashReporter &) = delete; + ~CrashReporter() = default; + + void init(const QString &dsn, + const QString &release, + const QString &databasePath, + PersonalSettings::CrashReportingMode consentMode); + [[nodiscard]] bool isReady() const; + void shutdown(); + void giveConsent(); + void revokeConsent(); + + /** + * @brief SDK-independent gating decision, unit-testable in OFF builds. + */ + static bool isReportingEnabled(const QString &dsn, PersonalSettings::CrashReportingMode consentMode); + + static QString sanitizeLogLine(const QString &line); + static QString redactCockatriceUrls(const QString &line); + +private: + bool ready = false; + QString sentryDatabasePath; +}; + +#endif diff --git a/cockatrice/src/interface/logger.cpp b/cockatrice/src/interface/logger.cpp index 87fdcbd8c..864840f4e 100644 --- a/cockatrice/src/interface/logger.cpp +++ b/cockatrice/src/interface/logger.cpp @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include #include #include @@ -18,8 +20,11 @@ #define LOGGER_MAX_ENTRIES 128 #define LOGGER_FILENAME "qdebug.txt" +#define LOGGER_SESSION_MARKER "Log session started at" +#define LOGGER_PERSISTENT_MAX_BYTES (512 * 1024) +#define LOGGER_PERSISTENT_KEEP_LINES 2000 -Logger::Logger() : logToFileEnabled(false) +Logger::Logger() : logToFileEnabled(false), persistentLogEnabled(false) { logBuffer.append(getClientVersion()); logBuffer.append(getSystemArchitecture()); @@ -35,6 +40,7 @@ Logger::Logger() : logToFileEnabled(false) Logger::~Logger() { closeLogfileSession(); + closePersistentLogSession(); logBuffer.clear(); } @@ -82,6 +88,137 @@ void Logger::closeLogfileSession() fileHandle.close(); } +void Logger::setPersistentLogPath(const QString &path) +{ + QMutexLocker locker(&mutex); + if (path.isEmpty() || (persistentLogEnabled && persistentLogPath == path)) { + return; + } + + QDir().mkpath(QFileInfo(path).absolutePath()); + persistentLogPath = path; + openPersistentLog(path); +} + +void Logger::prunePersistentLog(const QString &path) +{ + QFileInfo info(path); + if (!info.exists() || info.size() <= LOGGER_PERSISTENT_MAX_BYTES) { + return; + } + + QFile file(path); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { + return; + } + QList allLines; + QTextStream in(&file); + while (!in.atEnd()) { + allLines.append(in.readLine()); + } + file.close(); + + const int start = + allLines.size() > LOGGER_PERSISTENT_KEEP_LINES ? allLines.size() - LOGGER_PERSISTENT_KEEP_LINES : 0; + if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate | QIODevice::Text)) { + return; + } + QTextStream out(&file); + for (int i = start; i < allLines.size(); ++i) { + out << allLines.at(i) << Qt::endl; + } + file.close(); +} + +void Logger::openPersistentLog(const QString &path) +{ + prunePersistentLog(path); + + persistentFileHandle.setFileName(path); + if (!persistentFileHandle.open(QIODevice::WriteOnly | QIODevice::Append | QIODevice::Text)) { + qWarning() << "Logger failed to open persistent log" << path << "for writing"; + return; + } + persistentFileStream.setDevice(&persistentFileHandle); + persistentFileStream << LOGGER_SESSION_MARKER << " " << QDateTime::currentDateTime().toString() << Qt::endl; + persistentFileStream << getClientVersion() << Qt::endl; + persistentFileStream << getSystemArchitecture() << Qt::endl; + persistentFileStream << getClientInstallInfo() << Qt::endl; + persistentFileStream.flush(); + persistentLogEnabled = true; +} + +void Logger::closePersistentLogSession() +{ + if (!persistentLogEnabled) { + return; + } + + persistentLogEnabled = false; + persistentFileStream << "Log session closed at " << QDateTime::currentDateTime().toString() << Qt::endl; + persistentFileStream.flush(); + persistentFileHandle.close(); +} + +QList Logger::getPreviousSessionTail(int maxLines) const +{ + QString path; + { + QMutexLocker locker(&mutex); + path = persistentLogPath; + } + if (path.isEmpty()) { + return {}; + } + return extractPreviousSessionTail(path, maxLines); +} + +QList Logger::extractPreviousSessionTail(const QString &filePath, int maxLines) +{ + QList result; + if (maxLines <= 0) { + return result; + } + + QFile file(filePath); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { + return result; + } + QList allLines; + QTextStream in(&file); + while (!in.atEnd()) { + allLines.append(in.readLine()); + } + file.close(); + + QList markerIndexes; + for (int i = 0; i < allLines.size(); ++i) { + if (allLines.at(i).contains(QLatin1String(LOGGER_SESSION_MARKER))) { + markerIndexes.append(i); + } + } + + QList tail; + if (markerIndexes.isEmpty()) { + tail = allLines; + } else if (markerIndexes.size() == 1) { + // Only the current session has been recorded; there is no previous session yet. + return result; + } else { + const int previousStart = markerIndexes.at(markerIndexes.size() - 2); + const int currentStart = markerIndexes.last(); + for (int i = previousStart; i < currentStart; ++i) { + tail.append(allLines.at(i)); + } + } + + const int start = tail.size() > maxLines ? tail.size() - maxLines : 0; + for (int i = start; i < tail.size(); ++i) { + result.append(tail.at(i)); + } + return result; +} + void Logger::log(QtMsgType /* type */, const QMessageLogContext & /* ctx */, const QString &message) { QMetaObject::invokeMethod(this, "internalLog", Qt::QueuedConnection, Q_ARG(const QString &, message)); @@ -102,6 +239,11 @@ void Logger::internalLog(const QString &message) if (logToFileEnabled) { fileStream << message << Qt::endl; // Print to fileStream } + + if (persistentLogEnabled) { + persistentFileStream << message << Qt::endl; + persistentFileStream.flush(); + } } QString Logger::getSystemArchitecture() diff --git a/cockatrice/src/interface/logger.h b/cockatrice/src/interface/logger.h index 2287da804..cc7cee046 100644 --- a/cockatrice/src/interface/logger.h +++ b/cockatrice/src/interface/logger.h @@ -49,6 +49,9 @@ public: { return logBuffer; } + void setPersistentLogPath(const QString &path); + QList getPreviousSessionTail(int maxLines = 60) const; + static QList extractPreviousSessionTail(const QString &filePath, int maxLines = 60); private: Logger(); @@ -61,11 +64,18 @@ private: QTextStream fileStream; QFile fileHandle; QList logBuffer; - QMutex mutex; + mutable QMutex mutex; + QString persistentLogPath; + bool persistentLogEnabled; + QTextStream persistentFileStream; + QFile persistentFileHandle; protected: void openLogfileSession(); void closeLogfileSession(); + void openPersistentLog(const QString &path); + void prunePersistentLog(const QString &path); + void closePersistentLogSession(); protected slots: void internalLog(const QString &message); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_crash_report.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_crash_report.cpp new file mode 100644 index 000000000..7fc69e12b --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_crash_report.cpp @@ -0,0 +1,152 @@ +#include "dlg_crash_report.h" + +#include "../../../client/crash/crash_reporter.h" +#include "../../../client/settings/cache_settings.h" +#include "../../logger.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +DlgCrashReport::DlgCrashReport(QWidget *parent) : QDialog(parent) +{ + infoLabel = new QLabel(this); + infoLabel->setWordWrap(true); + + accountLabel = new QLabel(this); + accountLabel->setWordWrap(true); + + previewArea = new QPlainTextEdit(this); + previewArea->setReadOnly(true); + previewArea->setPlainText(buildRedactedPreview()); + + rememberBox = new QCheckBox(this); + + sendButton = new QPushButton(this); + sendButton->setDefault(true); + connect(sendButton, &QPushButton::clicked, this, &DlgCrashReport::actSend); + + dontSendButton = new QPushButton(this); + dontSendButton->setAutoDefault(false); + connect(dontSendButton, &QPushButton::clicked, this, &DlgCrashReport::actDontSend); + + auto *buttonLayout = new QHBoxLayout; + buttonLayout->addWidget(rememberBox); + buttonLayout->addStretch(); + buttonLayout->addWidget(dontSendButton); + buttonLayout->addWidget(sendButton); + + auto *mainLayout = new QVBoxLayout; + mainLayout->addWidget(infoLabel); + mainLayout->addWidget(accountLabel); + mainLayout->addWidget(previewArea); + mainLayout->addLayout(buttonLayout); + setLayout(mainLayout); + + resize(700, 500); + retranslateUi(); +} + +void DlgCrashReport::changeEvent(QEvent *event) +{ + if (event->type() == QEvent::LanguageChange) { + retranslateUi(); + } + QDialog::changeEvent(event); +} + +void DlgCrashReport::retranslateUi() +{ + setWindowTitle(tr("Send crash report")); + infoLabel->setText( + tr("Cockatrice closed unexpectedly. The preview below shows what a crash report would contain (mock).")); + accountLabel->setText(tr("No account needed to send a crash report.")); + rememberBox->setText(tr("Remember my choice")); + sendButton->setText(tr("Send")); + dontSendButton->setText(tr("Don't send")); +} + +QString DlgCrashReport::buildRedactedPreview() +{ + Logger &logger = Logger::getInstance(); + + QStringList header; + header.append(logger.getClientVersion()); + header.append(logger.getSystemArchitecture()); + header.append(logger.getSystemLocale()); + header.append(logger.getClientInstallInfo()); + header.append(QStringLiteral("---")); + + QList logLines = logger.getPreviousSessionTail(); + if (logLines.isEmpty()) { + logLines = logger.getLogBuffer(); + } + const int previewLines = 60; + int start = logLines.size() > previewLines ? logLines.size() - previewLines : 0; + + QStringList lines = header; + for (int i = start; i < logLines.size(); ++i) { + lines.append(CrashReporter::redactCockatriceUrls(CrashReporter::sanitizeLogLine(logLines.at(i)))); + } + return lines.join(QStringLiteral("\n")); +} + +QString DlgCrashReport::writeMockReport(const QString &previewText) +{ + Logger &logger = Logger::getInstance(); + + QJsonObject report; + report[QStringLiteral("mock")] = true; + report[QStringLiteral("timestamp")] = QDateTime::currentDateTimeUtc().toString(Qt::ISODate); + report[QStringLiteral("clientVersion")] = logger.getClientVersion(); + report[QStringLiteral("systemArchitecture")] = logger.getSystemArchitecture(); + report[QStringLiteral("systemLocale")] = logger.getSystemLocale(); + report[QStringLiteral("installInfo")] = logger.getClientInstallInfo(); + report[QStringLiteral("logPreview")] = previewText; + + QString fileName = QStringLiteral("crash-report-mock-%1.json") + .arg(QDateTime::currentDateTimeUtc().toString(QStringLiteral("yyyyMMdd-HHmmss"))); + QString filePath = QDir::tempPath() + QStringLiteral("/") + fileName; + + QFile file(filePath); + if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { + return QString(); + } + file.write(QJsonDocument(report).toJson()); + file.close(); + return filePath; +} + +void DlgCrashReport::actSend() +{ + if (rememberBox->isChecked()) { + SettingsCache::instance().personal().setCrashReportingMode(PersonalSettings::CrashReportingMode::AlwaysSend); + } + const QString savedPath = writeMockReport(previewArea->toPlainText()); + if (savedPath.isEmpty()) { + qCWarning(CrashReportLog) << "Could not save the mock crash report locally"; + } else { + qCInfo(CrashReportLog) << "Mock crash report saved locally to" << savedPath; + } + accept(); +} + +void DlgCrashReport::actDontSend() +{ + if (rememberBox->isChecked()) { + SettingsCache::instance().personal().setCrashReportingMode(PersonalSettings::CrashReportingMode::NeverSend); + } + reject(); +} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_crash_report.h b/cockatrice/src/interface/widgets/dialogs/dlg_crash_report.h new file mode 100644 index 000000000..576eb3cf9 --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_crash_report.h @@ -0,0 +1,51 @@ +/** + * @file dlg_crash_report.h + * @ingroup Dialogs + */ + +#ifndef DLG_CRASH_REPORT_H +#define DLG_CRASH_REPORT_H + +#include +#include +#include + +class QCheckBox; +class QLabel; +class QPlainTextEdit; +class QPushButton; + +/** + * @brief MOCK crash-report dialog (UI + settings only, no network). + * + * The Send path writes a redacted JSON file to the temp dir instead of + * uploading anything. Real Sentry upload comes in a later phase. + */ +class DlgCrashReport : public QDialog +{ + Q_OBJECT +public: + explicit DlgCrashReport(QWidget *parent = nullptr); + + static QString buildRedactedPreview(); + static QString writeMockReport(const QString &previewText); + +protected: + void changeEvent(QEvent *event) override; + +private: + QLabel *infoLabel; + QLabel *accountLabel; + QPlainTextEdit *previewArea; + QCheckBox *rememberBox; + QPushButton *sendButton; + QPushButton *dontSendButton; + + void retranslateUi(); + +private slots: + void actSend(); + void actDontSend(); +}; + +#endif diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp index fa8938b32..a19417661 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp @@ -133,6 +133,27 @@ GeneralSettingsPage::GeneralSettingsPage() versionGroupBox = new QGroupBox; versionGroupBox->setLayout(versionGrid); + // crash reporting consent (stored in personal.ini, not debug.ini) + crashReportingBox.addItem(""); // texts set in retranslateUi + crashReportingBox.addItem(""); + crashReportingBox.addItem(""); + crashReportingBox.setCurrentIndex(static_cast(settings.personal().getCrashReportingMode())); + + connect(&crashReportingBox, qOverload(&QComboBox::currentIndexChanged), this, [](int index) { + if (index < 0 || index > static_cast(PersonalSettings::CrashReportingMode::NeverSend)) { + return; + } + SettingsCache::instance().personal().setCrashReportingMode( + static_cast(index)); + }); + + auto *crashReportingGrid = new QGridLayout; + crashReportingGrid->addWidget(&crashReportingLabel, 0, 0); + crashReportingGrid->addWidget(&crashReportingBox, 0, 1); + + crashReportingGroupBox = new QGroupBox; + crashReportingGroupBox->setLayout(crashReportingGrid); + // card database settings startupCardUpdateCheckBehaviorSelector.addItem(""); // these will be set in retranslateUI startupCardUpdateCheckBehaviorSelector.addItem(""); @@ -336,6 +357,7 @@ GeneralSettingsPage::GeneralSettingsPage() auto *mainLayout = new QVBoxLayout; mainLayout->addWidget(languageGroupBox); mainLayout->addWidget(versionGroupBox); + mainLayout->addWidget(crashReportingGroupBox); mainLayout->addWidget(cardDatabaseGroupBox); mainLayout->addWidget(startupGroupBox); mainLayout->addWidget(pathsGroupBox); @@ -569,13 +591,19 @@ void GeneralSettingsPage::retranslateUi() tr("English and selected card language")); advertiseTranslationPageLabel.setText( QString("%2").arg(WIKI_TRANSLATION_FAQ).arg(tr("How to help with translations"))); - versionGroupBox->setTitle(tr("Version settings")); updateReleaseChannelLabel.setText(tr("Update channel")); startupUpdateCheckCheckBox.setText(tr("Check for client updates on startup")); updateNotificationCheckBox.setText(tr("Notify if a feature supported by the server is missing in my client")); newVersionOracleCheckBox.setText(tr("Automatically run Oracle when running a new version of Cockatrice")); + crashReportingGroupBox->setTitle(tr("Crash reporting")); + crashReportingLabel.setText(tr("Send crash reports:")); + crashReportingBox.setItemText(static_cast(PersonalSettings::CrashReportingMode::Ask), tr("Ask every time")); + crashReportingBox.setItemText(static_cast(PersonalSettings::CrashReportingMode::AlwaysSend), + tr("Always send")); + crashReportingBox.setItemText(static_cast(PersonalSettings::CrashReportingMode::NeverSend), tr("Never send")); + // We can't change the strings after they're put into the QComboBox, so this is our workaround int oldIndex = updateReleaseChannelBox.currentIndex(); updateReleaseChannelBox.clear(); diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h index fcf1c78d0..20564c968 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h @@ -49,6 +49,7 @@ private slots: private: QGroupBox *languageGroupBox; QGroupBox *versionGroupBox; + QGroupBox *crashReportingGroupBox; QGroupBox *cardDatabaseGroupBox; QGroupBox *startupGroupBox; QGroupBox *pathsGroupBox; @@ -70,6 +71,9 @@ private: QCheckBox updateNotificationCheckBox; QCheckBox newVersionOracleCheckBox; + QLabel crashReportingLabel; + QComboBox crashReportingBox; + QLabel startupCardUpdateCheckBehaviorLabel; QComboBox startupCardUpdateCheckBehaviorSelector; QLabel cardUpdateCheckIntervalLabel; diff --git a/cockatrice/src/main.cpp b/cockatrice/src/main.cpp index 3f63efc3b..38ec3d137 100644 --- a/cockatrice/src/main.cpp +++ b/cockatrice/src/main.cpp @@ -20,6 +20,7 @@ #include "main.h" +#include "client/crash/crash_reporter.h" #include "client/network/update/card_spoiler/spoiler_background_updater.h" #include "client/settings/cache_settings.h" #include "client/sound_engine.h" @@ -30,6 +31,7 @@ #include "interface/logger.h" #include "interface/pixel_map_generator.h" #include "interface/theme_manager.h" +#include "interface/widgets/dialogs/dlg_crash_report.h" #include "interface/window_main.h" #include "libcockatrice/card/database/card_database.h" #include "libcockatrice/rng/rng_abstract.h" @@ -44,6 +46,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -259,6 +264,46 @@ int main(int argc, char *argv[]) qApp->setAttribute(Qt::AA_DontShowIconsInMenus, true); #endif + // Crash reporting (increment 1): init the SDK early so a crash anywhere below is captured into the + // local database. Uploads stay consent-gated (require_user_consent, armed in CrashReporter::init): + // AlwaysSend gives consent here so cached envelopes flush at startup; any other mode starts + // unconsented and the dialog below drives give/revoke. An empty DSN skips the SDK entirely and + // the mock fallback further below behaves exactly as before. + CrashReporter crashReporter; + const QString crashDatabasePath = SettingsCache::instance().getSettingsPath() + QStringLiteral("sentry"); + { + QString crashDsn = QString::fromUtf8(qgetenv("COCKATRICE_SENTRY_DSN")); + if (crashDsn.isEmpty()) { + crashDsn = QString::fromLatin1(COCKATRICE_SENTRY_DSN); + } + crashReporter.init(crashDsn, QString::fromLatin1(VERSION_STRING), crashDatabasePath, + SettingsCache::instance().personal().getCrashReportingMode()); + if (crashReporter.isReady() && SettingsCache::instance().personal().getCrashReportingMode() == + PersonalSettings::CrashReportingMode::AlwaysSend) { + // Silent opt-in: cached crash envelopes flush automatically once consent is given. + crashReporter.giveConsent(); + } + // Mid-session mode changes must reach the SDK: without this, switching from AlwaysSend to + // NeverSend (or Ask) leaves consent given until restart, so a later crash in this session + // would still upload after the user opted out. crashReporter outlives main(), so capturing + // it here is safe. + QObject::connect(&SettingsCache::instance().personal(), &PersonalSettings::crashReportingModeChanged, + [&crashReporter](PersonalSettings::CrashReportingMode mode) { + if (mode == PersonalSettings::CrashReportingMode::AlwaysSend) { + crashReporter.giveConsent(); + } else { + crashReporter.revokeConsent(); + } + }); + } + + // Persistent session log: keeps this run's logs on disk so the next-startup crash dialog can + // attach the crashed session's tail (the in-memory buffer is lost on crash). Opened in Append + // mode with a per-launch session marker; read back via Logger::getPreviousSessionTail(). + const QString persistentLogPath = + SettingsCache::instance().getSettingsPath() + QStringLiteral("logs/cockatrice.log"); + Logger::getInstance().setPersistentLogPath(persistentLogPath); + // Translations #ifdef Q_OS_MAC translationPath = qApp->applicationDirPath() + "/../Resources/translations"; @@ -276,7 +321,9 @@ int main(int argc, char *argv[]) parser.addOptions( {{{"c", "connect"}, QCoreApplication::translate("main", "Connect on startup"), "user:pass@host:port"}, - {{"d", "debug-output"}, QCoreApplication::translate("main", "Debug to file")}}); + {{"d", "debug-output"}, QCoreApplication::translate("main", "Debug to file")}, + {"simulate-crash", + QCoreApplication::translate("main", "Show the mock crash-report dialog (nothing is sent)")}}); parser.process(app); @@ -413,6 +460,51 @@ int main(int argc, char *argv[]) ui.show(); qCInfo(MainLog) << "ui.show() finished"; + // MOCK crash-report flow (UI + settings only): no sentry-native upload, no network. + // Triggered by --simulate-crash or by leftover Windows crash dumps. Handled dumps are deleted + // after Send / Don't send (or a silent NeverSend skip) so one crash never prompts twice; on + // non-Windows nothing writes cockatrice.crash.*.dmp yet, so --simulate-crash is the trigger there. + { + QString crashDumpDir; +#ifdef Q_OS_WIN + // Mirrors the writer (CockatriceUnhandledExceptionFilter): it appends CrashDumps/Cockatrice + // to %LOCALAPPDATA%, or, when its known-folder lookup fails, to the temp directory walked up + // two parent_path() steps. %LOCALAPPDATA% is this scan's stand-in for FOLDERID_LocalAppData: + // both come from the same profile, so when the lookup fails the variable is normally unset. + QString crashDumpRootPath = QString::fromLocal8Bit(qgetenv("LOCALAPPDATA")); + if (crashDumpRootPath.isEmpty()) { + // The writer's fallback expression, verbatim — QDir::cdUp() is existence-checked and + // would silently leave the path alone here, parent_path() is not. + const std::filesystem::path fallback = std::filesystem::temp_directory_path().parent_path().parent_path(); + crashDumpRootPath = QString::fromStdWString(fallback.wstring()); + } + crashDumpDir = QDir(crashDumpRootPath).absoluteFilePath(QStringLiteral("CrashDumps/Cockatrice")); +#else + crashDumpDir = QDir::tempPath(); +#endif + const QStringList crashDumps = + QDir(crashDumpDir).entryList({QStringLiteral("cockatrice.crash.*.dmp")}, QDir::Files); + if (parser.isSet("simulate-crash") || !crashDumps.isEmpty()) { + const PersonalSettings::CrashReportingMode crashMode = + SettingsCache::instance().personal().getCrashReportingMode(); + if (crashMode == PersonalSettings::CrashReportingMode::AlwaysSend) { + crashReporter.giveConsent(); + DlgCrashReport::writeMockReport(DlgCrashReport::buildRedactedPreview()); + } else if (crashMode == PersonalSettings::CrashReportingMode::Ask) { + DlgCrashReport crashDialog(&ui); + if (crashDialog.exec() == QDialog::Accepted) { + crashReporter.giveConsent(); + } else { + crashReporter.revokeConsent(); + } + } + // NeverSend: skip silently (no dialog, nothing written or sent). + for (const QString &dump : crashDumps) { + QFile::remove(crashDumpDir + QStringLiteral("/") + dump); + } + } + } + // force shortcuts to be shown/hidden in right-click menus, regardless of system defaults qApp->setAttribute(Qt::AA_DontShowShortcutsInContextMenus, !SettingsCache::instance().userInterface().getShowShortcuts()); @@ -437,6 +529,8 @@ int main(int argc, char *argv[]) int ret = app.exec(); + crashReporter.shutdown(); + qCInfo(MainLog) << "Event loop finished, terminating..."; delete rng; PingPixmapGenerator::clear(); diff --git a/libcockatrice_settings/libcockatrice/settings/debug_settings.cpp b/libcockatrice_settings/libcockatrice/settings/debug_settings.cpp index c36e7187e..709155b1c 100644 --- a/libcockatrice_settings/libcockatrice/settings/debug_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/debug_settings.cpp @@ -34,4 +34,4 @@ int DebugSettings::getLocalGamePlayerCount() const QString DebugSettings::getDeckPathForPlayer(const QString &playerName) const { return getValue(playerName, "localgame", "deck").toString(); -} \ No newline at end of file +} diff --git a/libcockatrice_settings/libcockatrice/settings/personal_settings.cpp b/libcockatrice_settings/libcockatrice/settings/personal_settings.cpp index 682144485..79d06ddd4 100644 --- a/libcockatrice_settings/libcockatrice/settings/personal_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/personal_settings.cpp @@ -51,3 +51,24 @@ void PersonalSettings::setSeenTips(const QList &_seenTips) } setValue(QVariant::fromValue(storedTipList), "seenTips", "tipOfDay"); } + +PersonalSettings::CrashReportingMode PersonalSettings::getCrashReportingMode() const +{ + const int raw = getValue("crashReportingMode", QString(), QString(), 0).toInt(); + switch (raw) { + case 1: + return CrashReportingMode::AlwaysSend; + case 2: + return CrashReportingMode::NeverSend; + case 0: + default: + // Clamp corrupt ini values back to Ask; the QSettings key string is unchanged (no migration). + return CrashReportingMode::Ask; + } +} + +void PersonalSettings::setCrashReportingMode(CrashReportingMode _crashReportingMode) +{ + setValue(static_cast(_crashReportingMode), "crashReportingMode"); + emit crashReportingModeChanged(_crashReportingMode); +} diff --git a/libcockatrice_settings/libcockatrice/settings/personal_settings.h b/libcockatrice_settings/libcockatrice/settings/personal_settings.h index 0819af41b..0cd51cdd0 100644 --- a/libcockatrice_settings/libcockatrice/settings/personal_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/personal_settings.h @@ -24,8 +24,19 @@ public: void setShowTipsOnStartup(bool _showTipsOnStartup); void setSeenTips(const QList &_seenTips); + enum class CrashReportingMode + { + Ask = 0, ///< Show the crash-report dialog after an unclean exit. + AlwaysSend = 1, ///< Upload crash reports without asking. + NeverSend = 2 ///< Never upload; stay silent. + }; + + [[nodiscard]] CrashReportingMode getCrashReportingMode() const; + void setCrashReportingMode(CrashReportingMode _crashReportingMode); + signals: void langChanged(); + void crashReportingModeChanged(CrashReportingMode mode); public: explicit PersonalSettings(const QString &settingPath, QObject *parent = nullptr); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8811b8623..aac4d4818 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -422,6 +422,21 @@ if(WITH_CLIENT) ${CMAKE_SOURCE_DIR}/cockatrice/src ${CMAKE_SOURCE_DIR}/libcockatrice_utility ) + # CrashReporter is client code; DEFINITIONS keeps the test hermetic by compiling the + # no-op path (no sentry-native headers needed) regardless of WITH_CRASH_REPORTING. + cockatrice_add_test( + NAME + crash_reporter_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/crash/crash_reporter.cpp + crash_reporter_test.cpp + LIBS + libcockatrice_settings + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/cockatrice/src + DEFINITIONS + COCKATRICE_WITH_CRASH_REPORTING=0 + ) endif() add_subdirectory(card_zone_algorithms) diff --git a/tests/crash_reporter_test.cpp b/tests/crash_reporter_test.cpp new file mode 100644 index 000000000..c52a2e495 --- /dev/null +++ b/tests/crash_reporter_test.cpp @@ -0,0 +1,87 @@ +#include "client/crash/crash_reporter.h" + +#include +#include +#include +#include + +namespace +{ + +TEST(CrashReporterTest, SanitizeLogLineStripsAnsiColorCodes) +{ + ASSERT_EQ(CrashReporter::sanitizeLogLine(QStringLiteral("\033[32mhello\033[0m")), QStringLiteral("hello")); + ASSERT_EQ(CrashReporter::sanitizeLogLine(QStringLiteral("\033[1;31mfatal\033[0m")), QStringLiteral("fatal")); + ASSERT_EQ(CrashReporter::sanitizeLogLine(QStringLiteral("plain line")), QStringLiteral("plain line")); +} + +TEST(CrashReporterTest, RedactCockatriceUrlsKeepsSchemeAndHostOnly) +{ + ASSERT_EQ( + CrashReporter::redactCockatriceUrls(QStringLiteral("open cockatrice://opendeck?token=secret&user=me now")), + QStringLiteral("open cockatrice://opendeck now")); +} + +TEST(CrashReporterTest, RedactCockatriceUrlsIsIdempotent) +{ + // A secret-free URL must not match again after redaction (regression: this hung forever). + ASSERT_EQ(CrashReporter::redactCockatriceUrls(QStringLiteral("open cockatrice://opendeck now")), + QStringLiteral("open cockatrice://opendeck now")); +} + +TEST(CrashReporterTest, RedactCockatriceUrlsLeavesOtherTextAlone) +{ + ASSERT_EQ(CrashReporter::redactCockatriceUrls(QStringLiteral("https://cockatrice.github.io/?token=secret")), + QStringLiteral("https://cockatrice.github.io/?token=secret")); + ASSERT_EQ(CrashReporter::sanitizeLogLine(QStringLiteral("")), QStringLiteral("")); +} + +TEST(CrashReporterTest, InitSkippedWithoutDsn) +{ + CrashReporter reporter; + reporter.init(QString(), QStringLiteral("3.1.2"), QStringLiteral("/tmp/cockatrice-crash-test"), + PersonalSettings::CrashReportingMode::Ask); + ASSERT_FALSE(reporter.isReady()); + reporter.shutdown(); + ASSERT_FALSE(reporter.isReady()); +} + +TEST(CrashReporterTest, InitSkippedWhenNeverSend) +{ + CrashReporter reporter; + reporter.init(QStringLiteral("https://key@sentry.io/1"), QStringLiteral("3.1.2"), + QStringLiteral("/tmp/cockatrice-crash-test"), PersonalSettings::CrashReportingMode::NeverSend); + ASSERT_FALSE(reporter.isReady()); + reporter.shutdown(); +} + +// The init() tests above compile to no-ops when the SDK is off, so they pass +// regardless of the gating; the gating decision itself is covered here, in both +// build modes, through the SDK-independent helper init() consults. +TEST(CrashReporterTest, ReportingEnabledGating) +{ + using Mode = PersonalSettings::CrashReportingMode; + const QString dsn = QStringLiteral("https://key@sentry.io/1"); + ASSERT_FALSE(CrashReporter::isReportingEnabled(QString(), Mode::Ask)); + ASSERT_FALSE(CrashReporter::isReportingEnabled(QString(), Mode::AlwaysSend)); + ASSERT_FALSE(CrashReporter::isReportingEnabled(QString(), Mode::NeverSend)); + ASSERT_TRUE(CrashReporter::isReportingEnabled(dsn, Mode::Ask)); + ASSERT_TRUE(CrashReporter::isReportingEnabled(dsn, Mode::AlwaysSend)); + ASSERT_FALSE(CrashReporter::isReportingEnabled(dsn, Mode::NeverSend)); +} + +TEST(CrashReporterTest, ShutdownWithoutInitIsSafe) +{ + CrashReporter reporter; + ASSERT_FALSE(reporter.isReady()); + reporter.shutdown(); + ASSERT_FALSE(reporter.isReady()); +} + +} // namespace + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index 0d4cdbafe..aea7f1d54 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -367,6 +368,32 @@ TEST_F(SettingsDefaultsTest, Personal_ShowTipsOnStartup_Default) ASSERT_EQ(s.getShowTipsOnStartup(), true); } +TEST_F(SettingsDefaultsTest, Personal_CrashReportingMode_Default) +{ + PersonalSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getCrashReportingMode(), PersonalSettings::CrashReportingMode::Ask); +} + +TEST_F(SettingsDefaultsTest, Personal_CrashReportingMode_SetAndGet) +{ + PersonalSettings s(settingsPath, nullptr); + s.setCrashReportingMode(PersonalSettings::CrashReportingMode::AlwaysSend); + ASSERT_EQ(s.getCrashReportingMode(), PersonalSettings::CrashReportingMode::AlwaysSend); + s.setCrashReportingMode(PersonalSettings::CrashReportingMode::NeverSend); + ASSERT_EQ(s.getCrashReportingMode(), PersonalSettings::CrashReportingMode::NeverSend); + s.setCrashReportingMode(PersonalSettings::CrashReportingMode::Ask); + ASSERT_EQ(s.getCrashReportingMode(), PersonalSettings::CrashReportingMode::Ask); +} + +TEST_F(SettingsDefaultsTest, Personal_CrashReportingMode_ClampsCorruptValue) +{ + QSettings raw(settingsPath + "personal.ini", QSettings::IniFormat); + raw.setValue("personal/crashReportingMode", 99); + raw.sync(); + PersonalSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getCrashReportingMode(), PersonalSettings::CrashReportingMode::Ask); +} + // --- DownloadSettings --- TEST_F(SettingsDefaultsTest, Download_PicDownload_Default)