From 62c4cfde89ea8aec3ecf0a7871ca5304536ef6b2 Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Thu, 30 Jun 2022 20:51:05 +0200 Subject: [PATCH] improve media upload error messages (#2602) --- .../components/compose/ComposeActivity.kt | 26 ++++++++++++------- .../components/compose/ComposeViewModel.kt | 2 +- .../tusky/components/compose/MediaUploader.kt | 18 ++++++++++--- .../tusky/util/ThrowableExtensions.kt | 26 +++++++++++++++++++ .../tusky/viewmodel/EditProfileViewModel.kt | 20 ++------------ app/src/main/res/layout/activity_compose.xml | 1 + 6 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt index 1d4703406..370f89c35 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt @@ -161,7 +161,7 @@ class ComposeActivity : val uriNew = result.uriContent if (result.isSuccessful && uriNew != null) { viewModel.cropImageItemOld?.let { itemOld -> - val size = getMediaSize(getApplicationContext().getContentResolver(), uriNew) + val size = getMediaSize(contentResolver, uriNew) lifecycleScope.launch { viewModel.addMediaToQueue( @@ -407,8 +407,13 @@ class ComposeActivity : enableButton(binding.composeAddMediaButton, active, active) enablePollButton(media.isNullOrEmpty()) }.subscribe() - viewModel.uploadError.observe { - displayTransientError(R.string.error_media_upload_sending) + viewModel.uploadError.observe { throwable -> + Log.w(TAG, "media upload failed", throwable) + if (throwable is UploadServerError) { + displayTransientError(throwable.errorMessage) + } else { + displayTransientError(R.string.error_media_upload_sending) + } } viewModel.setupComplete.observe { // Focus may have changed during view model setup, ensure initial focus is on the edit field @@ -553,19 +558,23 @@ class ComposeActivity : super.onSaveInstanceState(outState) } - private fun displayTransientError(@StringRes stringId: Int) { - val bar = Snackbar.make(binding.activityCompose, stringId, Snackbar.LENGTH_LONG) + private fun displayTransientError(errorMessage: String) { + val bar = Snackbar.make(binding.activityCompose, errorMessage, Snackbar.LENGTH_LONG) // necessary so snackbar is shown over everything bar.view.elevation = resources.getDimension(R.dimen.compose_activity_snackbar_elevation) + bar.setAnchorView(R.id.composeBottomBar) bar.show() } + private fun displayTransientError(@StringRes stringId: Int) { + displayTransientError(getString(stringId)) + } private fun toggleHideMedia() { this.viewModel.toggleMarkSensitive() } private fun updateSensitiveMediaToggle(markMediaSensitive: Boolean, contentWarningShown: Boolean) { - if (viewModel.media.value.isNullOrEmpty()) { + if (viewModel.media.value.isEmpty()) { binding.composeHideMediaButton.hide() } else { binding.composeHideMediaButton.show() @@ -904,11 +913,10 @@ class ComposeActivity : // Currently the only supported lossless format is png. val mimeType: String? = contentResolver.getType(item.uri) val isPng: Boolean = mimeType != null && mimeType.endsWith("/png") - val context = getApplicationContext() - val tempFile = createNewImageFile(context, if (isPng) ".png" else ".jpg") + val tempFile = createNewImageFile(this, if (isPng) ".png" else ".jpg") // "Authority" must be the same as the android:authorities string in AndroidManifest.xml - val uriNew = FileProvider.getUriForFile(context, BuildConfig.APPLICATION_ID + ".fileprovider", tempFile) + val uriNew = FileProvider.getUriForFile(this, BuildConfig.APPLICATION_ID + ".fileprovider", tempFile) viewModel.cropImageItemOld = item diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt index f15565d03..a7e1779cf 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt @@ -219,7 +219,7 @@ class ComposeViewModel @Inject constructor( val contentWarningChanged = showContentWarning.value!! && !contentWarning.isNullOrEmpty() && !startingContentWarning.startsWith(contentWarning.toString()) - val mediaChanged = !media.value.isNullOrEmpty() + val mediaChanged = media.value.isNotEmpty() val pollChanged = poll.value != null return modifiedInitialState || textChanged || contentWarningChanged || mediaChanged || pollChanged diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt index de7141a15..324540d12 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt @@ -23,7 +23,7 @@ import android.util.Log import android.webkit.MimeTypeMap import androidx.core.content.FileProvider import androidx.core.net.toUri -import at.connyduck.calladapter.networkresult.getOrThrow +import at.connyduck.calladapter.networkresult.fold import com.keylesspalace.tusky.BuildConfig import com.keylesspalace.tusky.R import com.keylesspalace.tusky.components.compose.ComposeActivity.QueuedMedia @@ -32,6 +32,7 @@ import com.keylesspalace.tusky.network.ProgressRequestBody import com.keylesspalace.tusky.util.MEDIA_SIZE_UNKNOWN import com.keylesspalace.tusky.util.getImageSquarePixels import com.keylesspalace.tusky.util.getMediaSize +import com.keylesspalace.tusky.util.getServerErrorMessage import com.keylesspalace.tusky.util.randomAlphanumericString import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -73,6 +74,7 @@ class AudioSizeException : Exception() class VideoSizeException : Exception() class MediaTypeException : Exception() class CouldNotOpenFileException : Exception() +class UploadServerError(val errorMessage: String) : Exception() class MediaUploader @Inject constructor( private val context: Context, @@ -223,8 +225,16 @@ class MediaUploader @Inject constructor( null } - val result = mediaUploadApi.uploadMedia(body, description).getOrThrow() - send(UploadEvent.FinishedEvent(result.id)) + mediaUploadApi.uploadMedia(body, description).fold({ result -> + send(UploadEvent.FinishedEvent(result.id)) + }, { throwable -> + val errorMessage = throwable.getServerErrorMessage() + if (errorMessage == null) { + throw throwable + } else { + throw UploadServerError(errorMessage) + } + }) awaitClose() } } @@ -241,7 +251,7 @@ class MediaUploader @Inject constructor( } private companion object { - private const val TAG = "MediaUploaderImpl" + private const val TAG = "MediaUploader" private const val STATUS_VIDEO_SIZE_LIMIT = 41943040 // 40MiB private const val STATUS_AUDIO_SIZE_LIMIT = 41943040 // 40MiB private const val STATUS_IMAGE_SIZE_LIMIT = 8388608 // 8MiB diff --git a/app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt b/app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt new file mode 100644 index 000000000..26f962554 --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt @@ -0,0 +1,26 @@ +package com.keylesspalace.tusky.util + +import org.json.JSONException +import org.json.JSONObject +import retrofit2.HttpException + +/** + * checks if this throwable indicates an error causes by a 4xx/5xx server response and + * tries to retrieve the error message the server sent + * @return the error message, or null if this is no server error or it had no error message + */ +fun Throwable.getServerErrorMessage(): String? { + if (this is HttpException) { + val errorResponse = response()?.errorBody()?.string() + return if (!errorResponse.isNullOrBlank()) { + try { + JSONObject(errorResponse).getString("error") + } catch (e: JSONException) { + null + } + } else { + null + } + } + return null +} diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt index 72835acc4..bc7f435df 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt @@ -32,6 +32,7 @@ import com.keylesspalace.tusky.util.Error import com.keylesspalace.tusky.util.Loading import com.keylesspalace.tusky.util.Resource import com.keylesspalace.tusky.util.Success +import com.keylesspalace.tusky.util.getServerErrorMessage import com.keylesspalace.tusky.util.randomAlphanumericString import kotlinx.coroutines.launch import okhttp3.MediaType.Companion.toMediaTypeOrNull @@ -39,9 +40,6 @@ import okhttp3.MultipartBody import okhttp3.RequestBody import okhttp3.RequestBody.Companion.asRequestBody import okhttp3.RequestBody.Companion.toRequestBody -import org.json.JSONException -import org.json.JSONObject -import retrofit2.HttpException import java.io.File import javax.inject.Inject @@ -156,21 +154,7 @@ class EditProfileViewModel @Inject constructor( eventHub.dispatch(ProfileEditedEvent(newProfileData)) }, { throwable -> - if (throwable is HttpException) { - val errorResponse = throwable.response()?.errorBody()?.string() - val errorMsg = if (!errorResponse.isNullOrBlank()) { - try { - JSONObject(errorResponse).optString("error", "") - } catch (e: JSONException) { - null - } - } else { - null - } - saveData.postValue(Error(errorMessage = errorMsg)) - } else { - saveData.postValue(Error()) - } + saveData.postValue(Error(errorMessage = throwable.getServerErrorMessage())) } ) } diff --git a/app/src/main/res/layout/activity_compose.xml b/app/src/main/res/layout/activity_compose.xml index 3219578b3..958de1b8d 100644 --- a/app/src/main/res/layout/activity_compose.xml +++ b/app/src/main/res/layout/activity_compose.xml @@ -239,6 +239,7 @@ app:layout_behavior="com.google.android.material.bottomsheet.BottomSheetBehavior" />