Skip to content

Google Play Service error handle #2900

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

Closed
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import dagger.hilt.android.qualifiers.ApplicationContext
import javax.inject.Inject
import javax.inject.Singleton
import timber.log.Timber

private val INSTALL_API_REQUEST_CODE = GoogleApiAvailability::class.java.hashCode() and 0xffff

Expand All @@ -37,29 +38,30 @@
* Displays a dialog to install Google Play Services, if missing. Throws an error if install not
* possible or cancelled.
*/
suspend fun installGooglePlayServices() {
suspend fun installGooglePlayServices(): Boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a Boolean return value means the caller needs to address both return value and thrown exceptions as failure states. Since we need to catch exceptions anyway, why not rely on those for error states?

val status = googleApiAvailability.isGooglePlayServicesAvailable(context)
if (status == ConnectionResult.SUCCESS) return
if (status == ConnectionResult.SUCCESS) return true

val requestCode = INSTALL_API_REQUEST_CODE
startResolution(status, requestCode, GooglePlayServicesMissingException())
getNextResult(requestCode)
return startResolution(status, requestCode)
}

private fun startResolution(status: Int, requestCode: Int, throwable: Throwable) {
if (!googleApiAvailability.isUserResolvableError(status)) throw throwable
private suspend fun startResolution(status: Int, requestCode: Int): Boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here.

if (!googleApiAvailability.isUserResolvableError(status)) return false

activityStreams.withActivity {
googleApiAvailability.showErrorDialogFragment(it, status, requestCode) { throw throwable }
return try {
activityStreams.withActivity { activity ->
googleApiAvailability.showErrorDialogFragment(activity, status, requestCode, null)
}
getNextResult(requestCode)
} catch (e: Exception) {
Timber.e(e, "Activity result was not successful for requestCode: $requestCode")
false
}
}

private suspend fun getNextResult(requestCode: Int) {
private suspend fun getNextResult(requestCode: Int): Boolean {

Check warning on line 63 in ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt#L63

Added line #L63 was not covered by tests
val result = activityStreams.getNextActivityResult(requestCode)
if (!result.isOk()) {
error("Activity result failed: requestCode = $requestCode, result = $result")
}
return result.isOk()

Check warning on line 65 in ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt

View check run for this annotation

Codecov / codecov/patch

ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt#L65

Added line #L65 was not covered by tests
}

class GooglePlayServicesMissingException : Error("Google play services not available")
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import android.view.View
import android.view.ViewGroup
import androidx.lifecycle.lifecycleScope
import com.google.android.ground.R
import com.google.android.ground.system.GoogleApiManager
import com.google.android.ground.ui.common.AbstractFragment
import com.google.android.ground.ui.common.EphemeralPopups
import dagger.hilt.android.AndroidEntryPoint
Expand Down Expand Up @@ -54,7 +53,9 @@ class StartupFragment : AbstractFragment() {
try {
viewModel.initializeLogin()
} catch (t: Throwable) {
onInitFailed(t)
Timber.e(t, "Failed to launch app")
popups.ErrorPopup().show(R.string.google_api_install_failed)
requireActivity().finish()
}
}
}
Expand All @@ -63,12 +64,4 @@ class StartupFragment : AbstractFragment() {
dismissProgressDialog()
super.onPause()
}

private fun onInitFailed(t: Throwable) {
Timber.e(t, "Failed to launch app")
if (t is GoogleApiManager.GooglePlayServicesMissingException) {
popups.ErrorPopup().show(R.string.google_api_install_failed)
}
requireActivity().finish()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,12 @@ internal constructor(
) : AbstractViewModel() {

/** Checks & installs Google Play Services and initializes the login flow. */
suspend fun initializeLogin() {
googleApiManager.installGooglePlayServices()
suspend fun initializeLogin(): Boolean {
val isGooglePlayServicesAvailable = googleApiManager.installGooglePlayServices()
if (!isGooglePlayServicesAvailable) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here. This entire condition goes away if installGooglePlayServices throws an exception.

error("Google Play Services installation failed")
}
userRepository.init()
return true
}
}
Loading