Fix crash on Android 9–11 from expedited sync workers (#78)#79
Merged
Conversation
On API < 31, WorkManager backs an expedited work request with a
foreground service and calls CoroutineWorker.getForegroundInfo() to get
the notification to display. The default implementation throws
IllegalStateException("Not implemented"), which crashed the app — most
visibly at startup, where LibraryMirrorPushScheduler enqueues an
expedited library-mirror push. On API 31+ expedited work uses
JobScheduler, so getForegroundInfo() is never called and the crash
never surfaced there.
Override getForegroundInfo() in both expedited CoroutineWorkers
(SyncWorker, LibraryMirrorPushWorker) to return a ForegroundInfo backed
by a silent, low-importance notification on a dedicated channel. The
notification only appears on Android ≤ 11 while an expedited worker
runs and auto-clears on completion; behavior on Android 12+ is
unchanged (no foreground service, no notification).
No manifest changes needed: FOREGROUND_SERVICE is already merged
transitively from work-runtime, and the foreground-service path only
executes on API ≤ 30, so targetSdk 34's foreground-service-type and
DATA_SYNC permission requirements don't apply.
Adds regression tests in both modules under @config(sdk = [28])
asserting getForegroundInfo() returns a valid notification instead of
throwing.
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
1 participant
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.
Summary
Fixes #78 — a crash on Android 9 (and any Android < 12), most visibly at app startup.
Root cause
On API < 31, WorkManager backs an expedited work request with a foreground service and calls
CoroutineWorker.getForegroundInfo()to obtain the notification to display. The defaultCoroutineWorkerimplementation throwsIllegalStateException("Not implemented"):On API 31+ expedited work uses JobScheduler expedited jobs, so
getForegroundInfo()is never called — which is why the crash only surfaced on older devices.Two workers opt into expedited work and were missing the override:
LibraryMirrorPushWorker— enqueued expedited at app start (LibraryMirrorPushScheduler.enqueueOneTime(expedited = true)), the most likely culprit for crash-on-launch.SyncWorker— enqueued expedited for user-initiated "Sync now".The reporter's
OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUESTdoesn't help here: that policy only governs quota exhaustion, not the foreground-service requirement on pre-12.Fix
Override
getForegroundInfo()in both workers to return aForegroundInfobacked by a silent, low-importance (IMPORTANCE_LOW) notification on a dedicated channel.Resulting behavior
getForegroundInfo()is never called, no notification.Non-expedited triggers (ambient sync, daily periodic library push,
enqueueAfterLibraryChange) never run as a foreground service, so they show no notification on any version.No manifest changes needed:
FOREGROUND_SERVICEis already merged transitively fromwork-runtime, and the foreground-service path only executes on API ≤ 30, sotargetSdk 34's foreground-service-type /DATA_SYNCpermission requirements don't apply. NoPOST_NOTIFICATIONSconcern either (that's API 33+, where this path never runs).Tests
Regression tests in both modules under
@Config(sdk = [28])(Android 9) assertinggetForegroundInfo()returns a valid notification instead of throwing. Addedwork.testingas a test dependency to:data:sync.Verified locally via
scripts/dgradle :data:sync:testDebugUnitTest :data:library:testDebugUnitTest— BUILD SUCCESSFUL.