refactor(diagnostics): remove bug report breadcrumbs

This commit is contained in:
2026-08-02 19:18:18 +02:00
parent 7cb2270d56
commit 877083a3ed
12 changed files with 14 additions and 242 deletions

View File

@@ -8,14 +8,6 @@ export type InputFailure = {
export type InputResult<T> = { ok: true; value: T } | InputFailure;
export type NormalizedProperties = Record<string, string>;
export interface NormalizedBreadcrumb {
name: string;
timestampMillis: number;
properties: NormalizedProperties;
}
export interface NormalizedDevice {
deviceName: string;
deviceModel: string;
@@ -36,16 +28,12 @@ export interface NormalizedBugPayload {
contact: string;
logs: string;
device: NormalizedDevice;
breadcrumbs: NormalizedBreadcrumb[];
schemaVersion: 1;
}
export const MAX_LOG_BYTES = 192 * 1024;
export const MAX_BREADCRUMBS_JSON_BYTES = 16_000;
export const MAX_DEVICE_JSON_BYTES = 4_000;
const MAX_PROPERTIES = 12;
const MAX_BREADCRUMBS = 40;
const MISSING = Symbol("missing");
const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
const UTF8_ENCODER = new TextEncoder();
@@ -177,8 +165,6 @@ export function normalizeBug(body: JsonObject): InputResult<NormalizedBugPayload
if (!logs.ok) return logs;
const device = normalizeDevice(pick(body, ["device"]));
if (!device.ok) return device;
const breadcrumbs = normalizeBreadcrumbs(pick(body, ["breadcrumbs"]));
if (!breadcrumbs.ok) return breadcrumbs;
const version = schemaVersion(body);
if (!version.ok) return version;
@@ -194,62 +180,10 @@ export function normalizeBug(body: JsonObject): InputResult<NormalizedBugPayload
contact: contact.value,
logs: includeLogs.value === true ? logs.value : "",
device: device.value,
breadcrumbs: breadcrumbs.value,
schemaVersion: version.value,
});
}
function normalizeBreadcrumbs(raw: unknown | typeof MISSING): InputResult<NormalizedBreadcrumb[]> {
if (raw === MISSING) return success([]);
if (!Array.isArray(raw)) return failure(400, "invalid_breadcrumbs");
const breadcrumbs: NormalizedBreadcrumb[] = [];
for (const item of raw.slice(0, MAX_BREADCRUMBS)) {
if (!isPlainObject(item)) return failure(400, "invalid_breadcrumbs");
const name = stringField(item, ["name"], 64, "invalid_breadcrumbs", true, true);
if (!name.ok) return name;
const timestamp = timestampField(item, ["timestampMillis", "timestamp_millis", "ts"]);
if (!timestamp.ok) return failure(400, "invalid_breadcrumbs");
const properties = normalizeProperties(
pick(item, ["properties", "props"]),
"invalid_breadcrumbs",
);
if (!properties.ok) return properties;
breadcrumbs.push({
name: name.value,
timestampMillis: timestamp.value,
properties: properties.value,
});
if (jsonBytes(breadcrumbs) > MAX_BREADCRUMBS_JSON_BYTES) {
breadcrumbs.pop();
break;
}
}
return success(breadcrumbs);
}
function normalizeProperties(
raw: unknown | typeof MISSING,
error: string,
): InputResult<NormalizedProperties> {
if (raw === MISSING) return success({});
if (!isPlainObject(raw)) return failure(400, error);
const entries: Array<[string, string]> = [];
const normalizedKeys = new Set<string>();
for (const [key, value] of Object.entries(raw).slice(0, MAX_PROPERTIES)) {
if (typeof value !== "string") return failure(400, error);
const normalizedKey = truncateUtf8(key, 40);
if (normalizedKey.length === 0 || normalizedKeys.has(normalizedKey)) {
return failure(400, error);
}
normalizedKeys.add(normalizedKey);
entries.push([normalizedKey, truncateUtf8(value, 128)]);
}
return success(Object.fromEntries(entries));
}
function normalizeDevice(raw: unknown | typeof MISSING): InputResult<NormalizedDevice> {
if (raw === MISSING) raw = {};
if (!isPlainObject(raw)) return failure(400, "invalid_device");

View File

@@ -35,8 +35,8 @@ export async function storeBug(
`INSERT INTO bugs (
id, received_at, occurred_at, install_id, app_version, platform,
what_happened, expected, steps, contact, logs_r2_key,
device_json, breadcrumbs_json, status, schema_version
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'open', ?)
device_json, status, schema_version
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'open', ?)
ON CONFLICT(id) DO NOTHING`,
)
.bind(
@@ -52,7 +52,6 @@ export async function storeBug(
payload.contact,
logsKey,
JSON.stringify(payload.device),
JSON.stringify(payload.breadcrumbs),
payload.schemaVersion,
)
.run();

View File

@@ -1,6 +1,5 @@
import { describe, expect, it } from "vitest";
import {
MAX_BREADCRUMBS_JSON_BYTES,
MAX_DEVICE_JSON_BYTES,
MAX_LOG_BYTES,
normalizeBug,
@@ -119,20 +118,11 @@ describe("normalizers", () => {
});
});
it("keeps logs, breadcrumbs, and device JSON within valid byte budgets", () => {
const properties = Object.fromEntries(
Array.from({ length: 12 }, (_, index) => [`key-${index}-${"\u0000".repeat(40)}`, "\u0000".repeat(128)]),
);
const breadcrumbs = Array.from({ length: 40 }, (_, index) => ({
name: `crumb-${index}`,
timestamp_millis: index,
properties,
}));
it("keeps logs and device JSON within valid byte budgets", () => {
const result = normalizeBug(
bugPayload({
include_logs: true,
logs: "😀".repeat(60_000),
breadcrumbs,
device: {
device_name: "\u0000".repeat(200),
device_model: "\u0000".repeat(200),
@@ -145,14 +135,9 @@ describe("normalizers", () => {
expect(result.ok).toBe(true);
if (!result.ok) return;
const breadcrumbsJson = JSON.stringify(result.value.breadcrumbs);
const deviceJson = JSON.stringify(result.value.device);
expect(ENCODER.encode(result.value.logs).byteLength).toBe(MAX_LOG_BYTES);
expect(ENCODER.encode(breadcrumbsJson).byteLength).toBeLessThanOrEqual(
MAX_BREADCRUMBS_JSON_BYTES,
);
expect(ENCODER.encode(deviceJson).byteLength).toBeLessThanOrEqual(MAX_DEVICE_JSON_BYTES);
expect(JSON.parse(breadcrumbsJson)).toEqual(result.value.breadcrumbs);
expect(JSON.parse(deviceJson)).toEqual(result.value.device);
});
@@ -218,7 +203,6 @@ function bugPayload(overrides: Record<string, unknown> = {}): Record<string, unk
contact: "",
logs: "",
device: {},
breadcrumbs: [],
schema_version: 1,
...overrides,
};

View File

@@ -121,7 +121,7 @@ describe("diagnostics Worker", () => {
const row = await env.DB.prepare(
`SELECT occurred_at AS occurredAt, logs_r2_key AS logsKey,
device_json AS deviceJson, breadcrumbs_json AS breadcrumbsJson
device_json AS deviceJson
FROM bugs WHERE id = ?`,
)
.bind(id)
@@ -129,7 +129,6 @@ describe("diagnostics Worker", () => {
occurredAt: number;
logsKey: string;
deviceJson: string;
breadcrumbsJson: string;
}>();
expect(row?.occurredAt).toBe(3);
expect(JSON.parse(row?.deviceJson ?? "null")).toEqual({
@@ -139,9 +138,6 @@ describe("diagnostics Worker", () => {
network: "offline",
batteryLevel: "90%",
});
expect(JSON.parse(row?.breadcrumbsJson ?? "null")).toEqual([
{ name: "opened", timestampMillis: 2, properties: {} },
]);
expect(await (await env.BLOBS.get(row?.logsKey ?? "missing"))?.text()).toBe("first logs");
const objects = await env.BLOBS.list({ prefix: `bugs/${id}/` });
@@ -198,15 +194,15 @@ describe("diagnostics Worker", () => {
`INSERT INTO bugs
(id, received_at, occurred_at, install_id, app_version, platform,
what_happened, expected, steps, contact, logs_r2_key,
device_json, breadcrumbs_json, status, schema_version)
VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', ?, '{}', '[]', 'open', 1)`,
device_json, status, schema_version)
VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', ?, '{}', 'open', 1)`,
).bind(oldBugId, oldReceivedAt, oldReceivedAt, INSTALL_ID, oldBugKey),
env.DB.prepare(
`INSERT INTO bugs
(id, received_at, occurred_at, install_id, app_version, platform,
what_happened, expected, steps, contact, logs_r2_key,
device_json, breadcrumbs_json, status, schema_version)
VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', NULL, '{}', '[]', 'open', 1)`,
device_json, status, schema_version)
VALUES (?, ?, ?, ?, '', '', 'failed', 'worked', '', '', NULL, '{}', 'open', 1)`,
).bind(currentBugId, Date.now(), Date.now(), INSTALL_ID),
]);
@@ -283,10 +279,10 @@ describe("diagnostics Worker", () => {
INSERT INTO bugs (
id, received_at, occurred_at, install_id, app_version, platform,
what_happened, expected, steps, contact, logs_r2_key,
device_json, breadcrumbs_json, status, schema_version
device_json, status, schema_version
)
SELECT 'retention-backlog-' || printf('%04d', value), ?, ?, ?, '', '',
'failed', 'worked', '', '', NULL, '{}', '[]', 'open', 1
'failed', 'worked', '', '', NULL, '{}', 'open', 1
FROM sequence`,
)
.bind(oldReceivedAt, oldReceivedAt, INSTALL_ID)
@@ -358,7 +354,6 @@ function bugPayload(id: string, logs: string): Record<string, unknown> {
network: "offline",
batteryLevel: "90%",
},
breadcrumbs: [{ name: "opened", timestampMillis: 2, properties: {} }],
schemaVersion: 1,
};
}
@@ -382,7 +377,6 @@ function normalizedBug(id: string, logs: string): NormalizedBugPayload {
network: "offline",
batteryLevel: "90%",
},
breadcrumbs: [],
schemaVersion: 1,
};
}

View File

@@ -1,47 +0,0 @@
package com.vnidrop.app.diagnostics
import com.vnidrop.app.logging.platformNowMillis
import kotlin.concurrent.atomics.AtomicReference
import kotlin.concurrent.atomics.ExperimentalAtomicApi
/**
* Fixed-size ring of high-level app breadcrumbs for crash / bug context.
* Always in-memory only; never auto-uploaded without policy + transport.
*
* Updates are best-effort under concurrency; losing a breadcrumb is preferable
* to blocking a dying process on a lock.
*/
@OptIn(ExperimentalAtomicApi::class)
class BreadcrumbBuffer(
private val capacity: Int = DefaultCapacity,
) {
init {
require(capacity > 0) { "capacity must be positive" }
}
private val items = AtomicReference<List<Breadcrumb>>(emptyList())
fun add(name: String, properties: Map<String, String> = emptyMap(), timestampMillis: Long = platformNowMillis()) {
val sanitizedName = sanitizeDiagnosticName(name)
if (sanitizedName.isBlank()) return
val crumb = Breadcrumb(
name = sanitizedName,
timestampMillis = timestampMillis,
properties = sanitizeDiagnosticProperties(properties),
)
while (true) {
val current = items.load()
if (items.compareAndSet(current, (current + crumb).takeLast(capacity))) return
}
}
fun snapshot(): List<Breadcrumb> = items.load()
fun clear() {
items.store(emptyList())
}
companion object {
const val DefaultCapacity = 40
}
}

View File

@@ -21,7 +21,6 @@ data class BugReportDraft(
class BugReportService(
private val preferencesRepository: PreferencesRepository,
private val transport: DiagnosticsTransport,
private val breadcrumbs: BreadcrumbBuffer,
private val appVersion: String,
private val platform: String,
private val logReader: () -> String = {
@@ -53,7 +52,6 @@ class BugReportService(
network = deviceInfo?.network?.takeUtf8Bytes(96),
batteryLevel = deviceInfo?.batteryLevel?.takeUtf8Bytes(64),
),
breadcrumbs = breadcrumbs.snapshot(),
)
}

View File

@@ -13,7 +13,6 @@ import kotlinx.coroutines.launch
class DiagnosticsCoordinator(
val preferencesRepository: PreferencesRepository,
val transport: DiagnosticsTransport,
val breadcrumbs: BreadcrumbBuffer,
val bugReports: BugReportService,
private val scope: CoroutineScope,
) {
@@ -32,18 +31,15 @@ class DiagnosticsCoordinator(
scope: CoroutineScope,
transport: DiagnosticsTransport = NoOpDiagnosticsTransport(),
): DiagnosticsCoordinator {
val breadcrumbs = BreadcrumbBuffer()
val bugReports = BugReportService(
preferencesRepository = preferencesRepository,
transport = transport,
breadcrumbs = breadcrumbs,
appVersion = appVersion,
platform = platform,
)
return DiagnosticsCoordinator(
preferencesRepository = preferencesRepository,
transport = transport,
breadcrumbs = breadcrumbs,
bugReports = bugReports,
scope = scope,
)

View File

@@ -8,8 +8,6 @@ internal object DiagnosticsJson {
internal const val MaxInstallIdBytes = 80
internal const val MaxAppVersionBytes = 40
internal const val MaxPlatformBytes = 40
private const val MaxBreadcrumbsJsonBytes = 16_000
private const val MaxBreadcrumbs = 40
fun bugBody(report: BugReport): String {
val logs = if (report.includeLogs) report.logs else ""
@@ -72,47 +70,7 @@ internal object DiagnosticsJson {
appendJsonField("network", report.device.network.orEmpty())
append(',')
appendJsonField("batteryLevel", report.device.batteryLevel.orEmpty())
append("},")
append("\"breadcrumbs\":")
appendBreadcrumbs(report.breadcrumbs)
append('}')
}
private fun StringBuilder.appendBreadcrumbs(crumbs: List<Breadcrumb>) {
append('[')
var encodedBytes = 2
var appended = 0
for (crumb in crumbs) {
if (appended == MaxBreadcrumbs) break
val name = sanitizeDiagnosticName(crumb.name)
if (name.isBlank() || crumb.timestampMillis < 0) continue
val encoded = buildString {
append('{')
appendJsonField("name", name)
append(',')
append("\"timestampMillis\":")
append(crumb.timestampMillis)
append(',')
append("\"properties\":")
appendStringMap(crumb.properties)
append('}')
}
val additionBytes = encoded.encodeToByteArray().size + if (appended == 0) 0 else 1
if (encodedBytes + additionBytes > MaxBreadcrumbsJsonBytes) break
if (appended > 0) append(',')
append(encoded)
encodedBytes += additionBytes
appended += 1
}
append(']')
}
private fun StringBuilder.appendStringMap(map: Map<String, String>) {
append('{')
sanitizeDiagnosticProperties(map).entries.forEachIndexed { index, (key, value) ->
if (index > 0) append(',')
appendJsonField(key, value)
}
append('}')
}

View File

@@ -5,12 +5,6 @@ package com.vnidrop.app.diagnostics
* intentionally abstracted; nothing here assumes a network backend.
*/
data class Breadcrumb(
val name: String,
val timestampMillis: Long,
val properties: Map<String, String> = emptyMap(),
)
data class DeviceSnapshot(
val deviceName: String?,
val deviceModel: String?,
@@ -32,7 +26,6 @@ data class BugReport(
val includeLogs: Boolean,
val logs: String,
val device: DeviceSnapshot,
val breadcrumbs: List<Breadcrumb>,
val schemaVersion: Int = DiagnosticsSchemaVersion,
)

View File

@@ -1,30 +1,11 @@
package com.vnidrop.app.diagnostics
internal const val MaxDiagnosticProperties = 12
internal const val MaxDiagnosticPropertyKeyBytes = 40
internal const val MaxDiagnosticPropertyValueBytes = 128
internal const val MaxDiagnosticNameBytes = 64
internal fun sanitizeDiagnosticsInstallId(value: String): String {
val trimmed = value.trim()
if (trimmed.any { it.code < 0x20 || it.code == 0x7f }) return ""
return trimmed.takeUtf8Bytes(DiagnosticsJson.MaxInstallIdBytes)
}
internal fun sanitizeDiagnosticName(name: String): String =
name.takeUtf8Bytes(MaxDiagnosticNameBytes)
internal fun sanitizeDiagnosticProperties(properties: Map<String, String>): Map<String, String> {
val sanitized = LinkedHashMap<String, String>(minOf(properties.size, MaxDiagnosticProperties))
for ((rawKey, rawValue) in properties) {
val key = rawKey.takeUtf8Bytes(MaxDiagnosticPropertyKeyBytes)
if (key.isEmpty() || key in sanitized) continue
sanitized[key] = LogRedactor.redact(rawValue).takeUtf8Bytes(MaxDiagnosticPropertyValueBytes)
if (sanitized.size == MaxDiagnosticProperties) break
}
return sanitized
}
internal fun String.takeUtf8Bytes(maxBytes: Int): String {
require(maxBytes >= 0) { "maxBytes must not be negative" }
val encoded = encodeToByteArray()

View File

@@ -204,22 +204,11 @@ class DiagnosticsTest {
assertTrue(redacted.contains("[redacted-endpoint]"))
}
@Test
fun breadcrumbBufferKeepsOnlyLatestEntries() {
val buffer = BreadcrumbBuffer(capacity = 3)
buffer.add("a")
buffer.add("b")
buffer.add("c")
buffer.add("d")
assertEquals(listOf("b", "c", "d"), buffer.snapshot().map { it.name })
}
@Test
fun bugReportRequiresWhatAndExpected() = runTest {
val service = BugReportService(
preferencesRepository = fakePrefs(),
transport = RecordingDiagnosticsTransport(),
breadcrumbs = BreadcrumbBuffer(),
appVersion = "1.0",
platform = "Test",
logReader = { "logs" },
@@ -238,7 +227,6 @@ class DiagnosticsTest {
val service = BugReportService(
preferencesRepository = fakePrefs(),
transport = throwingTransport,
breadcrumbs = BreadcrumbBuffer(),
appVersion = "1.0",
platform = "Test",
)
@@ -255,7 +243,6 @@ class DiagnosticsTest {
val service = BugReportService(
preferencesRepository = fakePrefs(),
transport = transport,
breadcrumbs = BreadcrumbBuffer(),
appVersion = "1.0",
platform = "Test",
logReader = { LogRedactor.redact("ticket=abcdefghijklmnopqrstuvwxyz012345 plain") },
@@ -286,7 +273,6 @@ class DiagnosticsTest {
val service = BugReportService(
preferencesRepository = fakePrefs(),
transport = RecordingDiagnosticsTransport(),
breadcrumbs = BreadcrumbBuffer(),
appVersion = "1.0",
platform = "Test",
logReader = { rawLogs },
@@ -304,7 +290,6 @@ class DiagnosticsTest {
val service = BugReportService(
preferencesRepository = fakePrefs(),
transport = RecordingDiagnosticsTransport(),
breadcrumbs = BreadcrumbBuffer(),
appVersion = "1.0",
platform = "Test",
logReader = { "🙂".repeat(60_000) },
@@ -333,7 +318,6 @@ class DiagnosticsTest {
includeLogs = includeLogs,
logs = logs,
device = DeviceSnapshot(null, null, "OS", null, null),
breadcrumbs = emptyList(),
)
private fun fakePrefs() = FakePreferencesRepository(

View File

@@ -18,7 +18,6 @@ import com.vnidrop.app.core.ShareAccessPolicy
import com.vnidrop.app.core.Transfer
import com.vnidrop.app.core.TransferDirection
import com.vnidrop.app.core.TransferStatus
import com.vnidrop.app.diagnostics.BreadcrumbBuffer
import com.vnidrop.app.diagnostics.BugReportService
import com.vnidrop.app.diagnostics.DiagnosticsTransport
import com.vnidrop.app.diagnostics.NoOpDiagnosticsTransport
@@ -989,7 +988,6 @@ class ViewModelsTest {
BugReportService(
preferencesRepository = preferences,
transport = transport,
breadcrumbs = BreadcrumbBuffer(),
appVersion = "1.0",
platform = "Test",
logReader = { "sample log line" },