From f5782dccbd2f97f1f909ed8b15e368f6a6544812 Mon Sep 17 00:00:00 2001 From: "Marco Antonio J. Costa" Date: Sun, 26 Jul 2026 12:07:32 -0300 Subject: [PATCH] Upload pending crash reports as opt-in telemetry A crash report is worth nothing sitting on the player's disk. On startup, drain the crash_report_*.txt files the handler left behind to the endpoint named by CRASH_TELEMETRY_URL in Ja2 Settings; an empty or absent key turns the feature off entirely. The first launch asks the player once and remembers the answer in telemetry.consent -- declined means the reports simply keep accumulating locally. This lands in its own translation unit rather than in more of debug_win_util.cpp. Everything in that file runs inside a faulting thread and may not allocate; everything here runs at startup with a healthy heap and is ordinary code. Two files keep the no-heap rule easy to see and easy to hold. The draining runs on a detached thread. The uploads are synchronous WinHttp calls with seconds-long timeouts, and this sits on the startup path, so on the main thread an unreachable endpoint is a stall the player watches before the splash screen. Nothing waits on the result: if the player quits first the process exits from under the thread, which costs nothing, since an interrupted upload leaves the file on disk and it goes out next launch. The consent prompt stays on the main thread on purpose -- it is a question, and a question has to be asked before anything is sent. Which reports get deleted is chosen so that a mistake cannot destroy them. A file goes away on 2xx, and on 400/413/415, i.e. content the server will never accept. Everything else keeps it: no connection, 5xx, and notably the 403/404 of a mistyped CRASH_TELEMETRY_URL, which would otherwise silently eat every player's crash history. Bounds all round: every WinHttp phase has a timeout, a report over 256 KB is not one of ours and never goes on the wire, at most 20 uploads per launch so a crash-looping build cannot turn startup into an upload session, and reports older than 30 days are dropped unsent. Co-Authored-By: Claude Opus 5 --- CMakeLists.txt | 1 + sgp/CMakeLists.txt | 1 + sgp/crash_telemetry.cpp | 174 ++++++++++++++++++++++++++++++++++++++++ sgp/debug_util.h | 6 ++ sgp/sgp.cpp | 5 ++ 5 files changed, 187 insertions(+) create mode 100644 sgp/crash_telemetry.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 32fd27a41..2b53d15c7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -130,6 +130,7 @@ set(Ja2_Libraries "dbghelp.lib" "winmm.lib" "ws2_32.lib" +"winhttp.lib" bfVFS Lua Multiplayer diff --git a/sgp/CMakeLists.txt b/sgp/CMakeLists.txt index 706f7ac48..35db8757d 100644 --- a/sgp/CMakeLists.txt +++ b/sgp/CMakeLists.txt @@ -2,6 +2,7 @@ set(sgpSrc "${CMAKE_CURRENT_SOURCE_DIR}/Button Sound Control.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/Button System.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/Compression.cpp" +"${CMAKE_CURRENT_SOURCE_DIR}/crash_telemetry.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/Cursor Control.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/DEBUG.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/debug_util.cpp" diff --git a/sgp/crash_telemetry.cpp b/sgp/crash_telemetry.cpp new file mode 100644 index 000000000..daf6d79f9 --- /dev/null +++ b/sgp/crash_telemetry.cpp @@ -0,0 +1,174 @@ +// Crash telemetry: upload the crash_report_*.txt files the crash handler left +// behind, on the next launch. +// +// Deliberately a separate translation unit from debug_win_util.cpp. Everything in +// there runs inside a faulting thread and may not allocate; everything here runs at +// startup with a healthy heap and is ordinary code. Keeping the two apart keeps the +// no-heap rule easy to see and easy to hold. + +#if defined(_MSC_VER) + +#include "debug_util.h" + +#include +#include +#include // _beginthreadex for the detached upload thread + +#include + +namespace { + +// Persisted consent: 1 = yes, 0 = no, -1 = not asked yet. +int readConsent() { + HANDLE h = CreateFileA("telemetry.consent", GENERIC_READ, FILE_SHARE_READ, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (h == INVALID_HANDLE_VALUE) return -1; + char c = 0; DWORD got = 0; + ReadFile(h, &c, 1, &got, NULL); + CloseHandle(h); + return (got == 1 && c == '1') ? 1 : 0; +} + +void writeConsent(bool yes) { + HANDLE h = CreateFileA("telemetry.consent", GENERIC_WRITE, 0, NULL, + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + if (h == INVALID_HANDLE_VALUE) return; + DWORD w; WriteFile(h, yes ? "1" : "0", 1, &w, NULL); + CloseHandle(h); +} + +// A report bigger than this is not one of ours; never put it on the wire. +const DWORD kMaxReportBytes = 256 * 1024; + +// POST one report file to url. Returns the HTTP status, or 0 if the request never +// completed (no connection, DNS failure, timeout) — see reportIsSettled(). +DWORD postReport(const wchar_t* url, const char* path) { + HANDLE fh = CreateFileA(path, GENERIC_READ, FILE_SHARE_READ, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (fh == INVALID_HANDLE_VALUE) return 0; + DWORD size = GetFileSize(fh, NULL); + if (size > kMaxReportBytes) { CloseHandle(fh); return 413; } + std::vector body(size ? size : 1); + DWORD got = 0; + BOOL read_ok = ReadFile(fh, body.data(), size, &got, NULL); + CloseHandle(fh); + if (!read_ok || got != size) return 0; + + URL_COMPONENTS uc = {}; uc.dwStructSize = sizeof(uc); + wchar_t host[256] = {}, urlpath[1024] = {}; + uc.lpszHostName = host; uc.dwHostNameLength = _countof(host); + uc.lpszUrlPath = urlpath; uc.dwUrlPathLength = _countof(urlpath); + if (!WinHttpCrackUrl(url, 0, 0, &uc)) return 400; + + HINTERNET hSession = WinHttpOpen(L"JA2-1.13-crash-telemetry", + WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, 0); + if (!hSession) return 0; + // Bound every phase. Even off the main thread these must not hang forever: the + // thread holds the report files open-ish and we want the queue drained or given + // up on within seconds, not to leave a socket parked for the whole session. + WinHttpSetTimeouts(hSession, 5000, 5000, 10000, 15000); + + DWORD status = 0; + if (HINTERNET hConnect = WinHttpConnect(hSession, host, uc.nPort, 0)) { + DWORD flags = (uc.nScheme == INTERNET_SCHEME_HTTPS) ? WINHTTP_FLAG_SECURE : 0; + if (HINTERNET hReq = WinHttpOpenRequest(hConnect, L"POST", urlpath, NULL, + WINHTTP_NO_REFERER, WINHTTP_DEFAULT_ACCEPT_TYPES, flags)) { + if (WinHttpSendRequest(hReq, L"Content-Type: text/plain\r\n", (DWORD)-1, + body.data(), size, size, 0) && + WinHttpReceiveResponse(hReq, NULL)) { + DWORD len = sizeof(status); + WinHttpQueryHeaders(hReq, + WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, + WINHTTP_HEADER_NAME_BY_INDEX, &status, &len, WINHTTP_NO_HEADER_INDEX); + } + WinHttpCloseHandle(hReq); + } + WinHttpCloseHandle(hConnect); + } + WinHttpCloseHandle(hSession); + return status; +} + +// Whether a report is done with, i.e. safe to delete. Uploaded (2xx), or rejected +// as content the server will never take (a malformed or oversized body). Anything +// else — no connection, 5xx, and notably 404/403 from a mistyped or misconfigured +// CRASH_TELEMETRY_URL — keeps the file, so a bad setting loses nobody's report. +bool reportIsSettled(DWORD status) { + return (status >= 200 && status < 300) || + status == 400 || status == 413 || status == 415; +} + +// Reports older than this are stale: the crash they describe is long since shipped +// past, and a player who was offline for a season should not upload a season of them. +const DWORD kMaxReportAgeDays = 30; + +bool olderThan(const FILETIME& ft, DWORD days) { + FILETIME now; + GetSystemTimeAsFileTime(&now); + ULARGE_INTEGER t = { ft.dwLowDateTime, ft.dwHighDateTime }; + ULARGE_INTEGER n = { now.dwLowDateTime, now.dwHighDateTime }; + if (n.QuadPart <= t.QuadPart) return false; // clock skew: treat as fresh + return (n.QuadPart - t.QuadPart) > days * 24ULL * 60 * 60 * 10000000ULL; +} + +// One launch drains at most this many, so a crash-looping build cannot turn startup +// into a long upload session. The rest wait for the next launch. +const int kMaxUploadsPerRun = 20; + +wchar_t s_telemetryUrl[512]; + +// Drains the pending reports. Runs detached: if the player quits first the process +// exits from under it, which costs nothing — an interrupted upload leaves the file +// on disk and it goes out next launch. +unsigned __stdcall telemetryThread(void*) { + WIN32_FIND_DATAA fd; + HANDLE hFind = FindFirstFileA("crash_report_*.txt", &fd); + if (hFind == INVALID_HANDLE_VALUE) return 0; + int sent = 0; + do { + if (olderThan(fd.ftLastWriteTime, kMaxReportAgeDays)) { + DeleteFileA(fd.cFileName); + continue; + } + if (sent++ >= kMaxUploadsPerRun) break; + if (reportIsSettled(postReport(s_telemetryUrl, fd.cFileName))) + DeleteFileA(fd.cFileName); + } while (FindNextFileA(hFind, &fd)); + FindClose(hFind); + return 0; +} + +} // anonymous namespace + +namespace sgp { +void processCrashTelemetry(const wchar_t* url) { + if (url == NULL || url[0] == L'\0') return; // no endpoint configured: feature off + + int consent = readConsent(); + if (consent < 0) { // first run: ask once, remember the answer + int r = MessageBoxW(NULL, + L"This build can send crash reports to the developers to help fix bugs.\n" + L"A report contains only technical crash data \x2014 no personal information.\n\n" + L"Send crash reports automatically?", + L"Jagged Alliance 2 v1.13 \x2014 Crash Reporting", + MB_YESNO | MB_ICONQUESTION); + writeConsent(r == IDYES); + consent = (r == IDYES) ? 1 : 0; + } + if (consent != 1) return; // declined: leave reports on disk, accumulating + + // Hand the draining to a detached thread. The uploads are synchronous WinHttp + // calls with seconds-long timeouts, and this runs on the startup path: on the + // main thread a slow or unreachable endpoint is a stall the player sees before + // the splash screen. Nothing waits on the result, so it can take as long as it + // takes. The consent prompt above stays here, on purpose — that one is a + // question, and a question has to be asked before anything is sent. + lstrcpynW(s_telemetryUrl, url, ARRAYSIZE(s_telemetryUrl)); + // _beginthreadex, not CreateThread: the upload path uses the CRT (std::vector), + // which wants its per-thread state set up and torn down. + uintptr_t t = _beginthreadex(NULL, 0, telemetryThread, NULL, 0, NULL); + if (t) CloseHandle((HANDLE)t); +} +} // namespace sgp + +#endif // _MSC_VER diff --git a/sgp/debug_util.h b/sgp/debug_util.h index 38abec514..7a577d8b4 100644 --- a/sgp/debug_util.h +++ b/sgp/debug_util.h @@ -69,6 +69,12 @@ namespace sgp // stamp into crash reports, so a report can be tied to whoever raises it with // us on Discord. Empty or unset simply omits the field. Call once at startup. void setCrashUserHandle(const wchar_t* handle); + + // Startup crash-telemetry pass (heap is healthy here — never call from a + // crash handler). If url is empty the feature is off. On first run, asks the + // player for consent (native dialog) and remembers it; if granted, POSTs each + // pending crash_report_*.txt to url and deletes the ones that upload cleanly. + void processCrashTelemetry(const wchar_t* url); } #endif // BASE_DEBUG_UTIL_H_ diff --git a/sgp/sgp.cpp b/sgp/sgp.cpp index 90036b721..75c1a4dc7 100644 --- a/sgp/sgp.cpp +++ b/sgp/sgp.cpp @@ -948,6 +948,11 @@ void GetRuntimeSettings( ) oProps.initFromIniFile(GAME_INI_FILE); PopulateSectionFromCommandLine(oProps, "Ja2 Settings"); + // Upload crash reports from previous runs (first launch asks the player). + // Empty CRASH_TELEMETRY_URL = off. Runs here, at startup, never in a crash. + sgp::processCrashTelemetry( + oProps.getStringProperty("Ja2 Settings", L"CRASH_TELEMETRY_URL").c_str()); + // Optional player handle stamped into crash reports written from here on, so a // report can be tied to whoever raises it with us. Unset = field omitted. sgp::setCrashUserHandle(