From 2a5df54ae42980924b659e72a4eafbfd484e0148 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 15 Jun 2023 14:15:06 +0200 Subject: [PATCH 1/3] Fix crash: show an error message with a Retry button when there is no network when displaying the BootstrapBottomSheet. --- .../crypto/recover/BootstrapActions.kt | 1 + .../crypto/recover/BootstrapBottomSheet.kt | 5 ++ .../crypto/recover/BootstrapErrorFragment.kt | 54 ++++++++++++++ .../recover/BootstrapSharedViewModel.kt | 71 ++++++++++++------- .../features/crypto/recover/BootstrapStep.kt | 2 + .../res/layout/fragment_bootstrap_error.xml | 33 +++++++++ 6 files changed, 141 insertions(+), 25 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapErrorFragment.kt create mode 100644 vector/src/main/res/layout/fragment_bootstrap_error.xml diff --git a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapActions.kt b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapActions.kt index 395b4d04756..fd61bdd5dd4 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapActions.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapActions.kt @@ -20,6 +20,7 @@ import im.vector.app.core.platform.VectorViewModelAction import java.io.OutputStream sealed class BootstrapActions : VectorViewModelAction { + object Retry : BootstrapActions() // Navigation diff --git a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapBottomSheet.kt b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapBottomSheet.kt index be02737ef8b..586f60beaf2 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapBottomSheet.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapBottomSheet.kt @@ -212,6 +212,11 @@ class BootstrapBottomSheet : VectorBaseBottomSheetDialogFragment { + views.bootstrapIcon.isVisible = true + views.bootstrapTitleText.text = getString(R.string.bottom_sheet_setup_secure_backup_title) + showFragment(BootstrapErrorFragment::class) + } } super.invalidate() } diff --git a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapErrorFragment.kt b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapErrorFragment.kt new file mode 100644 index 00000000000..26b29d449ad --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapErrorFragment.kt @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.crypto.recover + +import android.view.LayoutInflater +import android.view.ViewGroup +import com.airbnb.mvrx.parentFragmentViewModel +import com.airbnb.mvrx.withState +import dagger.hilt.android.AndroidEntryPoint +import im.vector.app.R +import im.vector.app.core.epoxy.onClick +import im.vector.app.core.extensions.setTextOrHide +import im.vector.app.core.platform.VectorBaseFragment +import im.vector.app.databinding.FragmentBootstrapErrorBinding + +@AndroidEntryPoint +class BootstrapErrorFragment : + VectorBaseFragment() { + + override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): FragmentBootstrapErrorBinding { + return FragmentBootstrapErrorBinding.inflate(inflater, container, false) + } + + val sharedViewModel: BootstrapSharedViewModel by parentFragmentViewModel() + + override fun invalidate() = withState(sharedViewModel) { state -> + when (state.step) { + is BootstrapStep.Error -> { + views.bootstrapDescriptionText.setTextOrHide(errorFormatter.toHumanReadable(state.step.error)) + } + else -> { + // Should not happen, show a generic error + views.bootstrapDescriptionText.setTextOrHide(getString(R.string.unknown_error)) + } + } + views.bootstrapRetryButton.onClick { + sharedViewModel.handle(BootstrapActions.Retry) + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSharedViewModel.kt b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSharedViewModel.kt index b3d83b948b9..c9c2c5ce9ac 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSharedViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSharedViewModel.kt @@ -54,6 +54,7 @@ import org.matrix.android.sdk.api.session.crypto.keysbackup.extractCurveKeyFromR import org.matrix.android.sdk.api.session.crypto.keysbackup.toKeysVersionResult import org.matrix.android.sdk.api.session.securestorage.RawBytesKeySpec import org.matrix.android.sdk.api.session.uia.DefaultBaseAuth +import timber.log.Timber import java.io.OutputStream import kotlin.coroutines.Continuation import kotlin.coroutines.resumeWithException @@ -118,37 +119,48 @@ class BootstrapSharedViewModel @AssistedInject constructor( } } SetupMode.NORMAL -> { - // need to check if user have an existing keybackup - setState { - copy(step = BootstrapStep.CheckingMigration) - } + checkMigration() + } + } + } + + private fun checkMigration() { + // need to check if user have an existing keybackup + setState { + copy(step = BootstrapStep.CheckingMigration) + } - // We need to check if there is an existing backup - viewModelScope.launch(Dispatchers.IO) { - val version = tryOrNull { session.cryptoService().keysBackupService().getCurrentVersion() }?.toKeysVersionResult() - if (version == null) { - // we just resume plain bootstrap - doesKeyBackupExist = false + // We need to check if there is an existing backup + viewModelScope.launch(Dispatchers.IO) { + try { + val version = tryOrNull { session.cryptoService().keysBackupService().getCurrentVersion() }?.toKeysVersionResult() + if (version == null) { + // we just resume plain bootstrap + doesKeyBackupExist = false + setState { + copy(step = BootstrapStep.FirstForm(keyBackUpExist = doesKeyBackupExist, methods = this.secureBackupMethod)) + } + } else { + // we need to get existing backup passphrase/key and convert to SSSS + val keyVersion = tryOrNull { + session.cryptoService().keysBackupService().getVersion(version.version) + } + if (keyVersion == null) { + // strange case... just finish? + _viewEvents.post(BootstrapViewEvents.Dismiss(false)) + } else { + doesKeyBackupExist = true + isBackupCreatedFromPassphrase = keyVersion.getAuthDataAsMegolmBackupAuthData()?.privateKeySalt != null setState { copy(step = BootstrapStep.FirstForm(keyBackUpExist = doesKeyBackupExist, methods = this.secureBackupMethod)) } - } else { - // we need to get existing backup passphrase/key and convert to SSSS - val keyVersion = tryOrNull { - session.cryptoService().keysBackupService().getVersion(version.version) - } - if (keyVersion == null) { - // strange case... just finish? - _viewEvents.post(BootstrapViewEvents.Dismiss(false)) - } else { - doesKeyBackupExist = true - isBackupCreatedFromPassphrase = keyVersion.getAuthDataAsMegolmBackupAuthData()?.privateKeySalt != null - setState { - copy(step = BootstrapStep.FirstForm(keyBackUpExist = doesKeyBackupExist, methods = this.secureBackupMethod)) - } - } } } + } catch (failure: Throwable) { + Timber.e(failure, "Error while checking key backup") + setState { + copy(step = BootstrapStep.Error(failure)) + } } } } @@ -268,6 +280,9 @@ class BootstrapSharedViewModel @AssistedInject constructor( copy(step = BootstrapStep.AccountReAuth(stringProvider.getString(R.string.authentication_error))) } } + BootstrapActions.Retry -> { + checkMigration() + } } } @@ -568,6 +583,12 @@ class BootstrapSharedViewModel @AssistedInject constructor( ) } } + is BootstrapStep.Error -> { + // do we let you cancel from here? + if (state.canLeave) { + _viewEvents.post(BootstrapViewEvents.SkipBootstrap(state.passphrase != null)) + } + } } } diff --git a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapStep.kt b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapStep.kt index 3975f0e9a23..b807acc0ba2 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapStep.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapStep.kt @@ -105,6 +105,8 @@ sealed class BootstrapStep { object Initializing : BootstrapStep() data class SaveRecoveryKey(val isSaved: Boolean) : BootstrapStep() object DoneSuccess : BootstrapStep() + + data class Error(val error: Throwable) : BootstrapStep() } fun BootstrapStep.GetBackupSecretForMigration.useKey(): Boolean { diff --git a/vector/src/main/res/layout/fragment_bootstrap_error.xml b/vector/src/main/res/layout/fragment_bootstrap_error.xml new file mode 100644 index 00000000000..02c944fed2c --- /dev/null +++ b/vector/src/main/res/layout/fragment_bootstrap_error.xml @@ -0,0 +1,33 @@ + + + + + +