feat: pluggable crash report upload with state tracking - #4
Conversation
Add an opt-in mechanism to upload local crash reports (JVM stack traces and native Breakpad minidumps) through a consumer-supplied interface, tracking the upload state of each report so nothing is sent twice and failures are retried. - CrashReportUploader: backend-agnostic transport interface implemented by the host app; the library adds no INTERNET permission of its own. - Room-backed state store (PENDING/UPLOADING/UPLOADED/FAILED + attempt count). - CrashReportScanner reconciles both report locations (crashReports/*.txt by suffix and core_dump/*.dmp) with the store on each run. - CrashUploadWorker (WorkManager) drains uploadable reports off the crashing thread with a network constraint and exponential-backoff retry. - Public API on CrashReporter: setUploader / setUploadEnabled (off by default) / setDeleteAfterUpload / setMaxUploadAttempts / uploadPendingReports. - Remove leftover System.out.println debug lines from initialize/initNative. - Robolectric unit tests for the DAO, scanner, and worker; docs/upload.md. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
📝 WalkthroughWalkthroughAdds an opt-in crash report upload pipeline. Reports are discovered from disk, tracked in Room, processed by WorkManager through a configurable uploader, retried after failures, and optionally deleted after successful upload. CrashReporter exposes configuration and triggering APIs, with tests and documentation. ChangesCrash report upload pipeline
Breakpad tooling notes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant CrashReporter
participant CrashUploadManager
participant WorkManager
participant CrashUploadWorker
participant CrashReportUploader
Application->>CrashReporter: register uploader and enable uploads
CrashReporter->>CrashUploadManager: configure upload pipeline
CrashUploadManager->>WorkManager: enqueue constrained worker
WorkManager->>CrashUploadWorker: execute background upload
CrashUploadWorker->>CrashReportUploader: upload report
CrashReportUploader-->>CrashUploadWorker: return success or failure
CrashUploadWorker-->>WorkManager: return success or retry
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
crashreporter/docs/breadpad.md (1)
3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the tooling references authoritative and reproducible.
The Breakpad link points to a mutable branch, while the Chapter01 link is a third-party sample with Mac-only prebuilt tools and example-specific paths; its own README directs users to upstream Breakpad for building tools. (github.com) Prefer linking directly to the relevant upstream build/dump-inspection documentation, optionally retaining Chapter01 as an example, and pinning references to a known revision where reproducibility matters.
Suggested documentation update
### Fetch the source and build the tools -https://github.com/google/breakpad +[Breakpad source and build instructions](https://github.com/google/breakpad) ### Check the dump file with tools -https://github.com/AndroidAdvanceWithGeektime/Chapter01 +[Chapter01 sample tooling](https://github.com/AndroidAdvanceWithGeektime/Chapter01)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crashreporter/docs/breadpad.md` around lines 3 - 7, Update the documentation references in breadpad.md to link directly to upstream Breakpad build and dump-inspection documentation instead of relying on the mutable repository branch and third-party Chapter01 tooling; optionally retain Chapter01 only as an explicitly labeled example, and pin upstream references to a known revision where reproducibility is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crashreporter/build.gradle`:
- Around line 67-77: Change the androidx.test.ext:junit dependency declaration
from implementation to testImplementation in the Gradle dependencies block,
keeping it scoped only to tests and leaving the other dependency configurations
unchanged.
In
`@crashreporter/src/main/java/com/balsikandar/crashreporter/CrashReporter.java`:
- Around line 50-56: Update the directory creation in initNative to call
mkdirs() instead of mkdir() for coreFolder, ensuring the full crash-report path
is created when parent directories are missing while preserving the existing
existence check and Breakpad initialization.
In
`@crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashReportDao.kt`:
- Around line 27-35: Update the getUploadable and countUploadable queries to
include UPLOADING alongside PENDING and FAILED, so interrupted uploads are
eligible for retry while preserving the existing maxAttempts filtering and
ordering.
In
`@crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt`:
- Around line 45-50: Update CrashUploadWorker’s upload loop to check isStopped
at the start of each iteration and break before processing further reports; in
its catch block, also break when the exception is an InterruptedException or
isStopped is true, preventing cancellation from being recorded as a retryable
upload failure. Apply both changes at
crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt
lines 45-50 and 68-72.
- Around line 59-62: Update the deleteAfterUpload cleanup in CrashUploadWorker
so dao.deleteByPath(entity.filePath) runs only when file.delete() succeeds or
the file is already absent; retain the database row when deletion fails while
the file still exists.
---
Nitpick comments:
In `@crashreporter/docs/breadpad.md`:
- Around line 3-7: Update the documentation references in breadpad.md to link
directly to upstream Breakpad build and dump-inspection documentation instead of
relying on the mutable repository branch and third-party Chapter01 tooling;
optionally retain Chapter01 only as an explicitly labeled example, and pin
upstream references to a known revision where reproducibility is required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ead2fd5-73a6-4d8c-a732-221650f2fc21
📒 Files selected for processing (18)
crashreporter/build.gradlecrashreporter/docs/breadpad.mdcrashreporter/docs/upload.mdcrashreporter/src/main/java/com/balsikandar/crashreporter/CrashReporter.javacrashreporter/src/main/java/com/balsikandar/crashreporter/upload/CrashReportUploader.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/CrashUploadManager.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/ReportType.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/Converters.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashReportDao.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashReportDatabase.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashReportEntity.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashReportScanner.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.ktcrashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/UploadStatus.ktcrashreporter/src/test/java/com/balsikandar/crashreporter/upload/internal/CrashReportDaoTest.ktcrashreporter/src/test/java/com/balsikandar/crashreporter/upload/internal/CrashReportScannerTest.ktcrashreporter/src/test/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorkerTest.ktcrashreporter/src/test/resources/robolectric.properties
| // Crash-report upload state tracking + background upload | ||
| implementation 'androidx.room:room-runtime:2.6.1' | ||
| kapt 'androidx.room:room-compiler:2.6.1' | ||
| implementation 'androidx.work:work-runtime:2.9.1' | ||
|
|
||
| testImplementation 'junit:junit:4.13.2' | ||
| implementation 'androidx.test.ext:junit:1.1.5' | ||
| testImplementation 'androidx.room:room-testing:2.6.1' | ||
| testImplementation 'org.robolectric:robolectric:4.11.1' | ||
| testImplementation 'androidx.work:work-testing:2.9.1' | ||
| testImplementation 'androidx.test:core:1.5.0' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win
Do not leak test dependencies into the production artifact.
The androidx.test.ext:junit dependency is being added using implementation. Because this is an Android library project, this configuration bundles the test dependency into the compile classpath of any application consuming the library. Change it to testImplementation to restrict it exclusively to the testing scope.
🐛 Proposed fix
testImplementation 'junit:junit:4.13.2'
- implementation 'androidx.test.ext:junit:1.1.5'
+ testImplementation 'androidx.test.ext:junit:1.1.5'
testImplementation 'androidx.room:room-testing:2.6.1'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Crash-report upload state tracking + background upload | |
| implementation 'androidx.room:room-runtime:2.6.1' | |
| kapt 'androidx.room:room-compiler:2.6.1' | |
| implementation 'androidx.work:work-runtime:2.9.1' | |
| testImplementation 'junit:junit:4.13.2' | |
| implementation 'androidx.test.ext:junit:1.1.5' | |
| testImplementation 'androidx.room:room-testing:2.6.1' | |
| testImplementation 'org.robolectric:robolectric:4.11.1' | |
| testImplementation 'androidx.work:work-testing:2.9.1' | |
| testImplementation 'androidx.test:core:1.5.0' | |
| // Crash-report upload state tracking + background upload | |
| implementation 'androidx.room:room-runtime:2.6.1' | |
| kapt 'androidx.room:room-compiler:2.6.1' | |
| implementation 'androidx.work:work-runtime:2.9.1' | |
| testImplementation 'junit:junit:4.13.2' | |
| testImplementation 'androidx.test.ext:junit:1.1.5' | |
| testImplementation 'androidx.room:room-testing:2.6.1' | |
| testImplementation 'org.robolectric:robolectric:4.11.1' | |
| testImplementation 'androidx.work:work-testing:2.9.1' | |
| testImplementation 'androidx.test:core:1.5.0' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crashreporter/build.gradle` around lines 67 - 77, Change the
androidx.test.ext:junit dependency declaration from implementation to
testImplementation in the Gradle dependencies block, keeping it scoped only to
tests and leaving the other dependency configurations unchanged.
| private static void initNative(Context context) { | ||
| String fileDir = crashReportPath == null ? CrashUtil.getDefaultPath(context) : crashReportPath; | ||
| String fileDir = resolveCrashReportRoot(context); | ||
| File coreFolder = new File(new File(fileDir), "core_dump"); | ||
| if (!coreFolder.exists()) | ||
| coreFolder.mkdir(); | ||
|
|
||
| System.out.println(">>> core dump path : " + coreFolder.getAbsolutePath()); | ||
| CrashReporter.initBreakpad(coreFolder.getAbsolutePath()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use mkdirs() to safely handle missing parent directories.
If a custom crashReportSavePath is supplied and its parent directories do not exist yet, mkdir() will fail silently. This would prevent the core_dump folder from being created and minidumps from being saved. Using mkdirs() guarantees the entire path is created.
🛠️ Proposed fix
private static void initNative(Context context) {
String fileDir = resolveCrashReportRoot(context);
File coreFolder = new File(new File(fileDir), "core_dump");
if (!coreFolder.exists())
- coreFolder.mkdir();
+ coreFolder.mkdirs();
CrashReporter.initBreakpad(coreFolder.getAbsolutePath());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static void initNative(Context context) { | |
| String fileDir = crashReportPath == null ? CrashUtil.getDefaultPath(context) : crashReportPath; | |
| String fileDir = resolveCrashReportRoot(context); | |
| File coreFolder = new File(new File(fileDir), "core_dump"); | |
| if (!coreFolder.exists()) | |
| coreFolder.mkdir(); | |
| System.out.println(">>> core dump path : " + coreFolder.getAbsolutePath()); | |
| CrashReporter.initBreakpad(coreFolder.getAbsolutePath()); | |
| private static void initNative(Context context) { | |
| String fileDir = resolveCrashReportRoot(context); | |
| File coreFolder = new File(new File(fileDir), "core_dump"); | |
| if (!coreFolder.exists()) | |
| coreFolder.mkdirs(); | |
| CrashReporter.initBreakpad(coreFolder.getAbsolutePath()); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 51-51: Prevent path traversal
Context: new File(new File(fileDir), "core_dump")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal'). Security best practice.
(path-traversal-java)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crashreporter/src/main/java/com/balsikandar/crashreporter/CrashReporter.java`
around lines 50 - 56, Update the directory creation in initNative to call
mkdirs() instead of mkdir() for coreFolder, ensuring the full crash-report path
is created when parent directories are missing while preserving the existing
existence check and Breakpad initialization.
| @Query( | ||
| "SELECT * FROM crash_reports " + | ||
| "WHERE status IN ('PENDING', 'FAILED') AND attemptCount < :maxAttempts " + | ||
| "ORDER BY createdAt ASC" | ||
| ) | ||
| fun getUploadable(maxAttempts: Int): List<CrashReportEntity> | ||
|
|
||
| @Query("SELECT COUNT(*) FROM crash_reports WHERE status IN ('PENDING', 'FAILED') AND attemptCount < :maxAttempts") | ||
| fun countUploadable(maxAttempts: Int): Int |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Prevent crash reports from getting permanently stuck in the UPLOADING state.
If the application process is killed while a report is actively in the UPLOADING state, it will remain UPLOADING in the database forever. Because getUploadable and countUploadable only select PENDING and FAILED reports, that crash report will never be retried, resulting in silent data loss.
To seamlessly recover interrupted uploads, include 'UPLOADING' in these queries (assuming WorkManager is configured to prevent concurrent worker instances), or explicitly reset UPLOADING to PENDING at the start of the worker run.
🛠️ Proposed fix
`@Query`(
"SELECT * FROM crash_reports " +
- "WHERE status IN ('PENDING', 'FAILED') AND attemptCount < :maxAttempts " +
+ "WHERE status IN ('PENDING', 'FAILED', 'UPLOADING') AND attemptCount < :maxAttempts " +
"ORDER BY createdAt ASC"
)
fun getUploadable(maxAttempts: Int): List<CrashReportEntity>
- `@Query`("SELECT COUNT(*) FROM crash_reports WHERE status IN ('PENDING', 'FAILED') AND attemptCount < :maxAttempts")
+ `@Query`("SELECT COUNT(*) FROM crash_reports WHERE status IN ('PENDING', 'FAILED', 'UPLOADING') AND attemptCount < :maxAttempts")
fun countUploadable(maxAttempts: Int): Int📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Query( | |
| "SELECT * FROM crash_reports " + | |
| "WHERE status IN ('PENDING', 'FAILED') AND attemptCount < :maxAttempts " + | |
| "ORDER BY createdAt ASC" | |
| ) | |
| fun getUploadable(maxAttempts: Int): List<CrashReportEntity> | |
| @Query("SELECT COUNT(*) FROM crash_reports WHERE status IN ('PENDING', 'FAILED') AND attemptCount < :maxAttempts") | |
| fun countUploadable(maxAttempts: Int): Int | |
| `@Query`( | |
| "SELECT * FROM crash_reports " + | |
| "WHERE status IN ('PENDING', 'FAILED', 'UPLOADING') AND attemptCount < :maxAttempts " + | |
| "ORDER BY createdAt ASC" | |
| ) | |
| fun getUploadable(maxAttempts: Int): List<CrashReportEntity> | |
| `@Query`("SELECT COUNT(*) FROM crash_reports WHERE status IN ('PENDING', 'FAILED', 'UPLOADING') AND attemptCount < :maxAttempts") | |
| fun countUploadable(maxAttempts: Int): Int |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashReportDao.kt`
around lines 27 - 35, Update the getUploadable and countUploadable queries to
include UPLOADING alongside PENDING and FAILED, so interrupted uploads are
eligible for retry while preserving the existing maxAttempts filtering and
ordering.
There was a problem hiding this comment.
Items in UPLOADING state shouldn't be counted in the Uploadable set ?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e kt . crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal --exec cat -n {}
echo "---manager---"
fd CrashUploadManager.kt crashreporter/src/main/java/com/balsikandar/crashreporter/upload --exec cat -n {}Length of output: 17905
@rayworks Good catch, and you're right to push back on that.
Looking more closely at CrashUploadWorker.doWork(), getUploadable() is only queried once at the top of the run, then the loop marks each entity UPLOADING immediately before uploading and finalizes it before moving on — so within a single run there's no risk of an in-flight item being re-selected. Permanently adding 'UPLOADING' to the query (as I originally proposed) would only matter across runs, but it opens exactly the hazard you're describing: if uploadNow() (which uses ExistingWorkPolicy.REPLACE) is called while a previous run is still actively uploading something, a second run could pick up and re-upload a file the first run hasn't finished with yet.
A better fix is to keep UPLOADING out of the uploadable set, and instead reclaim stale UPLOADING rows once, at the very start of doWork(), before reconciliation. Since normal scheduling uses ExistingWorkPolicy.KEEP (no two runs pending/running at once) and REPLACE cancels the outstanding request, any row still UPLOADING when a new run starts is leftover from a run whose process died mid-upload — never one that's still alive — so it's safe to reset it back to PENDING:
// CrashReportDao.kt
`@Query`("UPDATE crash_reports SET status = 'PENDING' WHERE status = 'UPLOADING'")
fun resetStuckUploads()// CrashUploadWorker.kt, at the top of doWork(), before CrashReportScanner.reconcile(...)
dao.resetStuckUploads()This recovers interrupted uploads without ever letting UPLOADING rows be double-selected by a genuinely concurrent run, addressing your concern directly. Let me know if you'd like me to fold this into a plan for the actual code change.
✏️ Learnings added
Learnt from: rayworks
Repo: rayworks/CrashReporter PR: 4
File: crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashReportDao.kt:27-35
Timestamp: 2026-07-15T08:53:01.217Z
Learning: In `crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt`, `CrashUploadWorker.doWork()` calls `dao.getUploadable(maxAttempts)` only once at the top of each run, then marks/finalizes each entity's status synchronously inside the loop before moving to the next — so within a single run, marking an entity `UPLOADING` never causes it to be re-selected. In `crashreporter/src/main/java/com/balsikandar/crashreporter/upload/CrashUploadManager.kt`, uploads are scheduled via `WorkManager.enqueueUniqueWork(UNIQUE_WORK_NAME, policy, request)`; normal scheduling (`maybeEnqueue()`) uses `ExistingWorkPolicy.KEEP`, while `uploadNow()` uses `ExistingWorkPolicy.REPLACE` — the only path that could race with an actively running worker. Preferred fix for crash reports stuck in `UPLOADING` forever (e.g., process killed mid-upload): reset `UPLOADING` rows back to `PENDING` once at the start of `doWork()` (before reconciliation), rather than permanently including `'UPLOADING'` in the `getUploadable`/`countUploadable` queries, since the latter risks double-uploading a file that's still actively being processed by another run.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| for (entity in dao.getUploadable(maxAttempts)) { | ||
| val file = File(entity.filePath) | ||
| if (!file.exists()) { | ||
| dao.deleteByPath(entity.filePath) | ||
| continue | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Respect WorkManager cancellation to prevent spurious failures and attempt-count exhaustion.
If the system cancels the worker (e.g., due to suddenly lost network constraints), the thread is interrupted. Ignoring this cancellation causes the worker to loop through all remaining reports, fail them immediately, and increment their attempt counts—falsely penalizing reports for a system-level constraint loss.
crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt#L45-L50: Addif (isStopped) breakat the start of theforloop to safely abort remaining items.crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt#L68-L72: Inside the catch block, explicitly checkif (e is InterruptedException || isStopped) breakso that an interrupted ongoing upload isn't treated as a retryable failure.
📍 Affects 1 file
crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt#L45-L50(this comment)crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt#L68-L72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt`
around lines 45 - 50, Update CrashUploadWorker’s upload loop to check isStopped
at the start of each iteration and break before processing further reports; in
its catch block, also break when the exception is an InterruptedException or
isStopped is true, preventing cancellation from being recorded as a retryable
upload failure. Apply both changes at
crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt
lines 45-50 and 68-72.
| if (deleteAfterUpload) { | ||
| file.delete() | ||
| dao.deleteByPath(entity.filePath) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent duplicate uploads if file deletion fails.
If file.delete() returns false due to an I/O glitch, the database row is still unconditionally deleted. On the subsequent run, CrashReportScanner will rediscover the file on disk and re-insert it as a brand-new PENDING report, causing an infinite loop of duplicate uploads.
Only delete the database record if the file was successfully deleted or genuinely no longer exists.
🛠️ Proposed fix
- if (deleteAfterUpload) {
- file.delete()
- dao.deleteByPath(entity.filePath)
- }
+ if (deleteAfterUpload) {
+ if (file.delete() || !file.exists()) {
+ dao.deleteByPath(entity.filePath)
+ } else {
+ Log.w(TAG, "Failed to delete uploaded report: ${file.name}")
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (deleteAfterUpload) { | |
| file.delete() | |
| dao.deleteByPath(entity.filePath) | |
| } | |
| if (deleteAfterUpload) { | |
| if (file.delete() || !file.exists()) { | |
| dao.deleteByPath(entity.filePath) | |
| } else { | |
| Log.w(TAG, "Failed to delete uploaded report: ${file.name}") | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crashreporter/src/main/java/com/balsikandar/crashreporter/upload/internal/CrashUploadWorker.kt`
around lines 59 - 62, Update the deleteAfterUpload cleanup in CrashUploadWorker
so dao.deleteByPath(entity.filePath) runs only when file.delete() succeeds or
the file is already absent; retain the database row when deletion fails while
the file still exists.
1 participant
Summary
Adds an opt-in mechanism to upload local crash reports to a backend and track the upload state of each report. Previously the library only captured reports (JVM
*_crash.txt/*_exception.txtand native Breakpadcore_dump/*.dmp) and surfaced them on-device — there was no egress path and no state beyond the filename suffix.The library stays backend-agnostic: the host app supplies the transport via
CrashReportUploader; the library owns discovery, state, and retry. NoINTERNETpermission is added to the library manifest.What's included
CrashReportUploader— consumer-implementedupload(File, ReportType): Boolean.PENDING → UPLOADING → UPLOADED/FAILED), attempt count, timestamps, last error (crashreporter_uploads.db, internal storage).CrashReportScanner— reconciles both report locations with the store on each run; prunes rows for deleted files.CrashUploadWorker(WorkManager) — drains uploadable reports off the crashing thread, with aCONNECTEDconstraint and exponential-backoff retry up tomaxUploadAttempts(default 5).CrashReporter:setUploader,setUploadEnabled(off by default),setDeleteAfterUpload,setMaxUploadAttempts,uploadPendingReports.System.out.println(">>>…")debug lines frominitialize/initNative.docs/upload.md.Design notes
uploadPendingReports()while the app is running).Application.onCreate).Testing
./gradlew :crashreporter:testDebugUnitTest→ passing (11 new Robolectric tests across DAO, scanner, worker; success / false / throw / no-uploader paths). Robolectric pinned to SDK 33 since 4.11.1 has no sandbox for compileSdk 35.assembleDebugwas not run in this environment — worth a full assemble before release.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests