Play Store reporting Medic app crashes

[garethbowen]

Describe the bug
Play Store is reporting the "Medic" android app is crashing for 15% of users many with a android.app.Fragment$InstantiationException. The Medic app isn't "production" but we should investigate to ensure this doesn't impact anyone else, and prioritise accordingly.

To Reproduce
Not sure how to reproduce it.

Expected behavior
No crashes.

Logs

Exception java.lang.RuntimeException: Unable to start activity ComponentInfo{org.medicmobile.webapp.mobile/org.medicmobile.webapp.mobile.EmbeddedBrowserActivity}: android.app.Fragment$InstantiationException: Unable to instantiate fragment t.v: could not find Fragment constructor
  at android.app.ActivityThread.performLaunchActivity (ActivityThread.java:3491)
  at android.app.ActivityThread.handleLaunchActivity (ActivityThread.java:3643)
  at android.app.ActivityThread.handleRelaunchActivityInner (ActivityThread.java:5505)
  at android.app.ActivityThread.handleRelaunchActivity (ActivityThread.java:5404)
  at android.app.servertransaction.ActivityRelaunchItem.execute (ActivityRelaunchItem.java:69)
  at android.app.servertransaction.TransactionExecutor.executeCallbacks (TransactionExecutor.java:135)
  at android.app.servertransaction.TransactionExecutor.execute (TransactionExecutor.java:95)
  at android.app.ClientTransactionHandler.executeTransaction (ClientTransactionHandler.java:58)
  at android.app.ActivityThread.handleRelaunchActivityLocally (ActivityThread.java:5464)
  at android.app.ActivityThread.access$3400 (ActivityThread.java:244)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:2115)
  at android.os.Handler.dispatchMessage (Handler.java:106)
  at android.os.Looper.loop (Looper.java:223)
  at android.app.ActivityThread.main (ActivityThread.java:7707)
  at java.lang.reflect.Method.invoke
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:612)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:997)
Caused by android.app.Fragment$InstantiationException: Unable to instantiate fragment t.v: could not find Fragment constructor
  at android.app.Fragment.instantiate (Fragment.java:562)
  at android.app.FragmentContainer.instantiate (FragmentContainer.java:53)
  at android.app.FragmentState.instantiate (FragmentState.java:77)
  at android.app.FragmentManagerImpl.restoreAllState (FragmentManager.java:2867)
  at android.app.FragmentController.restoreAllState (FragmentController.java:142)
  at android.app.Activity.onCreate (Activity.java:1585)
  at org.medicmobile.webapp.mobile.EmbeddedBrowserActivity.onCreate
  at android.app.Activity.performCreate (Activity.java:8000)
  at android.app.Activity.performCreate (Activity.java:7984)
  at android.app.Instrumentation.callActivityOnCreate (Instrumentation.java:1309)
  at android.app.ActivityThread.performLaunchActivity (ActivityThread.java:3464)
Caused by java.lang.NoSuchMethodException: t.v.<init> []
  at java.lang.Class.getConstructor0 (Class.java:2332)
  at java.lang.Class.getConstructor (Class.java:1728)
  at android.app.Fragment.instantiate (Fragment.java:543)

Environment

• Instance: ?
• Android Version: A range...
• App Version: recent

Additional context
More info here: https://play.google.com/console/developers/7987044497930745454/app/4973604959552855131/vitals/crashes?errorType=CRASH&appProcessState=Foreground&days=28

[sugat009]

Updates:
Putting this on pause status for the time being as this issue seems to be a bit more time-consuming than initially assumed. I'll pick up this issue once I have finished my current ongoing tasks in the app services team.
So far my attempts to recreate this issue have been unsuccessful. I've tried this on a wide range of devices, both Android Studio emulators and physical devices, in various Android versions.
Things to do when resuming this:

1. As Gareth suggested, instead of recreating the issue, we could start addressing the issue based on the error stack trace instead.
2. Reach out to project officers and survey if this issue has been reported by any CHWs.(I'll be doing this shortly.)

[garethbowen]

I've had a look through the stacktrace. All search results are coming back saying every fragment needs a public empty constructor. This is confirmed by the docs: https://developer.android.com/reference/android/app/Fragment.html#Fragment()

Default constructor. Every fragment must have an empty constructor, so it can be instantiated when restoring its activity's state. It is strongly recommended that subclasses do not have other constructors with parameters, since these constructors will not be called when the fragment is re-instantiated; instead, arguments can be supplied by the caller with setArguments(Bundle) and later retrieved by the Fragment with getArguments().

Applications should generally not implement a constructor. Prefer onAttach(android.content.Context) instead. It is the first place application code can run where the fragment is ready to be used - the point where the fragment is actually associated with its context. Some applications may also want to implement onInflate(Activity, AttributeSet, Bundle) to retrieve attributes from a layout resource, although note this happens when the fragment is attached.

Following this thread and I noticed that the EmbeddedBrowserActivity.onCreate method that's throwing the Exception does set up the OpenSettingsDialogFragment here.

If you go to the OpenSettingsDialogFragment source you will in fact see we specify a constructor with args and do not have a default public no arg constructor.

So I think this is the smoking gun - we're doing exactly what the docs say we must not do. Funnily enough we're even suppressing the lint error that would have picked this up.

There's even a hint about how to reproduce this in this line of the docs...

It is strongly recommended that subclasses do not have other constructors with parameters, since these constructors will not be called when the fragment is re-instantiated

What I think might be happening is the app is returning from some sort of hibernation state and Android is reconstructing this Fragment and finding no constructor for it to call. This is still quite hard to reproduce because I don't know how to force an app into hibernation, but it's a start. Maybe there's a way in the emulator to force it?

Anyway, my approach to fix this would be...

1. Remove the lint suppression, run lint, and see that it picks up the problem.
2. Remove the constructor - best practice is not to provide a constructor so there's a default public one available and having it there makes it tempting to do the wrong thing again.
3. Find a different way to provide the arg that's currently passed in. I think there are two clear possibilities here: (a) Use the setArguments method which is built to do exactly what we're doing. Or (b) We only call the argument in one place - can we refactor it so that onTouch call is in the EmbeddedBrowserActivity and calls a method on the OpenSettingsDialogFragment? I think this would be cleaner so we're not holding on to a copy of the view, but it's also more work to refactor.
4. (optional) The FragmentTransation we're using is deprecated (see the notice on the docs) so we should look at refactoring it to recommended approach. I've marked this optional because it might make fixing the bug easier, but I suspect it's a bit of a diversion and we should do that in a separate effort.

@sugat009 Does that all make sense? Can you start on this list without necessarily reproducing the issue yourself?

[sugat009]

@garethbowen Thanks for such a detailed investigation and explanation. I'll pick this up after my work in the app services team is done.