keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
390 stars 108 forks source link

bug(android): error when background update completes but cannot notify #11550

Open sentry-io[bot] opened 4 months ago

sentry-io[bot] commented 4 months ago

Sentry Issue: KEYMAN-ANDROID-3Q8

onLexicalModelInstalled where context is null.

@jahorton here.

Based on my analysis of where this error is generated in the overall app/keyboard control flow... it appears that this is called when a lexical model is updated, but the context corresponding to the entity that requested the update is no longer valid.

The logging line in question was originally added in #3499... to investigate errors we saw before we started using Sentry. Whatever those errors were... is likely now lost to history, though.

My interpretation of this scenario is that we'd previously triggered a background update, which has now succeeded... but only after the host's lifetime is completed.

References:

https://github.com/keymanapp/keyman/blob/b068f94537e6bf8703068143473aec7d30ff584c/android/KMEA/app/src/main/java/com/keyman/engine/logic/ResourcesUpdateTool.java#L446-L449

Note the .isFinishing check:

https://developer.android.com/reference/android/app/Activity#isFinishing()

https://stackoverflow.com/q/10847526

I know Activity.finish() method calls somewhere in the way to Activity.onDestroy(), and also removing the activity from stack [...]

From the Android docs link above:

isFinishing Check to see whether this activity is in the process of finishing, either because you called finish() on it or someone else has requested that it finished. [...]

jahorton commented 3 months ago

See also #10112 - it triggers the same kind of error, but for packages in general. Also includes a repro!

jahorton commented 3 months ago

Based on observations about #10112's repro, I feel that these two issues should pretty low priority. No user-facing functionality is affected save, possibly, for a 'toast' notification about the update completing.

mcdurdin commented 3 months ago

OK with lowpri, triage to 19.0 if you like.