fix(auth): fix "Already resumed" crash in phone auth SMS auto-verification - #2447
fix(auth): fix "Already resumed" crash in phone auth SMS auto-verification#2447demolaf wants to merge 1 commit into
Conversation
19c6f52 to
655b983
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request improves phone number verification by handling coroutine cancellation and multiple callbacks more robustly. It transitions from suspendCoroutine to suspendCancellableCoroutine and introduces an AtomicBoolean to prevent multiple callbacks from resuming the continuation. Additionally, user-initiated cancellation now clears the pending loading state and propagates the CancellationException directly rather than emitting an error state. Comprehensive unit tests have been added to cover these scenarios. The review feedback suggests registering an invokeOnCancellation listener to mark the operation as resolved upon cancellation, which prevents late-arriving callbacks from attempting to resume the cancelled continuation.
655b983 to
233082c
Compare
Fixes #2446.
DefaultVerifier.verifyPhoneNumberadapted Firebase'sOnVerificationStateChangedCallbacks— which can fire more than once per request — onto a single-shotsuspendCoroutine, resuming the same continuation fromonVerificationCompleted,onVerificationFailedandonCodeSent. On the SMS auto-retrieval path Firebase deliversonCodeSentand thenonVerificationCompleted, so the second resume threwIllegalStateException: Already resumedon the main thread inside Firebase's own dispatcher, where nothing in the library can catch it. A singleAtomicBooleanlatch now lets the first callback resolve the continuation and drops later ones with a log line.Switching to
suspendCancellableCoroutinealso made cancellation reachable for the first time, soverifyPhoneNumbernow rethrowsCancellationExceptionrather than reporting it as an auth error, and clears a pendingLoadingstate so navigating away mid-request can't leave the UI spinning.Added
PhoneAuthDefaultVerifierTestcovering every callback ordering plus cancellation — verified the multi-callback cases fail on the old code and pass with the fix.