mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-05 17:43:23 -08:00
[CrashReporting] Breakpad capture, session detection and symbol upload (#7523)
* [CrashReporting] Breakpad everywhere, session detection, symbol CI (inc 3) Breakpad on all platforms (no crashpad packaging); remove the Windows MiniDumpWithFullMemory filter and the installer WER LocalDumps full-dump registration. QLockFile per-process session tracking replaces the Windows-only .dmp scan, placed after parser.process and the instance hand-off so help/version exits and handed-off secondaries never leak a lock. CI: Release-only DSN bake from SENTRY_DSN secret, tag-only sentry-cli symbol upload (org cockatrice, project native). Tests: 533/533 ctest green, format clean, code gate PASS. * [CrashReporting] Enable http_retry, harden symbol upload, fix NSIS context Sets sentry_options_set_http_retry so the cache_keep-held crash envelope in Ask mode is written with a retry filename and actually flushes on giveConsent() instead of sitting in <db>/cache until pruning. Adds continue-on-error to both sentry-cli install steps so a Sentry registry outage or broken Git Bash install cannot block release assets. Cleans the legacy WER CrashDumps folder under SetShellVarContext current (under all it resolves to ProgramData, not the user's AppData) in installer and uninstaller; registry delete works in either context. * [CrashReporting] Extract the startup crash-report flow from main() per review * [CrashReporting] Break up the CrashReporter class doc per review * [CrashReporting] Drop the invalid --release from the symbol upload `debug-files upload` never accepted `--release` (checked against the pinned sentry-cli 3.8.0: the subcommand declares --org/--project and a set of type/id/scan options, and execute() reads no release; the only global flags are --url/--header/--auth-token/--log-level/--quiet/--allow-failure). Clap rejects the unknown flag before any upload happens, so every tag build has been shipping without symbols, with continue-on-error keeping the release pipeline green. Release association is also the wrong mechanism: debug information files are matched to events by debug ID via the event's debug_meta. The Sentry release of a native event comes from the event itself (COCKATRICE_SENTRY_RELEASE), not from the DIF upload. Remove the flag and the now-unused SENTRY_RELEASE env from both invocations (Linux, and Windows/macOS). The symbol upload stays continue-on-error (a Sentry outage must not block the release assets); revisit once it has proven itself on a tag. * [CrashReporting] Split debug information out of packaged release binaries -g is required so the CI symbol upload has DWARF to send, but it was left in the shipped binaries too: the Linux Release client is 563 MiB, 552 MiB of it debug info, and every distribution channel pays for that. Keep the build tree unchanged (CI uploads symbols from build/) and strip only the packaged copy. A CPack pre-build script extracts each ELF's debug info to COCKATRICE_DEBUG_SYMBOL_DIR and then runs `strip --strip-debug --strip-unneeded`. .eh_frame is allocatable and survives, so unwinding is unaffected, and the build ID is identical in the stripped binary and the sidecar. Measured on a local Release build (GCC -O2 -g, WITH_CRASH_REPORTING=ON), bytes: binary build tree packaged before packaged after .debug sidecar cockatrice 590,722,560 590,722,560 28,086,480 579,623,576 oracle 53,432,608 53,432,608 1,649,888 52,075,344 tar.gz of both trees: 289,964,521 -> 19,018,058 (~15x smaller). Only ELF (Linux/BSD) is handled: Windows already links /DEBUG with separate PDBs, and macOS extracts a dSYM from the bundle. Opt-out via COCKATRICE_SPLIT_DEBUG_SYMBOLS; skipped with a warning when objcopy/strip are missing. CPACK_PRE_BUILD_SCRIPTS (not CPACK_INSTALL_SCRIPTS, which runs before staging) invokes it, and the script refuses to run unless CPACK_TEMPORARY_DIRECTORY is a real CPack staging tree so a missing variable can never turn its glob into a filesystem-wide walk. * [CrashReporting] Upload Windows PDBs from where Ninja actually puts them The Windows branch of the symbol upload looked in build/<target>/Release, which is the Visual Studio generator's layout. The Windows leg uses Ninja, which is single-config: linker PDBs land in build/<target>/, next to the binaries — the same paths the "[Windows] Upload PDBs" artifact step below globs with if-no-files-found: error. So paths was always empty on Windows, the step printed "No symbol paths found, skipping symbol upload" and exited 0: a silent no-op behind continue-on-error. Every Windows build would then report "A required debug information file was missing" for cockatrice.exe with a PDB-style debug id (GUID-age), exactly as if no upload step existed at all. Use the artifact step's globs. Also turn the empty-path case into a failure in both symbol steps: a silent skip is indistinguishable from a successful upload (which is how the wrong path went unnoticed), and continue-on-error already keeps a broken upload from failing the release. Verified by extracting both steps' run blocks and executing the Windows branch against a Ninja-style tree: it now passes the three linker PDBs to sentry-cli (and not the CMakeFiles/*.pdb intermediates), and exits non-zero with an ::error:: annotation when there is nothing to upload. * [CrashReporting] Archive PDBs on tag builds so a failed upload is recoverable The "[Windows] Upload PDBs" artifact step skipped tag builds (github.ref_type != 'tag'), so for a release the symbols existed only inside the Sentry upload — which is continue-on-error. A failed upload therefore lost the ability to symbolicate that release permanently: no rebuild produces the same PDB, since the debug id is a per-build GUID+age. That is how every Windows crash reported so far became unreadable. Archive the PDBs for tag builds as well, and order the step after "Upload to release" so a missing PDB can never block a release from being published. Retention stays at the repository default; nothing consumes the artifact automatically, it is the manual recovery path (gh run download <run> -n <name>-PDBs, then sentry-cli debug-files upload). Verified by re-parsing the workflow and asserting the step order (release -> PDBs -> attest) and the broadened condition. * [CrashReporting] Upload the Windows executables with their PDBs Sentry needs two things per image on Windows: symbolication and stack unwinding. The PDB covers symbolication, but for a 64-bit PE the Call Frame Information (.pdata/.xdata) lives in the executable and is not copied into the PDB, so uploading PDBs alone leaves the image at "Stack Unwinding: Missing". That is exactly what the event's Images panel showed for cockatrice.exe, while Sentry's own Microsoft symbol server resolved KERNELBASE.dll and ucrtbase.dll from the same dump. sentry-cli also needs the pair in the same invocation: only then does it associate the PE's code identifier with the PDB's debug identifier, otherwise "the identifiers diverge and Sentry might not be able to resolve the correct file for symbolication" (docs: Debug Identifiers). Do the same for macOS, where the dSYM carries the debug information but the Mach-O carries the unwind information, and include the executables in the symbol artifact so a re-upload still has the pair to offer. Verified by extracting the step and running both branches against stub harnesses: Windows passes the three exe/pdb pairs in a single sentry-cli invocation and still fails loudly on an empty tree; macOS passes the dSYM and the Mach-O together. * [CrashReporting] Upload the shipped Qt runtime DLLs too Qt ships no PDBs with its MSVC SDK, so every Qt frame in a crash report resolved to nothing: an image with neither PDB nor PE has no unwind information (on 64-bit Windows the Call Frame Information lives in the PE and is not copied into the PDB) and no symbol source to fall back on. The list is required-runtime.txt, the manifest .ci/compile.sh already asserts the installer against, so what gets uploaded is exactly what an install carries. Qt frames now get unwinding plus export-table names; line numbers still need Qt's debug information files, which the SDK omits. The guards are not cosmetic: the step runs under `bash -eo pipefail`, where a sed over a missing file or an unmatched ls glob aborts the whole step, and continue-on-error makes that look identical to a successful upload. * [CrashReporting] Upload only the binaries, archive symbols only on tags The Linux leg handed sentry-cli whole build directories: 471 candidate files, of which 7 were object files that can never match a stack frame. Only the three linked binaries carry the build-ids Sentry looks up; the packaged copy is split and stripped but keeps that same build-id. The Windows symbol archive ran on every packaging build even though only tag builds upload to Sentry, so a PR build paid for an artifact nothing could ever consume. Tags keep it as the durable copy for a failed upload. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL
GitHub
parent
ad0fd2d4b0
commit
9f59f5c495
9 changed files with 648 additions and 156 deletions
|
|
@ -46,9 +46,6 @@
|
|||
#include <QCoreApplication>
|
||||
#include <QCryptographicHash>
|
||||
#include <QDebug>
|
||||
#include <QDialog>
|
||||
#include <QDir>
|
||||
#include <QFile>
|
||||
#include <QFileInfo>
|
||||
#include <QGuiApplication>
|
||||
#include <QIcon>
|
||||
|
|
@ -108,60 +105,6 @@ static void CockatriceLogger(QtMsgType type, const QMessageLogContext &ctx, cons
|
|||
Logger::getInstance().log(type, ctx, logMessage);
|
||||
}
|
||||
|
||||
#ifdef Q_OS_WIN
|
||||
// clang-format off
|
||||
#include <Windows.h>
|
||||
#include <DbgHelp.h>
|
||||
#include <ShlObj.h>
|
||||
#include <ctime>
|
||||
#include <filesystem>
|
||||
|
||||
#pragma comment(lib, "DbgHelp.lib") // Link the DbgHelp library
|
||||
// clang-format on
|
||||
|
||||
LONG WINAPI CockatriceUnhandledExceptionFilter(EXCEPTION_POINTERS *exceptionPointers)
|
||||
{
|
||||
std::filesystem::path path;
|
||||
|
||||
// Find %LOCALAPPDATA% (or cheat at finding it)
|
||||
wchar_t *localAppDataFolder;
|
||||
if (SHGetKnownFolderPath(FOLDERID_LocalAppData, KF_FLAG_CREATE, NULL, &localAppDataFolder) != S_OK) {
|
||||
path = std::filesystem::temp_directory_path().parent_path().parent_path();
|
||||
} else {
|
||||
path = std::filesystem::path(localAppDataFolder);
|
||||
}
|
||||
|
||||
// Plan on writing crash files into %LOCALAPPDATA%/CrashDumps/Cockatrice/cockatrice.crash.*.dmp
|
||||
path /= "CrashDumps";
|
||||
path /= "Cockatrice";
|
||||
if (!std::filesystem::exists(path)) {
|
||||
std::filesystem::create_directories(path);
|
||||
}
|
||||
path /= "cockatrice.crash." + std::to_string(std::time(0)) + ".dmp";
|
||||
|
||||
// Create and write crash files
|
||||
#ifdef UNICODE
|
||||
HANDLE hDumpFile =
|
||||
CreateFile(path.wstring().c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
|
||||
#else
|
||||
HANDLE hDumpFile =
|
||||
CreateFile(path.string().c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
|
||||
#endif
|
||||
|
||||
MINIDUMP_EXCEPTION_INFORMATION mei;
|
||||
mei.ExceptionPointers = exceptionPointers;
|
||||
mei.ThreadId = GetCurrentThreadId();
|
||||
mei.ClientPointers = 1;
|
||||
|
||||
MiniDumpWriteDump(GetCurrentProcess(), GetCurrentProcessId(), hDumpFile, MiniDumpWithFullMemory, &mei, nullptr,
|
||||
nullptr);
|
||||
|
||||
CloseHandle(hDumpFile);
|
||||
|
||||
return EXCEPTION_EXECUTE_HANDLER;
|
||||
}
|
||||
#endif
|
||||
|
||||
void installNewTranslator()
|
||||
{
|
||||
QString lang = SettingsCache::instance().personal().getLang();
|
||||
|
|
@ -202,11 +145,52 @@ static QString redactActivationUrl(const QString &url)
|
|||
return parsed.scheme() + "://" + parsed.host();
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Run the startup crash-report flow for an unclean exit (or --simulate-crash).
|
||||
*
|
||||
* With a configured DSN the SDK owns capture and upload (consent-gated); with an
|
||||
* empty DSN the flow falls back to a local mock file. AlwaysSend uploads silently
|
||||
* (the log was folded into the pending native envelope(s) at startup); Ask offers
|
||||
* the dialog; NeverSend skips silently.
|
||||
*
|
||||
* @param attachedEnvelopes number of pending native envelopes that already carry the
|
||||
* redacted log, so an AlwaysSend run only sends a standalone
|
||||
* log event when no crash event existed.
|
||||
*/
|
||||
static void runCrashReportFlow(CrashReporter &crashReporter, QWidget *parent, int attachedEnvelopes)
|
||||
{
|
||||
const PersonalSettings::CrashReportingMode crashMode = SettingsCache::instance().personal().getCrashReportingMode();
|
||||
const bool crashReady = crashReporter.isReady();
|
||||
if (crashMode == PersonalSettings::CrashReportingMode::AlwaysSend) {
|
||||
if (crashReady) {
|
||||
// Silent path: the redacted log was folded into the pending native crash
|
||||
// envelope(s) at startup and consent already released them. Only when no
|
||||
// pending crash event existed (e.g. --simulate-crash) does a standalone
|
||||
// log event still need to go out here.
|
||||
if (attachedEnvelopes == 0) {
|
||||
const QString crashPreview = DlgCrashReport::buildRedactedPreview();
|
||||
if (!crashReporter.sendReport(crashPreview)) {
|
||||
const QString savedPath = DlgCrashReport::writeMockReport(crashPreview);
|
||||
qCWarning(MainLog) << "Failed to send crash report, saved locally to" << savedPath;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
DlgCrashReport::writeMockReport(DlgCrashReport::buildRedactedPreview());
|
||||
}
|
||||
} else if (crashMode == PersonalSettings::CrashReportingMode::Ask) {
|
||||
// Real mode: Send gives consent + captures the event, Don't send revokes
|
||||
// consent; the dialog owns both via the reporter.
|
||||
DlgCrashReport crashDialog(parent, crashReady, crashReady ? &crashReporter : nullptr);
|
||||
crashDialog.exec();
|
||||
}
|
||||
// NeverSend: skip silently (no dialog, nothing written or sent).
|
||||
}
|
||||
|
||||
int main(int argc, char *argv[])
|
||||
{
|
||||
#ifdef Q_OS_WIN
|
||||
SetUnhandledExceptionFilter(CockatriceUnhandledExceptionFilter);
|
||||
#endif
|
||||
// No custom unhandled-exception filter: sentry owns native crash capture on all
|
||||
// platforms. (The legacy MiniDumpWithFullMemory writer was removed as a privacy
|
||||
// liability; unclean exits are detected via CrashReporter::beginSession below.)
|
||||
|
||||
// Logging setup
|
||||
#ifdef Q_OS_APPLE
|
||||
|
|
@ -404,6 +388,16 @@ int main(int argc, char *argv[])
|
|||
}
|
||||
}
|
||||
|
||||
// Unclean-exit detection (cross-platform QLockFile session tracking, replacing the
|
||||
// legacy Windows-only .dmp scan): a leftover session lock means the previous run died
|
||||
// without reaching endSession(). Our own lock is held until the event loop finishes.
|
||||
// Deliberately placed after parser.process() (--help/--version exit inside it and must
|
||||
// not leak a lock) and after the instance hand-off (a handed-off secondary exits
|
||||
// immediately and must never take a lock). Note this counts any death without
|
||||
// endSession() as a crash, including SIGTERM at logout, Task Manager kills and
|
||||
// session-end shutdowns; in AlwaysSend those send a report too.
|
||||
const bool uncleanExit = crashReporter.beginSession(crashDatabasePath);
|
||||
|
||||
rng = new RNG_SFMT(CryptoUtil::randomUInt64());
|
||||
themeManager = new ThemeManager;
|
||||
soundEngine = new SoundEngine;
|
||||
|
|
@ -497,63 +491,11 @@ int main(int argc, char *argv[])
|
|||
ui.show();
|
||||
qCInfo(MainLog) << "ui.show() finished";
|
||||
|
||||
// Crash-report flow: with a configured DSN the SDK owns capture and upload
|
||||
// (consent-gated); with an empty DSN the dialog falls back to a local mock
|
||||
// file and behaves exactly as before. Triggered by --simulate-crash or by
|
||||
// leftover Windows crash dumps, which are consumed after handling so one
|
||||
// crash never prompts twice.
|
||||
{
|
||||
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();
|
||||
const bool crashReady = crashReporter.isReady();
|
||||
if (crashMode == PersonalSettings::CrashReportingMode::AlwaysSend) {
|
||||
if (crashReady) {
|
||||
// Silent path: the redacted log was folded into the pending native crash
|
||||
// envelope(s) at startup and consent already released them. Only when no
|
||||
// pending crash event existed (e.g. --simulate-crash) does a standalone
|
||||
// log event still need to go out here.
|
||||
if (crashLogAttachedEnvelopes == 0) {
|
||||
const QString crashPreview = DlgCrashReport::buildRedactedPreview();
|
||||
if (!crashReporter.sendReport(crashPreview)) {
|
||||
const QString savedPath = DlgCrashReport::writeMockReport(crashPreview);
|
||||
qCWarning(MainLog) << "Failed to send crash report, saved locally to" << savedPath;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
DlgCrashReport::writeMockReport(DlgCrashReport::buildRedactedPreview());
|
||||
}
|
||||
} else if (crashMode == PersonalSettings::CrashReportingMode::Ask) {
|
||||
// Real mode: Send gives consent + captures the event, Don't
|
||||
// send revokes consent; the dialog owns both via the reporter.
|
||||
DlgCrashReport crashDialog(&ui, crashReady, crashReady ? &crashReporter : nullptr);
|
||||
crashDialog.exec();
|
||||
}
|
||||
// NeverSend: skip silently (no dialog, nothing written or sent).
|
||||
// Handled dumps are consumed so one crash never prompts twice.
|
||||
for (const QString &dump : crashDumps) {
|
||||
QFile::remove(crashDumpDir + QStringLiteral("/") + dump);
|
||||
}
|
||||
}
|
||||
// Crash-report flow: the trigger is --simulate-crash or an unclean previous exit
|
||||
// (leftover session lock; the old %LOCALAPPDATA%/CrashDumps .dmp scan is gone with
|
||||
// the exception filter). See runCrashReportFlow() for the per-mode handling.
|
||||
if (parser.isSet("simulate-crash") || uncleanExit) {
|
||||
runCrashReportFlow(crashReporter, &ui, crashLogAttachedEnvelopes);
|
||||
}
|
||||
|
||||
// force shortcuts to be shown/hidden in right-click menus, regardless of system defaults
|
||||
|
|
@ -580,8 +522,6 @@ int main(int argc, char *argv[])
|
|||
|
||||
int ret = app.exec();
|
||||
|
||||
crashReporter.shutdown();
|
||||
|
||||
qCInfo(MainLog) << "Event loop finished, terminating...";
|
||||
delete rng;
|
||||
PingPixmapGenerator::clear();
|
||||
|
|
@ -589,5 +529,10 @@ int main(int argc, char *argv[])
|
|||
UserLevelPixmapGenerator::clear();
|
||||
ManaSymbolPixmapGenerator::clear();
|
||||
|
||||
// Released last: crashes anywhere in the teardown above (a common place for Qt apps to
|
||||
// crash) are still captured by the SDK and detected as unclean exits on the next launch.
|
||||
crashReporter.shutdown();
|
||||
crashReporter.endSession();
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue