From 8b93e75b9f1b0deb2a24f6c282306c3dde1916f3 Mon Sep 17 00:00:00 2001 From: Bob Date: Tue, 8 Sep 2026 05:03:05 +0000 Subject: [PATCH 1/2] feat(vitals): persist comparable error-cluster reports Git-Session-Id: 7941 --- .github/workflows/collect-play.yml | 11 +- README.md | 61 ++++++ crash_reports.py | 160 +++++++++++++++ crash_signatures.py | 115 +++++++++++ fixtures/crashes/README.md | 19 ++ fixtures/crashes/managed-npe.txt | 4 + fixtures/crashes/native-jni-abort.txt | 8 + fixtures/crashes/nested-inflation.txt | 12 ++ fixtures/crashes/rust-native-synthetic.txt | 8 + test_crash_reports.py | 221 +++++++++++++++++++++ test_crash_signatures.py | 160 +++++++++++++++ vitals.py | 106 +++++++++- 12 files changed, 872 insertions(+), 13 deletions(-) create mode 100644 crash_reports.py create mode 100644 crash_signatures.py create mode 100644 fixtures/crashes/README.md create mode 100644 fixtures/crashes/managed-npe.txt create mode 100644 fixtures/crashes/native-jni-abort.txt create mode 100644 fixtures/crashes/nested-inflation.txt create mode 100644 fixtures/crashes/rust-native-synthetic.txt create mode 100644 test_crash_reports.py create mode 100644 test_crash_signatures.py diff --git a/.github/workflows/collect-play.yml b/.github/workflows/collect-play.yml index 65da7d7e..5770a488 100644 --- a/.github/workflows/collect-play.yml +++ b/.github/workflows/collect-play.yml @@ -21,16 +21,21 @@ jobs: - name: Install dependencies if: env.HAS_SA == 'true' run: uv sync - - name: Collect vitals + install base + - name: Collect vitals, error reports + install base if: env.HAS_SA == 'true' + shell: bash env: PLAY_SA_JSON: ${{ secrets.PLAY_SA_JSON }} PLAY_BUCKET: ${{ secrets.PLAY_BUCKET }} - GOOGLE_APPLICATION_CREDENTIALS: ${{ github.workspace }}/play-sa.json + GOOGLE_APPLICATION_CREDENTIALS: ${{ runner.temp }}/play-sa.json run: | + trap 'rm -f "$GOOGLE_APPLICATION_CREDENTIALS"' EXIT printf '%s' "$PLAY_SA_JSON" > "$GOOGLE_APPLICATION_CREDENTIALS" uv run vitals.py crash-rate --days 30 --update data/android-crash-rate.csv uv run vitals.py anr-rate --days 30 --update data/android-anr-rate.csv + # Sanitized, ranked error snapshots; API failures fail this collection. + uv run vitals.py errors --type all --days 7 --limit 100 --stacktraces \ + --update-dir data/android-errors # installed.csv = Active Device Installs; android-ratings.csv = store # rating over time — both from the bulk reports (#17). if [ -n "$PLAY_BUCKET" ]; then @@ -43,13 +48,13 @@ jobs: # skipping, which is how the vitals feed stalled unnoticed for a week. uv run play_tracks.py --update data/android-tracks.csv \ || echo "::warning::play_tracks.py failed (exit $?) - track state not updated" - rm -f "$GOOGLE_APPLICATION_CREDENTIALS" - name: Commit if: env.HAS_SA == 'true' run: | git config --local user.email "noreply@github.com" git config --local user.name "GitHub Action" git add data/android-crash-rate.csv data/android-anr-rate.csv data/android/installed.csv data/android-ratings.csv + git add data/android-errors/current.json data/android-errors/current.md data/android-errors/history/ # android-tracks.csv may not exist yet if play_tracks.py lacked permission [ -f data/android-tracks.csv ] && git add data/android-tracks.csv || true git diff --cached --quiet || git commit -m "chore: update Play Console stats" diff --git a/README.md b/README.md index f24bbfaa..cdcf764d 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ These data are updated automatically in CI: - `stats-assets.csv` - Per-asset download counts (source for per-platform / per-version breakdowns) - `android/installed.csv` - Android install base (Active Device Installs, Play Console bulk reports) - `android-crash-rate.csv` / `android-anr-rate.csv` - Android vitals (Play Developer Reporting API) + - `android-errors/current.md` / `current.json` - Ranked error issues and comparisons; immutable JSON snapshots in `android-errors/history/` - `android-ratings.csv` - Android Play Store rating over time (Total Average Rating, bulk reports) The following is manually updated: @@ -47,6 +48,66 @@ Needs a Google Cloud service account granted "view app quality / Android vitals" access in the Play Console; point at the key with `--credentials` or `GOOGLE_APPLICATION_CREDENTIALS`. See the module docstring for setup. +### Error reports for Android release triage + +The daily Play collector writes a seven-day, top-100 report to +`data/android-errors/current.md`. The Android release owner reads this alongside +`android-tracks.csv` and Play Console's per-version vitals before promoting a build: + +```sh +uv run vitals.py errors --type all --days 7 --limit 100 --stacktraces --markdown +uv run vitals.py errors --type all --days 7 --limit 100 --stacktraces --json +uv run vitals.py errors --type all --days 7 --limit 100 --stacktraces --update-dir data/android-errors +``` + +Start with the highest report counts and increases, inspect the retained sample +frames, and link each actionable cluster to its existing `ActivityWatch/aw-android` +issue or open one with the report's stable ID and evidence. Counts are reports +within the recorded query window, not unique users or severity scores. Severity +is `unclassified` until triage. Confirm that an affected version reached users +using `android-tracks.csv`, then check Play Console's per-version vitals before +claiming a release fixed an issue. Samples describe observed reports; they cannot +prove coverage of every affected or fixed version. + +JSON schema version 1 records `package`, `collected_at`, `query` (type, days, +limit, and hour-aligned UTC start/end), `coverage`, `clusters`, and `comparison`. +`--days` defaults to 1. Each cluster has an `identity`, `identity_source`, +`report_count`, sanitized `trace`, optional sample version/time provenance, and +optional disposition. Play's full issue resource name is the primary identity; +a deterministic heuristic fingerprint is used only when that name is absent. +Google warns that its [alpha issue grouping can change identities](https://developers.google.com/play/developer/reporting/reference/rest/v1beta1/vitals.errors.issues). + +Raw `reportText`, exception messages, local paths, and credentials are excluded +from persisted reports. Structural sample frames and provenance are retained; +frame extraction is a heuristic because Google does not guarantee the +[report text's machine-readable format](https://developers.google.com/play/developer/reporting/reference/rest/v1beta1/vitals.errors.reports). + +Comparisons require the same package, issue type, window duration, and result +limit. Daily seven-day windows overlap, so count changes compare rolling windows. +A missing issue is **not observed**, never automatically fixed: it may fall +outside the window or top-N limit, or Play may regroup it. + +`--update-dir` writes `current.json`, `current.md`, and an append-only +`history/-.json` snapshot. API failures fail collection +instead of publishing an empty success. Maintain triage decisions separately in +`data/android-errors/dispositions.json` (automatically read if present), or pass +`--dispositions PATH`: + +```json +{ + "apps/net.activitywatch.android/ISSUE_ID": { + "status": "investigating", + "issue_url": "https://github.com/ActivityWatch/aw-android/issues/210", + "note": "Confirm affected version codes before release promotion." + } +} +``` + +Use actual identities from the report. The collector never overwrites this file; +review and commit disposition changes manually. The workflow stages only the +generated current files and history directory, and removes its temporary +service-account key even when collection fails. + ## Play release tracks `releases.csv` records when a version was *tagged*, which is not when it diff --git a/crash_reports.py b/crash_reports.py new file mode 100644 index 00000000..0b61964a --- /dev/null +++ b/crash_reports.py @@ -0,0 +1,160 @@ +"""Versioned Play error snapshots and an operator-facing report; no raw traces on disk.""" +from __future__ import annotations + +import hashlib +import html +import json +import os +import tempfile +from pathlib import Path + +from crash_signatures import fallback_fingerprint, normalize_trace + + +def json_text(value: dict) -> str: + return json.dumps(value, indent=2, sort_keys=True, ensure_ascii=False) + "\n" + + +def build_snapshot(package: str, query: dict, issues: list[dict], samples: dict, + collected_at: str, truncated: bool, dispositions: dict | None = None) -> dict: + clusters = [] + for issue in issues: + issue_id = issue.get("name") or None + sample = samples.get(issue_id, {}) + trace = sample.get("reportText", "") + fingerprint = None if issue_id else fallback_fingerprint( + issue.get("type", ""), issue.get("cause", ""), issue.get("location", ""), trace) + identity = issue_id or fingerprint + raw_count = issue.get("errorReportCount") + version = sample.get("appVersion", {}) + clusters.append({ + "identity": identity, + "identity_source": "play_issue" if issue_id else "heuristic_fingerprint", + "issue_id": issue_id, + "fingerprint": fingerprint, + "type": issue.get("type", ""), + "report_count": int(raw_count) if raw_count is not None else None, + "severity": "unclassified", + "cause": issue.get("cause", ""), + "location": issue.get("location", ""), + "issue_url": issue.get("issueUri", ""), + "last_report_time": issue.get("lastErrorReportTime"), + "trace": normalize_trace(trace), + "sample": { + "report_id": sample.get("name"), + "event_time": sample.get("eventTime"), + "version_code": version.get("versionCode"), + "version_name": version.get("versionName"), + } if sample else None, + "disposition": (dispositions or {}).get(identity, {}), + }) + clusters.sort(key=lambda row: ( + -(row["report_count"] if row["report_count"] is not None else -1), row["identity"])) + return { + "schema_version": 1, + "package": package, + "collected_at": collected_at, + "query": query, + "coverage": {"returned": len(clusters), "truncated": truncated}, + "count_source": "Play errorReportCount within query interval and filter", + "clusters": clusters, + } + + +def compare_snapshots(current: dict, previous: dict | None) -> dict: + if previous is None: + return {"status": "no_baseline"} + if previous.get("schema_version") != 1: + raise ValueError("Unsupported previous snapshot schema_version") + # Compare equal-sized rolling windows, never pretend differing selections are a trend. + keys = ("days", "type", "limit") + if (current["package"] != previous["package"] or + any(current["query"][k] != previous["query"][k] for k in keys)): + return {"status": "incompatible_query", "previous_collected_at": previous["collected_at"]} + old = {row["identity"]: row for row in previous["clusters"]} + new = {row["identity"]: row for row in current["clusters"]} + deltas = {} + for identity in sorted(old.keys() & new.keys()): + before, after = old[identity]["report_count"], new[identity]["report_count"] + deltas[identity] = after - before if before is not None and after is not None else None + return { + "status": "comparable", + "previous_collected_at": previous["collected_at"], + "newly_observed": sorted(new.keys() - old.keys()), + "not_observed": sorted(old.keys() - new.keys()), + "report_count_deltas": deltas, + } + + +def _cell(value) -> str: + return html.escape(str(value or ""), quote=False).replace("|", "\\|").replace("\n", " ") + + +def render_markdown(snapshot: dict) -> str: + query, comparison = snapshot["query"], snapshot.get("comparison", {}) + deltas = comparison.get("report_count_deltas", {}) + lines = [ + f"# Android error clusters: {_cell(snapshot['package'])}", "", + f"Collected: {snapshot['collected_at']}. Window: {query['start_time']} → {query['end_time']} (UTC).", + f"Type: {query['type']}; ranked by reports; limit: {query['limit']}; " + f"truncated: {str(snapshot['coverage']['truncated']).lower()}.", "", + "Counts are reports in the query window, not users or lifetime totals. " + "Severity is unclassified. Play-reported causes are root-cause hypotheses, not confirmed diagnoses.", + "Missing clusters are not observed in this selection; they are not proven fixed. " + "Play may regroup issues and change their IDs.", "", + f"Comparison: {comparison.get('status', 'no_baseline')}. " + f"Previous collection: {comparison.get('previous_collected_at', 'none')}.", "", + "| Reports | Δ reports | Type | Play-reported cause / location | Sample version | Identity | Disposition |", + "| ---: | ---: | --- | --- | --- | --- | --- |", + ] + for row in snapshot["clusters"]: + delta = deltas.get(row["identity"]) + sample, disposition = row["sample"] or {}, row["disposition"] + count = row["report_count"] if row["report_count"] is not None else "unknown" + version = " / ".join(str(sample[k]) for k in ("version_code", "version_name") if sample.get(k)) + disposition_text = " — ".join(str(disposition[k]) for k in ("status", "issue_url", "note") + if disposition.get(k)) + lines.append("| " + " | ".join(_cell(value) for value in ( + str(count), f"{delta:+d}" if delta is not None else "—", row["type"], + f"{row['cause']} / {row['location']}", version or "unavailable", + row["identity"], disposition_text)) + " |") + if not snapshot["clusters"]: + lines.extend(["", "No error issues returned for this window."]) + if comparison.get("newly_observed"): + lines.extend(["", "Newly observed: " + ", ".join(map(_cell, comparison["newly_observed"]))]) + if comparison.get("not_observed"): + lines.extend(["", "Not observed: " + ", ".join(map(_cell, comparison["not_observed"]))]) + lines.extend(["", "Sample versions identify one report, not all affected versions. " + "Use the Play issue and release-track data to verify a fix in the shipped version."]) + return "\n".join(lines) + "\n" + + +def persist_snapshot(directory: Path, snapshot: dict) -> Path: + """Retain every distinct snapshot before replacing the current report.""" + history = directory / "history" + history.mkdir(parents=True, exist_ok=True) + payload = json_text(snapshot) + digest = hashlib.sha256(payload.encode()).hexdigest()[:16] + stamp = snapshot["collected_at"].replace(":", "").replace("-", "") + archive = history / f"{stamp}-{digest}.json" + # Publish a complete file without replacing a historical snapshot. A failed + # write leaves only a temporary file, never a truncated immutable archive. + with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8", dir=history, + prefix=".", suffix=".tmp", delete=False) as stream: + temporary_archive = Path(stream.name) + try: + stream.write(payload) + stream.flush() + os.fsync(stream.fileno()) + try: + os.link(temporary_archive, archive) + except FileExistsError: + if archive.read_text(encoding="utf-8") != payload: + raise ValueError(f"Existing snapshot differs from its content hash: {archive}") + finally: + temporary_archive.unlink(missing_ok=True) + for name, text in (("current.json", payload), ("current.md", render_markdown(snapshot))): + temporary = directory / f".{name}.{digest}.tmp" + temporary.write_text(text, encoding="utf-8") + temporary.replace(directory / name) + return archive diff --git a/crash_signatures.py b/crash_signatures.py new file mode 100644 index 00000000..cc4c37d6 --- /dev/null +++ b/crash_signatures.py @@ -0,0 +1,115 @@ +"""Conservative structural signatures for reports without a Play issue ID. + +These are versioned heuristics, not authoritative crash clusters. Play's +reportText is human-readable and not machine-stable; unsupported lines are +dropped, so incomplete/unsymbolicated reports can share a fallback signature. +No exception messages, source paths, or unparsed text enter the signature. +""" +from __future__ import annotations + +import hashlib +import json +import re + + +_IDENT = r"[A-Za-z_$][A-Za-z0-9_$]*" +_JAVA_METHOD = rf"(?:{_IDENT}\.)+(?:{_IDENT}||)" +_CLASS = rf"(?:{_IDENT}\.)*(?:[A-Z_$][A-Za-z0-9_$]*)?(?:Exception|Error|Throwable)" +_CAUSE = re.compile( + rf'^(?:(?:Caused by:\s*|Exception\s+|Exception in thread "[^"]*"\s+))?' + rf"({_CLASS})(?::|$)" +) +_SIGNAL_NAME = r"SIG(?:ABRT|SEGV|BUS|ILL|FPE|TRAP|SYS|KILL|QUIT)" +_SIGNAL = re.compile( + rf"^(?:(?:Fatal\s+)?signal\s+\d+\s+\(({_SIGNAL_NAME})\)|({_SIGNAL_NAME})(?::|$))" +) +_LOGCAT = re.compile( + r"^(?:(?:\d{4}-)?\d{2}-\d{2}\s+)?\d{2}:\d{2}:\d{2}(?:\.\d+)?\s+" + r"(?:(?:\d+\s+){2})?[VDIWEFAS]\s+[^:\s]+\s*:\s*(.*)$" +) +_JAVA_FRAME = re.compile(rf"^at\s+({_JAVA_METHOD})\s*\([^()]*\)$") +_NATIVE_FRAME = re.compile( + r"^#\d+\s+(?:pc\s+[0-9a-fA-F]+\s+\S+|\.\.\.)\s+" + r"(?:\(offset 0x[0-9a-fA-F]+\)\s+)?\(([^()]*)\)" + r"(?:\s+\(BuildId:\s*[0-9a-fA-F]+\))?$" +) +_ABBREVIATED_JAVA = re.compile(rf"^#\d+\s+\.\.\.\s+({_JAVA_METHOD})$") +_RUST_FRAME = re.compile(r"^\d+:\s+(?:0x[0-9a-fA-F]+\s+-\s+)?(\S+::\S+)$") +_NATIVE_SYMBOL = re.compile(rf"{_IDENT}(?:::{_IDENT})*") +_RUST_HASH = re.compile(r"::h[0-9a-f]{16}$") +_OFFSET = re.compile(r"\+(?:0x[0-9a-fA-F]+|\d+)$") + + +def _native_symbol(value: str) -> str | None: + symbol = _RUST_HASH.sub("", _OFFSET.sub("", value.strip())) + return symbol if _NATIVE_SYMBOL.fullmatch(symbol) else None + + +def normalize_trace(text: str) -> dict[str, list[str]]: + """Extract ordered cause classes/signals and stack symbols, dropping prose. + + Supports Java/Kotlin frames, Android native frames, and simple symbolicated + Rust backtraces. Complex C++/Rust demangled symbols are deliberately omitted. + This reduces incidental data exposure; it is not a general PII redactor. + """ + causes: list[str] = [] + frames: list[str] = [] + for raw_line in text.splitlines(): + line = raw_line.strip() + logcat = _LOGCAT.fullmatch(line) + if logcat: + line = logcat.group(1).strip() + cause = _CAUSE.match(line) + if cause: + causes.append(cause.group(1)) + continue + signal = _SIGNAL.match(line) + if signal: + causes.append(signal.group(1) or signal.group(2)) + continue + java = _JAVA_FRAME.fullmatch(line) or _ABBREVIATED_JAVA.fullmatch(line) + if java: + frames.append(java.group(1)) + continue + native = _NATIVE_FRAME.fullmatch(line) or _RUST_FRAME.fullmatch(line) + if native: + symbol = _native_symbol(native.group(1)) + if symbol: + frames.append(symbol) + return {"causes": causes, "frames": frames} + + +def _normalize_location(location: str) -> str: + """Keep only a method, qualified native symbol, or shared-library basename.""" + location = location.strip() + java = re.fullmatch(rf"({_JAVA_METHOD})(?:\s*\([^()]*\))?", location) + if java: + return java.group(1) + if "::" in location or location.startswith("Java_"): + return _native_symbol(location) or "" + library = location.rsplit("/", 1)[-1] + if re.fullmatch(r"lib[A-Za-z0-9_-]+\.so", library): + return library + return "" + + +def fallback_fingerprint( + issue_type: str, cause: str, location: str, trace: str = "" +) -> str: + """Hash structural evidence when Play did not provide an issue ID. + + Changes to this normalization contract require a new version prefix. Missing + structure can collide; report consumers must label this key as heuristic. + """ + known_types = {"CRASH", "ANR", "NON_FATAL"} + issue_type = issue_type.strip().upper() + if issue_type == "APPLICATION_NOT_RESPONDING": + issue_type = "ANR" + payload = { + "type": issue_type if issue_type in known_types else "UNKNOWN", + "cause": normalize_trace(cause)["causes"], + "location": _normalize_location(location), + "trace": normalize_trace(trace), + } + encoded = json.dumps(payload, sort_keys=True, separators=(",", ":")).encode() + return "fallback:v1:" + hashlib.sha256(encoded).hexdigest() diff --git a/fixtures/crashes/README.md b/fixtures/crashes/README.md new file mode 100644 index 00000000..dc4d3729 --- /dev/null +++ b/fixtures/crashes/README.md @@ -0,0 +1,19 @@ +# Crash signature fixtures + +These are small structural excerpts, not production reports. Messages, device +details, process/thread IDs, timestamps, and addresses are replaced or omitted. +Only exception classes and code symbols relevant to extraction remain. + +| Fixture | Evidence | +| --- | --- | +| `managed-npe.txt` | [ActivityWatch/aw-android#185](https://github.com/ActivityWatch/aw-android/issues/185): ChromeWatcher managed NPE; source excerpt has no message, so a redaction placeholder was added to exercise message removal. | +| `nested-inflation.txt` | [ActivityWatch/aw-android#210](https://github.com/ActivityWatch/aw-android/issues/210): RuntimeException, nested InflateException, and UnsupportedOperationException. | +| `native-jni-abort.txt` | [ActivityWatch/aw-android#220](https://github.com/ActivityWatch/aw-android/issues/220): SIGABRT in the libaw_sync path with `syncBoth` JNI evidence; redundant unsymbolicated frames omitted. This report does **not** establish a Rust panic. | +| `rust-native-synthetic.txt` | Explicitly synthetic Rust backtrace, using illustrative `aw_sync` symbols; not a captured ActivityWatch failure or evidence of a particular bug. | + +Sources were read on 2026-09-08. The normalized fingerprint is a **heuristic +fallback** only. Play issue IDs remain primary. Google's +[ErrorReport reference](https://developers.google.com/play/developer/reporting/reference/rest/v1beta1/vitals.errors.reports#ErrorReport) +describes `reportText` as human-readable and warns that its format can change. +Unsupported and unsymbolicated lines are discarded rather than preserved as raw +text. Complex demangled C++/Rust symbols are outside this extractor's scope. diff --git a/fixtures/crashes/managed-npe.txt b/fixtures/crashes/managed-npe.txt new file mode 100644 index 00000000..dfb0246a --- /dev/null +++ b/fixtures/crashes/managed-npe.txt @@ -0,0 +1,4 @@ +Exception java.lang.NullPointerException: [redacted message] + at net.activitywatch.android.watcher.ChromeWatcher.onAccessibilityEvent (ChromeWatcher.kt:76) + at android.accessibilityservice.AccessibilityService$2.onAccessibilityEvent (AccessibilityService.java:2744) + ... diff --git a/fixtures/crashes/native-jni-abort.txt b/fixtures/crashes/native-jni-abort.txt new file mode 100644 index 00000000..76d778af --- /dev/null +++ b/fixtures/crashes/native-jni-abort.txt @@ -0,0 +1,8 @@ +00:00:00.000 I SyncInterface: [redacted operation] +00:00:00.001 F DEBUG : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr -------- +00:00:00.001 F DEBUG : pid: 12345, tid: 12346, name: [redacted thread] +00:00:00.001 F DEBUG : #00 pc 0000000000000010 libc.so (abort+156) +00:00:00.001 F DEBUG : #01 pc 0000000000000020 base.apk (offset 0x1000) +00:00:00.001 F DEBUG : #15 pc 0000000000000030 base.apk (Java_net_activitywatch_android_SyncInterface_syncBoth+260) +00:00:00.001 F DEBUG : #25 ... SyncInterface$syncBothAsync$2.invoke +00:00:00.001 F DEBUG : #33 ... SyncInterface.performSyncAsync$lambda$4 diff --git a/fixtures/crashes/nested-inflation.txt b/fixtures/crashes/nested-inflation.txt new file mode 100644 index 00000000..7c82e4ce --- /dev/null +++ b/fixtures/crashes/nested-inflation.txt @@ -0,0 +1,12 @@ +FATAL EXCEPTION: main +Process: net.activitywatch.android, PID: 12345 +java.lang.RuntimeException: [redacted activity and message] + android.view.InflateException: [redacted resource and message] + at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:4164) + at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:4322) + ... +Caused by: java.lang.UnsupportedOperationException: [redacted attribute and theme] + at android.content.res.TypedArray.getColorStateList(TypedArray.java:600) + at android.widget.TextView.readTextAppearance(TextView.java:4535) + at android.widget.TextView.(TextView.java:1468) + at androidx.appcompat.widget.AppCompatTextView.(AppCompatTextView.java:115) diff --git a/fixtures/crashes/rust-native-synthetic.txt b/fixtures/crashes/rust-native-synthetic.txt new file mode 100644 index 00000000..04c870b1 --- /dev/null +++ b/fixtures/crashes/rust-native-synthetic.txt @@ -0,0 +1,8 @@ +thread '[redacted]' panicked at /private/example.rs:42:5: +[synthetic message containing private path /data/user/secret/database.sqlite] +stack backtrace: + 0: std::panicking::begin_panic::h0123456789abcdef + at /rustc/0123456789abcdef/library/std/src/panicking.rs:10:5 + 1: 0x0000000000001234 - aw_sync::sync::sync_run::hfedcba9876543210 + at /private/checkout/aw-sync/src/sync.rs:123:4 + 2: aw_sync::jni::sync_both::h0123456789abcdef diff --git a/test_crash_reports.py b/test_crash_reports.py new file mode 100644 index 00000000..86b37548 --- /dev/null +++ b/test_crash_reports.py @@ -0,0 +1,221 @@ +"""Offline API/CLI/report contracts; response identifiers and messages are synthetic.""" +from __future__ import annotations + +import copy +import json + +import pytest +import requests +from click.testing import CliRunner + +import crash_reports +import vitals + +QUERY = {"days": 7, "type": "all", "limit": 100, + "start_time": "2026-09-01T04:00:00Z", "end_time": "2026-09-08T04:00:00Z"} +ISSUE = {"name": "apps/net.activitywatch.android/123", "type": "CRASH", + "cause": "java.lang.NullPointerException", "location": "ChromeWatcher.onAccessibilityEvent", + "errorReportCount": "791", "distinctUsers": "99", + "issueUri": "https://play.google.com/console/issue/123", + "lastErrorReportTime": "2026-09-08T01:00:00Z"} +REPORT = {"name": "apps/net.activitywatch.android/sample123", "issue": ISSUE["name"], + "type": "CRASH", "eventTime": "2026-09-08T01:00:00Z", + "appVersion": {"versionCode": "27", "versionName": "0.12.1"}, + "deviceModel": {"deviceId": {"buildDevice": "PRIVATE_DEVICE"}}, + "vcsInformation": "PRIVATE_BUILD_PATH", + "reportText": "java.lang.NullPointerException: PRIVATE_MESSAGE\n" + " at net.activitywatch.android.ChromeWatcher.onAccessibilityEvent(ChromeWatcher.kt:76)"} + + +def snapshot(issues=None, query=None): + return crash_reports.build_snapshot( + "net.activitywatch.android", query or QUERY, issues if issues is not None else [ISSUE], + {ISSUE["name"]: REPORT}, "2026-09-08T04:20:00Z", False) + + +class Response: + def __init__(self, payload, status=200): + self.payload, self.status = payload, status + + def json(self): + return self.payload + + def raise_for_status(self): + if self.status != 200: + raise requests.HTTPError(f"HTTP {self.status}") + + +def test_snapshot_keeps_play_identity_and_only_allowlisted_sample_fields(monkeypatch): + def forbidden(*args): + pytest.fail("Play issue identity must never be replaced by a local hash") + monkeypatch.setattr(crash_reports, "fallback_fingerprint", forbidden) + result = snapshot() + row = result["clusters"][0] + assert row["identity"] == ISSUE["name"] + assert row["fingerprint"] is None + assert row["report_count"] == 791 + assert row["sample"] == {"report_id": REPORT["name"], "event_time": REPORT["eventTime"], + "version_code": "27", "version_name": "0.12.1"} + payload = crash_reports.json_text(result) + assert "PRIVATE" not in payload + assert "reportText" not in payload + assert "distinctUsers" not in payload + assert row["severity"] == "unclassified" + + +def test_snapshot_order_and_fallback_are_deterministic(): + second = dict(ISSUE, name="apps/net.activitywatch.android/222", errorReportCount="999") + assert crash_reports.json_text(snapshot([ISSUE, second])) == crash_reports.json_text( + snapshot([second, ISSUE])) + imported = {k: v for k, v in ISSUE.items() if k != "name"} + row = snapshot([imported])["clusters"][0] + assert row["identity_source"] == "heuristic_fingerprint" + assert row["identity"] == row["fingerprint"] + assert row["identity"] == snapshot([imported])["clusters"][0]["identity"] + + +def test_unknown_count_is_not_zero(): + missing = {k: v for k, v in ISSUE.items() if k != "errorReportCount"} + result = snapshot([missing]) + assert result["clusters"][0]["report_count"] is None + assert "unknown" in crash_reports.render_markdown(result) + + +def test_comparison_tracks_movement_without_declaring_missing_clusters_fixed(): + old = snapshot([ISSUE, dict(ISSUE, name="apps/net.activitywatch.android/gone")]) + new = snapshot([dict(ISSUE, errorReportCount="7"), + dict(ISSUE, name="apps/net.activitywatch.android/new")]) + comparison = crash_reports.compare_snapshots(new, old) + assert comparison["report_count_deltas"][ISSUE["name"]] == -784 + assert comparison["not_observed"] == ["apps/net.activitywatch.android/gone"] + assert comparison["newly_observed"] == ["apps/net.activitywatch.android/new"] + new["comparison"] = comparison + rendered = crash_reports.render_markdown(new) + assert "-784" in rendered + assert "not proven fixed" in rendered + assert "not all affected versions" in rendered + + +@pytest.mark.parametrize("field,value", [("days", 1), ("type", "anr"), ("limit", 10)]) +def test_different_query_is_not_a_count_trend(field, value): + result = crash_reports.compare_snapshots(snapshot(query={**QUERY, field: value}), snapshot()) + assert result["status"] == "incompatible_query" + assert "report_count_deltas" not in result + + +def test_persistence_retains_changed_same_time_snapshots_and_dispositions(tmp_path): + old = snapshot() + first = crash_reports.persist_snapshot(tmp_path, old) + original = first.read_bytes() + assert crash_reports.persist_snapshot(tmp_path, old) == first + assert len(list((tmp_path / "history").glob("*.json"))) == 1 + decision = {ISSUE["name"]: {"status": "investigating", "issue_url": "https://github.com/ActivityWatch/aw-android/issues/185"}} + disposition_file = tmp_path / "dispositions.json" + disposition_file.write_text(json.dumps(decision)) + newer = copy.deepcopy(old) + newer["clusters"][0]["report_count"] = 5 + newer["comparison"] = crash_reports.compare_snapshots(newer, old) + second = crash_reports.persist_snapshot(tmp_path, newer) + assert first != second + assert first.read_bytes() == original + assert json.loads((tmp_path / "current.json").read_text()) == newer + assert json.loads(disposition_file.read_text()) == decision + assert "-786" in (tmp_path / "current.md").read_text() + + +def test_fetch_paginates_and_records_top_n_truncation(monkeypatch): + monkeypatch.setattr(vitals, "_access_token", lambda _: "test-token") + calls = [] + responses = iter([ + {"errorIssues": [ISSUE], "nextPageToken": "page2"}, + {"errorIssues": [dict(ISSUE, name="second"), dict(ISSUE, name="third")]}, + ]) + def get(url, **kwargs): + calls.append(copy.deepcopy(kwargs["params"])) + return Response(next(responses)) + monkeypatch.setattr(vitals.requests, "get", get) + rows, _, truncated = vitals.fetch_error_clusters("pkg", "all", 2, None, QUERY) + assert len(rows) == 2 and truncated + assert calls[0]["orderBy"] == "errorReportCount desc" + assert calls[1]["pageToken"] == "page2" + assert calls[0]["interval.startTime.day"] == 1 + assert calls[0]["interval.endTime.hours"] == 4 + assert calls[0]["interval.endTime.timeZone.id"] == "UTC" + assert "filter" not in calls[0] + + +def test_corrupt_existing_archive_is_reported_not_silently_reused(tmp_path): + result = snapshot() + archive = crash_reports.persist_snapshot(tmp_path, result) + archive.write_text('{"schema_') # An interrupted writer from an older version. + with pytest.raises(ValueError, match="Existing snapshot differs"): + crash_reports.persist_snapshot(tmp_path, result) + assert archive.read_text() == '{"schema_' # Never overwrite historical evidence. + assert not list((tmp_path / "history").glob("*.tmp")) + + +def test_sample_uses_same_query_window(monkeypatch): + def get(url, **kwargs): + assert kwargs["params"]["filter"] == "errorIssueId = 123" + assert kwargs["params"]["interval.startTime.day"] == 1 + return Response({"errorReports": [REPORT]}) + monkeypatch.setattr(vitals.requests, "get", get) + assert vitals.fetch_sample_report("pkg", "123", "token", QUERY) == REPORT + + +def test_cli_json_empty_is_parseable_and_human_default_is_preserved(monkeypatch): + runner = CliRunner() + monkeypatch.setattr(vitals, "fetch_error_clusters", lambda *args: ([], "token", False)) + result = runner.invoke(vitals.cli, ["errors", "--json"]) + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["clusters"] == [] + assert runner.invoke(vitals.cli, ["errors"]).stdout == "No error issues found.\n" + monkeypatch.setattr(vitals, "fetch_error_clusters", lambda *args: ([ISSUE], "token", False)) + result = runner.invoke(vitals.cli, ["errors"]) + assert result.stdout == ("1. [CRASH] 791 reports ChromeWatcher.onAccessibilityEvent\n" + " java.lang.NullPointerException\n\n") + + +def test_cli_missing_play_id_uses_fallback_even_when_samples_requested(monkeypatch): + issue = {k: v for k, v in ISSUE.items() if k != "name"} + monkeypatch.setattr(vitals, "fetch_error_clusters", lambda *args: ([issue], "token", False)) + def forbidden(*args): + pytest.fail("Cannot request a sample without a Play issue ID") + monkeypatch.setattr(vitals, "fetch_sample_report", forbidden) + result = CliRunner().invoke(vitals.cli, ["errors", "--json", "--stacktraces"]) + assert result.exit_code == 0, result.output + row = json.loads(result.stdout)["clusters"][0] + assert row["identity_source"] == "heuristic_fingerprint" + assert row["sample"] is None + + +def test_cli_consumer_preserves_history_and_decisions_on_refresh_and_failure(monkeypatch, tmp_path): + monkeypatch.setattr(vitals, "fetch_error_clusters", lambda *args: ([ISSUE], "token", False)) + monkeypatch.setattr(vitals, "fetch_sample_report", lambda *args: REPORT) + (tmp_path / "dispositions.json").write_text(json.dumps({ISSUE["name"]: {"status": "linked"}})) + runner = CliRunner() + command = ["errors", "--json", "--stacktraces", "--update-dir", str(tmp_path)] + first = runner.invoke(vitals.cli, command) + assert first.exit_code == 0, first.output + assert json.loads(first.stdout)["clusters"][0]["disposition"] == {"status": "linked"} + second = runner.invoke(vitals.cli, command) + assert second.exit_code == 0, second.output + assert json.loads(second.stdout)["comparison"]["report_count_deltas"][ISSUE["name"]] == 0 + baseline = (tmp_path / "current.json").read_bytes() + archives = list((tmp_path / "history").glob("*.json")) + def fail(*args): + raise requests.HTTPError("HTTP 403") + monkeypatch.setattr(vitals, "fetch_sample_report", fail) + failed = runner.invoke(vitals.cli, command) + assert failed.exit_code != 0 and "403" in failed.output + assert (tmp_path / "current.json").read_bytes() == baseline + assert list((tmp_path / "history").glob("*.json")) == archives + + +def test_cli_rejects_unsupported_baseline_without_overwriting(tmp_path, monkeypatch): + (tmp_path / "current.json").write_text('{"schema_version": 999}') + monkeypatch.setattr(vitals, "fetch_error_clusters", lambda *args: ([], "token", False)) + result = CliRunner().invoke(vitals.cli, ["errors", "--update-dir", str(tmp_path)]) + assert result.exit_code != 0 + assert "Unsupported previous snapshot" in result.output + assert not (tmp_path / "history").exists() diff --git a/test_crash_signatures.py b/test_crash_signatures.py new file mode 100644 index 00000000..67f38d6b --- /dev/null +++ b/test_crash_signatures.py @@ -0,0 +1,160 @@ +"""Offline structural/noise regression tests; fixtures contain no live reports.""" +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from crash_signatures import fallback_fingerprint, normalize_trace + + +FIXTURES = Path(__file__).parent / "fixtures" / "crashes" + + +def trace(name: str) -> str: + return (FIXTURES / f"{name}.txt").read_text() + + +def fingerprint(text: str) -> str: + return fallback_fingerprint("CRASH", "", "", text) + + +def test_managed_npe_retains_discriminating_app_frame(): + assert normalize_trace(trace("managed-npe")) == { + "causes": ["java.lang.NullPointerException"], + "frames": [ + "net.activitywatch.android.watcher.ChromeWatcher.onAccessibilityEvent", + "android.accessibilityservice.AccessibilityService$2.onAccessibilityEvent", + ], + } + + +def test_v1_fingerprint_contract_is_stable(): + # An intentional normalization change must bump the version, not silently + # reinterpret historical reports under the existing fingerprint namespace. + assert fingerprint(trace("managed-npe")) == ( + "fallback:v1:96a2fada8fc9d092cfcfc96cef2e37bc5bfb6fc383afb655d52632f0f58f234e" + ) + + +def test_nested_inflation_retains_cause_order_and_constructor(): + normalized = normalize_trace(trace("nested-inflation")) + assert normalized["causes"] == [ + "java.lang.RuntimeException", + "android.view.InflateException", + "java.lang.UnsupportedOperationException", + ] + assert "android.widget.TextView." in normalized["frames"] + reversed_causes = trace("nested-inflation").replace( + "java.lang.RuntimeException", "java.lang.UnsupportedOperationException" + ).replace("Caused by: java.lang.UnsupportedOperationException", "Caused by: java.lang.RuntimeException") + assert fingerprint(reversed_causes) != fingerprint(trace("nested-inflation")) + + +def test_native_abort_retains_signal_and_jni_not_unsymbolicated_addresses(): + assert normalize_trace(trace("native-jni-abort")) == { + "causes": ["SIGABRT"], + "frames": [ + "abort", + "Java_net_activitywatch_android_SyncInterface_syncBoth", + "SyncInterface$syncBothAsync$2.invoke", + "SyncInterface.performSyncAsync$lambda$4", + ], + } + + +def test_synthetic_rust_preserves_functions_not_build_hashes(): + assert normalize_trace(trace("rust-native-synthetic")) == { + "causes": [], + "frames": [ + "std::panicking::begin_panic", + "aw_sync::sync::sync_run", + "aw_sync::jni::sync_both", + ], + } + + +@pytest.mark.parametrize("fixture", [ + "managed-npe", "nested-inflation", "native-jni-abort", "rust-native-synthetic" +]) +def test_incidental_noise_does_not_split_family(fixture): + original = trace(fixture) + changed = original.replace("[redacted", "[another-secret").replace("12345", "98989") + changed = changed.replace("00:00:00.001", "23:59:59.999").replace(":76)", ":999)") + changed = changed.replace("+260", "+0x100").replace("0000000000000030", "0000000098765432") + changed = changed.replace("0123456789abcdef", "ffffffffffffffff") + changed = changed.replace("/private/checkout", "/data/user/another-private-path") + changed += "\nDevice: private-device\nversionCode: 99\nBuildId: deadbeef\n" + assert fingerprint(changed) == fingerprint(original) + + +@pytest.mark.parametrize(("fixture", "before", "after"), [ + ("managed-npe", "NullPointerException", "IllegalStateException"), + ("managed-npe", "ChromeWatcher.onAccessibilityEvent", "ChromeWatcher.onInterrupt"), + ("native-jni-abort", "SIGABRT", "SIGSEGV"), + ("native-jni-abort", "syncBoth+260", "syncPush+260"), + ("rust-native-synthetic", "aw_sync::sync::sync_run", "aw_sync::sync::sync_push"), +]) +def test_structural_changes_split_family(fixture, before, after): + original = trace(fixture) + assert fingerprint(original.replace(before, after)) != fingerprint(original) + + +def test_messages_and_malformed_lines_are_not_retained(): + private = "private@example.test /data/user/secret token=secret-value" + text = f'''Exception in thread "{private}" java.lang.NullPointerException: {private} + at net.activitywatch.android.MainActivity.onCreate (/private/project/MainActivity.kt:19) + caused by arbitrary prose {private} + at {private} + #15 pc 0123 /private/libaw_sync.so ({private}) + #00 pc 4567 /private/base.apk (offset 0x1234) + 1: {private} + Process: {private} + https://{private} + ''' + assert normalize_trace(text) == { + "causes": ["java.lang.NullPointerException"], + "frames": ["net.activitywatch.android.MainActivity.onCreate"], + } + assert "private" not in json.dumps(normalize_trace(text)) + + +def test_native_frame_ignores_path_build_id_and_offset(): + first = "#00 pc 1234 /data/user/private/libaw_sync.so (aw_sync::sync::run+16) (BuildId: abcd)" + second = "#00 pc abcdef /another/path/libaw_sync.so (aw_sync::sync::run+0x12) (BuildId: fedc)" + assert normalize_trace(first) == {"causes": [], "frames": ["aw_sync::sync::run"]} + assert fingerprint(first) == fingerprint(second) + + +def test_full_logcat_prefix_and_fatal_signal(): + text = "09-08 12:34:56.789 12345 12346 F libc : Fatal signal 6 (SIGABRT) in tid 12346 (private)" + assert normalize_trace(text) == {"causes": ["SIGABRT"], "frames": []} + + +def test_fallback_uses_normalized_metadata_and_versioned_digest(): + first = fallback_fingerprint( + "CRASH", "java.lang.NullPointerException: secret-one", + "ChromeWatcher.onAccessibilityEvent (ChromeWatcher.kt:76)" + ) + second = fallback_fingerprint( + " crash ", "java.lang.NullPointerException: secret-two", + "ChromeWatcher.onAccessibilityEvent (/private/ChromeWatcher.kt:99)" + ) + assert first == second + assert first.startswith("fallback:v1:") + assert len(first.removeprefix("fallback:v1:")) == 64 + assert fallback_fingerprint("ANR", "java.lang.NullPointerException", "") != first + + +def test_api_anr_type_matches_filter_alias_and_does_not_become_unknown(): + api_type = fallback_fingerprint("APPLICATION_NOT_RESPONDING", "", "") + assert api_type == fallback_fingerprint("ANR", "", "") + assert api_type != fallback_fingerprint("UNKNOWN", "", "") + + +def test_empty_or_unsupported_reports_have_explicitly_heuristic_collision(): + assert normalize_trace("not a stack trace") == {"causes": [], "frames": []} + assert fallback_fingerprint("CRASH", "secret-one", "path/one") == fallback_fingerprint( + "CRASH", "secret-two", "path/two" + ) diff --git a/vitals.py b/vitals.py index 63d5634e..0b914bf1 100644 --- a/vitals.py +++ b/vitals.py @@ -35,12 +35,17 @@ from __future__ import annotations import csv +import json import os -from datetime import date, timedelta +from datetime import date, datetime, timedelta, timezone +from pathlib import Path import click import requests +from crash_reports import (build_snapshot, compare_snapshots, json_text, + persist_snapshot, render_markdown) + SCOPE = "https://www.googleapis.com/auth/playdeveloperreporting" BASE = "https://playdeveloperreporting.googleapis.com/v1beta1" DEFAULT_PACKAGE = "net.activitywatch.android" @@ -251,25 +256,54 @@ def anr_rate(package, credentials, days, metric, as_csv, update_path, dry_run): def fetch_error_issues(package, issue_type, limit, credentials): """Top error clusters (crash/ANR) with cause, location, and report count.""" + issues, token, _ = fetch_error_clusters(package, issue_type, limit, credentials) + return issues, token + + +def _interval_params(query): + params = {} + if query: + for key, name in (("start_time", "startTime"), ("end_time", "endTime")): + dt = datetime.fromisoformat(query[key].replace("Z", "+00:00")) + for field, value in (("year", dt.year), ("month", dt.month), + ("day", dt.day), ("hours", dt.hour)): + params[f"interval.{name}.{field}"] = value + params[f"interval.{name}.timeZone.id"] = "UTC" + return params + + +def fetch_error_clusters(package, issue_type, limit, credentials, query=None): token = _access_token(credentials) base = f"{BASE}/apps/{package}" - params = {"pageSize": limit} + params = {"pageSize": limit, "orderBy": "errorReportCount desc", **_interval_params(query)} if issue_type and issue_type != "all": params["filter"] = f"errorIssueType = {issue_type.upper()}" - r = requests.get(f"{base}/errorIssues:search", - headers={"Authorization": f"Bearer {token}"}, params=params, timeout=30) - r.raise_for_status() - return r.json().get("errorIssues", []), token + issues = [] + while True: + r = requests.get(f"{base}/errorIssues:search", + headers={"Authorization": f"Bearer {token}"}, params=params, timeout=30) + r.raise_for_status() + response = r.json() + issues.extend(response.get("errorIssues", [])) + next_page = response.get("nextPageToken") + if len(issues) >= limit or not next_page: + return issues[:limit], token, bool(next_page or len(issues) > limit) + params["pageToken"] = next_page def fetch_sample_stacktrace(package, issue_id, token): + return fetch_sample_report(package, issue_id, token).get("reportText", "") + + +def fetch_sample_report(package, issue_id, token, query=None): base = f"{BASE}/apps/{package}" r = requests.get(f"{base}/errorReports:search", headers={"Authorization": f"Bearer {token}"}, - params={"pageSize": 1, "filter": f"errorIssueId = {issue_id}"}, timeout=30) + params={"pageSize": 1, "filter": f"errorIssueId = {issue_id}", + **_interval_params(query)}, timeout=30) r.raise_for_status() reports = r.json().get("errorReports", []) - return reports[0].get("reportText", "") if reports else "" + return reports[0] if reports else {} @cli.command("errors") @@ -277,10 +311,62 @@ def fetch_sample_stacktrace(package, issue_id, token): @click.option("--credentials", envvar="GOOGLE_APPLICATION_CREDENTIALS") @click.option("--type", "issue_type", default="crash", type=click.Choice(["crash", "anr", "all"]), show_default=True) -@click.option("--limit", default=10, show_default=True) +@click.option("--limit", default=10, type=click.IntRange(1, 1000), show_default=True) @click.option("--stacktraces", is_flag=True, help="Include a sample stacktrace per cluster.") -def errors(package, credentials, issue_type, limit, stacktraces): +@click.option("--json", "as_json", is_flag=True, help="Emit a versioned, sanitized JSON snapshot.") +@click.option("--markdown", is_flag=True, help="Emit a ranked Markdown report.") +@click.option("--days", default=1, type=click.IntRange(1, 30), show_default=True, + help="UTC window for structured reports.") +@click.option("--update-dir", type=click.Path(path_type=Path, file_okay=False), + help="Persist current JSON/Markdown and immutable snapshot history.") +@click.option("--previous", type=click.Path(path_type=Path, exists=True, dir_okay=False), + help="Compare with this snapshot (otherwise update-dir/current.json).") +@click.option("--dispositions", type=click.Path(path_type=Path, exists=True, dir_okay=False), + help="Identity-keyed status/issue_url/note JSON (otherwise update-dir/dispositions.json).") +def errors(package, credentials, issue_type, limit, stacktraces, as_json, markdown, + days, update_dir, previous, dispositions): """Top crash/ANR clusters (cause, location, report count) from Android vitals.""" + if as_json and markdown: + raise click.UsageError("Choose --json or --markdown, not both.") + if previous or dispositions: + if not (as_json or markdown or update_dir): + raise click.UsageError("--previous/--dispositions require structured output.") + if as_json or markdown or update_dir: + now = datetime.now(timezone.utc) + end = now.replace(minute=0, second=0, microsecond=0) + query = {"days": days, "type": issue_type, "limit": limit, + "start_time": (end - timedelta(days=days)).isoformat().replace("+00:00", "Z"), + "end_time": end.isoformat().replace("+00:00", "Z")} + if update_dir: + previous = previous or (update_dir / "current.json") + dispositions = dispositions or (update_dir / "dispositions.json") + try: + old = json.loads(previous.read_text()) if previous and previous.exists() else None + decisions = (json.loads(dispositions.read_text()) + if dispositions and dispositions.exists() else {}) + if not isinstance(decisions, dict) or any( + not isinstance(row, dict) or set(row) - {"status", "issue_url", "note"} + or any(not isinstance(value, str) for value in row.values()) + for row in decisions.values()): + raise ValueError("Dispositions must map identities to status/issue_url/note strings") + issues, token, truncated = fetch_error_clusters( + package, issue_type, limit, credentials, query) + samples = {issue["name"]: fetch_sample_report( + package, issue["name"].split("/")[-1], token, query) + for issue in issues if issue.get("name")} if stacktraces else {} + snapshot = build_snapshot(package, query, issues, samples, + now.isoformat().replace("+00:00", "Z"), truncated, decisions) + snapshot["comparison"] = compare_snapshots(snapshot, old) + if update_dir: + archive = persist_snapshot(update_dir, snapshot) + click.echo(f"Saved {archive}", err=True) + except (ValueError, OSError, requests.RequestException) as exc: + raise click.ClickException(str(exc)) from exc + if as_json: + click.echo(json_text(snapshot), nl=False) + elif markdown or not update_dir: + click.echo(render_markdown(snapshot), nl=False) + return issues, token = fetch_error_issues(package, issue_type, limit, credentials) if not issues: click.echo("No error issues found.") From bde73ae17b49eb53ca5973225f9f683685774dad Mon Sep 17 00:00:00 2001 From: Bob Date: Tue, 8 Sep 2026 05:15:57 +0000 Subject: [PATCH 2/2] fix(vitals): publish current error reports as one transaction If current.md failed after current.json was replaced, the two live reports could describe different snapshots. Stage both files, replace them in order, and restore the previous pair (or delete a first-time partial) on failure. --- crash_reports.py | 46 +++++++++++++++++++++++++++++++++++++++---- test_crash_reports.py | 43 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/crash_reports.py b/crash_reports.py index 0b61964a..7a736d09 100644 --- a/crash_reports.py +++ b/crash_reports.py @@ -153,8 +153,46 @@ def persist_snapshot(directory: Path, snapshot: dict) -> Path: raise ValueError(f"Existing snapshot differs from its content hash: {archive}") finally: temporary_archive.unlink(missing_ok=True) - for name, text in (("current.json", payload), ("current.md", render_markdown(snapshot))): - temporary = directory / f".{name}.{digest}.tmp" - temporary.write_text(text, encoding="utf-8") - temporary.replace(directory / name) + _publish_current_reports(directory, digest, ( + ("current.json", payload), + ("current.md", render_markdown(snapshot)), + )) return archive + + +def _replace(source: Path, destination: Path) -> None: + source.replace(destination) + + +def _publish_current_reports(directory: Path, digest: str, + reports: tuple[tuple[str, str], ...]) -> None: + """Replace current.json and current.md together; roll back a partial publish.""" + published: list[tuple[Path, Path | None]] = [] + leftovers: list[Path] = [] + try: + for name, text in reports: + destination = directory / name + temporary = directory / f".{name}.{digest}.tmp" + leftovers.append(temporary) + temporary.write_text(text, encoding="utf-8") + backup = None + if destination.exists(): + backup = directory / f".{name}.{digest}.bak" + leftovers.append(backup) + backup.unlink(missing_ok=True) + try: + os.link(destination, backup) + except OSError: + backup.write_bytes(destination.read_bytes()) + _replace(temporary, destination) + published.append((destination, backup)) + except Exception: + for destination, backup in reversed(published): + if backup is not None: + _replace(backup, destination) + else: + destination.unlink(missing_ok=True) + raise + finally: + for path in leftovers: + path.unlink(missing_ok=True) diff --git a/test_crash_reports.py b/test_crash_reports.py index 86b37548..2afb7002 100644 --- a/test_crash_reports.py +++ b/test_crash_reports.py @@ -123,6 +123,49 @@ def test_persistence_retains_changed_same_time_snapshots_and_dispositions(tmp_pa assert "-786" in (tmp_path / "current.md").read_text() +def test_current_reports_roll_back_together_if_markdown_replace_fails(tmp_path, monkeypatch): + old = snapshot() + crash_reports.persist_snapshot(tmp_path, old) + previous_json = (tmp_path / "current.json").read_text() + previous_md = (tmp_path / "current.md").read_text() + newer = copy.deepcopy(old) + newer["clusters"][0]["report_count"] = 5 + original = crash_reports._replace + + def fail_markdown(source, destination): + if destination.name == "current.md": + raise OSError("simulated publish failure") + original(source, destination) + + monkeypatch.setattr(crash_reports, "_replace", fail_markdown) + with pytest.raises(OSError, match="simulated publish failure"): + crash_reports.persist_snapshot(tmp_path, newer) + assert (tmp_path / "current.json").read_text() == previous_json + assert (tmp_path / "current.md").read_text() == previous_md + assert not [path for path in tmp_path.iterdir() if path.name.startswith(".")] + monkeypatch.setattr(crash_reports, "_replace", original) + newer["comparison"] = crash_reports.compare_snapshots(newer, old) + crash_reports.persist_snapshot(tmp_path, newer) + assert json.loads((tmp_path / "current.json").read_text()) == newer + assert "-786" in (tmp_path / "current.md").read_text() + + +def test_partial_first_publish_does_not_leave_json_without_markdown(tmp_path, monkeypatch): + original = crash_reports._replace + + def fail_markdown(source, destination): + if destination.name == "current.md": + raise OSError("simulated publish failure") + original(source, destination) + + monkeypatch.setattr(crash_reports, "_replace", fail_markdown) + with pytest.raises(OSError, match="simulated publish failure"): + crash_reports.persist_snapshot(tmp_path, snapshot()) + assert not (tmp_path / "current.json").exists() + assert not (tmp_path / "current.md").exists() + assert not [path for path in tmp_path.iterdir() if path.name.startswith(".")] + + def test_fetch_paginates_and_records_top_n_truncation(monkeypatch): monkeypatch.setattr(vitals, "_access_token", lambda _: "test-token") calls = []