Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in [@ mozilla.appservices.places.uniffi.PlacesException$InternalPanic: at mozilla.appservices.places.uniffi.FfiConverterTypePlacesError.read(places.kt:3)] #4990

Closed
rvandermeulen opened this issue Jun 7, 2022 · 4 comments
Assignees

Comments

@rvandermeulen
Copy link
Contributor

rvandermeulen commented Jun 7, 2022

This is spiking in the Fenix 101 release, though it's been around since 99.

Crash report: https://crash-stats.mozilla.org/report/index/e928dc36-0a62-48f8-95de-1e98b0220607

Java stack trace:

mozilla.appservices.places.uniffi.PlacesException$InternalPanic
	at mozilla.appservices.places.uniffi.FfiConverterTypePlacesError.read(places.kt:3)
	at mozilla.appservices.places.uniffi.FfiConverterTypePlacesError$lift$1.invoke(places.kt:2)
	at mozilla.appservices.places.uniffi.FfiConverterTypePlacesError$lift$1.invoke(places.kt:1)
	at mozilla.appservices.places.uniffi.PlacesKt.liftFromRustBuffer(places.kt:2)
	at mozilla.appservices.places.uniffi.FfiConverterTypePlacesError.lift(places.kt:1)
	at mozilla.appservices.places.uniffi.PlacesException$ErrorHandler.lift(places.kt:2)
	at mozilla.appservices.places.uniffi.PlacesException$ErrorHandler.lift(places.kt:1)
	at mozilla.appservices.places.uniffi.PlacesConnection.getTopFrecentSiteInfos(places.kt:17)
	at mozilla.appservices.places.PlacesReaderConnection.getTopFrecentSiteInfos(PlacesConnection.kt:1)
	at mozilla.components.browser.storage.sync.PlacesHistoryStorage$getTopFrecentSites$2.invokeSuspend(PlacesHistoryStorage.kt:7)
	at mozilla.components.browser.storage.sync.PlacesHistoryStorage$getTopFrecentSites$2.invoke(PlacesHistoryStorage.kt:2)
	at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:1)
	at kotlinx.coroutines.BuildersKt.withContext(Unknown Source:9)
	at mozilla.components.feature.top.sites.DefaultTopSitesStorage.getTopSites(DefaultTopSitesStorage.kt:26)
	at org.mozilla.fenix.FenixApplication$initVisualCompletenessQueueAndQueueTasks$queueInitStorageAndServices$1$1.invokeSuspend(FenixApplication.kt:23)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:4)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:18)
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:1)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:10)

┆Issue is synchronized with this Jira Task
┆Story Points: 3
┆Sprint End Date: 2022-07-08

@rvandermeulen
Copy link
Contributor Author

@tarikeshaq
Copy link
Contributor

There is probably some investigation for the root cause (I'm guessing an SQL error of some type) however, the app (probably?) shouldn't crash when calling that API..

An immediate fix (to the crash, not the underlying problem) would be to handle the exceptions properly in the android-components call to getTopFrecentSites

One way to do that is to use the handler similar to how it's done in other API calls, like the recordObservation call for example, which will log to sentry but not crash on InternalPanic errors - this might make it easier to patch as well without needing us to cut a release over here in application-services

Generally speaking though, I see a bunch of calls in android-components that don't do the exceptions handling dance, and now I'm wondering if there is either an abstraction missing, or if we need to rethink up some of our exception handling

cc @mhammond and @bendk since you've been working on improving our error reporting

@tarikeshaq
Copy link
Contributor

FWIW, InternalPanic is very misleading, as it's not a panic at all... I've already renamed it to InternalError and it should be called that in the first firefox that used AS 93.1.0

@tarikeshaq
Copy link
Contributor

mozilla-mobile/android-components#12300 landed and should fix the crashes in 104 -

We have #4639 on file for properly dealing with corrupted databases, looking into the different Sentry messages related to this crash, it seems may are stemming from corrupted databases. I'll flag the corrupted databases bug to the team again and see if we can reprioratize it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants