fix(auth): stop reporting Block Store calls Play services doesn't support - #1670
Merged
Merged
Conversation
…port On devices whose Play services predates blockstore_retrieve_bytes_with_options v3, retrieveBytes fails with UnsupportedApiCallException. PlayBlockStoreBytes already swallows it and returns null, but trace(error = ...) forwards it to Bugsnag, so each launch filed 2-3 warnings (error 6abc0c6b991833fd09b525ad: 10 events, 3 users, older vivo and realme phones on Android 11/12). Log that case as a breadcrumb instead. Other failures are still reported, and return values are unchanged.
…upported Launch reads Block Store 2-3 times, and on these devices every read makes a Play services call that fails the same way. Remember the first UnsupportedApiCallException for the process and return null after that. There is no reliable up-front check: checkApiAvailability passes because the Block Store API exists, and the error reports is_fully_rolled_out=false, so a Play services version gate would also misjudge devices. Writes and deletes use different features and keep calling through.
runCatching also caught CancellationException, so a cancelled read, write or delete was traced as a Block Store failure and returned a fallback value instead of cancelling. In write, a cancel during the isEndToEndEncryptionAvailable check fell back to false and went on to storeBytes with cloud backup off. That call starts in Play services before its await sees the cancellation, and an unset backup flag deletes previously backed-up data on the next sync. Each failure path now calls ensureActive() first. It throws only when the calling coroutine is cancelled, so a Play services task that fails with its own CancellationException is still handled as a failure.
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
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.
Bugsnag error 6abc0c6b991833fd09b525ad is an
UnsupportedApiCallExceptionforblockstore_retrieve_bytes_with_optionsv3. It comes from devices whose Play services is too old forretrieveBytes(RetrieveBytesRequest): 10 events from 3 users so far, on older vivo and realme phones running Android 11 and 12.PlayBlockStoreBytesalready catches the failure and returnsnull, which the class documents as the intended fallback. The noise comes fromtrace(error = ...), which forwards any error toBugsnag.notify. Launch reads Block Store more than once, so each launch filed 2–3 warnings.Read, write and delete now go through
traceFailure, which logsUnsupportedApiCallExceptionas a breadcrumb and keeps reporting everything else. Return values are unchanged.After the first
UnsupportedApiCallExceptionfrom a read, later reads in the same process returnnullwithout calling Play services. There's no reliable check up front:checkApiAvailabilitypasses because the Block Store API itself exists, and the error reportsis_fully_rolled_out=false, so a Play services version gate would misjudge devices too. Writes and deletes use different features, and nothing shows them failing, so they still call through.Every failure path now calls
ensureActive()before handling the error, becauserunCatchingwas also catching coroutine cancellation. The write path is where this mattered most: a cancel during theisEndToEndEncryptionAvailablecheck fell back tofalseand went on to issuestoreByteswith cloud backup off, which deletes the existing cloud copy on the next sync.ensureActive()throws only when the calling coroutine is cancelled, so a Play services task that fails with its ownCancellationExceptionis still treated as a failure.There's no unit test: the class talks to Play services directly, and the existing tests use a fake behind
BlockStoreBytes.