fix: contain uncaught coroutine exception in AppLifecycleObserver foreground sync (#566)

[9thLevelSoftware]

Summary

Fixes the iOS-on-mac TestFlight 0.9.1 SIGABRT crash (issue #566) where an uncaught Kotlin coroutine exception from the foreground sync chain reached propagateExceptionFinalResort → processUnhandledException → abort() after wake.

Root cause

AppLifecycleObserver (App.kt) launches syncTriggerManager.onAppForeground() inside scope.launch { ... } using rememberCoroutineScope() — a plain Job with no SupervisorJob and no CoroutineExceptionHandler. Neither the launch body nor onAppForeground() had a try/catch, so any non-CancellationException throwable from the foreground sync chain (refreshPremiumStatusFromServer() / attemptSync() → Ktor/IO) propagated to the Kotlin/Native final-resort exception handler and aborted the process. This deviates from the codebase's established SupervisorJob containment pattern used by PortalAuthRepository, MigrationManager, KableBleRepository, etc.

RCA: GPT-5.5 xhigh — #566 (comment)

Changes

Primary fix — crash-prevention boundary (App.kt)
Wrap the scope.launch body in try/catch: rethrow CancellationException (preserve cancellation semantics), log + swallow all other throwables via Logger.e. This guarantees a foreground-sync throwable can never reach propagateExceptionFinalResort.

Defensive secondary fix (SyncTriggerManager.kt)
Wrap onAppForeground() body in try/catch: rethrow CancellationException, log via Logger.e and record the failure via onSyncFailure(e) so foreground sync failures are reflected in RetryState (enabling backoff/retry UI) rather than being silently swallowed by the outer catch.

Regression tests

• AppLifecycleCoroutineContainmentTest (new): pins the containment pattern at the coroutine-scope level — a non-CancellationException throwable does not reach the CoroutineExceptionHandler (would not abort), and CancellationException is rethrown so the launched job is cancelled.
• SyncTriggerManagerTest (extended): a raw throwable from refreshPremiumStatusFromServer() (the real crash path) is recorded in RetryState and does not propagate; CancellationException is rethrown and not recorded.

Test evidence

./gradlew :shared:testAndroidHostTest (JVM host unit tests, runs commonTest):

Test class	Tests	Failures	Errors
AppLifecycleCoroutineContainmentTest	2	0	0
SyncTriggerManagerTest	18	0	0
SyncBackoffTest	7	0	0
SyncFailureCapTest	8	0	0

All 35 tests pass, including the 4 new regression tests:

• nonCancellationThrowableDoesNotReachUncaughtHandler
• cancellationExceptionIsRethrownAndCancelsJob
• onAppForegroundRecordsFailureAndDoesNotPropagateWhenPremiumRefreshThrows
• onAppForegroundRethrowsCancellationException

Acceptance criteria

• A non-CancellationException throwable from onAppForeground() or any transitive call does not abort the process.
• The throwable is logged via Logger.e with call-site context.
• CancellationException is rethrown — cancellation semantics preserved.
• Foreground sync failures are recorded in RetryState (secondary fix).
• A regression test verifies the above and runs in CI (:shared:testAndroidHostTest).
• The fix applies to main.

Non-goals (per RCA)

• No change to sync logic, throttling, or backoff behavior.
• rememberCoroutineScope() is retained (Compose lifecycle-tied); the try/catch approach is lower-risk than swapping in a custom SupervisorJob scope.
• dSYM upload instrumentation deferred (separate task).

Fixes #566

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[gemini-code-assist]

Code Review

This pull request addresses a crash issue (#566) caused by uncaught coroutine exceptions during foreground sync. It introduces exception containment in both AppLifecycleObserver and SyncTriggerManager.onAppForeground() to catch and log general failures while rethrowing CancellationException to preserve coroutine cancellation semantics. Additionally, comprehensive regression tests have been added. The review feedback suggests several key improvements: removing the redundant outer try-catch block in App.kt, catching Exception instead of Throwable in SyncTriggerManager.kt to avoid swallowing critical JVM system errors, updating the test double to match, and deleting the redundant AppLifecycleCoroutineContainmentTest.kt file once the outer try-catch is removed.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The try-catch block in AppLifecycleObserver is redundant because syncTriggerManager.onAppForeground() is already fully wrapped in its own try-catch block that catches all non-cancellation exceptions, logs them, and handles the failure gracefully.

Additionally, keeping this outer try-catch introduces a maintainability risk: if any other code is added to this launch block in the future, any exceptions thrown by it would be caught here and logged with the misleading message "AppLifecycleObserver: onAppForeground failed".

We should simplify this by removing the redundant try-catch wrapper and directly calling syncTriggerManager.onAppForeground().

                    syncTriggerManager.onAppForeground()

[gemini-code-assist]

Catching Throwable is generally discouraged because it catches critical JVM system errors (such as OutOfMemoryError, StackOverflowError, or NoClassDefFoundError) which should not be swallowed or handled as transient sync failures. Swallowing these can leave the application in an unstable or corrupted state.

It is safer and more idiomatic to catch Exception instead, which covers all standard network, database, and serialization exceptions while allowing critical system errors to propagate.

Suggested change

	} catch (e: Throwable) {
	} catch (e: Exception) {

[gemini-code-assist]

To align with the production code change of catching Exception instead of Throwable, we should also update this test double's catch block to catch Exception.

Suggested change

	} catch (e: Throwable) {
	} catch (e: Exception) {

[gemini-code-assist]

If the redundant try-catch wrapper in App.kt is removed (as suggested in App.kt), this entire test class becomes redundant. The exception containment and cancellation propagation are already thoroughly tested in SyncTriggerManagerTest.kt (e.g., onAppForegroundRecordsFailureAndDoesNotPropagateWhenPremiumRefreshThrows and onAppForegroundRethrowsCancellationException). You can safely delete this file to keep the test suite clean and focused.