fix(updates): expedited request can't carry initial delay (crash hotfix)#552
fix(updates): expedited request can't carry initial delay (crash hotfix)#552rainxchzed merged 1 commit intomainfrom
Conversation
…Receiver against scheduling throws
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe pull request refactors work scheduling to improve robustness. ChangesWork Scheduling Configuration and Error Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Hotfix for the two crashes from PR #545.
Crash 1 — `Expedited jobs cannot be delayed`
```
java.lang.IllegalArgumentException: Expedited jobs cannot be delayed
at androidx.work.WorkRequest$Builder.build(WorkRequest.kt:305)
at zed.rainxch.core.data.services.UpdateScheduler.schedule(UpdateScheduler.kt:59)
```
`UpdateScheduler.schedule` was building a one-time `UpdateCheckWorker` with both `.setInitialDelay(1, TimeUnit.MINUTES)` (cold-start back-off, predates this PR) and `.setExpedited(...)` (added in PR #545 for aggressive-OEM headroom). WorkManager rejects the combination at `build()` time.
Fix: dropped `setInitialDelay`. Expedited dispatch is what we actually wanted on aggressive ROMs anyway — the 1-minute backoff was a minor optimisation, not a hard requirement. Network-constraint dispatch is fast enough on a fresh boot.
Crash 2 — `Broadcast already finished`
```
java.lang.IllegalStateException: Broadcast already finished
at android.content.BroadcastReceiver$PendingResult.sendFinished(BroadcastReceiver.java:313)
at android.app.ActivityThread.handleReceiver(ActivityThread.java:5591)
```
Caused by Crash 1's `IllegalArgumentException` escaping `BootReceiver.onReceive` after `pendingResult.finish()` had already run in the `finally` block. Android's broadcast pipeline saw the unhandled throw, called its own `sendFinished`, and tripped the already-finished guard.
Fix: added a defensive `catch (t: Throwable) { Logger.e(...) }` between `try` and `finally` in `BootReceiver`. Scheduling failures are logged and swallowed — they don't propagate to the framework. Crash 2 won't recur after Crash 1 is fixed, but the catch is the right shape for any future scheduler bug.
Test plan
Note
`scheduleBackgroundUpdateChecks` in `GithubStoreApp` was already wrapped in `try / Logger.e` so the same throw on cold start logged but didn't crash the app. The crash report came from BootReceiver's narrower error path.
Summary by CodeRabbit