1992849 - Crash in [@ java.lang.ClassCastException: at org.mozilla.fenix.components.PlayStoreReviewPromptController$$ExternalSyntheticLambda0.onComplete(R8$$SyntheticClass:112)]

Status: RESOLVED FIXED

(Firefox for Android :: Experimentation and Telemetry, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

This appears to be a new crash that started after the backout of bug 1986398. Marcin, can you please take a look? Seems like in general, we need to be more resilient to the possibility of Google Play not being available on devices we're installed on.

I'm also tentatively setting the status for Fx144 to affected, though at this point I'm only seeing crashes from Nightly builds. We can always change it later if that proves not to be the case.

Crash report: https://crash-stats.mozilla.org/report/index/18dfd847-235c-406b-8b9e-6a02f0251006

Top 10 frames:

0  org.mozilla.fenix.components.PlayStoreReviewPromptController$$ExternalSynthet...  onComplete  R8$$SyntheticClass:112
1  com.google.android.gms.tasks.zzi  run  com.google.android.gms:play-services-tasks@@18.4.0:10
2  android.os.Handler  handleCallback  Handler.java:991
3  android.os.Handler  dispatchMessage  Handler.java:102
4  android.os.Looper  loopOnce  Looper.java:232
5  android.os.Looper  loop  Looper.java:317
6  android.app.ActivityThread  main  ActivityThread.java:8934
7  java.lang.reflect.Method  invoke  Method.java:-2
8  com.android.internal.os.RuntimeInit$MethodAndArgsCaller  run  RuntimeInit.java:591
9  com.android.internal.os.ZygoteInit  main  ZygoteInit.java:911

aw) 	

java.lang.ClassCastException: com.google.android.play.core.review.internal.zzu cannot be cast to com.google.android.play.core.review.ReviewException
	at org.mozilla.fenix.components.PlayStoreReviewPromptController$$ExternalSyntheticLambda0.onComplete(R8$$SyntheticClass:112)
	at com.google.android.gms.tasks.zzi.run(com.google.android.gms:play-services-tasks@@18.4.0:10)
	at android.os.Handler.handleCallback(Handler.java:991)
	at android.os.Handler.dispatchMessage(Handler.java:102)
	at android.os.Looper.loopOnce(Looper.java:232)
	at android.os.Looper.loop(Looper.java:317)
	at android.app.ActivityThread.main(ActivityThread.java:8934)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:591)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:911)

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Bug 1992846 shows a bit of a different exception but appears to confirm that 144 is also affected.

java.io.NotSerializableException: com.google.android.gms.common.api.Status
	at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1240)
	at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1620)
	at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1581)
	at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1490)
	at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1234)
	at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1620)
	at java.io.ObjectOutputStream.defaultWriteObject(ObjectOutputStream.java:463)
	at java.lang.Throwable.writeObject(Throwable.java:1074)
	at java.lang.reflect.Method.invoke(Native Method)
	at java.io.ObjectStreamClass.invokeWriteObject(ObjectStreamClass.java:1079)
	at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1568)
	at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1490)
	at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1234)
	at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:354)
	at mozilla.components.lib.crash.CrashReporter.onCrash$lib_crash_release(CrashReporter.kt:334)
	at mozilla.components.lib.crash.handler.ExceptionHandler.uncaughtException(ExceptionHandler.kt:74)
	at java.lang.ThreadGroup.uncaughtException(ThreadGroup.java:1071)
	at java.lang.ThreadGroup.uncaughtException(ThreadGroup.java:1066)
	at java.lang.Thread.dispatchUncaughtException(Thread.java:2442)
	Suppressed: com.google.android.gms.tasks.RuntimeExecutionException: com.google.android.play.core.review.ReviewException: -1: Review Error(-1): The Play Store app is either not installed or not the official version. (https://developer.android.com/reference/com/google/android/play/core/review/model/ReviewErrorCode.html#PLAY_STORE_NOT_FOUND)
		at com.google.android.gms.tasks.zzw.getResult(com.google.android.gms:play-services-tasks@@18.3.2:27)
		at org.mozilla.fenix.components.PlayStoreReviewPromptController$$ExternalSyntheticLambda0.onComplete(R8$$SyntheticClass:144)
		at com.google.android.gms.tasks.zzi.run(com.google.android.gms:play-services-tasks@@18.3.2:10)
		at android.os.Handler.handleCallback(Handler.java:995)
		at android.os.Handler.dispatchMessage(Handler.java:103)
		at android.os.Looper.loopOnce(Looper.java:248)
		at android.os.Looper.loop(Looper.java:338)
		at android.app.ActivityThread.main(ActivityThread.java:9106)
		at java.lang.reflect.Method.invoke(Native Method)
		at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:593)
		at com.android.internal.os.ExecInit.main(ExecInit.java:50)
		at com.android.internal.os.RuntimeInit.nativeFinishInit(Native Method)
		at com.android.internal.os.RuntimeInit.main(RuntimeInit.java:371)
	Caused by: com.google.android.play.core.review.ReviewException: -1: Review Error(-1): The Play Store app is either not installed or not the official version. (https://developer.android.com/reference/com/google/android/play/core/review/model/ReviewErrorCode.html#PLAY_STORE_NOT_FOUND)
		at org.mozilla.fenix.components.PlayStoreReviewPromptController$tryPromptReview$reviewInfoFlow$1.invokeSuspend(PlayStoreReviewPromptController.kt:133)
		at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:9)
		at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:122)
		at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:4)
		at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:3)
		at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:94)

Comment 5

[Ted Campbell [:tcampbell]]

Issue introduced by Bug 1985507 (Fx144). That was then covered by 1986398 which was backed out, exposing original bug. Example of older crash on non-play-store phone https://crash-stats.mozilla.org/report/index/d933492a-e7c3-4adc-bee2-392800250907

Comment 6

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 1992849 - Do not call getResult when we do not complete successfully r=tcampbell,marcin

This patch fixes various bugs observed through our crash logs:

1. The documentation for Task.getResult says that it can throw
   multiple exceptions, so we shouldn't try to call it from a
   non-successful context. A simple attempt to pipe through the
   reviewErrorCode would allow telemetry to continue working, but
   a follow-up patch could make use of the propagated error code.
2. The Task.getException is not always a ReviewException, so
   type-casting it as such would mean that we crash at this point.
3. When we are in the case where the Play Store does not exist, we throw
   an ActivityNotFoundException and attempt to load the Play Store
   link to review the app (and I'm not sure why we would logically do
   that), we call tryLaunchPlayStoreReview which was written such that
   it would only navigate to the browser if called from the Settings. We
   change the direction to FromGlobal to circumvent this problem. Its
   unclear what the original intention might have been to use this value
   for if not telemetry.

Co-authored-by: Ted Campbell <[e-mail]>

Comment 7

[Ted Campbell [:tcampbell]]

Trigger criteria to show review prompt had different variants:

• Legacy: isDefaultBrowser && launchedFiveTimes
• Mixed: usedFourDaysThisWeek && (isDefaultBrowser || useBookmarks || launchedFiveTimes)
• jexl: usedFourDaysThisWeek && (isDefaultBrowser || useBookmarks)

Timeline:

• Initial - Review prompt is triggered by legacy criteria. Supports PlayStore and AOSP builds.
• Aug 05-07 - Bug 1976845, Bug 1979641, Bug 1976847 all land. Criteria changes Mixed criteria which is heavily restricted by the 4-of-7 condition.
• Aug 08 - Bug 1976872 lands. Switches to jexl trigger which is most restrictive.
• Aug 28 - Bug 1985507 lands. Introduces defect where non-play-store phones crash if prompt triggered due to mishandled exception
• Sep 05 - Bug 1986398 lands. Tries to detect review prompt not running and in that case doing fallback.
• Sep 15 - Fx144 goes to Beta.
• Oct 02 - Bug 1992079 lands to Fx 143,144,145. If Nimbus feature disabled, then use the Legacy conditions to trigger which is much more frequent.
• Oct 06 - Backout Bug 1986398. This removes the fallback, but defect from Bug 1985507 remains and Bug 1992079 keeps things happening often.

Patch above fixes Bug 1985507 to work correctly for AOSP (or many China variants for example) regardless of how sensitive triggers are.

Comment 8

[Ted Campbell [:tcampbell]]

Disabling Google Play Services and Google Play Store lets me reproduce these crashes on a normal phone. Using AOSP in an emulator also works.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c25e07ef5b3c
https://hg.mozilla.org/mozilla-central/rev/dc258c62ebc9
Do not call getResult when we do not complete successfully. r=tcampbell

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Nightly builds with this fix are building now and should be available from Google Play within the next few hours. Once we can confirm that the fix is working there, we'll figure out next steps.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jonalmeida, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox144 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Users without Google Play end up crashing when we try to show an in-app review prompt.
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a - manual testing was done by engineering and our affected audience has been vocal enough for us to know if the fix works for them.
• Risk associated with taking this patch: low
• Explanation of risk level: The patch turns definite cases into optional to avoid the crashing behaviour. We tested this patch on an AOSP emulator where we were able to reproduce the crashes and verified our fixes worked there.
• String changes made/needed: n/a
• Is Android affected?: yes

Comment 13

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 1992849 - Do not call getResult when we do not complete successfully r=tcampbell,marcin

This patch fixes various bugs observed through our crash logs:

1. The documentation for Task.getResult says that it can throw
   multiple exceptions, so we shouldn't try to call it from a
   non-successful context. A simple attempt to pipe through the
   reviewErrorCode would allow telemetry to continue working, but
   a follow-up patch could make use of the propagated error code.
2. The Task.getException is not always a ReviewException, so
   type-casting it as such would mean that we crash at this point.
3. When we are in the case where the Play Store does not exist, we throw
   an ActivityNotFoundException and attempt to load the Play Store
   link to review the app (and I'm not sure why we would logically do
   that), we call tryLaunchPlayStoreReview which was written such that
   it would only navigate to the browser if called from the Settings. We
   change the direction to FromGlobal to circumvent this problem. Its
   unclear what the original intention might have been to use this value
   for if not telemetry.

Original Revision: https://phabricator.services.mozilla.com/D267713

Comment 14

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/18537e77e192
https://hg.mozilla.org/releases/mozilla-beta/rev/f660a0d2ea6d

Comment 15

[Ted Campbell [:tcampbell]]

Updating info. The crashes actually began with Bug 1992079 (Oct 02)

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9518661 [details]
Bug 1992849 - Do not call getResult when we do not complete successfully r=tcampbell,marcin

Approved for 144.0rc2.

Comment 17

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/73da9c10290d
https://hg.mozilla.org/releases/mozilla-release/rev/1fe33561a4f3

Comment 18

[Adina Petridean QA [:apetridean]]

The QA team doesn’t have any de-Googled devices, but we verified on several others where no startup crashes occur: Google Pixel 7 (Android 16) and Motorola Nexus 6 (Android 8 ). Tests were performed on version 144.0rc2.