Repository navigation
Conversation
- Bound AccountManager token waits and make them interruptible - Add OkHttp call timeout, session wait timeout, and overall sync timeout - Record interrupted/busy syncs in history and report them to Crashlytics - Log sync stages, session holder, and WorkManager stop reason - Bump versionCode to 17205 and regenerate baseline profiles Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cancellation races can leak the session lock or duplicate history rows, while blocking Drive maintenance can outlast the sync deadline.
3 open issues
What changed in this PR
This PR hardens AppWatcher’s background update checks against stalls and adds diagnostics to investigate gaps in update notifications.
Changes:
- Adds bounded token requests, HTTP calls, session-lock waits, and sync execution, with cancellation handling.
- Records interrupted and abandoned syncs, exposes new history statuses, and logs session stages and worker stop reasons.
- Adds regression tests, increments
versionCodeto17205, and refreshes the release startup profile.
| File | Description |
|---|---|
app/src/test/java/com/anod/appwatcher/sync/UpdateCheckRegistrationTest.kt |
Tests timeout, cancellation, busy-session, and abandoned-history handling. |
app/src/test/java/com/anod/appwatcher/accounts/AuthTokenBlockingTest.kt |
Tests token interruption and session-wait timeout recovery. |
app/src/release/generated/baselineProfiles/startup-prof.txt |
Refreshes the generated release startup profile. |
app/src/main/java/com/anod/appwatcher/sync/UpdateCheck.kt |
Adds sync deadlines, interruption recording, and stage diagnostics. |
app/src/main/java/com/anod/appwatcher/sync/SyncWorker.kt |
Logs worker lifecycle and supplies stop reasons. |
app/src/main/java/com/anod/appwatcher/sync/SchedulesHistoryScreen.kt |
Displays interrupted and busy-session statuses. |
app/src/main/java/com/anod/appwatcher/database/SchedulesTable.kt |
Adds unfinished-history lookup and conditional interruption updates. |
app/src/main/java/com/anod/appwatcher/database/entities/Schedule.kt |
Defines the new failure statuses. |
app/src/main/java/com/anod/appwatcher/CrashlyticsExceptionFilter.kt |
Keeps interruption diagnostics reportable. |
app/src/main/java/com/anod/appwatcher/AppModule.kt |
Configures a two-minute OkHttp call timeout. |
app/src/main/java/com/anod/appwatcher/accounts/AuthTokenBlocking.kt |
Bounds token waits and makes blocking requests interruptible. |
app/src/main/java/com/anod/appwatcher/accounts/AuthAccountInitializer.kt |
Adds timed session acquisition and holder-stage diagnostics. |
app/build.gradle.kts |
Increments the release build number. |
Review effort: Balanced
Comment on lines
+160
to
+162
| withTimeoutOrNull(timeouts.syncMillis) { | ||
| performSerialized(manualSync, stopReason) | ||
| } ?: -1 |
Comment on lines
+228
to
+230
| val recorded = if (schedule.id == -1L) { | ||
| SchedulesTable.Queries.save(interrupted, database) | ||
| true |
- Remove timed session acquisition and busy status; keep single gate, bounded token/HTTP calls and active sync cap - Add account-allowlisted persistent diagnostic recorder (30 days, 10 MiB) with Settings toggle, share and delete - Record scheduler, worker, session, sync stage and startup state events - Pass diagnostic recorder explicitly to sync and settings components Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved concurrency and cancellation issues can leave callers suspended indefinitely, duplicate sync history, and block UI work, while a missing test dependency causes a timeout.
6 open issues
1 resolved since last review
Review effort: Balanced
| val result = CompletableDeferred<Unit>() | ||
| pendingResults.add(result) | ||
| try { | ||
| commands.send(Command.Operation(action, result)) |
Comment on lines
+162
to
+163
| playSessionCoordinator.withSession(owner) { | ||
| withTimeoutOrNull(timeouts.syncMillis) { |
- Bind diagnostics consent to the account hash in Preferences - Make recorder operations non-throwing and self-refreshing - Reduce SettingsViewModel diagnostics handling and reuse StartActivity - Fix missing diagnostics dependency in account selection test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Account switches can retain consent and buffered events, while recorder shutdown, lock-held disk I/O, and unbounded Drive transfers can hang operations or delay cancellation.
7 open issues
2 resolved since last review
Review effort: Balanced
Comment on lines
+76
to
+78
| is Command.Record -> synchronized(lock) { | ||
| if (!prefs.isDiagnosticsEnabled || command.account != accountHash || command.generation != generation) return@synchronized | ||
| store.append(command.record.encode(command.time, process), command.time) |
Comment on lines
+43
to
+45
| var isDiagnosticsEnabled: Boolean | ||
| get() = isDiagnosticsAllowed && devicePreferences.getString("diagnostic-consent", null) == | ||
| accountSnapshot?.name?.let(DiagnosticAccounts::hash) |
Comment on lines
+163
to
+165
| withTimeoutOrNull(timeouts.syncMillis) { | ||
| performSerialized(manualSync, stopReason) | ||
| } ?: -1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Beta users (and the maintainer) stop receiving update notifications until the app is opened. On-device refresh history shows every multi-day gap starting with a scheduled sync row that started but never finished.
Likely cause: a scheduled sync blocks forever on AccountManager
getAuthToken(...).result(no timeout, not cancellable) while holding thePlaySessionCoordinatormutex, so every later sync waits on the lock.Fixes
runInterruptible;OperationCanceledExceptioncaused by cancellation is surfaced as cancellation.callTimeoutof 2 minutes.Diagnostics
PlaySession,UpdateCheck, andSyncWorker.Release
versionCode17205; release baseline profiles regenerated on a physical device.Tests
Opt-in diagnostic logs
The AppLog-tee follow-up is currently in the worktree and has not been committed or pushed.