fix: contain workout-complete sync throws and isolate premium refresh failures (#569 review) - #570
Closed
cursor[bot] wants to merge 2 commits into
Closed
fix: contain workout-complete sync throws and isolate premium refresh failures (#569 review)#570cursor[bot] wants to merge 2 commits into
cursor[bot] wants to merge 2 commits into
Conversation
…eground sync (#566) AppLifecycleObserver (App.kt) launched syncTriggerManager.onAppForeground() in an unguarded scope.launch using rememberCoroutineScope() (plain Job, no SupervisorJob/CoroutineExceptionHandler). A non-CancellationException throwable from the foreground sync chain (refreshPremiumStatusFromServer / attemptSync -> Ktor/IO) propagated to propagateExceptionFinalResort -> processUnhandledException -> terminateWithUnhandledException -> abort(), killing the iOS-on-mac process with SIGABRT after wake (TestFlight 0.9.1 / 20260607234). Changes: - App.kt: wrap the scope.launch body in try/catch (rethrow CancellationException, log+swallow others via Logger.e). This is the crash-prevention boundary. - SyncTriggerManager.kt: wrap onAppForeground() body in try/catch (rethrow CancellationException, log via Logger.e and record via onSyncFailure) so foreground sync failures are recorded in RetryState for backoff/retry UI. - AppLifecycleCoroutineContainmentTest: pins the containment pattern at the coroutine-scope level (non-Cancellation throwable does not reach CoroutineExceptionHandler; CancellationException is rethrown and cancels job). - SyncTriggerManagerTest: regression tests that a raw throwable from refreshPremiumStatusFromServer() is recorded in RetryState and does not propagate, and that CancellationException is rethrown (not recorded). RCA: GPT-5.5 xhigh (issue #566 comment 4734443850). Fixes #566
… failures PR #569 fixed foreground SIGABRT (#566) but left two gaps: 1. onWorkoutCompleted() still called attemptSync() without containment. ActiveSessionEngine launches it via unguarded scope.launch after every workout save — the same Ktor/IO/SqlDelight throw path that aborted on foreground would crash post-workout. 2. onAppForeground() wrapped premium refresh and attemptSync in one try/catch, so a premium refresh throw skipped portal sync and incorrectly advanced backoff. Mirror the health body-weight pattern: isolate premium refresh, always attempt sync, only record onSyncFailure for attemptSync throws. Adds regression tests for both paths. Co-authored-by: Devil <9thLevelSoftware@users.noreply.github.com>
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
2 participants
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
Deep review of PR #569 (#566 foreground crash fix) found two high-severity gaps in the same coroutine exception-containment work.
Bug 1 — Workout-complete sync can still SIGABRT (crash)
Impact: Same class of iOS-on-mac
SIGABRTas #566, but triggered immediately after workout save when sync throws.Root cause: PR #569 contained exceptions in
onAppForeground()only.onWorkoutCompleted()still calledattemptSync()without containment, andActiveSessionEnginelaunches it via unguardedscope.launch { syncTriggerManager?.onWorkoutCompleted() }after every session persist. Any raw Ktor/IO/SqlDelight throwable fromsyncManager.sync()propagates toprocessUnhandledException→abort().Fix: Extract
attemptSyncSafely()(try/catch +onSyncFailure, rethrowCancellationException) and route bothonWorkoutCompleted()andonAppForeground()through it.Bug 2 — Premium refresh failure blocked portal sync + advanced backoff
Impact: After wake/resume, a transient premium refresh throw skipped
attemptSync()entirely and recorded a sync failure inRetryState, delaying legitimate portal sync for up to the backoff window.Root cause: PR #569 wrapped premium refresh and
attemptSyncin a single try/catch. Health body-weight sync already used an isolated catch; premium refresh did not.Fix: Add
refreshPremiumStatusFromConnectedPlatform()with the same isolated catch contract as health body-weight sync. Premium refresh failures are logged and swallowed; portal sync always proceeds.Validation
:shared:compileKotlinIosArm64and:shared:compileTestKotlinIosArm64passSyncTriggerManagerTestcases:onAppForegroundStillSyncsWhenPremiumRefreshThrowsonWorkoutCompletedRecordsFailureAndDoesNotPropagateWhenSyncThrowsBase
Builds on PR #569 (
fix/issue-566-foreground-coroutine-crash).