mozilla.appservices.autofill.AutofillException$OpenDatabaseException

[grigoryk]

This is our top crash right now for the 100 nightly cycle. Happens close to startup (as we're initializing the storage):

mozilla.appservices.autofill.AutofillException$OpenDatabaseException
	at mozilla.appservices.autofill.FfiConverterTypeAutofillError.read(autofill.kt:13)
	at mozilla.appservices.autofill.FfiConverterTypeAutofillError$lift$1.invoke(autofill.kt:2)
	at mozilla.appservices.autofill.FfiConverterTypeAutofillError$lift$1.invoke(autofill.kt:1)
	at mozilla.appservices.autofill.AutofillKt.liftFromRustBuffer(autofill.kt:2)
	at mozilla.appservices.autofill.FfiConverterTypeAutofillError.lift(autofill.kt:1)
	at mozilla.appservices.autofill.AutofillException$ErrorHandler.lift(autofill.kt:2)
	at mozilla.appservices.autofill.AutofillException$ErrorHandler.lift(autofill.kt:1)
	at mozilla.appservices.autofill.Store.<init>(autofill.kt:13)
	at mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage$conn$2.invoke(AutofillCreditCardsAddressesStorage.kt:5)
	at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:5)
	at mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage.getConn$service_sync_autofill_release(AutofillCreditCardsAddressesStorage.kt:1)
	at mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage$warmUp$2.invokeSuspend(AutofillCreditCardsAddressesStorage.kt:6)
	at mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage$warmUp$2.invoke(AutofillCreditCardsAddressesStorage.kt:2)
	at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:1)
	at kotlinx.coroutines.BuildersKt.withContext(Unknown Source)
	at org.mozilla.fenix.FenixApplication$initVisualCompletenessQueueAndQueueTasks$queueInitStorageAndServices$1$1.invokeSuspend(FenixApplication.kt:16)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:3)
	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 ┆epic: Triage ┆sprintEndDate: 2022-03-18

[grigoryk]

https://sentry.prod.mozaws.net/operations/firefox-nightly/issues/13180445/events/latest/ looks to be the same crash in Sentry. "Error executing SQL: database or disk is full" in particular is unhelpful.

[bendk]

If I'm reading the docs correctly (https://sqlite.org/rescode.html), this can occur if the temp directory gets filled up. I notice that places and logins both set PRAGMA temp_store = 2 specifically to avoid issues with the temp directory (some discussion here: https://github.com/mozilla/mentat/issues/505). Maybe this is the issue?

I notice there's also a bunch of other configuration differences between places, logins, and autofill. Would it make sense to define an sql-support function that sets a standard set of PRAGMAs?

[lougeniaC64]

@bendk Defining a standard set of pragmas for sql-support sounds like a great idea. Judging from issue #4435 I think a part of the issue is we haven't defined what the standard set of pragmas should be. But judging from the docs I think temp_store = 2 and journal_mode = WAL might be a good start.

Although it might be worth noting that push is the outlier.

[grigoryk]

Setting at least the PRAGMA temp_store = 2 was my suggestion as well, as a starting point for this 👍

[mhammond]

While I don't disagree that the pragmas should change, isn't a bigger problem that any SQL failure at startup is causing the app to crash?

[bendk]

It would be nice to have a generalized approach to this. Should we catch the error and create a store that always returns failures? Should the consumer catch the error and handle having no store?

[mhammond]

I suspect it will end up depending on the specific component and the specific request, and I'm mildly against having our components manage this (ie, this would exist purely in consumers) - but I don't think we need to get too sophisticated - eg, a log warning and the smallest amount of code to do the least worst thing seems reasonable. Eg, in this specific case, something like a silent failure to add a new autocomplete item is far from ideal, but far better than crashing, and it's arguable how far we go (eg, do we really want new strings and localizers etc to deal with this case?) Ultimately though, this is up to the app and not us.

An analogy is desktop with a corrupt sqlite places database - stuff gets very very weird, but there's both no attempt to repair this, nor is there special UI.

[mhammond]

I'm mildly against having our components manage this

uniffi decorators would possibly be a reasonable option :) But kotlin and swift changing to holding a nullable reference might be reasonable and sounds more appealing than handling it in rust. The pragma fix will probably drop this count a lot, so there's no urgency, but errors opening will still exist. Open to anything though :)

[bendk]

Yeah, I think it might be dependent on the component.

If it's possible to have a wrapper class, then I like having our components manage it. For example, maybe we could update LoginStore to store a Mutex<Option<LoginsDb>>. If that was None, then the get_by_base_domain() would log a warning and return 0 results. This way we only need to write that wrapper once, rather than once per consumer. That said, this may be impossible or too complicated.

If it's not possible, then I think the app needs to handle it at a higher level. Like implementing a GeckoLoginStorageDelegate for this case.

Seems like we should get some input from the app developers and maybe product? @grigoryk what do you think?

[bendk]

Re-opening this one, since the longer term question is still open.

[mhammond]

Moved to bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1866353

Change performed by the Move to Bugzilla add-on.