From b2c7b60691377ca697e4603073c233a6c1fd24fc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 22:58:26 +0000 Subject: [PATCH 01/12] feat: persist crash logs to disk so they survive app restart (#231) Installs a Thread.UncaughtExceptionHandler that captures the last logcat lines, custom log source entries, and recent network requests alongside the exception's stack trace, then always delegates to whatever handler was previously installed so other crash reporters keep working. Records are written synchronously to the no-backup directory and surfaced in a new Crash tab on the next launch, with copy/share/delete support. --- AGENTS.md | 1 + .../ms/square/debugoverlay/DebugOverlay.kt | 67 +++++- .../bugreport/model/BugReportSnapshot.kt | 2 + .../internal/crash/CrashHandler.kt | 95 ++++++++ .../internal/crash/CrashRecord.kt | 43 ++++ .../internal/crash/CrashRecordExporter.kt | 53 +++++ .../internal/crash/CrashRecordStorage.kt | 116 ++++++++++ .../crash/CrashRecordTextFormatter.kt | 51 ++++ .../data/DebugOverlayDataRepository.kt | 36 +++ .../internal/data/source/LogcatDataSource.kt | 10 + .../internal/ui/CrashLogDetailScreen.kt | 218 ++++++++++++++++++ .../internal/ui/CrashLogTabContent.kt | 177 ++++++++++++++ .../internal/ui/DebugPanelDialog.kt | 11 +- .../src/main/res/values/strings.xml | 12 + .../res/xml/debugoverlay_bugreport_paths.xml | 6 +- .../internal/crash/CrashHandlerTest.kt | 125 ++++++++++ .../internal/crash/CrashRecordStorageTest.kt | 80 +++++++ .../crash/CrashRecordTextFormatterTest.kt | 79 +++++++ .../data/source/LogcatDataSourceTest.kt | 22 ++ docs/ARCHITECTURE.md | 2 + 20 files changed, 1202 insertions(+), 4 deletions(-) create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt create mode 100644 debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt create mode 100644 debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt create mode 100644 debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt create mode 100644 debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt diff --git a/AGENTS.md b/AGENTS.md index edc879bd..15738d24 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,6 +31,7 @@ A Jetpack Compose library that displays real-time debug information as an overla | Create new extension | Implement `LogSource` or `NetworkRequestSource`, self-register in `init` block | | Modify bug report flow | `BugReportGenerator.kt` (orchestration), `BugReportDraftStorage.kt` (persistence) | | Change metrics display | `internal/ui/DebugOverlayPanel.kt` (compact overlay), `DebugOverlayPanelDataSource.kt` (aggregation) | +| Modify crash persistence | `internal/crash/CrashHandler.kt` (capture), `CrashRecordStorage.kt` (persistence), `CrashLogTabContent.kt`/`CrashLogDetailScreen.kt` (UI) | ## Build Commands diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt index c66aef59..7e620aff 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt @@ -12,8 +12,11 @@ import com.ms.square.debugoverlay.internal.InternalDebugOverlayApi import com.ms.square.debugoverlay.internal.Logger import com.ms.square.debugoverlay.internal.OverlayViewManager import com.ms.square.debugoverlay.internal.bugreport.BugReportGenerator +import com.ms.square.debugoverlay.internal.bugreport.DefaultAppInfoProvider import com.ms.square.debugoverlay.internal.bugreport.IntentShareExporter import com.ms.square.debugoverlay.internal.bugreport.validateFilename +import com.ms.square.debugoverlay.internal.crash.CrashHandler +import com.ms.square.debugoverlay.internal.crash.DefaultCrashRecordStorage import com.ms.square.debugoverlay.internal.data.DebugOverlayDataRepository import com.ms.square.debugoverlay.internal.ui.DebugPanelActivity import com.ms.square.debugoverlay.internal.util.checkMainThread @@ -102,9 +105,35 @@ public object DebugOverlay { repository = repository, activityProvider = viewManager ) + + installCrashHandler(application, repository) } } + /** + * Chains a [CrashHandler] in front of whatever [Thread.UncaughtExceptionHandler] was + * previously installed (the platform default, or another crash reporter like + * Crashlytics), so persisting a crash record never interferes with existing crash + * handling. Captured exactly once here; never re-installed on [configure] calls, since + * [CrashHandler] reads [config] live via lambdas. + */ + private fun installCrashHandler(application: Application, repository: DebugOverlayDataRepository) { + val previousHandler = Thread.getDefaultUncaughtExceptionHandler() + val cachedAppInfo = runCatching { DefaultAppInfoProvider.getAppInfo(application) }.getOrNull() + Thread.setDefaultUncaughtExceptionHandler( + CrashHandler( + previousHandler = previousHandler, + storage = DefaultCrashRecordStorage(application), + cachedAppInfo = cachedAppInfo, + logcatSnapshotProvider = { repository.logcatSnapshotSync() }, + customLogSnapshotProvider = { repository.customLogSnapshotSync() }, + networkRequestsSnapshotProvider = { repository.networkRequestsSnapshotSync() }, + isEnabled = { config.persistCrashLogs }, + maxLogLines = { config.maxCrashLogLines } + ) + ) + } + // CopyOnWriteArrayList enables lock-free iteration during bug report generation // synchronized block in addBugReportContributor ensures atomic duplicate detection internal val bugReportContributors = CopyOnWriteArrayList() @@ -243,12 +272,38 @@ public object DebugOverlay { field = value } + /** + * Whether to persist the last log lines and the exception's stack trace to disk when + * an uncaught exception occurs, so they survive process death and can be reviewed or + * exported from the Crash tab on the next launch. + * + * Default is `true`. + */ + public var persistCrashLogs: Boolean = initial.persistCrashLogs + + /** + * Maximum number of recent entries kept per source (Logcat, custom log source, + * network requests) in a persisted crash record. + * + * Default is [Config.DEFAULT_MAX_CRASH_LOG_LINES] (100). + * + * @throws IllegalArgumentException if assigned a negative value. + */ + @IntRange(from = 0) + public var maxCrashLogLines: Int = initial.maxCrashLogLines + set(value) { + require(value >= 0) { "maxCrashLogLines must be non-negative, was $value" } + field = value + } + internal fun build(): Config = Config( overlayMode = overlayMode, networkRequestSource = networkRequestSource, customLogSource = customLogSource, bugReportExporter = bugReportExporter, - maxLogcatEntries = maxLogcatEntries + maxLogcatEntries = maxLogcatEntries, + persistCrashLogs = persistCrashLogs, + maxCrashLogLines = maxCrashLogLines ) } @@ -269,6 +324,11 @@ public object DebugOverlay { * Default is the built-in share sheet exporter. * @property maxLogcatEntries Maximum number of entries kept in the built-in Logcat * tab buffer. Default is [DEFAULT_MAX_LOGCAT_ENTRIES]. + * @property persistCrashLogs Whether to persist the last log lines and the exception's + * stack trace to disk on an uncaught exception, reviewable from the Crash tab on the + * next launch. Default is `true`. + * @property maxCrashLogLines Maximum number of recent entries kept per source in a + * persisted crash record. Default is [DEFAULT_MAX_CRASH_LOG_LINES]. * * @see configure */ @@ -278,10 +338,15 @@ public object DebugOverlay { val customLogSource: LogSource? = null, val bugReportExporter: BugReportExporter = IntentShareExporter, val maxLogcatEntries: Int = DEFAULT_MAX_LOGCAT_ENTRIES, + val persistCrashLogs: Boolean = true, + val maxCrashLogLines: Int = DEFAULT_MAX_CRASH_LOG_LINES, ) { public companion object { /** Default value for [maxLogcatEntries]. */ public const val DEFAULT_MAX_LOGCAT_ENTRIES: Int = 300 + + /** Default value for [maxCrashLogLines]. */ + public const val DEFAULT_MAX_CRASH_LOG_LINES: Int = 100 } } } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/model/BugReportSnapshot.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/model/BugReportSnapshot.kt index b2e82b58..70e26e4a 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/model/BugReportSnapshot.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/model/BugReportSnapshot.kt @@ -6,11 +6,13 @@ import com.ms.square.debugoverlay.internal.data.model.DeviceInfo import com.ms.square.debugoverlay.internal.data.model.JankStatsUiState import com.ms.square.debugoverlay.model.LogEntry import com.ms.square.debugoverlay.model.NetworkRequest +import kotlinx.serialization.Serializable /** * Bundled custom log source data. * Ensures logs and source name are always provided together. */ +@Serializable internal data class CustomLogSourceData(val logs: List, val sourceName: String) /** diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt new file mode 100644 index 00000000..af05323e --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt @@ -0,0 +1,95 @@ +package com.ms.square.debugoverlay.internal.crash + +import android.os.Process +import com.ms.square.debugoverlay.internal.Logger +import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo +import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData +import com.ms.square.debugoverlay.model.LogEntry +import com.ms.square.debugoverlay.model.NetworkRequest +import kotlin.system.exitProcess + +/** + * Persists a [CrashRecord] to disk on an uncaught exception, then always delegates to + * [previousHandler] so the app's normal crash behavior (and any other installed crash + * reporter, e.g. Crashlytics) is unaffected. + * + * Installed once by [com.ms.square.debugoverlay.DebugOverlay.install]. All data reads + * are non-suspending, in-memory snapshots — this must never spawn a subprocess, hop a + * dispatcher, or otherwise risk not completing before the process dies. + * + * @param previousHandler The handler that was installed before this one, captured once + * at install time. Never re-fetched, so a handler installed by another SDK *after* + * DebugOverlay is never clobbered. + * @param storage Where the crash record is written. + * @param cachedAppInfo App info captured once at install time (immutable for the process + * lifetime) — avoids PackageManager calls in the crash path. + * @param logcatSnapshotProvider Non-suspending snapshot of the in-memory Logcat buffer. + * @param customLogSnapshotProvider Non-suspending snapshot of the custom log source, if any. + * @param networkRequestsSnapshotProvider Non-suspending snapshot of recent network requests. + * @param isEnabled Whether persistence is currently enabled. Read live at crash time so + * toggling [com.ms.square.debugoverlay.DebugOverlay.configure] doesn't require + * reinstalling this handler. + * @param maxLogLines Maximum number of entries kept per log/request source, read live. + */ +internal class CrashHandler( + private val previousHandler: Thread.UncaughtExceptionHandler?, + private val storage: CrashRecordStorage, + private val cachedAppInfo: AppInfo?, + private val logcatSnapshotProvider: () -> List, + private val customLogSnapshotProvider: () -> CustomLogSourceData?, + private val networkRequestsSnapshotProvider: () -> List, + private val isEnabled: () -> Boolean, + private val maxLogLines: () -> Int, +) : Thread.UncaughtExceptionHandler { + + @Suppress("TooGenericExceptionCaught") + override fun uncaughtException(thread: Thread, throwable: Throwable) { + try { + if (isEnabled()) { + storage.writeSync(buildCrashRecord(thread, throwable)) + } + } catch (t: Throwable) { + // Deliberately broad: capture failure must never prevent the delegate call below + // from running, since that's what keeps other crash reporters (e.g. Crashlytics) + // and the platform's own crash handling working. + runCatching { Logger.e("CrashHandler failed to capture crash record", t) } + } finally { + delegateToPreviousHandler(thread, throwable) + } + } + + private fun buildCrashRecord(thread: Thread, throwable: Throwable): CrashRecord { + val maxLines = maxLogLines().coerceAtLeast(0) + val customLogSnapshot = customLogSnapshotProvider()?.let { data -> + data.copy(logs = data.logs.takeLast(maxLines)) + } + return CrashRecord( + timestampMs = System.currentTimeMillis(), + threadName = thread.name, + exceptionType = throwable.javaClass.name, + message = throwable.message, + stackTrace = throwable.stackTraceToString(), + appInfo = cachedAppInfo, + logcatLogs = logcatSnapshotProvider().takeLast(maxLines), + customLogSourceData = customLogSnapshot, + networkRequests = networkRequestsSnapshotProvider().takeLast(maxLines) + ) + } + + private fun delegateToPreviousHandler(thread: Thread, throwable: Throwable) { + val handler = previousHandler + if (handler != null) { + handler.uncaughtException(thread, throwable) + } else { + // Unreachable on real devices: the platform always installs a default handler + // before Application.onCreate(). Guards test doubles / edge-case environments + // where none was ever installed, so the process still terminates. + Process.killProcess(Process.myPid()) + exitProcess(EXIT_CODE_UNCAUGHT_EXCEPTION) + } + } + + private companion object { + const val EXIT_CODE_UNCAUGHT_EXCEPTION = 10 + } +} diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt new file mode 100644 index 00000000..90945c3b --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt @@ -0,0 +1,43 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo +import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData +import com.ms.square.debugoverlay.model.LogEntry +import com.ms.square.debugoverlay.model.NetworkRequest +import kotlinx.serialization.Serializable +import java.io.File +import java.util.UUID + +/** + * A crash captured by [CrashHandler] and persisted to disk so it survives process death. + * + * @param version Schema version, for forward-compatible reads via `ignoreUnknownKeys`. + * @param timestampMs When the exception was caught (epoch millis). + * @param threadName Name of the thread that crashed. + * @param exceptionType Fully-qualified exception class name. + * @param message The exception's message, if any. + * @param stackTrace Full stack trace text, including any "Caused by" chain. + * @param appInfo App info captured at install time (immutable for the process lifetime). + * @param logcatLogs Recent logcat entries leading up to the crash. + * @param customLogSourceData Recent custom log source entries, null if none registered. + * @param networkRequests Recent network requests leading up to the crash. + */ +@Serializable +internal data class CrashRecord( + val version: Int = 1, + val id: String = UUID.randomUUID().toString(), + val timestampMs: Long, + val threadName: String, + val exceptionType: String, + val message: String?, + val stackTrace: String, + val appInfo: AppInfo?, + val logcatLogs: List, + val customLogSourceData: CustomLogSourceData?, + val networkRequests: List, +) + +/** A [CrashRecord] paired with the file it was loaded from. */ +internal data class CrashRecordInfo(val filePath: String, val record: CrashRecord) { + val file: File get() = File(filePath) +} diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt new file mode 100644 index 00000000..6a9a232c --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt @@ -0,0 +1,53 @@ +package com.ms.square.debugoverlay.internal.crash + +import android.content.Context +import android.content.Intent +import androidx.core.content.FileProvider +import com.ms.square.debugoverlay.core.R +import com.ms.square.debugoverlay.internal.Logger +import com.ms.square.debugoverlay.internal.util.checkFolderExists +import com.ms.square.debugoverlay.internal.util.formatFilenameTimestamp +import com.ms.square.debugoverlay.internal.util.runCatchingNonCancellation +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import java.io.File + +private const val PROVIDER_AUTHORITY_SUFFIX = ".debugoverlay.bugreport.provider" +private const val EXPORTS_SUBDIR = "debugoverlay_crash_exports" + +/** + * Shares a [CrashRecord] as a plain-text file via Android's share sheet. + * + * Writes to a temp file under [Context.getCacheDir] (rather than putting the text + * directly in [Intent.EXTRA_TEXT]) to avoid binder IPC size limits when logs are large, + * and reuses the same FileProvider authority already declared for bug report sharing. + */ +internal object CrashRecordExporter { + + suspend fun share(context: Context, record: CrashRecord): Boolean = withContext(Dispatchers.IO) { + runCatchingNonCancellation { + val exportsDir = File(context.cacheDir, EXPORTS_SUBDIR).also { it.checkFolderExists() } + val file = File(exportsDir, "crash_${formatFilenameTimestamp(record.timestampMs)}.txt") + file.writeText(formatCrashRecordAsText(record)) + + val authority = "${context.packageName}$PROVIDER_AUTHORITY_SUFFIX" + val uri = FileProvider.getUriForFile(context, authority, file) + + val intent = Intent(Intent.ACTION_SEND).apply { + type = "text/plain" + putExtra(Intent.EXTRA_STREAM, uri) + putExtra(Intent.EXTRA_SUBJECT, record.exceptionType) + addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) + addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + } + val chooserTitle = context.getString(R.string.debugoverlay_share_crash_log) + withContext(Dispatchers.Main) { + context.startActivity(Intent.createChooser(intent, chooserTitle).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)) + } + true + }.getOrElse { e -> + Logger.w("Failed to share crash record", e) + false + } + } +} diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt new file mode 100644 index 00000000..0f79beae --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt @@ -0,0 +1,116 @@ +package com.ms.square.debugoverlay.internal.crash + +import android.content.Context +import com.ms.square.debugoverlay.internal.Logger +import com.ms.square.debugoverlay.internal.util.checkFolderExists +import com.ms.square.debugoverlay.internal.util.runCatchingNonCancellation +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlinx.serialization.json.Json +import java.io.File +import java.util.UUID + +private const val CRASH_RECORDS_SUBDIR = "debugoverlay_crash_records" +internal const val DEFAULT_MAX_CRASH_RECORDS = 5 + +/** + * Persists [CrashRecord]s to disk so they survive process death, and lets the next + * launch discover and review them. + */ +internal sealed interface CrashRecordStorage { + /** + * Writes [record] to disk and evicts old records beyond the retention limit. + * + * Must be safe to call synchronously from [Thread.UncaughtExceptionHandler.uncaughtException]: + * no suspension, no dispatcher hop. Any failure is swallowed and logged — the caller + * must be able to unconditionally proceed to the previous crash handler afterward. + */ + fun writeSync(record: CrashRecord) + + /** Loads all persisted crash records, most recent first. */ + suspend fun listCrashRecords(): List + + /** Deletes a single persisted crash record. */ + suspend fun deleteCrashRecord(info: CrashRecordInfo) +} + +/** + * Default implementation of [CrashRecordStorage] using the app's no-backup data directory. + * + * Records are flat JSON files (no per-record folder needed, unlike bug report drafts, + * since a crash record is a single self-contained blob) named `crash__.json` + * so lexicographic filename order matches chronological order. + * + * @param context Application context for no-backup directory access + * @param maxRecords Maximum number of records retained; oldest evicted first + */ +internal class DefaultCrashRecordStorage( + private val context: Context, + private val maxRecords: Int = DEFAULT_MAX_CRASH_RECORDS, +) : CrashRecordStorage { + + private val json = Json { ignoreUnknownKeys = true } + + // Plain JVM monitor, not a coroutines Mutex: writeSync() runs outside any coroutine + // context (it's called directly from uncaughtException()), and guards against two + // near-simultaneous crashes on different threads racing on the directory listing. + private val writeLock = Any() + + private val recordsDir by lazy { + File(context.noBackupFilesDir, CRASH_RECORDS_SUBDIR).also { + it.checkFolderExists() + } + } + + override fun writeSync(record: CrashRecord) { + synchronized(writeLock) { + val file = File(recordsDir, fileNameFor(record)) + file.writeText(json.encodeToString(CrashRecord.serializer(), record)) + evictOldRecordsLocked() + } + } + + private fun fileNameFor(record: CrashRecord) = "crash_${record.timestampMs}_${UUID.randomUUID()}.json" + + // Must be called while holding writeLock. + private fun evictOldRecordsLocked() { + val files = recordsDir.listFiles()?.filter { it.isFile } ?: return + if (files.size <= maxRecords) return + files.sortedDescending().drop(maxRecords).forEach { it.delete() } + } + + override suspend fun listCrashRecords(): List = withContext(Dispatchers.IO) { + val files = recordsDir.listFiles()?.filter { it.isFile }?.sortedDescending() ?: emptyList() + files.mapNotNull { file -> loadRecord(file)?.let { CrashRecordInfo(file.absolutePath, it) } } + } + + override suspend fun deleteCrashRecord(info: CrashRecordInfo): Unit = withContext(Dispatchers.IO) { + val file = info.file + if (!isDirectChildOfRecordsDir(file)) { + Logger.w("Refusing to delete crash record outside records directory: ${file.absolutePath}") + return@withContext + } + if (file.exists() && !file.delete()) { + Logger.w("Failed to delete crash record: ${file.absolutePath}") + } + } + + /** + * Checks if the given file is a direct child of [recordsDir]. + * Uses canonical paths to resolve symlinks and ".." traversal attacks. + */ + private fun isDirectChildOfRecordsDir(file: File): Boolean = runCatching { + file.canonicalFile.parentFile == recordsDir.canonicalFile + }.getOrElse { e -> + Logger.w("Failed to resolve canonical path for safety check: ${e.javaClass.simpleName} - ${e.message}") + false + } + + private fun loadRecord(file: File): CrashRecord? = + runCatchingNonCancellation { + json.decodeFromString(CrashRecord.serializer(), file.readText()) + }.getOrElse { e -> + Logger.w("Failed to parse crash record ${file.name}: ${e.message}") + null + } +} diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt new file mode 100644 index 00000000..e7bcf073 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt @@ -0,0 +1,51 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.ms.square.debugoverlay.internal.util.formatFullTimestamp +import com.ms.square.debugoverlay.internal.util.toClipboardText +import com.ms.square.debugoverlay.model.NetworkRequest + +private const val SEPARATOR_WIDTH = 80 + +/** + * Formats a [CrashRecord] as human-readable plain text for sharing (e.g. to a teammate + * or a GitHub issue), mirroring [com.ms.square.debugoverlay.formatBugReportMarkdown]'s + * intent of a readable rendering rather than raw JSON. + */ +internal fun formatCrashRecordAsText(record: CrashRecord): String = buildString { + appendLine("=".repeat(SEPARATOR_WIDTH)) + appendLine("${record.exceptionType}: ${record.message ?: "(no message)"}") + appendLine("=".repeat(SEPARATOR_WIDTH)) + appendLine("Time: ${formatFullTimestamp(record.timestampMs)}") + appendLine("Thread: ${record.threadName}") + record.appInfo?.let { appInfo -> + appendLine("Package: ${appInfo.packageName}") + appInfo.versionName?.let { appendLine("Version: $it (${appInfo.versionCode})") } + } + appendLine() + + appendLine("--- STACK TRACE ---") + appendLine(record.stackTrace) + + if (record.logcatLogs.isNotEmpty()) { + appendLine() + appendLine("--- LOGCAT (${record.logcatLogs.size}) ---") + record.logcatLogs.forEach { appendLine(it.toClipboardText()) } + } + + record.customLogSourceData?.let { customLogs -> + if (customLogs.logs.isNotEmpty()) { + appendLine() + appendLine("--- ${customLogs.sourceName.uppercase()} (${customLogs.logs.size}) ---") + customLogs.logs.forEach { appendLine(it.toClipboardText()) } + } + } + + if (record.networkRequests.isNotEmpty()) { + appendLine() + appendLine("--- NETWORK REQUESTS (${record.networkRequests.size}) ---") + record.networkRequests.forEach { appendLine(it.toSummaryLine()) } + } +} + +private fun NetworkRequest.toSummaryLine(): String = + "${formatFullTimestamp(timestampMs)} $method $url -> ${statusCode ?: "?"} (${durationMs}ms)" diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt index e82dd3af..75a61ade 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt @@ -7,6 +7,9 @@ import com.ms.square.debugoverlay.LogSource import com.ms.square.debugoverlay.NetworkRequestSource import com.ms.square.debugoverlay.NoOpNetworkRequestSource import com.ms.square.debugoverlay.internal.Logger +import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData +import com.ms.square.debugoverlay.internal.crash.CrashRecordInfo +import com.ms.square.debugoverlay.internal.crash.DefaultCrashRecordStorage import com.ms.square.debugoverlay.internal.data.model.AppExitInfo import com.ms.square.debugoverlay.internal.data.model.DeviceInfo import com.ms.square.debugoverlay.internal.data.model.JankStatsUiState @@ -20,6 +23,7 @@ import com.ms.square.debugoverlay.internal.util.throttleLatest import com.ms.square.debugoverlay.model.LogEntry import com.ms.square.debugoverlay.model.NetworkRequest import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.Flow @@ -29,7 +33,9 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -47,6 +53,7 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop private val deviceInfoDataSource = DeviceInfoDataSource(context, scope) private val jankStatsDataSource = JankStatsDataSource() private val appExitDataSource = AppExitDataSource(context, scope) + private val crashRecordStorage = DefaultCrashRecordStorage(context) init { scope.launch { @@ -99,6 +106,19 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop val appExitInfos: Flow> = appExitDataSource.appExitInfos + // Persisted crash records from a prior run. Queried once per subscription (Lazily): + // this run's own crashes can't change mid-session since a crash terminates the process. + val crashRecords: Flow> = flow { emit(crashRecordStorage.listCrashRecords()) } + .flowOn(Dispatchers.IO) + .stateIn(scope, SharingStarted.Lazily, emptyList()) + + val hasCrashRecords: StateFlow = crashRecords + .map { it.isNotEmpty() } + .distinctUntilChanged() + .stateIn(scope, SharingStarted.Eagerly, false) + + suspend fun deleteCrashRecord(info: CrashRecordInfo) = crashRecordStorage.deleteCrashRecord(info) + // Snapshot methods for bug reports (use cached value if available, otherwise query directly) suspend fun queryLogcatSnapshot(): List = logcatDataSource.queryLogcatSnapshot() suspend fun queryDeviceInfoSnapshot(): DeviceInfo = deviceInfoDataSource.queryDeviceInfoSnapshot() @@ -108,6 +128,22 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop val networkRequests: Flow> = currentNetworkRequestSource .flatMapLatest { source -> source.requests } + // Cached copy of the latest network requests, kept warm for synchronous reads + // (e.g. by CrashHandler, which cannot suspend). Mirrors customLogSourceLogs's + // SharingStarted.Eagerly pattern above. + private val networkRequestsSnapshot: StateFlow> = + networkRequests.stateIn(scope, SharingStarted.Eagerly, emptyList()) + + /** Non-suspending snapshot of the in-memory Logcat buffer. Safe to call from a crashing thread. */ + fun logcatSnapshotSync(): List = logcatDataSource.snapshotEntriesSync() + + /** Non-suspending snapshot of the custom log source's latest known logs, if one is registered. */ + fun customLogSnapshotSync(): CustomLogSourceData? = + customLogSourceName.value?.let { name -> CustomLogSourceData(customLogSourceLogs.value, name) } + + /** Non-suspending snapshot of the latest known network requests. */ + fun networkRequestsSnapshotSync(): List = networkRequestsSnapshot.value + fun setNetworkSource(source: NetworkRequestSource) { currentNetworkRequestSource.value = source } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt index ca159f70..3e138969 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt @@ -158,6 +158,16 @@ internal class LogcatDataSource( clearSignal.tryEmit(Unit) } + /** + * Returns a snapshot of the in-memory log buffer without suspending or spawning a + * subprocess. Safe to call from a crashing thread (e.g. [com.ms.square.debugoverlay.internal.crash.CrashHandler]). + * + * Unlike [queryLogcatSnapshot], this never falls back to a one-shot `logcat -t N` + * capture, so it returns an empty list if the Logcat tab was never subscribed to + * this process run. + */ + fun snapshotEntriesSync(): List = entries.toList() + /** * Returns a snapshot of logcat logs for bug reports. * Uses cached value if streaming was active (debug panel was viewed), otherwise captures directly. diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt new file mode 100644 index 00000000..5c9c8442 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt @@ -0,0 +1,218 @@ +package com.ms.square.debugoverlay.internal.ui + +import androidx.compose.foundation.horizontalScroll +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.text.selection.SelectionContainer +import androidx.compose.foundation.verticalScroll +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Delete +import androidx.compose.material.icons.filled.Share +import androidx.compose.material3.Button +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Scaffold +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.material3.TopAppBarDefaults +import androidx.compose.runtime.Composable +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalClipboard +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontFamily +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.unit.dp +import com.ms.square.debugoverlay.core.R +import com.ms.square.debugoverlay.internal.crash.CrashRecord +import com.ms.square.debugoverlay.internal.crash.CrashRecordExporter +import com.ms.square.debugoverlay.internal.crash.CrashRecordInfo +import com.ms.square.debugoverlay.internal.crash.formatCrashRecordAsText +import com.ms.square.debugoverlay.internal.util.copyToClipboard +import com.ms.square.debugoverlay.internal.util.formatFullTimestamp +import com.ms.square.debugoverlay.internal.util.toClipboardText +import kotlinx.coroutines.launch + +/** + * Detail screen for a single persisted crash record. + * + * Shows the exception type/message/thread, full stack trace, and the Logcat/custom + * log/network requests context captured at crash time, plus Copy/Share/Delete actions. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +internal fun CrashLogDetailScreen( + info: CrashRecordInfo, + onBack: () -> Unit, + onDelete: () -> Unit, + modifier: Modifier = Modifier, +) { + val record = info.record + val context = LocalContext.current + val scope = rememberCoroutineScope() + + Scaffold( + modifier = modifier.fillMaxSize(), + topBar = { + TopAppBar( + title = { + Column { + Text( + text = record.exceptionType, + style = MaterialTheme.typography.titleSmall, + color = MaterialTheme.colorScheme.error, + maxLines = 1 + ) + Text( + text = formatFullTimestamp(record.timestampMs), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 4.dp) + ) + } + }, + navigationIcon = { BackButton(onClick = onBack) }, + actions = { + IconButton(onClick = { scope.launch { CrashRecordExporter.share(context, record) } }) { + Icon( + imageVector = Icons.Default.Share, + contentDescription = stringResource(R.string.debugoverlay_share_crash_log) + ) + } + IconButton(onClick = onDelete) { + Icon( + imageVector = Icons.Default.Delete, + contentDescription = stringResource(R.string.debugoverlay_delete_crash_log) + ) + } + }, + colors = TopAppBarDefaults.topAppBarColors( + containerColor = MaterialTheme.colorScheme.surfaceContainer + ) + ) + } + ) { paddingValues -> + CrashLogDetailContent(info = info, modifier = Modifier.padding(paddingValues)) + } +} + +@Composable +private fun CrashLogDetailContent(info: CrashRecordInfo, modifier: Modifier = Modifier) { + val record = info.record + + Column(modifier = modifier.fillMaxSize()) { + SelectionContainer( + modifier = Modifier + .weight(1f) + .verticalScroll(rememberScrollState()) + ) { + Column( + modifier = Modifier.padding(16.dp), + verticalArrangement = Arrangement.spacedBy(16.dp) + ) { + record.message?.let { message -> + Surface( + shape = MaterialTheme.shapes.medium, + color = MaterialTheme.colorScheme.errorContainer, + modifier = Modifier.fillMaxWidth() + ) { + Text( + text = message, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onErrorContainer, + modifier = Modifier.padding(12.dp) + ) + } + } + + MonospaceSection( + title = stringResource(R.string.debugoverlay_crash_log_stack_trace), + text = record.stackTrace + ) + + if (record.logcatLogs.isNotEmpty()) { + MonospaceSection( + title = stringResource(R.string.debugoverlay_crash_log_logcat, record.logcatLogs.size), + text = record.logcatLogs.joinToString("\n") { it.toClipboardText() } + ) + } + + record.customLogSourceData?.let { customLogs -> + if (customLogs.logs.isNotEmpty()) { + MonospaceSection( + title = "${customLogs.sourceName} (${customLogs.logs.size})", + text = customLogs.logs.joinToString("\n") { it.toClipboardText() } + ) + } + } + + if (record.networkRequests.isNotEmpty()) { + MonospaceSection( + title = stringResource(R.string.debugoverlay_crash_log_network_requests, record.networkRequests.size), + text = record.networkRequests.joinToString("\n") { + "${formatFullTimestamp(it.timestampMs)} ${it.method} ${it.url} -> " + + "${it.statusCode ?: "?"} (${it.durationMs}ms)" + } + ) + } + } + } + + CopyButton(record = record) + } +} + +@Composable +private fun MonospaceSection(title: String, text: String, modifier: Modifier = Modifier) { + Column(modifier = modifier.fillMaxWidth()) { + Text( + text = title, + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + fontWeight = FontWeight.SemiBold, + modifier = Modifier.padding(bottom = 8.dp) + ) + Surface( + shape = MaterialTheme.shapes.small, + color = MaterialTheme.colorScheme.surfaceContainer, + modifier = Modifier.fillMaxWidth() + ) { + Text( + text = text, + style = MaterialTheme.typography.bodySmall, + fontFamily = FontFamily.Monospace, + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier + .horizontalScroll(rememberScrollState()) + .padding(12.dp) + ) + } + } +} + +@Composable +private fun CopyButton(record: CrashRecord, modifier: Modifier = Modifier) { + val clipboard = LocalClipboard.current + val context = LocalContext.current + val scope = rememberCoroutineScope() + + Button( + onClick = { + val clipboardLabel = context.getString(R.string.debugoverlay_crash_log_clipboard_label) + scope.copyToClipboard(clipboard, formatCrashRecordAsText(record), clipboardLabel) + }, + modifier = modifier + .fillMaxWidth() + .padding(16.dp) + ) { + Text(stringResource(R.string.debugoverlay_copy)) + } +} diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt new file mode 100644 index 00000000..20bde88d --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt @@ -0,0 +1,177 @@ +package com.ms.square.debugoverlay.internal.ui + +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.role +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import androidx.compose.ui.unit.sp +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.ms.square.debugoverlay.core.R +import com.ms.square.debugoverlay.internal.crash.CrashRecordInfo +import com.ms.square.debugoverlay.internal.util.formatRelativeTime +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.launch + +/** + * Crash tab content displaying [CrashRecordInfo]s persisted by + * [com.ms.square.debugoverlay.internal.crash.CrashHandler] from previous app runs. + * + * Only shown when at least one crash record exists (see `hasCrashRecords` gating in + * DebugPanelDialog), so an empty-state fallback is not needed for the list, only within it. + */ +@Composable +internal fun CrashLogTabContent( + crashRecordsFlow: Flow>, + onDeleteCrashRecord: suspend (CrashRecordInfo) -> Unit, + modifier: Modifier = Modifier, +) { + val crashRecords by crashRecordsFlow.collectAsStateWithLifecycle(initialValue = emptyList()) + var selected by remember { mutableStateOf(null) } + val scope = rememberCoroutineScope() + + DetailNavigation( + selectedItem = selected, + onBack = { selected = null }, + listContent = { + if (crashRecords.isEmpty()) { + EmptyCrashHistoryState() + } else { + CrashLogListScreen( + crashRecords = crashRecords, + onItemClick = { selected = it } + ) + } + }, + detailContent = { info -> + CrashLogDetailScreen( + info = info, + onBack = { selected = null }, + onDelete = { + scope.launch { + onDeleteCrashRecord(info) + selected = null + } + } + ) + }, + modifier = modifier + ) +} + +@Composable +private fun CrashLogListScreen( + crashRecords: List, + onItemClick: (CrashRecordInfo) -> Unit, + modifier: Modifier = Modifier, +) { + LazyColumn( + modifier = modifier.fillMaxSize(), + contentPadding = PaddingValues(16.dp), + verticalArrangement = Arrangement.spacedBy(12.dp) + ) { + items(crashRecords, key = { it.record.id }) { info -> + CrashLogItem(info = info, onClick = { onItemClick(info) }) + } + } +} + +@Composable +private fun CrashLogItem(info: CrashRecordInfo, onClick: () -> Unit, modifier: Modifier = Modifier) { + val record = info.record + val timeStamp = remember(record.timestampMs) { formatRelativeTime(record.timestampMs) } + val itemDescription = stringResource( + R.string.debugoverlay_crash_log_item_description, + record.exceptionType, + timeStamp + ) + + Surface( + modifier = modifier + .fillMaxWidth() + .semantics(mergeDescendants = true) { + contentDescription = itemDescription + role = Role.Button + } + .clickable { onClick() }, + shape = MaterialTheme.shapes.medium, + color = MaterialTheme.colorScheme.surfaceContainerLowest, + tonalElevation = 1.dp + ) { + Column(modifier = Modifier.padding(vertical = 12.dp, horizontal = 16.dp)) { + Text( + text = record.exceptionType, + style = MaterialTheme.typography.titleSmall, + color = MaterialTheme.colorScheme.error, + maxLines = 1, + overflow = TextOverflow.Ellipsis + ) + record.message?.let { message -> + Text( + text = message, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.padding(top = 4.dp) + ) + } + Text( + text = timeStamp, + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 8.dp) + ) + } + } +} + +@Composable +private fun EmptyCrashHistoryState(modifier: Modifier = Modifier) { + Box( + modifier = modifier.fillMaxSize(), + contentAlignment = Alignment.Center + ) { + Column(horizontalAlignment = Alignment.CenterHorizontally) { + Text("✅", fontSize = 48.sp) + Spacer(Modifier.height(16.dp)) + Text( + text = stringResource(R.string.debugoverlay_crash_log_empty_title), + style = MaterialTheme.typography.titleMedium + ) + Text( + text = stringResource(R.string.debugoverlay_crash_log_empty_subtitle), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + textAlign = TextAlign.Center, + modifier = Modifier.padding(top = 8.dp) + ) + } + } +} diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt index afd7e330..1c099153 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt @@ -66,6 +66,7 @@ private enum class BuiltInTab(@param:StringRes val titleResId: Int) { LOGCAT(R.string.debugoverlay_tab_logcat), CUSTOM_LOG(R.string.debugoverlay_tab_custom_log), // Fallback; UI uses dynamic title from source APP_EXITS(R.string.debugoverlay_tab_app_exits), + CRASH_LOG(R.string.debugoverlay_tab_crash_log), // Only shown when a persisted crash record exists NETWORK(R.string.debugoverlay_tab_network), JANKSTATS(R.string.debugoverlay_tab_jankstats), UI(R.string.debugoverlay_tab_ui), @@ -265,13 +266,15 @@ private fun bugReportButtonDescription(isCapturing: Boolean, draftCount: Int) = private fun DebugPanelContent(isCompactHeight: Boolean, modifier: Modifier = Modifier) { val repository = DebugOverlay.overlayDataRepository val hasCustomLogSource by repository.hasCustomLogSource.collectAsStateWithLifecycle() + val hasCrashRecords by repository.hasCrashRecords.collectAsStateWithLifecycle() val customLogSourceName by repository.customLogSourceName.collectAsStateWithLifecycle() val customTabs = (DebugOverlay.config.overlayMode as? OverlayMode.WithCustomTabs)?.customTabs.orEmpty() - // Build visible tabs: built-in tabs (with CUSTOM_LOG conditionally shown) + custom tabs - val visibleTabs = remember(hasCustomLogSource, customTabs) { + // Build visible tabs: built-in tabs (with CUSTOM_LOG/CRASH_LOG conditionally shown) + custom tabs + val visibleTabs = remember(hasCustomLogSource, hasCrashRecords, customTabs) { val builtIn = BuiltInTab.entries .filter { it != BuiltInTab.CUSTOM_LOG || hasCustomLogSource } + .filter { it != BuiltInTab.CRASH_LOG || hasCrashRecords } .map { PanelTab.BuiltIn(it) } builtIn + customTabs.map { PanelTab.Custom(it) } } @@ -345,6 +348,10 @@ private fun DebugPanelTabContent( exitInfosFlow = repository.appExitInfos, isSupported = repository.isAppExitSupported ) + BuiltInTab.CRASH_LOG -> CrashLogTabContent( + crashRecordsFlow = repository.crashRecords, + onDeleteCrashRecord = { repository.deleteCrashRecord(it) } + ) BuiltInTab.NETWORK -> NetworkTabContent( netStatsFlow = repository.netStats, networkRequestsFlow = repository.networkRequests, diff --git a/debugoverlay-core/src/main/res/values/strings.xml b/debugoverlay-core/src/main/res/values/strings.xml index 707de8f9..0eff3642 100644 --- a/debugoverlay-core/src/main/res/values/strings.xml +++ b/debugoverlay-core/src/main/res/values/strings.xml @@ -15,6 +15,7 @@ JankStats UI DeviceInfo + Crash Back @@ -52,6 +53,17 @@ Importance AppExit Info + + No Crashes Recorded + Crashes will appear here after your app\nrestarts following an unhandled exception. + Crash: %1$s, %2$s + Stack Trace + Logcat (%1$d) + Network Requests (%1$d) + Crash Info + Share Crash Log + Delete crash record + Search requests… DOWNLOADED diff --git a/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml b/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml index d3a68f38..34b5f96c 100644 --- a/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml +++ b/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml @@ -1,10 +1,14 @@ + diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt new file mode 100644 index 00000000..ada82fa7 --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt @@ -0,0 +1,125 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.google.common.truth.Truth.assertThat +import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData +import com.ms.square.debugoverlay.model.LogEntry +import com.ms.square.debugoverlay.model.LogLevel +import com.ms.square.debugoverlay.model.NetworkRequest +import org.junit.Test + +class CrashHandlerTest { + + private val previousHandler = FakeUncaughtExceptionHandler() + private val storage = FakeCrashRecordStorage() + + private fun createHandler( + logs: List = emptyList(), + customLogs: CustomLogSourceData? = null, + networkRequests: List = emptyList(), + isEnabled: Boolean = true, + maxLogLines: Int = 100, + ) = CrashHandler( + previousHandler = previousHandler, + storage = storage, + cachedAppInfo = null, + logcatSnapshotProvider = { logs }, + customLogSnapshotProvider = { customLogs }, + networkRequestsSnapshotProvider = { networkRequests }, + isEnabled = { isEnabled }, + maxLogLines = { maxLogLines } + ) + + @Test + fun `uncaughtException writes a crash record and delegates to the previous handler`() { + val handler = createHandler() + val thread = Thread.currentThread() + val throwable = IllegalStateException("boom") + + handler.uncaughtException(thread, throwable) + + val written = storage.written + assertThat(written).isNotNull() + assertThat(written!!.threadName).isEqualTo(thread.name) + assertThat(written.exceptionType).isEqualTo("java.lang.IllegalStateException") + assertThat(written.message).isEqualTo("boom") + assertThat(previousHandler.invokedWith).isEqualTo(thread to throwable) + } + + @Test + fun `uncaughtException does not write when disabled but still delegates`() { + val handler = createHandler(isEnabled = false) + + handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) + + assertThat(storage.written).isNull() + assertThat(previousHandler.invokedWith).isNotNull() + } + + @Test + fun `uncaughtException delegates to previous handler even when storage write throws`() { + storage.shouldThrowOnWrite = true + val handler = createHandler() + + handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) + + assertThat(previousHandler.invokedWith).isNotNull() + } + + @Test + fun `uncaughtException trims logs, custom logs, and network requests to maxLogLines`() { + val logs = (1..10).map { fakeLogEntry(it) } + val customLogs = CustomLogSourceData(logs = (1..10).map { fakeLogEntry(it) }, sourceName = "Timber") + val requests = (1..10).map { fakeNetworkRequest(it) } + val handler = createHandler(logs = logs, customLogs = customLogs, networkRequests = requests, maxLogLines = 3) + + handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) + + val written = storage.written!! + assertThat(written.logcatLogs).hasSize(3) + assertThat(written.logcatLogs).isEqualTo(logs.takeLast(3)) + assertThat(written.customLogSourceData!!.logs).hasSize(3) + assertThat(written.networkRequests).hasSize(3) + } + + private fun fakeLogEntry(index: Int) = LogEntry( + timestampMs = index.toLong(), + level = LogLevel.INFO, + tag = "Test", + pid = 1, + tid = 1, + threadName = "main", + message = "message $index" + ) + + private fun fakeNetworkRequest(index: Int) = NetworkRequest( + protocol = "http/1.1", + method = "GET", + url = "https://example.com/$index", + statusCode = 200, + durationMs = 10L, + responseSize = 100L, + requestSize = 0L, + timestampMs = index.toLong() + ) +} + +private class FakeCrashRecordStorage : CrashRecordStorage { + var written: CrashRecord? = null + var shouldThrowOnWrite: Boolean = false + + override fun writeSync(record: CrashRecord) { + if (shouldThrowOnWrite) error("simulated write failure") + written = record + } + + override suspend fun listCrashRecords(): List = emptyList() + override suspend fun deleteCrashRecord(info: CrashRecordInfo) = Unit +} + +private class FakeUncaughtExceptionHandler : Thread.UncaughtExceptionHandler { + var invokedWith: Pair? = null + + override fun uncaughtException(t: Thread, e: Throwable) { + invokedWith = t to e + } +} diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt new file mode 100644 index 00000000..dfc6ce43 --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt @@ -0,0 +1,80 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.io.File + +private const val BASE_TIMESTAMP_MS = 1_700_000_000_000L + +@RunWith(RobolectricTestRunner::class) +class CrashRecordStorageTest { + + private val storage = DefaultCrashRecordStorage( + context = RuntimeEnvironment.getApplication(), + maxRecords = 3 + ) + + @Test + fun `writeSync evicts oldest records beyond maxRecords`() = runTest { + repeat(5) { index -> storage.writeSync(fakeRecord(index)) } + + val records = storage.listCrashRecords() + + assertThat(records).hasSize(3) + } + + @Test + fun `listCrashRecords returns most recent first`() = runTest { + repeat(5) { index -> storage.writeSync(fakeRecord(index)) } + + val records = storage.listCrashRecords() + + // The 3 retained records are the 3 most recently written (indices 2, 3, 4), newest first. + assertThat(records.map { it.record.timestampMs }) + .containsExactly( + BASE_TIMESTAMP_MS + 4_000, + BASE_TIMESTAMP_MS + 3_000, + BASE_TIMESTAMP_MS + 2_000 + ) + .inOrder() + } + + @Test + fun `deleteCrashRecord removes the record`() = runTest { + storage.writeSync(fakeRecord(0)) + val info = storage.listCrashRecords().single() + + storage.deleteCrashRecord(info) + + assertThat(storage.listCrashRecords()).isEmpty() + } + + @Test + fun `deleteCrashRecord refuses to delete a file outside the records directory`() = runTest { + storage.writeSync(fakeRecord(0)) + val outsideFile = File.createTempFile("crash_outside", ".json") + outsideFile.writeText("not a real record") + val maliciousInfo = CrashRecordInfo(filePath = outsideFile.absolutePath, record = fakeRecord(0)) + + storage.deleteCrashRecord(maliciousInfo) + + assertThat(outsideFile.exists()).isTrue() + outsideFile.delete() + } + + private fun fakeRecord(index: Int) = CrashRecord( + timestampMs = BASE_TIMESTAMP_MS + index * 1_000L, + threadName = "main", + exceptionType = "java.lang.RuntimeException", + message = "boom $index", + stackTrace = "java.lang.RuntimeException: boom $index", + appInfo = null, + logcatLogs = emptyList(), + customLogSourceData = null, + networkRequests = emptyList() + ) +} diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt new file mode 100644 index 00000000..9ad3f56e --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt @@ -0,0 +1,79 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.google.common.truth.Truth.assertThat +import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData +import com.ms.square.debugoverlay.model.LogEntry +import com.ms.square.debugoverlay.model.LogLevel +import com.ms.square.debugoverlay.model.NetworkRequest +import org.junit.Test + +class CrashRecordTextFormatterTest { + + private val baseRecord = CrashRecord( + timestampMs = 1_700_000_000_000L, + threadName = "main", + exceptionType = "java.lang.IllegalStateException", + message = "boom", + stackTrace = "java.lang.IllegalStateException: boom\n\tat Foo.bar(Foo.kt:1)", + appInfo = null, + logcatLogs = emptyList(), + customLogSourceData = null, + networkRequests = emptyList() + ) + + @Test + fun `includes exception type, message, and stack trace`() { + val text = formatCrashRecordAsText(baseRecord) + + assertThat(text).contains("java.lang.IllegalStateException: boom") + assertThat(text).contains("--- STACK TRACE ---") + assertThat(text).contains("at Foo.bar(Foo.kt:1)") + } + + @Test + fun `omits log and network sections when empty`() { + val text = formatCrashRecordAsText(baseRecord) + + assertThat(text).doesNotContain("--- LOGCAT") + assertThat(text).doesNotContain("--- NETWORK REQUESTS") + } + + @Test + fun `includes logcat, custom log, and network sections when present`() { + val record = baseRecord.copy( + logcatLogs = listOf(fakeLogEntry("logcat line")), + customLogSourceData = CustomLogSourceData(logs = listOf(fakeLogEntry("timber line")), sourceName = "Timber"), + networkRequests = listOf( + NetworkRequest( + protocol = "http/1.1", + method = "GET", + url = "https://example.com", + statusCode = 200, + durationMs = 42L, + responseSize = 100L, + requestSize = 0L, + timestampMs = 1_700_000_000_000L + ) + ) + ) + + val text = formatCrashRecordAsText(record) + + assertThat(text).contains("--- LOGCAT (1) ---") + assertThat(text).contains("logcat line") + assertThat(text).contains("--- TIMBER (1) ---") + assertThat(text).contains("timber line") + assertThat(text).contains("--- NETWORK REQUESTS (1) ---") + assertThat(text).contains("GET https://example.com -> 200 (42ms)") + } + + private fun fakeLogEntry(message: String) = LogEntry( + timestampMs = 1_700_000_000_000L, + level = LogLevel.INFO, + tag = "Test", + pid = 1, + tid = 1, + threadName = "main", + message = message + ) +} diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt new file mode 100644 index 00000000..7ad5cb7c --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt @@ -0,0 +1,22 @@ +package com.ms.square.debugoverlay.internal.data.source + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import org.junit.Test + +class LogcatDataSourceTest { + + /** + * [LogcatDataSource.logs] is backed by `stateIn(SharingStarted.WhileSubscribed())`, so + * constructing the data source never starts the `logcat` subprocess on its own — + * only subscribing to [LogcatDataSource.logs] does. This keeps + * [LogcatDataSource.snapshotEntriesSync] safe to exercise without spawning a process. + */ + private val dataSource = LogcatDataSource(CoroutineScope(Job()), initialMaxEntries = 10) + + @Test + fun `snapshotEntriesSync returns empty list before Logcat has ever been subscribed`() { + assertThat(dataSource.snapshotEntriesSync()).isEmpty() + } +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7b407d30..849b7646 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -105,6 +105,7 @@ The overlay uses a synthetic `OverlayLifecycleOwner` to provide Compose lifecycl | **Bounded collections** | `EvictingQueue` prevents OOM from unbounded log/request accumulation | | **Main process only** | Overlay is per-process singleton; no multi-process complexity | | **Extensions depend on core** | Loose coupling via interfaces; core has no knowledge of extensions | +| **Crash handler always chains** | `CrashHandler` captures `Thread.getDefaultUncaughtExceptionHandler()` once at install time and always delegates to it (in a `finally` block, even if its own capture logic throws), so other crash reporters (e.g. Crashlytics) installed before or after DebugOverlay keep working. It never calls `Process.killProcess()`/`exitProcess()` itself. | ## Key Architectural Decisions @@ -117,6 +118,7 @@ The overlay uses a synthetic `OverlayLifecycleOwner` to provide Compose lifecycl | **Synthetic LifecycleOwner** | Enables Compose lifecycle APIs for overlay outside activity hierarchy | | **PixelCopy for screenshots** | Hardware-accelerated capture (API 26+); Canvas fallback for older | | **No-backup storage for drafts** | Bug report drafts persist across launches but don't sync to cloud | +| **No-backup storage for crash records** | Persisted crash records survive process death like drafts, in a separate directory with its own retention policy | ## Third-Party Dependencies From 1d5751191fb06c6eaab317160a082133a325f1bf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 00:26:56 +0000 Subject: [PATCH 02/12] refactor: drop unused Config.persistCrashLogs/maxCrashLogLines knobs Crash persistence is now always-on with a fixed internal line cap, matching the issue's ask instead of exposing configurability nobody requested. --- .../ms/square/debugoverlay/DebugOverlay.kt | 45 ++----------------- .../internal/crash/CrashHandler.kt | 15 +++---- .../internal/crash/CrashHandlerTest.kt | 14 +----- 3 files changed, 9 insertions(+), 65 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt index 7e620aff..72d653ef 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt @@ -114,8 +114,7 @@ public object DebugOverlay { * Chains a [CrashHandler] in front of whatever [Thread.UncaughtExceptionHandler] was * previously installed (the platform default, or another crash reporter like * Crashlytics), so persisting a crash record never interferes with existing crash - * handling. Captured exactly once here; never re-installed on [configure] calls, since - * [CrashHandler] reads [config] live via lambdas. + * handling. Captured exactly once here. */ private fun installCrashHandler(application: Application, repository: DebugOverlayDataRepository) { val previousHandler = Thread.getDefaultUncaughtExceptionHandler() @@ -127,9 +126,7 @@ public object DebugOverlay { cachedAppInfo = cachedAppInfo, logcatSnapshotProvider = { repository.logcatSnapshotSync() }, customLogSnapshotProvider = { repository.customLogSnapshotSync() }, - networkRequestsSnapshotProvider = { repository.networkRequestsSnapshotSync() }, - isEnabled = { config.persistCrashLogs }, - maxLogLines = { config.maxCrashLogLines } + networkRequestsSnapshotProvider = { repository.networkRequestsSnapshotSync() } ) ) } @@ -272,38 +269,12 @@ public object DebugOverlay { field = value } - /** - * Whether to persist the last log lines and the exception's stack trace to disk when - * an uncaught exception occurs, so they survive process death and can be reviewed or - * exported from the Crash tab on the next launch. - * - * Default is `true`. - */ - public var persistCrashLogs: Boolean = initial.persistCrashLogs - - /** - * Maximum number of recent entries kept per source (Logcat, custom log source, - * network requests) in a persisted crash record. - * - * Default is [Config.DEFAULT_MAX_CRASH_LOG_LINES] (100). - * - * @throws IllegalArgumentException if assigned a negative value. - */ - @IntRange(from = 0) - public var maxCrashLogLines: Int = initial.maxCrashLogLines - set(value) { - require(value >= 0) { "maxCrashLogLines must be non-negative, was $value" } - field = value - } - internal fun build(): Config = Config( overlayMode = overlayMode, networkRequestSource = networkRequestSource, customLogSource = customLogSource, bugReportExporter = bugReportExporter, - maxLogcatEntries = maxLogcatEntries, - persistCrashLogs = persistCrashLogs, - maxCrashLogLines = maxCrashLogLines + maxLogcatEntries = maxLogcatEntries ) } @@ -324,11 +295,6 @@ public object DebugOverlay { * Default is the built-in share sheet exporter. * @property maxLogcatEntries Maximum number of entries kept in the built-in Logcat * tab buffer. Default is [DEFAULT_MAX_LOGCAT_ENTRIES]. - * @property persistCrashLogs Whether to persist the last log lines and the exception's - * stack trace to disk on an uncaught exception, reviewable from the Crash tab on the - * next launch. Default is `true`. - * @property maxCrashLogLines Maximum number of recent entries kept per source in a - * persisted crash record. Default is [DEFAULT_MAX_CRASH_LOG_LINES]. * * @see configure */ @@ -338,15 +304,10 @@ public object DebugOverlay { val customLogSource: LogSource? = null, val bugReportExporter: BugReportExporter = IntentShareExporter, val maxLogcatEntries: Int = DEFAULT_MAX_LOGCAT_ENTRIES, - val persistCrashLogs: Boolean = true, - val maxCrashLogLines: Int = DEFAULT_MAX_CRASH_LOG_LINES, ) { public companion object { /** Default value for [maxLogcatEntries]. */ public const val DEFAULT_MAX_LOGCAT_ENTRIES: Int = 300 - - /** Default value for [maxCrashLogLines]. */ - public const val DEFAULT_MAX_CRASH_LOG_LINES: Int = 100 } } } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt index af05323e..db6672c9 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt @@ -26,10 +26,7 @@ import kotlin.system.exitProcess * @param logcatSnapshotProvider Non-suspending snapshot of the in-memory Logcat buffer. * @param customLogSnapshotProvider Non-suspending snapshot of the custom log source, if any. * @param networkRequestsSnapshotProvider Non-suspending snapshot of recent network requests. - * @param isEnabled Whether persistence is currently enabled. Read live at crash time so - * toggling [com.ms.square.debugoverlay.DebugOverlay.configure] doesn't require - * reinstalling this handler. - * @param maxLogLines Maximum number of entries kept per log/request source, read live. + * @param maxLogLines Maximum number of entries kept per log/request source. */ internal class CrashHandler( private val previousHandler: Thread.UncaughtExceptionHandler?, @@ -38,16 +35,13 @@ internal class CrashHandler( private val logcatSnapshotProvider: () -> List, private val customLogSnapshotProvider: () -> CustomLogSourceData?, private val networkRequestsSnapshotProvider: () -> List, - private val isEnabled: () -> Boolean, - private val maxLogLines: () -> Int, + private val maxLogLines: Int = DEFAULT_MAX_LOG_LINES, ) : Thread.UncaughtExceptionHandler { @Suppress("TooGenericExceptionCaught") override fun uncaughtException(thread: Thread, throwable: Throwable) { try { - if (isEnabled()) { - storage.writeSync(buildCrashRecord(thread, throwable)) - } + storage.writeSync(buildCrashRecord(thread, throwable)) } catch (t: Throwable) { // Deliberately broad: capture failure must never prevent the delegate call below // from running, since that's what keeps other crash reporters (e.g. Crashlytics) @@ -59,7 +53,7 @@ internal class CrashHandler( } private fun buildCrashRecord(thread: Thread, throwable: Throwable): CrashRecord { - val maxLines = maxLogLines().coerceAtLeast(0) + val maxLines = maxLogLines.coerceAtLeast(0) val customLogSnapshot = customLogSnapshotProvider()?.let { data -> data.copy(logs = data.logs.takeLast(maxLines)) } @@ -91,5 +85,6 @@ internal class CrashHandler( private companion object { const val EXIT_CODE_UNCAUGHT_EXCEPTION = 10 + const val DEFAULT_MAX_LOG_LINES = 100 } } diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt index ada82fa7..39a49eaa 100644 --- a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt @@ -16,7 +16,6 @@ class CrashHandlerTest { logs: List = emptyList(), customLogs: CustomLogSourceData? = null, networkRequests: List = emptyList(), - isEnabled: Boolean = true, maxLogLines: Int = 100, ) = CrashHandler( previousHandler = previousHandler, @@ -25,8 +24,7 @@ class CrashHandlerTest { logcatSnapshotProvider = { logs }, customLogSnapshotProvider = { customLogs }, networkRequestsSnapshotProvider = { networkRequests }, - isEnabled = { isEnabled }, - maxLogLines = { maxLogLines } + maxLogLines = maxLogLines ) @Test @@ -45,16 +43,6 @@ class CrashHandlerTest { assertThat(previousHandler.invokedWith).isEqualTo(thread to throwable) } - @Test - fun `uncaughtException does not write when disabled but still delegates`() { - val handler = createHandler(isEnabled = false) - - handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) - - assertThat(storage.written).isNull() - assertThat(previousHandler.invokedWith).isNotNull() - } - @Test fun `uncaughtException delegates to previous handler even when storage write throws`() { storage.shouldThrowOnWrite = true From 6f837a7c1380da34bc6e9d370db5405cb402a5ec Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 05:03:56 +0000 Subject: [PATCH 03/12] refactor: extract File.isDirectChildOf() shared by both storage classes CrashRecordStorage and BugReportDraftStorage each had an identical canonical-path safety check before delete, just under different names. --- .../internal/bugreport/BugReportDraftStorage.kt | 17 ++--------------- .../internal/crash/CrashRecordStorage.kt | 14 ++------------ .../square/debugoverlay/internal/util/Files.kt | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/BugReportDraftStorage.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/BugReportDraftStorage.kt index ed9b1c90..d5c74757 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/BugReportDraftStorage.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/BugReportDraftStorage.kt @@ -20,6 +20,7 @@ import com.ms.square.debugoverlay.internal.bugreport.model.BugReportState import com.ms.square.debugoverlay.internal.bugreport.model.DraftInfo import com.ms.square.debugoverlay.internal.bugreport.model.UserInput import com.ms.square.debugoverlay.internal.util.checkFolderExists +import com.ms.square.debugoverlay.internal.util.isDirectChildOf import com.ms.square.debugoverlay.internal.util.runCatchingNonCancellation import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow @@ -257,7 +258,7 @@ internal class DefaultBugReportDraftStorage( override suspend fun deleteFolder(folder: File): Unit = withContext(Dispatchers.IO) { val deleted = folderMutex.withLock { // Safety check: only delete folders that are direct children of our drafts directory - if (!isDirectChildOfDraftsDir(folder)) { + if (!folder.isDirectChildOf(draftsDir)) { Logger.w("Refusing to delete folder outside drafts directory: ${folder.absolutePath}") return@withLock false } @@ -281,20 +282,6 @@ internal class DefaultBugReportDraftStorage( } } - /** - * Checks if the given folder is a direct child of [draftsDir]. - * - * Uses canonical paths to resolve symlinks and ".." traversal attacks. - */ - private fun isDirectChildOfDraftsDir(folder: File): Boolean = runCatching { - val canonicalFolder = folder.canonicalFile - val canonicalDraftsDir = draftsDir.canonicalFile - canonicalFolder.parentFile == canonicalDraftsDir - }.getOrElse { e -> - Logger.w("Failed to resolve canonical path for safety check: ${e.javaClass.simpleName} - ${e.message}") - false - } - // ========== Draft Management ========== /** diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt index 0f79beae..d0c66a47 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt @@ -3,6 +3,7 @@ package com.ms.square.debugoverlay.internal.crash import android.content.Context import com.ms.square.debugoverlay.internal.Logger import com.ms.square.debugoverlay.internal.util.checkFolderExists +import com.ms.square.debugoverlay.internal.util.isDirectChildOf import com.ms.square.debugoverlay.internal.util.runCatchingNonCancellation import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -86,7 +87,7 @@ internal class DefaultCrashRecordStorage( override suspend fun deleteCrashRecord(info: CrashRecordInfo): Unit = withContext(Dispatchers.IO) { val file = info.file - if (!isDirectChildOfRecordsDir(file)) { + if (!file.isDirectChildOf(recordsDir)) { Logger.w("Refusing to delete crash record outside records directory: ${file.absolutePath}") return@withContext } @@ -95,17 +96,6 @@ internal class DefaultCrashRecordStorage( } } - /** - * Checks if the given file is a direct child of [recordsDir]. - * Uses canonical paths to resolve symlinks and ".." traversal attacks. - */ - private fun isDirectChildOfRecordsDir(file: File): Boolean = runCatching { - file.canonicalFile.parentFile == recordsDir.canonicalFile - }.getOrElse { e -> - Logger.w("Failed to resolve canonical path for safety check: ${e.javaClass.simpleName} - ${e.message}") - false - } - private fun loadRecord(file: File): CrashRecord? = runCatchingNonCancellation { json.decodeFromString(CrashRecord.serializer(), file.readText()) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/Files.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/Files.kt index 9c356d47..bdd8daf9 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/Files.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/Files.kt @@ -1,5 +1,6 @@ package com.ms.square.debugoverlay.internal.util +import com.ms.square.debugoverlay.internal.Logger import java.io.File /** @@ -9,3 +10,18 @@ import java.io.File internal fun File.checkFolderExists() { check(mkdirs() || isDirectory) { "Failed to create a folder: $absolutePath" } } + +/** + * Checks whether this file/folder is a direct child of [dir], resolving canonical paths + * to guard against symlinks and ".." traversal. + * + * Intended as a safety check before destructive operations (e.g. delete) on a + * caller-supplied path, to make sure it can't escape the directory it's expected to + * live in. + */ +internal fun File.isDirectChildOf(dir: File): Boolean = runCatching { + canonicalFile.parentFile == dir.canonicalFile +}.getOrElse { e -> + Logger.w("Failed to resolve canonical path for safety check: ${e.javaClass.simpleName} - ${e.message}") + false +} From 909382da2bdb8835973b682acc4dd706c3bf3756 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 05:13:36 +0000 Subject: [PATCH 04/12] feat: surface network error in crash export summary lines A failed request previously rendered identically to a successful one in the crash text export apart from a null status code. Appending the error title/message (not the full stack trace) keeps each line scannable while surfacing the one detail most likely to matter for crash triage. --- .../crash/CrashRecordTextFormatter.kt | 7 ++++-- .../crash/CrashRecordTextFormatterTest.kt | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt index e7bcf073..216a82be 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt @@ -47,5 +47,8 @@ internal fun formatCrashRecordAsText(record: CrashRecord): String = buildString } } -private fun NetworkRequest.toSummaryLine(): String = - "${formatFullTimestamp(timestampMs)} $method $url -> ${statusCode ?: "?"} (${durationMs}ms)" +private fun NetworkRequest.toSummaryLine(): String = buildString { + append(formatFullTimestamp(timestampMs)).append(' ').append(method).append(' ').append(url) + append(" -> ").append(statusCode ?: "?").append(" (").append(durationMs).append("ms)") + error?.let { append(" [ERROR: ${it.title}: ${it.message}]") } +} diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt index 9ad3f56e..e4a6b673 100644 --- a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt @@ -4,6 +4,7 @@ import com.google.common.truth.Truth.assertThat import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData import com.ms.square.debugoverlay.model.LogEntry import com.ms.square.debugoverlay.model.LogLevel +import com.ms.square.debugoverlay.model.NetworkError import com.ms.square.debugoverlay.model.NetworkRequest import org.junit.Test @@ -67,6 +68,29 @@ class CrashRecordTextFormatterTest { assertThat(text).contains("GET https://example.com -> 200 (42ms)") } + @Test + fun `appends error title and message for failed network requests`() { + val record = baseRecord.copy( + networkRequests = listOf( + NetworkRequest( + protocol = null, + method = "GET", + url = "https://example.com", + statusCode = null, + durationMs = 5_000L, + responseSize = null, + requestSize = 0L, + timestampMs = 1_700_000_000_000L, + error = NetworkError(title = "IOException", message = "Connection reset by peer") + ) + ) + ) + + val text = formatCrashRecordAsText(record) + + assertThat(text).contains("[ERROR: IOException: Connection reset by peer]") + } + private fun fakeLogEntry(message: String) = LogEntry( timestampMs = 1_700_000_000_000L, level = LogLevel.INFO, From ccb707ff94139c1b7226dd4fe133b801b6e7c8ff Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 05:44:46 +0000 Subject: [PATCH 05/12] perf: fetch crash-handler AppInfo off the app-startup critical path DebugOverlay.install() runs on the main thread before Application.onCreate() via AndroidX Startup, so the previous synchronous getAppInfo() call (multiple PackageManager IPC calls) directly delayed cold start. It's now fetched in a background coroutine and read via an AtomicReference-backed provider lambda; a crash before that completes just gets a null appInfo, same as any other best-effort field in this codebase. --- .../ms/square/debugoverlay/DebugOverlay.kt | 26 +++++++++++--- .../internal/crash/CrashHandler.kt | 10 +++--- .../internal/crash/CrashHandlerTest.kt | 34 ++++++++++++++++++- 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt index 72d653ef..e382c733 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt @@ -14,6 +14,7 @@ import com.ms.square.debugoverlay.internal.OverlayViewManager import com.ms.square.debugoverlay.internal.bugreport.BugReportGenerator import com.ms.square.debugoverlay.internal.bugreport.DefaultAppInfoProvider import com.ms.square.debugoverlay.internal.bugreport.IntentShareExporter +import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo import com.ms.square.debugoverlay.internal.bugreport.validateFilename import com.ms.square.debugoverlay.internal.crash.CrashHandler import com.ms.square.debugoverlay.internal.crash.DefaultCrashRecordStorage @@ -24,7 +25,9 @@ import com.ms.square.debugoverlay.internal.util.isMainProcess import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.atomic.AtomicReference /** * Internal entry point for DebugOverlay auto-installers. @@ -106,7 +109,7 @@ public object DebugOverlay { activityProvider = viewManager ) - installCrashHandler(application, repository) + installCrashHandler(application, scope, repository) } } @@ -115,15 +118,30 @@ public object DebugOverlay { * previously installed (the platform default, or another crash reporter like * Crashlytics), so persisting a crash record never interferes with existing crash * handling. Captured exactly once here. + * + * App info is fetched in the background on [scope] rather than inline here: [install] + * runs on the main thread before [Application.onCreate] (via AndroidX Startup), so any + * blocking work here — [DefaultAppInfoProvider.getAppInfo] does PackageManager IPC + * calls — directly delays app startup. [AtomicReference] gives the background write and + * the (possibly cross-thread, possibly-crashing-thread) read proper visibility without + * needing a coroutine to read it back. */ - private fun installCrashHandler(application: Application, repository: DebugOverlayDataRepository) { + private fun installCrashHandler( + application: Application, + scope: CoroutineScope, + repository: DebugOverlayDataRepository, + ) { val previousHandler = Thread.getDefaultUncaughtExceptionHandler() - val cachedAppInfo = runCatching { DefaultAppInfoProvider.getAppInfo(application) }.getOrNull() + val cachedAppInfo = AtomicReference(null) + scope.launch(Dispatchers.IO) { + cachedAppInfo.set(runCatching { DefaultAppInfoProvider.getAppInfo(application) }.getOrNull()) + } + Thread.setDefaultUncaughtExceptionHandler( CrashHandler( previousHandler = previousHandler, storage = DefaultCrashRecordStorage(application), - cachedAppInfo = cachedAppInfo, + cachedAppInfoProvider = cachedAppInfo::get, logcatSnapshotProvider = { repository.logcatSnapshotSync() }, customLogSnapshotProvider = { repository.customLogSnapshotSync() }, networkRequestsSnapshotProvider = { repository.networkRequestsSnapshotSync() } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt index db6672c9..4911e0ee 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt @@ -21,8 +21,10 @@ import kotlin.system.exitProcess * at install time. Never re-fetched, so a handler installed by another SDK *after* * DebugOverlay is never clobbered. * @param storage Where the crash record is written. - * @param cachedAppInfo App info captured once at install time (immutable for the process - * lifetime) — avoids PackageManager calls in the crash path. + * @param cachedAppInfoProvider Returns app info fetched once, off the main thread, shortly + * after install (see [com.ms.square.debugoverlay.DebugOverlay.install]) — never queried + * fresh here, since that would mean PackageManager IPC calls in the crash path. Returns + * null if a crash happens before that background fetch completes. * @param logcatSnapshotProvider Non-suspending snapshot of the in-memory Logcat buffer. * @param customLogSnapshotProvider Non-suspending snapshot of the custom log source, if any. * @param networkRequestsSnapshotProvider Non-suspending snapshot of recent network requests. @@ -31,7 +33,7 @@ import kotlin.system.exitProcess internal class CrashHandler( private val previousHandler: Thread.UncaughtExceptionHandler?, private val storage: CrashRecordStorage, - private val cachedAppInfo: AppInfo?, + private val cachedAppInfoProvider: () -> AppInfo?, private val logcatSnapshotProvider: () -> List, private val customLogSnapshotProvider: () -> CustomLogSourceData?, private val networkRequestsSnapshotProvider: () -> List, @@ -63,7 +65,7 @@ internal class CrashHandler( exceptionType = throwable.javaClass.name, message = throwable.message, stackTrace = throwable.stackTraceToString(), - appInfo = cachedAppInfo, + appInfo = cachedAppInfoProvider(), logcatLogs = logcatSnapshotProvider().takeLast(maxLines), customLogSourceData = customLogSnapshot, networkRequests = networkRequestsSnapshotProvider().takeLast(maxLines) diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt index 39a49eaa..e2adf5da 100644 --- a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt @@ -1,6 +1,7 @@ package com.ms.square.debugoverlay.internal.crash import com.google.common.truth.Truth.assertThat +import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData import com.ms.square.debugoverlay.model.LogEntry import com.ms.square.debugoverlay.model.LogLevel @@ -16,11 +17,12 @@ class CrashHandlerTest { logs: List = emptyList(), customLogs: CustomLogSourceData? = null, networkRequests: List = emptyList(), + appInfo: AppInfo? = null, maxLogLines: Int = 100, ) = CrashHandler( previousHandler = previousHandler, storage = storage, - cachedAppInfo = null, + cachedAppInfoProvider = { appInfo }, logcatSnapshotProvider = { logs }, customLogSnapshotProvider = { customLogs }, networkRequestsSnapshotProvider = { networkRequests }, @@ -43,6 +45,36 @@ class CrashHandlerTest { assertThat(previousHandler.invokedWith).isEqualTo(thread to throwable) } + @Test + fun `uncaughtException writes null appInfo when the background fetch has not completed yet`() { + val handler = createHandler(appInfo = null) + + handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) + + assertThat(storage.written!!.appInfo).isNull() + } + + @Test + fun `uncaughtException includes appInfo once the background fetch has completed`() { + val appInfo = AppInfo( + packageName = "com.test.app", + versionName = "1.0.0", + versionCode = 1, + targetSdkVersion = 34, + minSdkVersion = 21, + isDebuggable = true, + installerStore = "Unknown", + installerPackage = null, + firstInstallTime = 0L, + lastUpdateTime = 0L + ) + val handler = createHandler(appInfo = appInfo) + + handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) + + assertThat(storage.written!!.appInfo).isEqualTo(appInfo) + } + @Test fun `uncaughtException delegates to previous handler even when storage write throws`() { storage.shouldThrowOnWrite = true From 1c6d0daf3f164ffe013a035b3b66d6bd90661ad0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 06:06:15 +0000 Subject: [PATCH 06/12] simplify: drop the unreachable no-previous-handler fallback in CrashHandler The platform always installs a default UncaughtExceptionHandler before any app code runs, so previousHandler is never actually null here. Rather than reimplementing the platform's own kill-process fallback for a branch that can't be reached (and can't be safely unit-tested as written), just no-op: the crash record is already persisted by the time this runs. --- .../internal/crash/CrashHandler.kt | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt index 4911e0ee..04dca1a4 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt @@ -1,12 +1,12 @@ package com.ms.square.debugoverlay.internal.crash -import android.os.Process import com.ms.square.debugoverlay.internal.Logger import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData import com.ms.square.debugoverlay.model.LogEntry import com.ms.square.debugoverlay.model.NetworkRequest -import kotlin.system.exitProcess + +private const val DEFAULT_MAX_LOG_LINES = 100 /** * Persists a [CrashRecord] to disk on an uncaught exception, then always delegates to @@ -72,21 +72,11 @@ internal class CrashHandler( ) } + // previousHandler is effectively always non-null on real devices — the platform installs + // its own default handler before any app code runs, well before DebugOverlay.install() + // (see class doc). If it's ever null, the crash record above is already persisted; there's + // nothing more this library needs to do. private fun delegateToPreviousHandler(thread: Thread, throwable: Throwable) { - val handler = previousHandler - if (handler != null) { - handler.uncaughtException(thread, throwable) - } else { - // Unreachable on real devices: the platform always installs a default handler - // before Application.onCreate(). Guards test doubles / edge-case environments - // where none was ever installed, so the process still terminates. - Process.killProcess(Process.myPid()) - exitProcess(EXIT_CODE_UNCAUGHT_EXCEPTION) - } - } - - private companion object { - const val EXIT_CODE_UNCAUGHT_EXCEPTION = 10 - const val DEFAULT_MAX_LOG_LINES = 100 + previousHandler?.uncaughtException(thread, throwable) } } From 16add04d02dab8318481dec43d736ca5c6ea097e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 06:08:19 +0000 Subject: [PATCH 07/12] simplify: inline delegateToPreviousHandler into uncaughtException Down to one line after dropping the no-previous-handler fallback; no need for a separate function. --- .../debugoverlay/internal/crash/CrashHandler.kt | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt index 04dca1a4..9d43d950 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt @@ -50,7 +50,11 @@ internal class CrashHandler( // and the platform's own crash handling working. runCatching { Logger.e("CrashHandler failed to capture crash record", t) } } finally { - delegateToPreviousHandler(thread, throwable) + // previousHandler is effectively always non-null on real devices — the platform + // installs its own default handler before any app code runs, well before + // DebugOverlay.install() (see class doc). If it's ever null, the crash record above + // is already persisted; there's nothing more this library needs to do. + previousHandler?.uncaughtException(thread, throwable) } } @@ -71,12 +75,4 @@ internal class CrashHandler( networkRequests = networkRequestsSnapshotProvider().takeLast(maxLines) ) } - - // previousHandler is effectively always non-null on real devices — the platform installs - // its own default handler before any app code runs, well before DebugOverlay.install() - // (see class doc). If it's ever null, the crash record above is already persisted; there's - // nothing more this library needs to do. - private fun delegateToPreviousHandler(thread: Thread, throwable: Throwable) { - previousHandler?.uncaughtException(thread, throwable) - } } From 0dca7c89231d7e45b56d75dd2e3d0d492a54d635 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 06:18:48 +0000 Subject: [PATCH 08/12] perf: defer crash-record eviction off the synchronous crash-write path writeSync() (called from uncaughtException()) previously listed, sorted, and deleted old records on every write. Retention doesn't need real-time enforcement, so eviction now happens in listCrashRecords() instead - already off the crash path, on Dispatchers.IO, only when the Crash tab is opened. writeSync() is now just "build filename, write file." --- .../internal/crash/CrashRecordStorage.kt | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt index d0c66a47..93d02024 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt @@ -20,15 +20,23 @@ internal const val DEFAULT_MAX_CRASH_RECORDS = 5 */ internal sealed interface CrashRecordStorage { /** - * Writes [record] to disk and evicts old records beyond the retention limit. + * Writes [record] to disk. Does not evict old records — see [listCrashRecords]. * * Must be safe to call synchronously from [Thread.UncaughtExceptionHandler.uncaughtException]: - * no suspension, no dispatcher hop. Any failure is swallowed and logged — the caller - * must be able to unconditionally proceed to the previous crash handler afterward. + * no suspension, no dispatcher hop, no more work than writing this one file. Any failure + * is swallowed and logged — the caller must be able to unconditionally proceed to the + * previous crash handler afterward. */ fun writeSync(record: CrashRecord) - /** Loads all persisted crash records, most recent first. */ + /** + * Loads all persisted crash records, most recent first. + * + * Also evicts records beyond the retention limit first. Retention doesn't need real-time + * enforcement, so this is deferred here — off the crash path — rather than done in + * [writeSync]; between crashes, the on-disk count can transiently exceed the limit until + * this is next called. + */ suspend fun listCrashRecords(): List /** Deletes a single persisted crash record. */ @@ -53,8 +61,9 @@ internal class DefaultCrashRecordStorage( private val json = Json { ignoreUnknownKeys = true } // Plain JVM monitor, not a coroutines Mutex: writeSync() runs outside any coroutine - // context (it's called directly from uncaughtException()), and guards against two - // near-simultaneous crashes on different threads racing on the directory listing. + // context (it's called directly from uncaughtException()). Also guards the eviction + + // listing step in listCrashRecords() against a concurrent writeSync(), so a write and a + // list/evict can't race on the directory contents. private val writeLock = Any() private val recordsDir by lazy { @@ -67,7 +76,6 @@ internal class DefaultCrashRecordStorage( synchronized(writeLock) { val file = File(recordsDir, fileNameFor(record)) file.writeText(json.encodeToString(CrashRecord.serializer(), record)) - evictOldRecordsLocked() } } @@ -81,7 +89,13 @@ internal class DefaultCrashRecordStorage( } override suspend fun listCrashRecords(): List = withContext(Dispatchers.IO) { - val files = recordsDir.listFiles()?.filter { it.isFile }?.sortedDescending() ?: emptyList() + // Eviction happens here rather than in writeSync(): retention doesn't need real-time + // enforcement, only "eventually pruned back down" — deferring it off the crash path + // keeps writeSync() to the bare minimum needed before delegating to the previous handler. + val files = synchronized(writeLock) { + evictOldRecordsLocked() + recordsDir.listFiles()?.filter { it.isFile }?.sortedDescending() ?: emptyList() + } files.mapNotNull { file -> loadRecord(file)?.let { CrashRecordInfo(file.absolutePath, it) } } } From 94768511eaac0d1aa9c7c9dcd67c2df036ea6792 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 06:24:33 +0000 Subject: [PATCH 09/12] fix: refresh cached crash-records list after deleteCrashRecord() crashRecords was a one-shot flow cached via stateIn(Lazily), so deleting a record removed the file on disk but left the stale list in the UI until the app restarted. Switched to the same lazy-load-once + explicit-refresh pattern BugReportDraftStorage already uses for drafts, and re-run it after every delete. --- .../data/DebugOverlayDataRepository.kt | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt index 75a61ade..de22abfd 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt @@ -30,15 +30,17 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import java.util.concurrent.atomic.AtomicBoolean import kotlin.time.Duration.Companion.milliseconds /** Default name shown when a custom log source doesn't provide a source name. */ @@ -106,18 +108,35 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop val appExitInfos: Flow> = appExitDataSource.appExitInfos - // Persisted crash records from a prior run. Queried once per subscription (Lazily): - // this run's own crashes can't change mid-session since a crash terminates the process. - val crashRecords: Flow> = flow { emit(crashRecordStorage.listCrashRecords()) } - .flowOn(Dispatchers.IO) - .stateIn(scope, SharingStarted.Lazily, emptyList()) + // Persisted crash records from a prior run. Loaded once, lazily, on first subscription + // (mirrors BugReportDraftStorage's initDraftsIfNeeded()/refreshDrafts() pattern) — a + // crash terminates the process, so nothing can add a new record mid-session, but + // deleteCrashRecord() below explicitly re-syncs this after a deletion. + private val crashRecordsLoaded = AtomicBoolean(false) + private val _crashRecords = MutableStateFlow>(emptyList()) + + val crashRecords: Flow> = _crashRecords.asStateFlow() + .onStart { loadCrashRecordsIfNeeded() } val hasCrashRecords: StateFlow = crashRecords .map { it.isNotEmpty() } .distinctUntilChanged() .stateIn(scope, SharingStarted.Eagerly, false) - suspend fun deleteCrashRecord(info: CrashRecordInfo) = crashRecordStorage.deleteCrashRecord(info) + private suspend fun loadCrashRecordsIfNeeded() { + if (crashRecordsLoaded.compareAndSet(false, true)) { + refreshCrashRecords() + } + } + + private suspend fun refreshCrashRecords() { + _crashRecords.value = withContext(Dispatchers.IO) { crashRecordStorage.listCrashRecords() } + } + + suspend fun deleteCrashRecord(info: CrashRecordInfo) { + crashRecordStorage.deleteCrashRecord(info) + refreshCrashRecords() + } // Snapshot methods for bug reports (use cached value if available, otherwise query directly) suspend fun queryLogcatSnapshot(): List = logcatDataSource.queryLogcatSnapshot() From d87821021379193d9d10fca6c36246f59cd1d82e Mon Sep 17 00:00:00 2001 From: Manabu-GT Date: Sat, 8 Aug 2026 00:19:51 -0700 Subject: [PATCH 10/12] cr fixes --- AGENTS.md | 2 +- .../ms/square/debugoverlay/DebugOverlay.kt | 30 +---- .../internal/bugreport/IntentShareExporter.kt | 7 +- .../internal/bugreport/ui/BugReporterFab.kt | 5 +- .../internal/crash/CrashHandler.kt | 50 +------- .../internal/crash/CrashRecord.kt | 44 ++++++- .../internal/crash/CrashRecordBuilder.kt | 42 ++++++ .../internal/crash/CrashRecordExporter.kt | 6 +- .../internal/crash/CrashRecordStorage.kt | 54 +++++--- .../crash/CrashRecordTextFormatter.kt | 10 +- .../data/DebugOverlayDataRepository.kt | 110 ++++++++++------ .../internal/data/source/LogcatDataSource.kt | 84 ++++-------- .../ui/DraftBadge.kt => ui/CountBadge.kt} | 26 ++-- .../internal/ui/CrashLogDetailScreen.kt | 10 +- .../internal/ui/CrashLogTabContent.kt | 46 +++---- .../internal/ui/DebugPanelDialog.kt | 64 +++++++--- .../internal/util/FileProviders.kt | 20 +++ .../src/main/res/values/strings.xml | 1 + .../internal/crash/CrashHandlerTest.kt | 120 ++---------------- .../internal/crash/CrashRecordBuilderTest.kt | 100 +++++++++++++++ .../internal/crash/CrashRecordStorageTest.kt | 43 ++++++- .../crash/CrashRecordTextFormatterTest.kt | 4 +- .../data/source/LogcatDataSourceTest.kt | 22 ---- 23 files changed, 493 insertions(+), 407 deletions(-) create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt rename debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/{bugreport/ui/DraftBadge.kt => ui/CountBadge.kt} (57%) create mode 100644 debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt create mode 100644 debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilderTest.kt delete mode 100644 debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt diff --git a/AGENTS.md b/AGENTS.md index 15738d24..558c0f0f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ A Jetpack Compose library that displays real-time debug information as an overla | Create new extension | Implement `LogSource` or `NetworkRequestSource`, self-register in `init` block | | Modify bug report flow | `BugReportGenerator.kt` (orchestration), `BugReportDraftStorage.kt` (persistence) | | Change metrics display | `internal/ui/DebugOverlayPanel.kt` (compact overlay), `DebugOverlayPanelDataSource.kt` (aggregation) | -| Modify crash persistence | `internal/crash/CrashHandler.kt` (capture), `CrashRecordStorage.kt` (persistence), `CrashLogTabContent.kt`/`CrashLogDetailScreen.kt` (UI) | +| Modify crash persistence | `internal/crash/CrashHandler.kt` (chains the uncaught-exception handler), `CrashRecordBuilder.kt` (assembles the record), `CrashRecordStorage.kt` (persistence), `CrashLogTabContent.kt`/`CrashLogDetailScreen.kt` (UI). Data is gathered in `DebugOverlayDataRepository.writeCrashRecordSync()` | ## Build Commands diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt index e382c733..6d76a7ac 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt @@ -12,12 +12,9 @@ import com.ms.square.debugoverlay.internal.InternalDebugOverlayApi import com.ms.square.debugoverlay.internal.Logger import com.ms.square.debugoverlay.internal.OverlayViewManager import com.ms.square.debugoverlay.internal.bugreport.BugReportGenerator -import com.ms.square.debugoverlay.internal.bugreport.DefaultAppInfoProvider import com.ms.square.debugoverlay.internal.bugreport.IntentShareExporter -import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo import com.ms.square.debugoverlay.internal.bugreport.validateFilename import com.ms.square.debugoverlay.internal.crash.CrashHandler -import com.ms.square.debugoverlay.internal.crash.DefaultCrashRecordStorage import com.ms.square.debugoverlay.internal.data.DebugOverlayDataRepository import com.ms.square.debugoverlay.internal.ui.DebugPanelActivity import com.ms.square.debugoverlay.internal.util.checkMainThread @@ -25,9 +22,7 @@ import com.ms.square.debugoverlay.internal.util.isMainProcess import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.launch import java.util.concurrent.CopyOnWriteArrayList -import java.util.concurrent.atomic.AtomicReference /** * Internal entry point for DebugOverlay auto-installers. @@ -109,7 +104,7 @@ public object DebugOverlay { activityProvider = viewManager ) - installCrashHandler(application, scope, repository) + installCrashHandler(repository) } } @@ -118,33 +113,14 @@ public object DebugOverlay { * previously installed (the platform default, or another crash reporter like * Crashlytics), so persisting a crash record never interferes with existing crash * handling. Captured exactly once here. - * - * App info is fetched in the background on [scope] rather than inline here: [install] - * runs on the main thread before [Application.onCreate] (via AndroidX Startup), so any - * blocking work here — [DefaultAppInfoProvider.getAppInfo] does PackageManager IPC - * calls — directly delays app startup. [AtomicReference] gives the background write and - * the (possibly cross-thread, possibly-crashing-thread) read proper visibility without - * needing a coroutine to read it back. */ - private fun installCrashHandler( - application: Application, - scope: CoroutineScope, - repository: DebugOverlayDataRepository, - ) { + private fun installCrashHandler(repository: DebugOverlayDataRepository) { val previousHandler = Thread.getDefaultUncaughtExceptionHandler() - val cachedAppInfo = AtomicReference(null) - scope.launch(Dispatchers.IO) { - cachedAppInfo.set(runCatching { DefaultAppInfoProvider.getAppInfo(application) }.getOrNull()) - } Thread.setDefaultUncaughtExceptionHandler( CrashHandler( previousHandler = previousHandler, - storage = DefaultCrashRecordStorage(application), - cachedAppInfoProvider = cachedAppInfo::get, - logcatSnapshotProvider = { repository.logcatSnapshotSync() }, - customLogSnapshotProvider = { repository.customLogSnapshotSync() }, - networkRequestsSnapshotProvider = { repository.networkRequestsSnapshotSync() } + captureCrash = repository::writeCrashRecordSync ) ) } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/IntentShareExporter.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/IntentShareExporter.kt index af7bf48d..0494cc4d 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/IntentShareExporter.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/IntentShareExporter.kt @@ -3,20 +3,18 @@ package com.ms.square.debugoverlay.internal.bugreport import android.content.ClipData import android.content.Context import android.content.Intent -import androidx.core.content.FileProvider import com.ms.square.debugoverlay.BugReportExporter import com.ms.square.debugoverlay.core.R import com.ms.square.debugoverlay.formatBugReportMarkdown import com.ms.square.debugoverlay.internal.Logger import com.ms.square.debugoverlay.internal.bugreport.model.BugReportArchiveImpl +import com.ms.square.debugoverlay.internal.util.debugOverlayFileUri import com.ms.square.debugoverlay.internal.util.runCatchingNonCancellation import com.ms.square.debugoverlay.model.BugReport import com.ms.square.debugoverlay.model.ExportResult import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext -private const val PROVIDER_AUTHORITY_SUFFIX = ".debugoverlay.bugreport.provider" - /** * Default exporter that shares the bug report via Android's share sheet. * Uses Intent.ACTION_SEND with FileProvider for secure file sharing. @@ -30,8 +28,7 @@ internal object IntentShareExporter : BugReportExporter { UnsupportedOperationException("IntentShareExporter requires file-backed archive") ) } - val authority = "${context.packageName}$PROVIDER_AUTHORITY_SUFFIX" - val uri = FileProvider.getUriForFile(context, authority, file) + val uri = context.debugOverlayFileUri(file) val subject = context.getString(R.string.debugoverlay_bug_report_subject, file.nameWithoutExtension) val chooserTitle = context.getString(R.string.debugoverlay_share_bug_report) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/ui/BugReporterFab.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/ui/BugReporterFab.kt index d6e2bf10..fac18cd7 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/ui/BugReporterFab.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/ui/BugReporterFab.kt @@ -31,6 +31,7 @@ import androidx.compose.ui.unit.dp import com.ms.square.debugoverlay.DebugOverlay import com.ms.square.debugoverlay.core.R import com.ms.square.debugoverlay.internal.bugreport.BugReportGenerator +import com.ms.square.debugoverlay.internal.ui.CountBadge import kotlinx.coroutines.delay import kotlinx.coroutines.isActive import kotlinx.coroutines.launch @@ -155,8 +156,8 @@ internal fun BugReporterFab( // Badge positioned at top-right corner of FAB if (draftCount > 0 && fabState != BugReporterFabState.Processing) { - DraftCountBadge( - draftCount = draftCount, + CountBadge( + count = draftCount, modifier = Modifier.align(Alignment.TopEnd) ) } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt index 9d43d950..323c955c 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt @@ -1,49 +1,29 @@ package com.ms.square.debugoverlay.internal.crash import com.ms.square.debugoverlay.internal.Logger -import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo -import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData -import com.ms.square.debugoverlay.model.LogEntry -import com.ms.square.debugoverlay.model.NetworkRequest - -private const val DEFAULT_MAX_LOG_LINES = 100 /** - * Persists a [CrashRecord] to disk on an uncaught exception, then always delegates to + * Captures a crash record on an uncaught exception, then always delegates to * [previousHandler] so the app's normal crash behavior (and any other installed crash * reporter, e.g. Crashlytics) is unaffected. * - * Installed once by [com.ms.square.debugoverlay.DebugOverlay.install]. All data reads - * are non-suspending, in-memory snapshots — this must never spawn a subprocess, hop a - * dispatcher, or otherwise risk not completing before the process dies. + * Installed once by [com.ms.square.debugoverlay.DebugOverlay.install]. [captureCrash] must + * be non-suspending and should only read in-memory data to complete quickly before the process dies. * * @param previousHandler The handler that was installed before this one, captured once * at install time. Never re-fetched, so a handler installed by another SDK *after* * DebugOverlay is never clobbered. - * @param storage Where the crash record is written. - * @param cachedAppInfoProvider Returns app info fetched once, off the main thread, shortly - * after install (see [com.ms.square.debugoverlay.DebugOverlay.install]) — never queried - * fresh here, since that would mean PackageManager IPC calls in the crash path. Returns - * null if a crash happens before that background fetch completes. - * @param logcatSnapshotProvider Non-suspending snapshot of the in-memory Logcat buffer. - * @param customLogSnapshotProvider Non-suspending snapshot of the custom log source, if any. - * @param networkRequestsSnapshotProvider Non-suspending snapshot of recent network requests. - * @param maxLogLines Maximum number of entries kept per log/request source. + * @param captureCrash Builds and persists the crash record for the given thread/throwable. */ internal class CrashHandler( private val previousHandler: Thread.UncaughtExceptionHandler?, - private val storage: CrashRecordStorage, - private val cachedAppInfoProvider: () -> AppInfo?, - private val logcatSnapshotProvider: () -> List, - private val customLogSnapshotProvider: () -> CustomLogSourceData?, - private val networkRequestsSnapshotProvider: () -> List, - private val maxLogLines: Int = DEFAULT_MAX_LOG_LINES, + private val captureCrash: (Thread, Throwable) -> Unit, ) : Thread.UncaughtExceptionHandler { @Suppress("TooGenericExceptionCaught") override fun uncaughtException(thread: Thread, throwable: Throwable) { try { - storage.writeSync(buildCrashRecord(thread, throwable)) + captureCrash(thread, throwable) } catch (t: Throwable) { // Deliberately broad: capture failure must never prevent the delegate call below // from running, since that's what keeps other crash reporters (e.g. Crashlytics) @@ -57,22 +37,4 @@ internal class CrashHandler( previousHandler?.uncaughtException(thread, throwable) } } - - private fun buildCrashRecord(thread: Thread, throwable: Throwable): CrashRecord { - val maxLines = maxLogLines.coerceAtLeast(0) - val customLogSnapshot = customLogSnapshotProvider()?.let { data -> - data.copy(logs = data.logs.takeLast(maxLines)) - } - return CrashRecord( - timestampMs = System.currentTimeMillis(), - threadName = thread.name, - exceptionType = throwable.javaClass.name, - message = throwable.message, - stackTrace = throwable.stackTraceToString(), - appInfo = cachedAppInfoProvider(), - logcatLogs = logcatSnapshotProvider().takeLast(maxLines), - customLogSourceData = customLogSnapshot, - networkRequests = networkRequestsSnapshotProvider().takeLast(maxLines) - ) - } } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt index 90945c3b..5e4493be 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt @@ -4,6 +4,8 @@ import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData import com.ms.square.debugoverlay.model.LogEntry import com.ms.square.debugoverlay.model.NetworkRequest +import kotlinx.serialization.EncodeDefault +import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.Serializable import java.io.File import java.util.UUID @@ -11,19 +13,23 @@ import java.util.UUID /** * A crash captured by [CrashHandler] and persisted to disk so it survives process death. * - * @param version Schema version, for forward-compatible reads via `ignoreUnknownKeys`. + * @param version Schema version, for forward-compatible reads via `ignoreUnknownKeys`. Carries + * [EncodeDefault] because it always equals its default, which the serializer would otherwise + * omit — leaving stored records with no version to read. * @param timestampMs When the exception was caught (epoch millis). * @param threadName Name of the thread that crashed. * @param exceptionType Fully-qualified exception class name. * @param message The exception's message, if any. * @param stackTrace Full stack trace text, including any "Caused by" chain. - * @param appInfo App info captured at install time (immutable for the process lifetime). + * @param appInfo App info captured. * @param logcatLogs Recent logcat entries leading up to the crash. * @param customLogSourceData Recent custom log source entries, null if none registered. * @param networkRequests Recent network requests leading up to the crash. */ +@OptIn(ExperimentalSerializationApi::class) @Serializable internal data class CrashRecord( + @EncodeDefault val version: Int = 1, val id: String = UUID.randomUUID().toString(), val timestampMs: Long, @@ -34,7 +40,39 @@ internal data class CrashRecord( val appInfo: AppInfo?, val logcatLogs: List, val customLogSourceData: CustomLogSourceData?, - val networkRequests: List, + val networkRequests: List, +) + +/** + * The subset of a [NetworkRequest] a crash record keeps. + * + * Deliberately not [NetworkRequest] itself: that carries request/response bodies (up to 2MB + * each by default in the OkHttp extension) and [com.ms.square.debugoverlay.model.NetworkError] + * repeats the response body in its `stackTrace`. Persisting those could mean serializing 2+ + * megabytes on the crashing thread — most likely to fail exactly when the crash is an + * OutOfMemoryError — to store data neither the detail screen nor the text export ever renders. + * These are the only fields both consumers read. + */ +@Serializable +internal data class NetworkRequestSummary( + val timestampMs: Long, + val method: String, + val url: String, + val statusCode: Int?, + val durationMs: Long, + val errorTitle: String? = null, + val errorMessage: String? = null, +) + +/** Keeps only the fields a crash record renders — see [NetworkRequestSummary]. */ +internal fun NetworkRequest.toSummary() = NetworkRequestSummary( + timestampMs = timestampMs, + method = method, + url = url, + statusCode = statusCode, + durationMs = durationMs, + errorTitle = error?.title, + errorMessage = error?.message ) /** A [CrashRecord] paired with the file it was loaded from. */ diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt new file mode 100644 index 00000000..8d876e9e --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt @@ -0,0 +1,42 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo +import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData +import com.ms.square.debugoverlay.model.LogEntry +import com.ms.square.debugoverlay.model.NetworkRequest + +internal const val DEFAULT_MAX_LOG_LINES = 100 + +/** + * Assembles a [CrashRecord] from already-captured, in-memory data. + * + * Pure and non-suspending: every input is passed in, so this can run on the crashing thread + * and can be tested without a [android.content.Context]. Each log/request source is trimmed + * to the last [maxLogLines] entries, and network requests are reduced to + * [NetworkRequestSummary] so bodies never reach disk. + * + * @param appInfo App info fetched once shortly after install; null if the crash beat that + * background fetch. + */ +internal fun buildCrashRecord( + thread: Thread, + throwable: Throwable, + appInfo: AppInfo?, + logcatLogs: List, + customLogSourceData: CustomLogSourceData?, + networkRequests: List, + maxLogLines: Int = DEFAULT_MAX_LOG_LINES, +): CrashRecord { + val maxLines = maxLogLines.coerceAtLeast(0) + return CrashRecord( + timestampMs = System.currentTimeMillis(), + threadName = thread.name, + exceptionType = throwable.javaClass.name, + message = throwable.message, + stackTrace = throwable.stackTraceToString(), + appInfo = appInfo, + logcatLogs = logcatLogs.takeLast(maxLines), + customLogSourceData = customLogSourceData?.let { it.copy(logs = it.logs.takeLast(maxLines)) }, + networkRequests = networkRequests.takeLast(maxLines).map { it.toSummary() } + ) +} diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt index 6a9a232c..266a792f 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt @@ -2,17 +2,16 @@ package com.ms.square.debugoverlay.internal.crash import android.content.Context import android.content.Intent -import androidx.core.content.FileProvider import com.ms.square.debugoverlay.core.R import com.ms.square.debugoverlay.internal.Logger import com.ms.square.debugoverlay.internal.util.checkFolderExists +import com.ms.square.debugoverlay.internal.util.debugOverlayFileUri import com.ms.square.debugoverlay.internal.util.formatFilenameTimestamp import com.ms.square.debugoverlay.internal.util.runCatchingNonCancellation import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import java.io.File -private const val PROVIDER_AUTHORITY_SUFFIX = ".debugoverlay.bugreport.provider" private const val EXPORTS_SUBDIR = "debugoverlay_crash_exports" /** @@ -30,8 +29,7 @@ internal object CrashRecordExporter { val file = File(exportsDir, "crash_${formatFilenameTimestamp(record.timestampMs)}.txt") file.writeText(formatCrashRecordAsText(record)) - val authority = "${context.packageName}$PROVIDER_AUTHORITY_SUFFIX" - val uri = FileProvider.getUriForFile(context, authority, file) + val uri = context.debugOverlayFileUri(file) val intent = Intent(Intent.ACTION_SEND).apply { type = "text/plain" diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt index 93d02024..13da2aba 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt @@ -12,6 +12,8 @@ import java.io.File import java.util.UUID private const val CRASH_RECORDS_SUBDIR = "debugoverlay_crash_records" +private const val RECORD_SUFFIX = ".json" +private const val TEMP_SUFFIX = ".tmp" internal const val DEFAULT_MAX_CRASH_RECORDS = 5 /** @@ -23,9 +25,9 @@ internal sealed interface CrashRecordStorage { * Writes [record] to disk. Does not evict old records — see [listCrashRecords]. * * Must be safe to call synchronously from [Thread.UncaughtExceptionHandler.uncaughtException]: - * no suspension, no dispatcher hop, no more work than writing this one file. Any failure - * is swallowed and logged — the caller must be able to unconditionally proceed to the - * previous crash handler afterward. + * no suspension, no dispatcher hop, no more work than writing this one file. I/O failures + * propagate, so callers on the crash path must catch them and proceed to the previous crash + * handler regardless — see `CrashHandler.uncaughtException`. */ fun writeSync(record: CrashRecord) @@ -60,10 +62,9 @@ internal class DefaultCrashRecordStorage( private val json = Json { ignoreUnknownKeys = true } - // Plain JVM monitor, not a coroutines Mutex: writeSync() runs outside any coroutine - // context (it's called directly from uncaughtException()). Also guards the eviction + - // listing step in listCrashRecords() against a concurrent writeSync(), so a write and a - // list/evict can't race on the directory contents. + // Plain JVM monitor, not a coroutines Mutex: writeSync() is called directly from + // uncaughtException(), outside any coroutine. A single instance is shared with the crash + // path (see DebugOverlayDataRepository), so this also serialises writes against list/evict. private val writeLock = Any() private val recordsDir by lazy { @@ -74,18 +75,36 @@ internal class DefaultCrashRecordStorage( override fun writeSync(record: CrashRecord) { synchronized(writeLock) { + // Write to a temp file and rename, rather than writing the record in place: the + // process is already dying, so an in-place write that doesn't finish would leave a + // truncated file that occupies a retention slot forever and can never be parsed. + // Rename within the same directory is atomic, so a reader sees either no file or a + // complete one. Temp files are excluded from listing and cleaned up by eviction. val file = File(recordsDir, fileNameFor(record)) - file.writeText(json.encodeToString(CrashRecord.serializer(), record)) + val tempFile = File(recordsDir, "${file.name}$TEMP_SUFFIX") + tempFile.writeText(json.encodeToString(CrashRecord.serializer(), record)) + if (!tempFile.renameTo(file)) { + Logger.w("Failed to finalize crash record: ${file.name}") + tempFile.delete() + } } } private fun fileNameFor(record: CrashRecord) = "crash_${record.timestampMs}_${UUID.randomUUID()}.json" + private fun File.isRecordFile() = isFile && name.endsWith(RECORD_SUFFIX) + // Must be called while holding writeLock. private fun evictOldRecordsLocked() { - val files = recordsDir.listFiles()?.filter { it.isFile } ?: return + val allFiles = recordsDir.listFiles() ?: return + // Any temp file visible here is orphaned from a write that never finished: writeSync() + // holds the same lock for the whole write-and-rename, so none can be in flight now. + allFiles.filter { it.isFile && it.name.endsWith(TEMP_SUFFIX) }.forEach { it.delete() } + + val files = allFiles.filter { it.isRecordFile() } if (files.size <= maxRecords) return - files.sortedDescending().drop(maxRecords).forEach { it.delete() } + val staleRecords = files.sortedDescending().drop(maxRecords) + staleRecords.forEach { it.delete() } } override suspend fun listCrashRecords(): List = withContext(Dispatchers.IO) { @@ -94,7 +113,7 @@ internal class DefaultCrashRecordStorage( // keeps writeSync() to the bare minimum needed before delegating to the previous handler. val files = synchronized(writeLock) { evictOldRecordsLocked() - recordsDir.listFiles()?.filter { it.isFile }?.sortedDescending() ?: emptyList() + recordsDir.listFiles()?.filter { it.isRecordFile() }?.sortedDescending() ?: emptyList() } files.mapNotNull { file -> loadRecord(file)?.let { CrashRecordInfo(file.absolutePath, it) } } } @@ -110,11 +129,10 @@ internal class DefaultCrashRecordStorage( } } - private fun loadRecord(file: File): CrashRecord? = - runCatchingNonCancellation { - json.decodeFromString(CrashRecord.serializer(), file.readText()) - }.getOrElse { e -> - Logger.w("Failed to parse crash record ${file.name}: ${e.message}") - null - } + private fun loadRecord(file: File): CrashRecord? = runCatchingNonCancellation { + json.decodeFromString(CrashRecord.serializer(), file.readText()) + }.getOrElse { e -> + Logger.w("Failed to parse crash record ${file.name}: ${e.message}") + null + } } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt index 216a82be..46e899e6 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt @@ -2,7 +2,6 @@ package com.ms.square.debugoverlay.internal.crash import com.ms.square.debugoverlay.internal.util.formatFullTimestamp import com.ms.square.debugoverlay.internal.util.toClipboardText -import com.ms.square.debugoverlay.model.NetworkRequest private const val SEPARATOR_WIDTH = 80 @@ -43,12 +42,11 @@ internal fun formatCrashRecordAsText(record: CrashRecord): String = buildString if (record.networkRequests.isNotEmpty()) { appendLine() appendLine("--- NETWORK REQUESTS (${record.networkRequests.size}) ---") - record.networkRequests.forEach { appendLine(it.toSummaryLine()) } + record.networkRequests.forEach { appendLine(it.toExportLine()) } } } -private fun NetworkRequest.toSummaryLine(): String = buildString { - append(formatFullTimestamp(timestampMs)).append(' ').append(method).append(' ').append(url) - append(" -> ").append(statusCode ?: "?").append(" (").append(durationMs).append("ms)") - error?.let { append(" [ERROR: ${it.title}: ${it.message}]") } +private fun NetworkRequestSummary.toExportLine(): String { + val errorSuffix = errorTitle?.let { " [ERROR: $it: $errorMessage]" }.orEmpty() + return "${formatFullTimestamp(timestampMs)} $method $url -> ${statusCode ?: "?"} (${durationMs}ms)$errorSuffix" } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt index de22abfd..69f18ec5 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt @@ -7,9 +7,12 @@ import com.ms.square.debugoverlay.LogSource import com.ms.square.debugoverlay.NetworkRequestSource import com.ms.square.debugoverlay.NoOpNetworkRequestSource import com.ms.square.debugoverlay.internal.Logger +import com.ms.square.debugoverlay.internal.bugreport.DefaultAppInfoProvider import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData import com.ms.square.debugoverlay.internal.crash.CrashRecordInfo +import com.ms.square.debugoverlay.internal.crash.CrashRecordStorage import com.ms.square.debugoverlay.internal.crash.DefaultCrashRecordStorage +import com.ms.square.debugoverlay.internal.crash.buildCrashRecord import com.ms.square.debugoverlay.internal.data.model.AppExitInfo import com.ms.square.debugoverlay.internal.data.model.DeviceInfo import com.ms.square.debugoverlay.internal.data.model.JankStatsUiState @@ -19,6 +22,7 @@ import com.ms.square.debugoverlay.internal.data.source.DeviceInfoDataSource import com.ms.square.debugoverlay.internal.data.source.JankStatsDataSource import com.ms.square.debugoverlay.internal.data.source.LogcatDataSource import com.ms.square.debugoverlay.internal.data.source.NetStatsDataSource +import com.ms.square.debugoverlay.internal.util.runCatchingNonCancellation import com.ms.square.debugoverlay.internal.util.throttleLatest import com.ms.square.debugoverlay.model.LogEntry import com.ms.square.debugoverlay.model.NetworkRequest @@ -32,7 +36,6 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map @@ -40,13 +43,17 @@ import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import java.util.concurrent.atomic.AtomicBoolean import kotlin.time.Duration.Companion.milliseconds /** Default name shown when a custom log source doesn't provide a source name. */ internal const val DEFAULT_CUSTOM_LOG_SOURCE_NAME = "Custom" -internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScope, initialLogcatMaxEntries: Int) { +@Suppress("TooManyFunctions") +internal class DebugOverlayDataRepository( + private val context: Context, + private val scope: CoroutineScope, + initialLogcatMaxEntries: Int, +) { private val currentNetworkRequestSource = MutableStateFlow(NoOpNetworkRequestSource) private val logcatDataSource = LogcatDataSource(scope, initialMaxEntries = initialLogcatMaxEntries) @@ -55,7 +62,8 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop private val deviceInfoDataSource = DeviceInfoDataSource(context, scope) private val jankStatsDataSource = JankStatsDataSource() private val appExitDataSource = AppExitDataSource(context, scope) - private val crashRecordStorage = DefaultCrashRecordStorage(context) + + private val crashRecordStorage: CrashRecordStorage = DefaultCrashRecordStorage(context) init { scope.launch { @@ -96,7 +104,6 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop // Whether a custom log source is registered val hasCustomLogSource: StateFlow = customLogSource .map { it != null } - .distinctUntilChanged() .stateIn(scope, SharingStarted.Eagerly, false) val netStats: Flow = netStatsDataSource.stats @@ -108,38 +115,74 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop val appExitInfos: Flow> = appExitDataSource.appExitInfos - // Persisted crash records from a prior run. Loaded once, lazily, on first subscription - // (mirrors BugReportDraftStorage's initDraftsIfNeeded()/refreshDrafts() pattern) — a - // crash terminates the process, so nothing can add a new record mid-session, but - // deleteCrashRecord() below explicitly re-syncs this after a deletion. - private val crashRecordsLoaded = AtomicBoolean(false) - private val _crashRecords = MutableStateFlow>(emptyList()) + // null means "not read from disk yet", which the UI must not render as "no crashes" — + // otherwise the empty state flashes for a frame every time the Crash tab opens. + private val _crashRecords = MutableStateFlow?>(null) - val crashRecords: Flow> = _crashRecords.asStateFlow() - .onStart { loadCrashRecordsIfNeeded() } + val crashRecords: StateFlow?> = _crashRecords.asStateFlow() + .onStart { refreshCrashRecords() } + .stateIn(scope, SharingStarted.Lazily, null) - val hasCrashRecords: StateFlow = crashRecords - .map { it.isNotEmpty() } - .distinctUntilChanged() - .stateIn(scope, SharingStarted.Eagerly, false) + // Drives the Crash tab's count badge. Collecting this is enough to start the lazily-shared + // load above, so the panel doesn't have to hold the whole list (and recompose on every + // emission) just to show a number. + val crashRecordCount: StateFlow = crashRecords + .map { it?.size ?: 0 } + .stateIn(scope, SharingStarted.Lazily, 0) - private suspend fun loadCrashRecordsIfNeeded() { - if (crashRecordsLoaded.compareAndSet(false, true)) { - refreshCrashRecords() + private suspend fun refreshCrashRecords() { + _crashRecords.value = withContext(Dispatchers.IO) { + runCatchingNonCancellation { + crashRecordStorage.listCrashRecords() + }.onFailure { + Logger.e("Failed to refresh crash records", it) + }.getOrDefault(emptyList()) } } - private suspend fun refreshCrashRecords() { - _crashRecords.value = withContext(Dispatchers.IO) { crashRecordStorage.listCrashRecords() } + /** + * Builds a crash record from the current in-memory snapshots and persists it. + * + * Non-suspending and dispatcher-free so [CrashHandler] can call it directly from the + * crashing thread before delegating to the previous handler. The snapshots it reads are + * kept private: assembling the record here is the only reason they exist. + * + * App info is queried here rather than cached up front: it's two PackageManager IPC calls, + * cheap next to the file write below, and caching it would cost every app start for a read + * that happens at most once per process. Guarded separately so a failure costs the app info + * field, not the whole record. + */ + fun writeCrashRecordSync(thread: Thread, throwable: Throwable) { + crashRecordStorage.writeSync( + buildCrashRecord( + thread = thread, + throwable = throwable, + appInfo = runCatching { DefaultAppInfoProvider.getAppInfo(context) }.getOrNull(), + logcatLogs = logcatDataSource.queryLogcatSnapshot(), + customLogSourceData = customLogSnapshotSync(), + networkRequests = networkRequestsSnapshot.value + ) + ) } - suspend fun deleteCrashRecord(info: CrashRecordInfo) { - crashRecordStorage.deleteCrashRecord(info) - refreshCrashRecords() + /** + * Deletes a persisted crash record and re-syncs [crashRecords]. + * + * Runs on the repository's own [scope], not the caller's: when this ran on the crash tab's + * `rememberCoroutineScope()`, closing the panel mid-delete cancelled the coroutine between + * the delete and the refresh — `withContext` throws on return once the job is cancelled — so + * the file was gone while [crashRecords] still listed it. Nothing re-read it afterwards + * either, since the Lazily-shared `onStart` only runs on the first subscription. + */ + fun deleteCrashRecord(info: CrashRecordInfo) { + scope.launch { + crashRecordStorage.deleteCrashRecord(info) + refreshCrashRecords() + } } // Snapshot methods for bug reports (use cached value if available, otherwise query directly) - suspend fun queryLogcatSnapshot(): List = logcatDataSource.queryLogcatSnapshot() + fun queryLogcatSnapshot(): List = logcatDataSource.queryLogcatSnapshot() suspend fun queryDeviceInfoSnapshot(): DeviceInfo = deviceInfoDataSource.queryDeviceInfoSnapshot() suspend fun queryAppExitInfosSnapshot(): List = appExitDataSource.queryAppExitInfosSnapshot() @@ -147,22 +190,15 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop val networkRequests: Flow> = currentNetworkRequestSource .flatMapLatest { source -> source.requests } - // Cached copy of the latest network requests, kept warm for synchronous reads - // (e.g. by CrashHandler, which cannot suspend). Mirrors customLogSourceLogs's - // SharingStarted.Eagerly pattern above. + // Cached copy of the latest network requests, kept warm so writeCrashRecordSync() can read + // it without suspending. Mirrors customLogSourceLogs's SharingStarted.Eagerly pattern above. private val networkRequestsSnapshot: StateFlow> = networkRequests.stateIn(scope, SharingStarted.Eagerly, emptyList()) - /** Non-suspending snapshot of the in-memory Logcat buffer. Safe to call from a crashing thread. */ - fun logcatSnapshotSync(): List = logcatDataSource.snapshotEntriesSync() - - /** Non-suspending snapshot of the custom log source's latest known logs, if one is registered. */ - fun customLogSnapshotSync(): CustomLogSourceData? = + // Non-suspending snapshot of the custom log source's latest known logs, if one is registered. + private fun customLogSnapshotSync(): CustomLogSourceData? = customLogSourceName.value?.let { name -> CustomLogSourceData(customLogSourceLogs.value, name) } - /** Non-suspending snapshot of the latest known network requests. */ - fun networkRequestsSnapshotSync(): List = networkRequestsSnapshot.value - fun setNetworkSource(source: NetworkRequestSource) { currentNetworkRequestSource.value = source } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt index 3e138969..13a78e31 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt @@ -24,7 +24,6 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.isActive -import kotlinx.coroutines.withContext import java.io.BufferedReader import java.io.Closeable import java.io.IOException @@ -53,10 +52,9 @@ internal class LogcatDataSource( private val entries = EvictingQueue(initialMaxEntries) /** - * Maximum number of entries retained in the in-memory buffer and the count - * requested from logcat on next subscription. The currently-running subprocess - * keeps its original `-T N` arg until [WhileSubscribed][SharingStarted.WhileSubscribed] - * restarts the producer (panel reopen). + * Maximum number of entries retained in the in-memory buffer. Resizing takes effect on the + * queue immediately; the running subprocess keeps the `-T N` arg it was started with, which + * only affects how much history was replayed at process start. */ var maxEntries: Int @IntRange(from = 1) @@ -65,9 +63,8 @@ internal class LogcatDataSource( entries.capacity = value } - // Drops OS-replayed entries from before the last clear (e.g. when the producer - // restarts on panel reopen and `-T N` walks the OS ring buffer). - // Wall-clock epoch ms, matching `logcat -v ... epoch`. + // Drops OS-replayed entries from before the last clear, since `-T N` walks the OS ring + // buffer at producer start. Wall-clock epoch ms, matching `logcat -v ... epoch`. @Volatile private var clearMarkerMs: Long = 0L // Forces a downstream re-read after clear() so the UI sees `[]` instantly, @@ -83,9 +80,8 @@ internal class LogcatDataSource( * the queue's current state — the tick payload is irrelevant. */ private val producerSignal: Flow = flow { - // Each subscription session starts fresh — without this, the hoisted queue - // would accumulate duplicates as `logcat -T N` replays the OS ring buffer - // on every resubscribe (panel reopen). + // Starts fresh. With Eagerly sharing this runs once per process, but the clear is kept + // so a restarted producer can't double-count the history `logcat -T N` replays. entries.clear() var reader: BufferedReader? = null try { @@ -111,8 +107,7 @@ internal class LogcatDataSource( val line = reader.readLine() ?: break parser.parse(line)?.let { entry -> // Drop OS-replayed entries from before the last clear. `-T N` replays the - // last N ring-buffer lines on every subprocess start, including when the - // panel reopens after WhileSubscribed cancelled us. + // last N ring-buffer lines when the subprocess starts, which can predate a clear(). // (Rare caveat: a backward system-clock jump could mis-drop a real entry.) if (entry.timestampMs < clearMarkerMs) return@let entries.add(entry) @@ -131,7 +126,6 @@ internal class LogcatDataSource( /** * Stream logcat entries. Keeps last N entries in memory. - * Private StateFlow for direct .value access in [queryLogcatSnapshot]. * * `throttleLatest` is applied only to `producerSignal` so noisy producers * are rate-limited, while `clearSignal` flows straight through merge to @@ -145,7 +139,14 @@ internal class LogcatDataSource( .flowOn(Dispatchers.IO) .stateIn( scope, - started = SharingStarted.WhileSubscribed(), + // Eagerly, not WhileSubscribed: the buffer has to be warm when an uncaught exception + // arrives, and a crash rarely happens with the debug panel open. Under WhileSubscribed + // the subprocess only ran while the Logcat tab was visible, so crash records captured + // an empty log list for anyone who never opened the panel — and a stale one for anyone + // who had opened and closed it. Matches customLogSourceLogs/networkRequests, which the + // repository already shares eagerly for the same reason. Costs one `logcat -T N` + // subprocess for the process lifetime, with memory bounded by maxEntries. + started = SharingStarted.Eagerly, initialValue = emptyList() ) @@ -159,53 +160,16 @@ internal class LogcatDataSource( } /** - * Returns a snapshot of the in-memory log buffer without suspending or spawning a - * subprocess. Safe to call from a crashing thread (e.g. [com.ms.square.debugoverlay.internal.crash.CrashHandler]). + * Returns a snapshot of the in-memory log buffer without suspending or spawning a subprocess, + * so it stays safe to call while the process is dying — see + * [com.ms.square.debugoverlay.internal.data.DebugOverlayDataRepository.writeCrashRecordSync], + * which calls this from the crashing thread. * - * Unlike [queryLogcatSnapshot], this never falls back to a one-shot `logcat -t N` - * capture, so it returns an empty list if the Logcat tab was never subscribed to - * this process run. + * The buffer is filled from process start (the producer is shared eagerly), so this does not + * depend on the debug panel ever having been opened. Bug reports read it too. Entries are + * already filtered against [clear] as they are appended, so no post-filtering is needed here. */ - fun snapshotEntriesSync(): List = entries.toList() - - /** - * Returns a snapshot of logcat logs for bug reports. - * Uses cached value if streaming was active (debug panel was viewed), otherwise captures directly. - */ - suspend fun queryLogcatSnapshot(): List { - val cached = _logs.value - if (cached.isNotEmpty()) return cached - // drop anything captured before the last clear so a "clear → close panel → bug report" flow - // doesn't resurface pre-clear lines. - return captureLogcatOnce().filter { it.timestampMs >= clearMarkerMs } - } - - private suspend fun captureLogcatOnce(): List = withContext(Dispatchers.IO) { - buildList { - // -t N = fetch N recent lines and EXIT (vs -T which streams continuously) - val process = ProcessBuilder( - "logcat", - "-v", - "threadtime,printable,epoch", - "-t", - maxEntries.toString() - ).start() - - try { - InputStreamReader(process.inputStream).useLines { lines -> - lines.forEach { line -> - parser.parse(line)?.let { add(it) } - } - } - } catch (e: IOException) { - Logger.e("Failed to capture logcat snapshot", e) - } catch (e: SecurityException) { - Logger.e("Failed to capture logcat snapshot", e) - } finally { - process.safeDestroy() - } - } - } + fun queryLogcatSnapshot(): List = entries.toList() override fun close() { safeDestroyProcess() diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/ui/DraftBadge.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CountBadge.kt similarity index 57% rename from debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/ui/DraftBadge.kt rename to debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CountBadge.kt index 05714ce6..25c04d2f 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/bugreport/ui/DraftBadge.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CountBadge.kt @@ -1,4 +1,4 @@ -package com.ms.square.debugoverlay.internal.bugreport.ui +package com.ms.square.debugoverlay.internal.ui import androidx.compose.foundation.layout.offset import androidx.compose.material3.Badge @@ -11,21 +11,23 @@ import androidx.compose.ui.unit.dp private const val MAX_BADGE_COUNT = 9 /** - * Badge showing the number of saved drafts. + * Badge showing a count of items needing attention (saved drafts, crash records). * * Display logic: - * - 1 draft: dot only (no text) - * - 2-9 drafts: count as text - * - 10+ drafts: "9+" + * - 1 item: dot only (no text) + * - 2-9 items: count as text + * - 10+ items: "9+" * - * Uses M3 error color per convention for "items requiring attention". + * Uses M3 error color per convention for "items requiring attention". Purely decorative — + * callers own the accessible description on the element being badged, so the count isn't + * announced as a stray digit. * - * @param draftCount Number of drafts to display + * @param count Number of items to display; must be positive * @param modifier Modifier for positioning (use offset to fine-tune position) */ @Composable -internal fun DraftCountBadge(draftCount: Int, modifier: Modifier = Modifier) { - require(draftCount > 0) { "draftCount must be positive, got: $draftCount" } +internal fun CountBadge(count: Int, modifier: Modifier = Modifier) { + require(count > 0) { "count must be positive, got: $count" } Badge( // Default offset for top-right corner positioning modifier = modifier.offset(x = 1.dp, y = (-1).dp), @@ -34,9 +36,9 @@ internal fun DraftCountBadge(draftCount: Int, modifier: Modifier = Modifier) { contentColor = MaterialTheme.colorScheme.onError ) { val badgeText = when { - draftCount == 1 -> null // Dot only - draftCount > MAX_BADGE_COUNT -> "${MAX_BADGE_COUNT}+" - else -> draftCount.toString() + count == 1 -> null // Dot only + count > MAX_BADGE_COUNT -> "${MAX_BADGE_COUNT}+" + else -> count.toString() } badgeText?.let { Text(it) } } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt index 5c9c8442..529e3493 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt @@ -34,7 +34,6 @@ import androidx.compose.ui.unit.dp import com.ms.square.debugoverlay.core.R import com.ms.square.debugoverlay.internal.crash.CrashRecord import com.ms.square.debugoverlay.internal.crash.CrashRecordExporter -import com.ms.square.debugoverlay.internal.crash.CrashRecordInfo import com.ms.square.debugoverlay.internal.crash.formatCrashRecordAsText import com.ms.square.debugoverlay.internal.util.copyToClipboard import com.ms.square.debugoverlay.internal.util.formatFullTimestamp @@ -50,12 +49,11 @@ import kotlinx.coroutines.launch @OptIn(ExperimentalMaterial3Api::class) @Composable internal fun CrashLogDetailScreen( - info: CrashRecordInfo, + record: CrashRecord, onBack: () -> Unit, onDelete: () -> Unit, modifier: Modifier = Modifier, ) { - val record = info.record val context = LocalContext.current val scope = rememberCoroutineScope() @@ -100,14 +98,12 @@ internal fun CrashLogDetailScreen( ) } ) { paddingValues -> - CrashLogDetailContent(info = info, modifier = Modifier.padding(paddingValues)) + CrashLogDetailContent(record = record, modifier = Modifier.padding(paddingValues)) } } @Composable -private fun CrashLogDetailContent(info: CrashRecordInfo, modifier: Modifier = Modifier) { - val record = info.record - +private fun CrashLogDetailContent(record: CrashRecord, modifier: Modifier = Modifier) { Column(modifier = modifier.fillMaxSize()) { SelectionContainer( modifier = Modifier diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt index 20bde88d..da2e6284 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt @@ -19,7 +19,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -34,50 +33,54 @@ import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.ms.square.debugoverlay.core.R +import com.ms.square.debugoverlay.internal.crash.CrashRecord import com.ms.square.debugoverlay.internal.crash.CrashRecordInfo import com.ms.square.debugoverlay.internal.util.formatRelativeTime -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.launch +import kotlinx.coroutines.flow.StateFlow /** * Crash tab content displaying [CrashRecordInfo]s persisted by * [com.ms.square.debugoverlay.internal.crash.CrashHandler] from previous app runs. * - * Only shown when at least one crash record exists (see `hasCrashRecords` gating in - * DebugPanelDialog), so an empty-state fallback is not needed for the list, only within it. + * The tab is always present, so the empty state is the common case until the first crash — + * it's what tells the reader the feature exists and that a record appears only after a restart. + * + * @param crashRecordsFlow emits null until the records have been read from disk, which renders + * as blank rather than as the empty state — otherwise "No Crashes Recorded" flashes for a + * frame every time the tab opens, including when records do exist. */ @Composable internal fun CrashLogTabContent( - crashRecordsFlow: Flow>, - onDeleteCrashRecord: suspend (CrashRecordInfo) -> Unit, + crashRecordsFlow: StateFlow?>, + onDeleteCrashRecord: (CrashRecordInfo) -> Unit, modifier: Modifier = Modifier, ) { - val crashRecords by crashRecordsFlow.collectAsStateWithLifecycle(initialValue = emptyList()) + val crashRecords by crashRecordsFlow.collectAsStateWithLifecycle() var selected by remember { mutableStateOf(null) } - val scope = rememberCoroutineScope() DetailNavigation( selectedItem = selected, onBack = { selected = null }, listContent = { - if (crashRecords.isEmpty()) { - EmptyCrashHistoryState() - } else { - CrashLogListScreen( - crashRecords = crashRecords, + when { + // Not loaded yet: a directory listing is fast enough that a spinner would itself flash. + crashRecords == null -> Box(modifier = Modifier.fillMaxSize()) + crashRecords.isNullOrEmpty() -> EmptyCrashHistoryState() + else -> CrashLogListScreen( + crashRecords = crashRecords.orEmpty(), onItemClick = { selected = it } ) } }, detailContent = { info -> CrashLogDetailScreen( - info = info, + record = info.record, onBack = { selected = null }, onDelete = { - scope.launch { - onDeleteCrashRecord(info) - selected = null - } + // Fire-and-forget: the repository owns the deletion's lifetime, so navigating away + // immediately can't strand it half-done. + onDeleteCrashRecord(info) + selected = null } ) }, @@ -97,14 +100,13 @@ private fun CrashLogListScreen( verticalArrangement = Arrangement.spacedBy(12.dp) ) { items(crashRecords, key = { it.record.id }) { info -> - CrashLogItem(info = info, onClick = { onItemClick(info) }) + CrashLogItem(record = info.record, onClick = { onItemClick(info) }) } } } @Composable -private fun CrashLogItem(info: CrashRecordInfo, onClick: () -> Unit, modifier: Modifier = Modifier) { - val record = info.record +private fun CrashLogItem(record: CrashRecord, onClick: () -> Unit, modifier: Modifier = Modifier) { val timeStamp = remember(record.timestampMs) { formatRelativeTime(record.timestampMs) } val itemDescription = stringResource( R.string.debugoverlay_crash_log_item_description, diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt index 1c099153..3991bc8c 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt @@ -4,6 +4,7 @@ import android.view.View import androidx.annotation.StringRes import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -56,7 +57,6 @@ import com.ms.square.debugoverlay.OverlayMode import com.ms.square.debugoverlay.core.R import com.ms.square.debugoverlay.internal.bugreport.BugReportGenerator import com.ms.square.debugoverlay.internal.bugreport.ui.BugReportActivity -import com.ms.square.debugoverlay.internal.bugreport.ui.DraftCountBadge import com.ms.square.debugoverlay.internal.data.DEFAULT_CUSTOM_LOG_SOURCE_NAME import com.ms.square.debugoverlay.internal.data.DebugOverlayDataRepository import kotlinx.coroutines.isActive @@ -66,7 +66,7 @@ private enum class BuiltInTab(@param:StringRes val titleResId: Int) { LOGCAT(R.string.debugoverlay_tab_logcat), CUSTOM_LOG(R.string.debugoverlay_tab_custom_log), // Fallback; UI uses dynamic title from source APP_EXITS(R.string.debugoverlay_tab_app_exits), - CRASH_LOG(R.string.debugoverlay_tab_crash_log), // Only shown when a persisted crash record exists + CRASH_LOG(R.string.debugoverlay_tab_crash_log), // Always shown; badged with the record count NETWORK(R.string.debugoverlay_tab_network), JANKSTATS(R.string.debugoverlay_tab_jankstats), UI(R.string.debugoverlay_tab_ui), @@ -246,8 +246,8 @@ private fun BugReportButton(isCapturing: Boolean, draftCount: Int, onClick: () - // Badge positioned at top-right corner if (draftCount > 0 && !isCapturing) { - DraftCountBadge( - draftCount = draftCount, + CountBadge( + count = draftCount, modifier = Modifier.align(Alignment.TopEnd) ) } @@ -266,15 +266,18 @@ private fun bugReportButtonDescription(isCapturing: Boolean, draftCount: Int) = private fun DebugPanelContent(isCompactHeight: Boolean, modifier: Modifier = Modifier) { val repository = DebugOverlay.overlayDataRepository val hasCustomLogSource by repository.hasCustomLogSource.collectAsStateWithLifecycle() - val hasCrashRecords by repository.hasCrashRecords.collectAsStateWithLifecycle() + // Collecting this also starts the repository's lazily-shared load of persisted crash records. + val crashRecordCount by repository.crashRecordCount.collectAsStateWithLifecycle() val customLogSourceName by repository.customLogSourceName.collectAsStateWithLifecycle() val customTabs = (DebugOverlay.config.overlayMode as? OverlayMode.WithCustomTabs)?.customTabs.orEmpty() - // Build visible tabs: built-in tabs (with CUSTOM_LOG/CRASH_LOG conditionally shown) + custom tabs - val visibleTabs = remember(hasCustomLogSource, hasCrashRecords, customTabs) { + // Build visible tabs: built-in tabs (with CUSTOM_LOG conditionally shown) + custom tabs. + // CRASH_LOG is always shown — it gates on data, not capability, so hiding it until the first + // crash would hide the feature from anyone who hasn't crashed yet (and shift neighbouring tab + // indices when one arrives). Its own empty state explains the wait; see AppExitTabContent. + val visibleTabs = remember(hasCustomLogSource, customTabs) { val builtIn = BuiltInTab.entries .filter { it != BuiltInTab.CUSTOM_LOG || hasCustomLogSource } - .filter { it != BuiltInTab.CRASH_LOG || hasCrashRecords } .map { PanelTab.BuiltIn(it) } builtIn + customTabs.map { PanelTab.Custom(it) } } @@ -287,6 +290,7 @@ private fun DebugPanelContent(isCompactHeight: Boolean, modifier: Modifier = Mod visibleTabs = visibleTabs, selectedIndex = selectedIndex, customLogSourceName = customLogSourceName, + crashRecordCount = crashRecordCount, onTabSelected = { selectedIndex = it } ) DebugPanelTabContent( @@ -302,6 +306,7 @@ private fun DebugPanelTabRow( visibleTabs: List, selectedIndex: Int, customLogSourceName: String?, + crashRecordCount: Int, onTabSelected: (Int) -> Unit, ) { PrimaryScrollableTabRow( @@ -310,21 +315,42 @@ private fun DebugPanelTabRow( containerColor = Color.Transparent ) { visibleTabs.forEachIndexed { index, panelTab -> + val isBadgedCrashTab = panelTab is PanelTab.BuiltIn && + panelTab.tab == BuiltInTab.CRASH_LOG && + crashRecordCount > 0 + // Announce the count as part of the tab rather than letting the badge read out as a + // stray digit after the label. + val tabDescription = if (isBadgedCrashTab) { + stringResource(R.string.debugoverlay_tab_crash_log_description, crashRecordCount) + } else { + null + } + Tab( selected = index == selectedIndex, onClick = { onTabSelected(index) }, + modifier = if (tabDescription != null) { + Modifier.semantics(mergeDescendants = true) { contentDescription = tabDescription } + } else { + Modifier + }, text = { - Text( - text = when (panelTab) { - is PanelTab.BuiltIn -> if (panelTab.tab == BuiltInTab.CUSTOM_LOG) { - customLogSourceName ?: DEFAULT_CUSTOM_LOG_SOURCE_NAME - } else { - stringResource(panelTab.tab.titleResId) - } - is PanelTab.Custom -> panelTab.tab.title - }, - style = MaterialTheme.typography.labelLarge - ) + Row(verticalAlignment = Alignment.CenterVertically) { + Text( + text = when (panelTab) { + is PanelTab.BuiltIn -> if (panelTab.tab == BuiltInTab.CUSTOM_LOG) { + customLogSourceName ?: DEFAULT_CUSTOM_LOG_SOURCE_NAME + } else { + stringResource(panelTab.tab.titleResId) + } + is PanelTab.Custom -> panelTab.tab.title + }, + style = MaterialTheme.typography.labelLarge + ) + if (isBadgedCrashTab) { + CountBadge(count = crashRecordCount, modifier = Modifier.padding(start = 6.dp)) + } + } } ) } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt new file mode 100644 index 00000000..b816f0d3 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt @@ -0,0 +1,20 @@ +package com.ms.square.debugoverlay.internal.util + +import android.content.Context +import android.net.Uri +import androidx.core.content.FileProvider +import java.io.File + +// Must stay in sync with the provider authority declared in this library's AndroidManifest.xml +// ("${applicationId}.debugoverlay.bugreport.provider"). +private const val PROVIDER_AUTHORITY_SUFFIX = ".debugoverlay.bugreport.provider" + +/** + * Wraps [file] in a `content://` [Uri] served by DebugOverlay's own [FileProvider], for sharing + * exports (bug report archives, crash logs) with other apps. + * + * [file] must live under one of the paths declared in `res/xml/debugoverlay_bugreport_paths.xml`, + * or [FileProvider] throws [IllegalArgumentException]. + */ +internal fun Context.debugOverlayFileUri(file: File): Uri = + FileProvider.getUriForFile(this, "$packageName$PROVIDER_AUTHORITY_SUFFIX", file) diff --git a/debugoverlay-core/src/main/res/values/strings.xml b/debugoverlay-core/src/main/res/values/strings.xml index 0eff3642..ac222f9e 100644 --- a/debugoverlay-core/src/main/res/values/strings.xml +++ b/debugoverlay-core/src/main/res/values/strings.xml @@ -54,6 +54,7 @@ AppExit Info + Crash, %1$d recorded No Crashes Recorded Crashes will appear here after your app\nrestarts following an unhandled exception. Crash: %1$s, %2$s diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt index e2adf5da..f1e5fe0d 100644 --- a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt @@ -1,139 +1,41 @@ package com.ms.square.debugoverlay.internal.crash import com.google.common.truth.Truth.assertThat -import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo -import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData -import com.ms.square.debugoverlay.model.LogEntry -import com.ms.square.debugoverlay.model.LogLevel -import com.ms.square.debugoverlay.model.NetworkRequest import org.junit.Test class CrashHandlerTest { private val previousHandler = FakeUncaughtExceptionHandler() - private val storage = FakeCrashRecordStorage() + private var capturedWith: Pair? = null + private var shouldThrowOnCapture = false - private fun createHandler( - logs: List = emptyList(), - customLogs: CustomLogSourceData? = null, - networkRequests: List = emptyList(), - appInfo: AppInfo? = null, - maxLogLines: Int = 100, - ) = CrashHandler( + private val handler = CrashHandler( previousHandler = previousHandler, - storage = storage, - cachedAppInfoProvider = { appInfo }, - logcatSnapshotProvider = { logs }, - customLogSnapshotProvider = { customLogs }, - networkRequestsSnapshotProvider = { networkRequests }, - maxLogLines = maxLogLines + captureCrash = { thread, throwable -> + if (shouldThrowOnCapture) error("simulated capture failure") + capturedWith = thread to throwable + } ) @Test - fun `uncaughtException writes a crash record and delegates to the previous handler`() { - val handler = createHandler() + fun `uncaughtException captures the crash and delegates to the previous handler`() { val thread = Thread.currentThread() val throwable = IllegalStateException("boom") handler.uncaughtException(thread, throwable) - val written = storage.written - assertThat(written).isNotNull() - assertThat(written!!.threadName).isEqualTo(thread.name) - assertThat(written.exceptionType).isEqualTo("java.lang.IllegalStateException") - assertThat(written.message).isEqualTo("boom") + assertThat(capturedWith).isEqualTo(thread to throwable) assertThat(previousHandler.invokedWith).isEqualTo(thread to throwable) } @Test - fun `uncaughtException writes null appInfo when the background fetch has not completed yet`() { - val handler = createHandler(appInfo = null) - - handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) - - assertThat(storage.written!!.appInfo).isNull() - } - - @Test - fun `uncaughtException includes appInfo once the background fetch has completed`() { - val appInfo = AppInfo( - packageName = "com.test.app", - versionName = "1.0.0", - versionCode = 1, - targetSdkVersion = 34, - minSdkVersion = 21, - isDebuggable = true, - installerStore = "Unknown", - installerPackage = null, - firstInstallTime = 0L, - lastUpdateTime = 0L - ) - val handler = createHandler(appInfo = appInfo) - - handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) - - assertThat(storage.written!!.appInfo).isEqualTo(appInfo) - } - - @Test - fun `uncaughtException delegates to previous handler even when storage write throws`() { - storage.shouldThrowOnWrite = true - val handler = createHandler() + fun `uncaughtException delegates to previous handler even when capture throws`() { + shouldThrowOnCapture = true handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) assertThat(previousHandler.invokedWith).isNotNull() } - - @Test - fun `uncaughtException trims logs, custom logs, and network requests to maxLogLines`() { - val logs = (1..10).map { fakeLogEntry(it) } - val customLogs = CustomLogSourceData(logs = (1..10).map { fakeLogEntry(it) }, sourceName = "Timber") - val requests = (1..10).map { fakeNetworkRequest(it) } - val handler = createHandler(logs = logs, customLogs = customLogs, networkRequests = requests, maxLogLines = 3) - - handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) - - val written = storage.written!! - assertThat(written.logcatLogs).hasSize(3) - assertThat(written.logcatLogs).isEqualTo(logs.takeLast(3)) - assertThat(written.customLogSourceData!!.logs).hasSize(3) - assertThat(written.networkRequests).hasSize(3) - } - - private fun fakeLogEntry(index: Int) = LogEntry( - timestampMs = index.toLong(), - level = LogLevel.INFO, - tag = "Test", - pid = 1, - tid = 1, - threadName = "main", - message = "message $index" - ) - - private fun fakeNetworkRequest(index: Int) = NetworkRequest( - protocol = "http/1.1", - method = "GET", - url = "https://example.com/$index", - statusCode = 200, - durationMs = 10L, - responseSize = 100L, - requestSize = 0L, - timestampMs = index.toLong() - ) -} - -private class FakeCrashRecordStorage : CrashRecordStorage { - var written: CrashRecord? = null - var shouldThrowOnWrite: Boolean = false - - override fun writeSync(record: CrashRecord) { - if (shouldThrowOnWrite) error("simulated write failure") - written = record - } - - override suspend fun listCrashRecords(): List = emptyList() - override suspend fun deleteCrashRecord(info: CrashRecordInfo) = Unit } private class FakeUncaughtExceptionHandler : Thread.UncaughtExceptionHandler { diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilderTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilderTest.kt new file mode 100644 index 00000000..cf5cd47b --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilderTest.kt @@ -0,0 +1,100 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.google.common.truth.Truth.assertThat +import com.ms.square.debugoverlay.internal.bugreport.model.AppInfo +import com.ms.square.debugoverlay.internal.bugreport.model.CustomLogSourceData +import com.ms.square.debugoverlay.model.LogEntry +import com.ms.square.debugoverlay.model.LogLevel +import com.ms.square.debugoverlay.model.NetworkRequest +import org.junit.Test + +class CrashRecordBuilderTest { + + @Test + fun `buildCrashRecord captures the thread and exception details`() { + val thread = Thread.currentThread() + + val record = buildRecord(thread = thread, throwable = IllegalStateException("boom")) + + assertThat(record.threadName).isEqualTo(thread.name) + assertThat(record.exceptionType).isEqualTo("java.lang.IllegalStateException") + assertThat(record.message).isEqualTo("boom") + assertThat(record.stackTrace).contains("java.lang.IllegalStateException: boom") + } + + // appInfo is queried on the crash path and may fail, so null is a supported input. + @Test + fun `buildCrashRecord carries appInfo through, keeping null when it could not be fetched`() { + val appInfo = AppInfo( + packageName = "com.test.app", + versionName = "1.0.0", + versionCode = 1, + targetSdkVersion = 34, + minSdkVersion = 21, + isDebuggable = true, + installerStore = "Unknown", + installerPackage = null, + firstInstallTime = 0L, + lastUpdateTime = 0L + ) + + assertThat(buildRecord(appInfo = appInfo).appInfo).isEqualTo(appInfo) + assertThat(buildRecord(appInfo = null).appInfo).isNull() + } + + @Test + fun `buildCrashRecord trims logs, custom logs, and network requests to maxLogLines`() { + val logs = (1..10).map { fakeLogEntry(it) } + + val record = buildRecord( + logcatLogs = logs, + customLogSourceData = CustomLogSourceData(logs = (1..10).map { fakeLogEntry(it) }, sourceName = "Timber"), + networkRequests = (1..10).map { fakeNetworkRequest(it) }, + maxLogLines = 3 + ) + + assertThat(record.logcatLogs).hasSize(3) + assertThat(record.logcatLogs).isEqualTo(logs.takeLast(3)) + assertThat(record.customLogSourceData?.logs).hasSize(3) + assertThat(record.networkRequests).hasSize(3) + } + + private fun buildRecord( + thread: Thread = Thread.currentThread(), + throwable: Throwable = RuntimeException("boom"), + appInfo: AppInfo? = null, + logcatLogs: List = emptyList(), + customLogSourceData: CustomLogSourceData? = null, + networkRequests: List = emptyList(), + maxLogLines: Int = 100, + ) = buildCrashRecord( + thread = thread, + throwable = throwable, + appInfo = appInfo, + logcatLogs = logcatLogs, + customLogSourceData = customLogSourceData, + networkRequests = networkRequests, + maxLogLines = maxLogLines + ) + + private fun fakeLogEntry(index: Int) = LogEntry( + timestampMs = index.toLong(), + level = LogLevel.INFO, + tag = "Test", + pid = 1, + tid = 1, + threadName = "main", + message = "message $index" + ) + + private fun fakeNetworkRequest(index: Int) = NetworkRequest( + protocol = "http/1.1", + method = "GET", + url = "https://example.com/$index", + statusCode = 200, + durationMs = 10L, + responseSize = 100L, + requestSize = 0L, + timestampMs = index.toLong() + ) +} diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt index dfc6ce43..a91940fe 100644 --- a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt @@ -2,7 +2,9 @@ package com.ms.square.debugoverlay.internal.crash import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.test.runTest +import org.junit.Rule import org.junit.Test +import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.RuntimeEnvironment @@ -13,13 +15,20 @@ private const val BASE_TIMESTAMP_MS = 1_700_000_000_000L @RunWith(RobolectricTestRunner::class) class CrashRecordStorageTest { - private val storage = DefaultCrashRecordStorage( - context = RuntimeEnvironment.getApplication(), - maxRecords = 3 - ) + // Only for files written outside the app dirs: Robolectric already gives each test method its + // own sandbox for context.noBackupFilesDir and tears it down afterwards, so recordsDir needs + // no cleanup of its own. This rule deletes its contents pass or fail. + @get:Rule + val temporaryFolder = TemporaryFolder() + + private val context = RuntimeEnvironment.getApplication() + + private val storage = DefaultCrashRecordStorage(context = context, maxRecords = 3) + + private val recordsDir = File(context.noBackupFilesDir, "debugoverlay_crash_records") @Test - fun `writeSync evicts oldest records beyond maxRecords`() = runTest { + fun `listCrashRecords evicts oldest records beyond maxRecords`() = runTest { repeat(5) { index -> storage.writeSync(fakeRecord(index)) } val records = storage.listCrashRecords() @@ -27,6 +36,27 @@ class CrashRecordStorageTest { assertThat(records).hasSize(3) } + @Test + fun `writeSync persists the schema version`() = runTest { + storage.writeSync(fakeRecord(0)) + + val storedJson = recordsDir.listFiles().orEmpty().single().readText() + + assertThat(storedJson).contains("\"version\":1") + } + + @Test + fun `listCrashRecords ignores and cleans up a temp file left by an unfinished write`() = runTest { + storage.writeSync(fakeRecord(0)) + val orphanedTemp = File(recordsDir, "crash_${BASE_TIMESTAMP_MS}_orphan.json.tmp") + orphanedTemp.writeText("{\"partial\":") + + val records = storage.listCrashRecords() + + assertThat(records).hasSize(1) + assertThat(orphanedTemp.exists()).isFalse() + } + @Test fun `listCrashRecords returns most recent first`() = runTest { repeat(5) { index -> storage.writeSync(fakeRecord(index)) } @@ -56,14 +86,13 @@ class CrashRecordStorageTest { @Test fun `deleteCrashRecord refuses to delete a file outside the records directory`() = runTest { storage.writeSync(fakeRecord(0)) - val outsideFile = File.createTempFile("crash_outside", ".json") + val outsideFile = temporaryFolder.newFile("crash_outside.json") outsideFile.writeText("not a real record") val maliciousInfo = CrashRecordInfo(filePath = outsideFile.absolutePath, record = fakeRecord(0)) storage.deleteCrashRecord(maliciousInfo) assertThat(outsideFile.exists()).isTrue() - outsideFile.delete() } private fun fakeRecord(index: Int) = CrashRecord( diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt index e4a6b673..8c560a04 100644 --- a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt @@ -54,7 +54,7 @@ class CrashRecordTextFormatterTest { responseSize = 100L, requestSize = 0L, timestampMs = 1_700_000_000_000L - ) + ).toSummary() ) ) @@ -82,7 +82,7 @@ class CrashRecordTextFormatterTest { requestSize = 0L, timestampMs = 1_700_000_000_000L, error = NetworkError(title = "IOException", message = "Connection reset by peer") - ) + ).toSummary() ) ) diff --git a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt deleted file mode 100644 index 7ad5cb7c..00000000 --- a/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSourceTest.kt +++ /dev/null @@ -1,22 +0,0 @@ -package com.ms.square.debugoverlay.internal.data.source - -import com.google.common.truth.Truth.assertThat -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job -import org.junit.Test - -class LogcatDataSourceTest { - - /** - * [LogcatDataSource.logs] is backed by `stateIn(SharingStarted.WhileSubscribed())`, so - * constructing the data source never starts the `logcat` subprocess on its own — - * only subscribing to [LogcatDataSource.logs] does. This keeps - * [LogcatDataSource.snapshotEntriesSync] safe to exercise without spawning a process. - */ - private val dataSource = LogcatDataSource(CoroutineScope(Job()), initialMaxEntries = 10) - - @Test - fun `snapshotEntriesSync returns empty list before Logcat has ever been subscribed`() { - assertThat(dataSource.snapshotEntriesSync()).isEmpty() - } -} From e9070e7ee67c593774a3d8e8b9d3e05d03b04ab7 Mon Sep 17 00:00:00 2001 From: Manabu-GT Date: Sat, 8 Aug 2026 03:06:15 -0700 Subject: [PATCH 11/12] another tweaks --- .../src/main/AndroidManifest.xml | 10 +++-- .../internal/crash/CrashRecordBuilder.kt | 3 -- .../data/DebugOverlayDataRepository.kt | 37 +++++-------------- .../internal/data/source/LogcatDataSource.kt | 9 +---- .../internal/ui/DebugPanelDialog.kt | 8 +--- .../internal/util/FileProviders.kt | 6 +-- .../res/xml/debugoverlay_bugreport_paths.xml | 14 ------- .../xml/debugoverlay_file_provider_paths.xml | 15 ++++++++ 8 files changed, 38 insertions(+), 64 deletions(-) delete mode 100644 debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml create mode 100644 debugoverlay-core/src/main/res/xml/debugoverlay_file_provider_paths.xml diff --git a/debugoverlay-core/src/main/AndroidManifest.xml b/debugoverlay-core/src/main/AndroidManifest.xml index 3e25cf5c..ce0afd14 100644 --- a/debugoverlay-core/src/main/AndroidManifest.xml +++ b/debugoverlay-core/src/main/AndroidManifest.xml @@ -18,19 +18,23 @@ android:excludeFromRecents="true" /> + android:resource="@xml/debugoverlay_file_provider_paths" /> diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt index 8d876e9e..b7c9dfac 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt @@ -14,9 +14,6 @@ internal const val DEFAULT_MAX_LOG_LINES = 100 * and can be tested without a [android.content.Context]. Each log/request source is trimmed * to the last [maxLogLines] entries, and network requests are reduced to * [NetworkRequestSummary] so bodies never reach disk. - * - * @param appInfo App info fetched once shortly after install; null if the crash beat that - * background fetch. */ internal fun buildCrashRecord( thread: Thread, diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt index 69f18ec5..8ab34159 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt @@ -115,17 +115,19 @@ internal class DebugOverlayDataRepository( val appExitInfos: Flow> = appExitDataSource.appExitInfos - // null means "not read from disk yet", which the UI must not render as "no crashes" — - // otherwise the empty state flashes for a frame every time the Crash tab opens. + @OptIn(ExperimentalCoroutinesApi::class) + val networkRequests: StateFlow> = currentNetworkRequestSource + .flatMapLatest { source -> source.requests } + .stateIn(scope, SharingStarted.Eagerly, emptyList()) + + // null means "not read from disk yet" private val _crashRecords = MutableStateFlow?>(null) val crashRecords: StateFlow?> = _crashRecords.asStateFlow() .onStart { refreshCrashRecords() } .stateIn(scope, SharingStarted.Lazily, null) - // Drives the Crash tab's count badge. Collecting this is enough to start the lazily-shared - // load above, so the panel doesn't have to hold the whole list (and recompose on every - // emission) just to show a number. + // Drives the Crash tab's count badge. val crashRecordCount: StateFlow = crashRecords .map { it?.size ?: 0 } .stateIn(scope, SharingStarted.Lazily, 0) @@ -149,7 +151,7 @@ internal class DebugOverlayDataRepository( * * App info is queried here rather than cached up front: it's two PackageManager IPC calls, * cheap next to the file write below, and caching it would cost every app start for a read - * that happens at most once per process. Guarded separately so a failure costs the app info + * that might be unnecessary. Guarded separately so a failure costs the app info * field, not the whole record. */ fun writeCrashRecordSync(thread: Thread, throwable: Throwable) { @@ -159,20 +161,14 @@ internal class DebugOverlayDataRepository( throwable = throwable, appInfo = runCatching { DefaultAppInfoProvider.getAppInfo(context) }.getOrNull(), logcatLogs = logcatDataSource.queryLogcatSnapshot(), - customLogSourceData = customLogSnapshotSync(), - networkRequests = networkRequestsSnapshot.value + customLogSourceData = customLogSourceName.value?.let { name -> CustomLogSourceData(customLogSourceLogs.value, name) }, + networkRequests = networkRequests.value ) ) } /** * Deletes a persisted crash record and re-syncs [crashRecords]. - * - * Runs on the repository's own [scope], not the caller's: when this ran on the crash tab's - * `rememberCoroutineScope()`, closing the panel mid-delete cancelled the coroutine between - * the delete and the refresh — `withContext` throws on return once the job is cancelled — so - * the file was gone while [crashRecords] still listed it. Nothing re-read it afterwards - * either, since the Lazily-shared `onStart` only runs on the first subscription. */ fun deleteCrashRecord(info: CrashRecordInfo) { scope.launch { @@ -186,19 +182,6 @@ internal class DebugOverlayDataRepository( suspend fun queryDeviceInfoSnapshot(): DeviceInfo = deviceInfoDataSource.queryDeviceInfoSnapshot() suspend fun queryAppExitInfosSnapshot(): List = appExitDataSource.queryAppExitInfosSnapshot() - @OptIn(ExperimentalCoroutinesApi::class) - val networkRequests: Flow> = currentNetworkRequestSource - .flatMapLatest { source -> source.requests } - - // Cached copy of the latest network requests, kept warm so writeCrashRecordSync() can read - // it without suspending. Mirrors customLogSourceLogs's SharingStarted.Eagerly pattern above. - private val networkRequestsSnapshot: StateFlow> = - networkRequests.stateIn(scope, SharingStarted.Eagerly, emptyList()) - - // Non-suspending snapshot of the custom log source's latest known logs, if one is registered. - private fun customLogSnapshotSync(): CustomLogSourceData? = - customLogSourceName.value?.let { name -> CustomLogSourceData(customLogSourceLogs.value, name) } - fun setNetworkSource(source: NetworkRequestSource) { currentNetworkRequestSource.value = source } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt index 13a78e31..d8cc49bf 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt @@ -160,14 +160,7 @@ internal class LogcatDataSource( } /** - * Returns a snapshot of the in-memory log buffer without suspending or spawning a subprocess, - * so it stays safe to call while the process is dying — see - * [com.ms.square.debugoverlay.internal.data.DebugOverlayDataRepository.writeCrashRecordSync], - * which calls this from the crashing thread. - * - * The buffer is filled from process start (the producer is shared eagerly), so this does not - * depend on the debug panel ever having been opened. Bug reports read it too. Entries are - * already filtered against [clear] as they are appended, so no post-filtering is needed here. + * Returns a snapshot of the in-memory log buffer without suspending or spawning a subprocess. */ fun queryLogcatSnapshot(): List = entries.toList() diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt index 3991bc8c..81c0b9c2 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/DebugPanelDialog.kt @@ -66,7 +66,7 @@ private enum class BuiltInTab(@param:StringRes val titleResId: Int) { LOGCAT(R.string.debugoverlay_tab_logcat), CUSTOM_LOG(R.string.debugoverlay_tab_custom_log), // Fallback; UI uses dynamic title from source APP_EXITS(R.string.debugoverlay_tab_app_exits), - CRASH_LOG(R.string.debugoverlay_tab_crash_log), // Always shown; badged with the record count + CRASH_LOG(R.string.debugoverlay_tab_crash_log), NETWORK(R.string.debugoverlay_tab_network), JANKSTATS(R.string.debugoverlay_tab_jankstats), UI(R.string.debugoverlay_tab_ui), @@ -266,15 +266,11 @@ private fun bugReportButtonDescription(isCapturing: Boolean, draftCount: Int) = private fun DebugPanelContent(isCompactHeight: Boolean, modifier: Modifier = Modifier) { val repository = DebugOverlay.overlayDataRepository val hasCustomLogSource by repository.hasCustomLogSource.collectAsStateWithLifecycle() - // Collecting this also starts the repository's lazily-shared load of persisted crash records. val crashRecordCount by repository.crashRecordCount.collectAsStateWithLifecycle() val customLogSourceName by repository.customLogSourceName.collectAsStateWithLifecycle() val customTabs = (DebugOverlay.config.overlayMode as? OverlayMode.WithCustomTabs)?.customTabs.orEmpty() - // Build visible tabs: built-in tabs (with CUSTOM_LOG conditionally shown) + custom tabs. - // CRASH_LOG is always shown — it gates on data, not capability, so hiding it until the first - // crash would hide the feature from anyone who hasn't crashed yet (and shift neighbouring tab - // indices when one arrives). Its own empty state explains the wait; see AppExitTabContent. + // Build visible tabs: built-in tabs (with CUSTOM_LOG conditionally shown) + custom tabs val visibleTabs = remember(hasCustomLogSource, customTabs) { val builtIn = BuiltInTab.entries .filter { it != BuiltInTab.CUSTOM_LOG || hasCustomLogSource } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt index b816f0d3..a58488d2 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/util/FileProviders.kt @@ -6,14 +6,14 @@ import androidx.core.content.FileProvider import java.io.File // Must stay in sync with the provider authority declared in this library's AndroidManifest.xml -// ("${applicationId}.debugoverlay.bugreport.provider"). -private const val PROVIDER_AUTHORITY_SUFFIX = ".debugoverlay.bugreport.provider" +// ("${applicationId}.debugoverlay.fileprovider"). +private const val PROVIDER_AUTHORITY_SUFFIX = ".debugoverlay.fileprovider" /** * Wraps [file] in a `content://` [Uri] served by DebugOverlay's own [FileProvider], for sharing * exports (bug report archives, crash logs) with other apps. * - * [file] must live under one of the paths declared in `res/xml/debugoverlay_bugreport_paths.xml`, + * [file] must live under one of the paths declared in `res/xml/debugoverlay_file_provider_paths.xml`, * or [FileProvider] throws [IllegalArgumentException]. */ internal fun Context.debugOverlayFileUri(file: File): Uri = diff --git a/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml b/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml deleted file mode 100644 index 34b5f96c..00000000 --- a/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - diff --git a/debugoverlay-core/src/main/res/xml/debugoverlay_file_provider_paths.xml b/debugoverlay-core/src/main/res/xml/debugoverlay_file_provider_paths.xml new file mode 100644 index 00000000..da60f28d --- /dev/null +++ b/debugoverlay-core/src/main/res/xml/debugoverlay_file_provider_paths.xml @@ -0,0 +1,15 @@ + + + + + + From 05a5a71cf69cb051c17f56d05673d05fac24668b Mon Sep 17 00:00:00 2001 From: Manabu-GT Date: Sat, 8 Aug 2026 17:57:15 -0700 Subject: [PATCH 12/12] cr fixes --- .../ms/square/debugoverlay/DebugOverlay.kt | 7 ++--- .../internal/crash/CrashRecord.kt | 2 ++ .../internal/crash/CrashRecordExporter.kt | 15 +++++++---- .../internal/crash/CrashRecordStorage.kt | 15 +++++++++++ .../data/DebugOverlayDataRepository.kt | 4 ++- .../internal/data/source/LogcatDataSource.kt | 26 ++++++++++++++----- 6 files changed, 53 insertions(+), 16 deletions(-) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt index 6d76a7ac..63caeed1 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt @@ -247,9 +247,10 @@ public object DebugOverlay { public var bugReportExporter: BugReportExporter = initial.bugReportExporter /** - * Maximum number of entries kept in the built-in Logcat tab buffer. - * Also passed to `logcat -T N` / `-t N` so it controls how many lines the - * OS replays on producer start (panel open) and on bug-report snapshot. + * Maximum number of entries kept in the built-in Logcat tab buffer, which is also what + * bug reports and crash records read. + * + * Reassigning this resizes the buffer immediately. * * Default is [Config.DEFAULT_MAX_LOGCAT_ENTRIES] (300). Each entry holds a parsed * [com.ms.square.debugoverlay.model.LogEntry]. diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt index 5e4493be..33af425e 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt @@ -16,6 +16,7 @@ import java.util.UUID * @param version Schema version, for forward-compatible reads via `ignoreUnknownKeys`. Carries * [EncodeDefault] because it always equals its default, which the serializer would otherwise * omit — leaving stored records with no version to read. + * @param id unique identifier for the crash record * @param timestampMs When the exception was caught (epoch millis). * @param threadName Name of the thread that crashed. * @param exceptionType Fully-qualified exception class name. @@ -31,6 +32,7 @@ import java.util.UUID internal data class CrashRecord( @EncodeDefault val version: Int = 1, + @EncodeDefault val id: String = UUID.randomUUID().toString(), val timestampMs: Long, val threadName: String, diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt index 266a792f..bd0f4405 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.withContext import java.io.File private const val EXPORTS_SUBDIR = "debugoverlay_crash_exports" +private const val ID_SUFFIX_LENGTH = 8 /** * Shares a [CrashRecord] as a plain-text file via Android's share sheet. @@ -23,10 +24,16 @@ private const val EXPORTS_SUBDIR = "debugoverlay_crash_exports" */ internal object CrashRecordExporter { - suspend fun share(context: Context, record: CrashRecord): Boolean = withContext(Dispatchers.IO) { + /** + * Failures are logged, not reported: the caller has nothing to do with the outcome today. + * Surface it (a snackbar, as the bug report flow does) before giving this a return value + * — a discarded result reads as if someone is handling it. + */ + suspend fun share(context: Context, record: CrashRecord): Unit = withContext(Dispatchers.IO) { runCatchingNonCancellation { val exportsDir = File(context.cacheDir, EXPORTS_SUBDIR).also { it.checkFolderExists() } - val file = File(exportsDir, "crash_${formatFilenameTimestamp(record.timestampMs)}.txt") + val idSuffix = record.id.take(ID_SUFFIX_LENGTH) + val file = File(exportsDir, "crash_${formatFilenameTimestamp(record.timestampMs)}_$idSuffix.txt") file.writeText(formatCrashRecordAsText(record)) val uri = context.debugOverlayFileUri(file) @@ -42,10 +49,8 @@ internal object CrashRecordExporter { withContext(Dispatchers.Main) { context.startActivity(Intent.createChooser(intent, chooserTitle).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)) } - true - }.getOrElse { e -> + }.onFailure { e -> Logger.w("Failed to share crash record", e) - false } } } diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt index 13da2aba..4c116b94 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt @@ -1,6 +1,7 @@ package com.ms.square.debugoverlay.internal.crash import android.content.Context +import android.os.StrictMode import com.ms.square.debugoverlay.internal.Logger import com.ms.square.debugoverlay.internal.util.checkFolderExists import com.ms.square.debugoverlay.internal.util.isDirectChildOf @@ -74,6 +75,20 @@ internal class DefaultCrashRecordStorage( } override fun writeSync(record: CrashRecord) { + // This library ships in debug builds, which is exactly where teams enable StrictMode's + // detectDiskWrites(). With penaltyDeath() the policy would kill the process right here — + // before CrashHandler's finally block reaches the previous handler — silently swallowing + // the crash for the platform and any other reporter. Even penaltyLog would add noise at + // the worst possible moment. Permit writes on this thread only, and restore after. + val originalPolicy = StrictMode.allowThreadDiskWrites() + try { + writeRecordLocked(record) + } finally { + StrictMode.setThreadPolicy(originalPolicy) + } + } + + private fun writeRecordLocked(record: CrashRecord) { synchronized(writeLock) { // Write to a temp file and rename, rather than writing the record in place: the // process is already dying, so an in-place write that doesn't finish would leave a diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt index 8ab34159..f2fab51e 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/DebugOverlayDataRepository.kt @@ -161,7 +161,9 @@ internal class DebugOverlayDataRepository( throwable = throwable, appInfo = runCatching { DefaultAppInfoProvider.getAppInfo(context) }.getOrNull(), logcatLogs = logcatDataSource.queryLogcatSnapshot(), - customLogSourceData = customLogSourceName.value?.let { name -> CustomLogSourceData(customLogSourceLogs.value, name) }, + customLogSourceData = customLogSourceName.value?.let { name -> + CustomLogSourceData(customLogSourceLogs.value, name) + }, networkRequests = networkRequests.value ) ) diff --git a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt index d8cc49bf..2d818ce5 100644 --- a/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt @@ -31,6 +31,12 @@ import java.io.InputStreamReader import java.util.concurrent.TimeUnit import kotlin.time.Duration.Companion.milliseconds +// How much history `logcat -T N` replays when the reader starts. Fixed rather than tied to +// maxEntries: the reader starts at install() (process start), so this history predates the +// process — mostly the previous run's leftovers in the ring buffer. A small, constant amount +// of context is what's wanted; how much is retained afterwards is maxEntries' job. +private const val REPLAY_LINES = 100 + /** * This only reads current app logs, not other apps (such requires a signature-level permission -> READ_LOGS). */ @@ -53,8 +59,8 @@ internal class LogcatDataSource( /** * Maximum number of entries retained in the in-memory buffer. Resizing takes effect on the - * queue immediately; the running subprocess keeps the `-T N` arg it was started with, which - * only affects how much history was replayed at process start. + * queue immediately, and affects nothing else — how much history the OS replays at start is + * [REPLAY_LINES]. */ var maxEntries: Int @IntRange(from = 1) @@ -94,7 +100,7 @@ internal class LogcatDataSource( "-v", "threadtime,printable,epoch", "-T", - maxEntries.toString() + REPLAY_LINES.toString() ).start().also { synchronized(processLock) { currentProcess = it @@ -103,11 +109,17 @@ internal class LogcatDataSource( reader = InputStreamReader(process.inputStream).buffered() while (currentCoroutineContext().isActive) { - // readLine() returns null at end of stream, so exit early if a process dies unexpectedly - val line = reader.readLine() ?: break + // readLine() returns null at end of stream, so exit early if a process dies unexpectedly. + // Nothing restarts the producer (it is shared eagerly for the process lifetime), so log + // it — otherwise the buffer silently freezes and stale logs reach crash records too. + val line = reader.readLine() + if (line == null) { + Logger.w("logcat stream ended unexpectedly; log buffer is frozen for this process") + break + } parser.parse(line)?.let { entry -> - // Drop OS-replayed entries from before the last clear. `-T N` replays the - // last N ring-buffer lines when the subprocess starts, which can predate a clear(). + // Drop OS-replayed entries from before the last clear: the ring-buffer lines + // `-T` replays at subprocess start can predate a clear(). // (Rare caveat: a backward system-clock jump could mis-drop a real entry.) if (entry.timestampMs < clearMarkerMs) return@let entries.add(entry)