diff --git a/AGENTS.md b/AGENTS.md index edc879bd..558c0f0f 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` (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/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/DebugOverlay.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/DebugOverlay.kt index c66aef59..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 @@ -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.IntentShareExporter import com.ms.square.debugoverlay.internal.bugreport.validateFilename +import com.ms.square.debugoverlay.internal.crash.CrashHandler 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 +103,28 @@ public object DebugOverlay { repository = repository, activityProvider = viewManager ) + + installCrashHandler(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. + */ + private fun installCrashHandler(repository: DebugOverlayDataRepository) { + val previousHandler = Thread.getDefaultUncaughtExceptionHandler() + + Thread.setDefaultUncaughtExceptionHandler( + CrashHandler( + previousHandler = previousHandler, + captureCrash = repository::writeCrashRecordSync + ) + ) + } + // CopyOnWriteArrayList enables lock-free iteration during bug report generation // synchronized block in addBugReportContributor ensures atomic duplicate detection internal val bugReportContributors = CopyOnWriteArrayList() @@ -227,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/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/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/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/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 new file mode 100644 index 00000000..323c955c --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandler.kt @@ -0,0 +1,40 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.ms.square.debugoverlay.internal.Logger + +/** + * 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]. [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 captureCrash Builds and persists the crash record for the given thread/throwable. + */ +internal class CrashHandler( + private val previousHandler: Thread.UncaughtExceptionHandler?, + private val captureCrash: (Thread, Throwable) -> Unit, +) : Thread.UncaughtExceptionHandler { + + @Suppress("TooGenericExceptionCaught") + override fun uncaughtException(thread: Thread, throwable: Throwable) { + try { + 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) + // and the platform's own crash handling working. + runCatching { Logger.e("CrashHandler failed to capture crash record", t) } + } finally { + // 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) + } + } +} 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..33af425e --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecord.kt @@ -0,0 +1,83 @@ +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.EncodeDefault +import kotlinx.serialization.ExperimentalSerializationApi +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`. 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. + * @param message The exception's message, if any. + * @param stackTrace Full stack trace text, including any "Caused by" chain. + * @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, + @EncodeDefault + 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, +) + +/** + * 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. */ +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/CrashRecordBuilder.kt b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt new file mode 100644 index 00000000..b7c9dfac --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordBuilder.kt @@ -0,0 +1,39 @@ +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. + */ +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 new file mode 100644 index 00000000..bd0f4405 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordExporter.kt @@ -0,0 +1,56 @@ +package com.ms.square.debugoverlay.internal.crash + +import android.content.Context +import android.content.Intent +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 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. + * + * 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 { + + /** + * 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 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) + + 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)) + } + }.onFailure { e -> + Logger.w("Failed to share crash record", e) + } + } +} 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..4c116b94 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorage.kt @@ -0,0 +1,153 @@ +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 +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" +private const val RECORD_SUFFIX = ".json" +private const val TEMP_SUFFIX = ".tmp" +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. 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. 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) + + /** + * 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. */ + 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() 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 { + File(context.noBackupFilesDir, CRASH_RECORDS_SUBDIR).also { + it.checkFolderExists() + } + } + + 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 + // 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)) + 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 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 + val staleRecords = files.sortedDescending().drop(maxRecords) + staleRecords.forEach { it.delete() } + } + + override suspend fun listCrashRecords(): List = withContext(Dispatchers.IO) { + // 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.isRecordFile() }?.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 (!file.isDirectChildOf(recordsDir)) { + 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}") + } + } + + 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..46e899e6 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatter.kt @@ -0,0 +1,52 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.ms.square.debugoverlay.internal.util.formatFullTimestamp +import com.ms.square.debugoverlay.internal.util.toClipboardText + +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.toExportLine()) } + } +} + +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 e82dd3af..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 @@ -7,6 +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 @@ -16,29 +22,38 @@ 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 import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.awaitCancellation 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.flowOf import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext 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) @@ -48,6 +63,8 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop private val jankStatsDataSource = JankStatsDataSource() private val appExitDataSource = AppExitDataSource(context, scope) + private val crashRecordStorage: CrashRecordStorage = DefaultCrashRecordStorage(context) + init { scope.launch { try { @@ -87,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 @@ -99,15 +115,75 @@ internal class DebugOverlayDataRepository(context: Context, scope: CoroutineScop val appExitInfos: Flow> = appExitDataSource.appExitInfos + @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. + val crashRecordCount: StateFlow = crashRecords + .map { it?.size ?: 0 } + .stateIn(scope, SharingStarted.Lazily, 0) + + private suspend fun refreshCrashRecords() { + _crashRecords.value = withContext(Dispatchers.IO) { + runCatchingNonCancellation { + crashRecordStorage.listCrashRecords() + }.onFailure { + Logger.e("Failed to refresh crash records", it) + }.getOrDefault(emptyList()) + } + } + + /** + * 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 might be unnecessary. 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 = customLogSourceName.value?.let { name -> + CustomLogSourceData(customLogSourceLogs.value, name) + }, + networkRequests = networkRequests.value + ) + ) + } + + /** + * Deletes a persisted crash record and re-syncs [crashRecords]. + */ + 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() - @OptIn(ExperimentalCoroutinesApi::class) - val networkRequests: Flow> = currentNetworkRequestSource - .flatMapLatest { source -> source.requests } - 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..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 @@ -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 @@ -32,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,10 +58,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, and affects nothing else — how much history the OS replays at start is + * [REPLAY_LINES]. */ var maxEntries: Int @IntRange(from = 1) @@ -65,9 +69,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 +86,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 { @@ -98,7 +100,7 @@ internal class LogcatDataSource( "-v", "threadtime,printable,epoch", "-T", - maxEntries.toString() + REPLAY_LINES.toString() ).start().also { synchronized(processLock) { currentProcess = it @@ -107,12 +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 on every subprocess start, including when the - // panel reopens after WhileSubscribed cancelled us. + // 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) @@ -131,7 +138,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 +151,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,43 +172,9 @@ internal class LogcatDataSource( } /** - * Returns a snapshot of logcat logs for bug reports. - * Uses cached value if streaming was active (debug panel was viewed), otherwise captures directly. + * Returns a snapshot of the in-memory log buffer without suspending or spawning a subprocess. */ - 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 new file mode 100644 index 00000000..529e3493 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogDetailScreen.kt @@ -0,0 +1,214 @@ +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.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( + record: CrashRecord, + onBack: () -> Unit, + onDelete: () -> Unit, + modifier: Modifier = Modifier, +) { + 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(record = record, modifier = Modifier.padding(paddingValues)) + } +} + +@Composable +private fun CrashLogDetailContent(record: CrashRecord, modifier: Modifier = Modifier) { + 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..da2e6284 --- /dev/null +++ b/debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/ui/CrashLogTabContent.kt @@ -0,0 +1,179 @@ +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.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.CrashRecord +import com.ms.square.debugoverlay.internal.crash.CrashRecordInfo +import com.ms.square.debugoverlay.internal.util.formatRelativeTime +import kotlinx.coroutines.flow.StateFlow + +/** + * Crash tab content displaying [CrashRecordInfo]s persisted by + * [com.ms.square.debugoverlay.internal.crash.CrashHandler] from previous app runs. + * + * 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: StateFlow?>, + onDeleteCrashRecord: (CrashRecordInfo) -> Unit, + modifier: Modifier = Modifier, +) { + val crashRecords by crashRecordsFlow.collectAsStateWithLifecycle() + var selected by remember { mutableStateOf(null) } + + DetailNavigation( + selectedItem = selected, + onBack = { selected = null }, + listContent = { + 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( + record = info.record, + onBack = { selected = null }, + onDelete = { + // Fire-and-forget: the repository owns the deletion's lifetime, so navigating away + // immediately can't strand it half-done. + 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(record = info.record, onClick = { onItemClick(info) }) + } + } +} + +@Composable +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, + 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..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 @@ -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,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), NETWORK(R.string.debugoverlay_tab_network), JANKSTATS(R.string.debugoverlay_tab_jankstats), UI(R.string.debugoverlay_tab_ui), @@ -245,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) ) } @@ -265,6 +266,7 @@ 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 crashRecordCount by repository.crashRecordCount.collectAsStateWithLifecycle() val customLogSourceName by repository.customLogSourceName.collectAsStateWithLifecycle() val customTabs = (DebugOverlay.config.overlayMode as? OverlayMode.WithCustomTabs)?.customTabs.orEmpty() @@ -284,6 +286,7 @@ private fun DebugPanelContent(isCompactHeight: Boolean, modifier: Modifier = Mod visibleTabs = visibleTabs, selectedIndex = selectedIndex, customLogSourceName = customLogSourceName, + crashRecordCount = crashRecordCount, onTabSelected = { selectedIndex = it } ) DebugPanelTabContent( @@ -299,6 +302,7 @@ private fun DebugPanelTabRow( visibleTabs: List, selectedIndex: Int, customLogSourceName: String?, + crashRecordCount: Int, onTabSelected: (Int) -> Unit, ) { PrimaryScrollableTabRow( @@ -307,21 +311,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)) + } + } } ) } @@ -345,6 +370,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/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..a58488d2 --- /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.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_file_provider_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/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 +} diff --git a/debugoverlay-core/src/main/res/values/strings.xml b/debugoverlay-core/src/main/res/values/strings.xml index 707de8f9..ac222f9e 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,18 @@ Importance 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 + 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 deleted file mode 100644 index d3a68f38..00000000 --- a/debugoverlay-core/src/main/res/xml/debugoverlay_bugreport_paths.xml +++ /dev/null @@ -1,10 +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 @@ + + + + + + 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..f1e5fe0d --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashHandlerTest.kt @@ -0,0 +1,47 @@ +package com.ms.square.debugoverlay.internal.crash + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +class CrashHandlerTest { + + private val previousHandler = FakeUncaughtExceptionHandler() + private var capturedWith: Pair? = null + private var shouldThrowOnCapture = false + + private val handler = CrashHandler( + previousHandler = previousHandler, + captureCrash = { thread, throwable -> + if (shouldThrowOnCapture) error("simulated capture failure") + capturedWith = thread to throwable + } + ) + + @Test + fun `uncaughtException captures the crash and delegates to the previous handler`() { + val thread = Thread.currentThread() + val throwable = IllegalStateException("boom") + + handler.uncaughtException(thread, throwable) + + assertThat(capturedWith).isEqualTo(thread to throwable) + assertThat(previousHandler.invokedWith).isEqualTo(thread to throwable) + } + + @Test + fun `uncaughtException delegates to previous handler even when capture throws`() { + shouldThrowOnCapture = true + + handler.uncaughtException(Thread.currentThread(), RuntimeException("boom")) + + assertThat(previousHandler.invokedWith).isNotNull() + } +} + +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/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 new file mode 100644 index 00000000..a91940fe --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordStorageTest.kt @@ -0,0 +1,109 @@ +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 +import java.io.File + +private const val BASE_TIMESTAMP_MS = 1_700_000_000_000L + +@RunWith(RobolectricTestRunner::class) +class CrashRecordStorageTest { + + // 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 `listCrashRecords evicts oldest records beyond maxRecords`() = runTest { + repeat(5) { index -> storage.writeSync(fakeRecord(index)) } + + val records = storage.listCrashRecords() + + 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)) } + + 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 = 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() + } + + 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..8c560a04 --- /dev/null +++ b/debugoverlay-core/src/test/kotlin/com/ms/square/debugoverlay/internal/crash/CrashRecordTextFormatterTest.kt @@ -0,0 +1,103 @@ +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.NetworkError +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 + ).toSummary() + ) + ) + + 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)") + } + + @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") + ).toSummary() + ) + ) + + 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, + tag = "Test", + pid = 1, + tid = 1, + threadName = "main", + message = message + ) +} 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