From 903f6c54279771c9d0a0131c2430feb4c1e01ffb Mon Sep 17 00:00:00 2001 From: Shinokuni Date: Tue, 12 Nov 2024 17:15:59 +0100 Subject: [PATCH] Improve error handling in NewFeedScreen --- .../app/feeds/newfeed/NewFeedScreen.kt | 6 +- .../app/feeds/newfeed/NewFeedScreenModel.kt | 37 ++++------ .../app/util/accounterror/AccountError.kt | 73 +++++++++++++++++++ .../app/util/accounterror/FreshRSSError.kt | 57 +++++++++++++++ .../util/accounterror/NextcloudNewsError.kt | 64 ++++++++++++++++ app/src/main/res/values-fr/strings.xml | 3 + app/src/main/res/values/strings.xml | 3 + 7 files changed, 217 insertions(+), 26 deletions(-) create mode 100644 app/src/main/java/com/readrops/app/util/accounterror/AccountError.kt create mode 100644 app/src/main/java/com/readrops/app/util/accounterror/FreshRSSError.kt create mode 100644 app/src/main/java/com/readrops/app/util/accounterror/NextcloudNewsError.kt diff --git a/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreen.kt b/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreen.kt index ee0a255d..42f5787f 100644 --- a/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreen.kt +++ b/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreen.kt @@ -24,7 +24,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll -import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign @@ -34,7 +33,6 @@ import cafe.adriel.voyager.navigator.LocalNavigator import cafe.adriel.voyager.navigator.currentOrThrow import com.readrops.app.R import com.readrops.app.account.selection.adaptiveIconPainterResource -import com.readrops.app.util.ErrorMessage import com.readrops.app.util.components.AndroidScreen import com.readrops.app.util.components.DropdownBox import com.readrops.app.util.components.DropdownBoxValue @@ -208,11 +206,11 @@ class NewFeedScreen(val url: String? = null) : AndroidScreen() { } } - if (state.exception != null) { + if (state.error != null) { MediumSpacer() Text( - text = ErrorMessage.get(state.exception!!, LocalContext.current), + text = state.error!!, style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.error, textAlign = TextAlign.Center, diff --git a/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreenModel.kt b/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreenModel.kt index 7ba969fb..b9a51464 100644 --- a/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreenModel.kt +++ b/app/src/main/java/com/readrops/app/feeds/newfeed/NewFeedScreenModel.kt @@ -11,7 +11,7 @@ import com.readrops.api.utils.AuthInterceptor import com.readrops.api.utils.HtmlParser import com.readrops.app.R import com.readrops.app.repositories.BaseRepository -import com.readrops.app.util.ErrorMessage +import com.readrops.app.util.accounterror.AccountError import com.readrops.app.util.components.TextFieldError import com.readrops.db.Database import com.readrops.db.entities.Feed @@ -27,7 +27,6 @@ import kotlinx.coroutines.launch import org.koin.core.component.KoinComponent import org.koin.core.component.get import org.koin.core.parameter.parametersOf -import java.net.UnknownHostException class NewFeedScreenModel( private val database: Database, @@ -39,6 +38,8 @@ class NewFeedScreenModel( private val selectedAccountState = MutableStateFlow(state.value.selectedAccount) + private lateinit var accountError: AccountError + init { screenModelScope.launch(dispatcher) { database.accountDao() @@ -47,6 +48,8 @@ class NewFeedScreenModel( .collect { accounts -> val selectedAccount = accounts.find { it.isCurrentAccount } ?: accounts.first() + + accountError = AccountError.from(selectedAccount, context) selectedAccountState.update { selectedAccount } mutableState.update { newFeedState -> @@ -90,7 +93,7 @@ class NewFeedScreenModel( if (state.value.selectedResultsCount > 0) { mutableState.update { it.copy( - exception = null, + error = null, isLoading = true, parsingResults = state.value.parsingResults.map { parsingResult -> parsingResult.copy(error = null) @@ -132,7 +135,7 @@ class NewFeedScreenModel( private fun loadFeeds() { screenModelScope.launch(dispatcher) { - mutableState.update { it.copy(exception = null, isLoading = true) } + mutableState.update { it.copy(error = null, isLoading = true) } val url = state.value.url try { @@ -185,21 +188,11 @@ class NewFeedScreenModel( } } } catch (e: Exception) { - // TODO improve error handling for all accounts - when (e) { - is UnknownHostException -> mutableState.update { - it.copy( - urlError = TextFieldError.UnreachableUrl, - isLoading = false - ) - } - - else -> mutableState.update { - it.copy( - urlError = TextFieldError.NoRSSFeed, - isLoading = false - ) - } + mutableState.update { + it.copy( + error = accountError.newFeedMessage(e), + isLoading = false + ) } } } @@ -235,7 +228,7 @@ class NewFeedScreenModel( if (feed != null) { val error = errors[feed] - parsingResult.copy(error = ErrorMessage.get(error!!, context)) + parsingResult.copy(error = accountError.newFeedMessage(error!!)) } else { parsingResult } @@ -250,7 +243,7 @@ class NewFeedScreenModel( } else { mutableState.update { it.copy( - exception = errors.values.first(), + error = accountError.newFeedMessage(errors.values.first()), isLoading = false ) } @@ -337,7 +330,7 @@ data class State( val isAccountDropdownExpanded: Boolean = false, val isFoldersDropdownExpanded: Boolean = false, val urlError: TextFieldError? = null, - val exception: Exception? = null, + val error: String? = null, val isLoading: Boolean = false, val popScreen: Boolean = false, val parsingResults: List = listOf() diff --git a/app/src/main/java/com/readrops/app/util/accounterror/AccountError.kt b/app/src/main/java/com/readrops/app/util/accounterror/AccountError.kt new file mode 100644 index 00000000..4e563e9c --- /dev/null +++ b/app/src/main/java/com/readrops/app/util/accounterror/AccountError.kt @@ -0,0 +1,73 @@ +package com.readrops.app.util.accounterror + +import android.content.Context +import com.readrops.api.utils.exceptions.HttpException +import com.readrops.api.utils.exceptions.LoginFailedException +import com.readrops.api.utils.exceptions.ParseException +import com.readrops.api.utils.exceptions.UnknownFormatException +import com.readrops.app.R +import com.readrops.db.entities.account.Account +import com.readrops.db.entities.account.AccountType +import java.io.IOException +import java.net.UnknownHostException + +abstract class AccountError(protected val context: Context) { + + open fun newFeedMessage(exception: Exception): String = genericMessage(exception) + + open fun updateFeedMessage(exception: Exception): String = genericMessage(exception) + + open fun deleteFeedMessage(exception: Exception): String = genericMessage(exception) + + open fun newFolderMessage(exception: Exception): String = genericMessage(exception) + + open fun updateFolderMessage(exception: Exception): String = genericMessage(exception) + + open fun deleteFolderMessage(exception: Exception): String = genericMessage(exception) + + protected fun genericMessage(exception: Exception) = when (exception) { + is HttpException -> httpMessage(exception) + is UnknownHostException -> context.resources.getString(R.string.unreachable_url) + is NoSuchFileException -> context.resources.getString(R.string.unable_open_file) + is IOException -> context.resources.getString( + R.string.network_failure, + exception.message.orEmpty() + ) + + is ParseException, is UnknownFormatException -> context.resources.getString(R.string.processing_feed_error) + is LoginFailedException -> context.getString(R.string.login_failed) + else -> "${exception.javaClass.simpleName}: ${exception.message}" + } + + protected fun httpMessage(exception: HttpException): String { + return when (exception.code) { + in 400..499 -> { + when (exception.code) { + 400 -> context.resources.getString(R.string.http_error_400) + 401 -> context.resources.getString(R.string.http_error_401) + 403 -> context.resources.getString(R.string.http_error_403) + 404 -> context.resources.getString(R.string.http_error_404) + else -> context.resources.getString(R.string.http_error_4XX, exception.code) + } + } + + in 500..599 -> { + context.resources.getString(R.string.http_error_5XX, exception.code) + } + + else -> context.resources.getString(R.string.http_error, exception.code) + } + } + + companion object { + + fun from(account: Account, context: Context): AccountError = when (account.type) { + AccountType.FRESHRSS -> FreshRSSError(context) + AccountType.NEXTCLOUD_NEWS -> NextcloudNewsError(context) + else -> DefaultAccountError(context) + } + + class DefaultAccountError(context: Context) : AccountError(context) + } +} + diff --git a/app/src/main/java/com/readrops/app/util/accounterror/FreshRSSError.kt b/app/src/main/java/com/readrops/app/util/accounterror/FreshRSSError.kt new file mode 100644 index 00000000..acc6319e --- /dev/null +++ b/app/src/main/java/com/readrops/app/util/accounterror/FreshRSSError.kt @@ -0,0 +1,57 @@ +package com.readrops.app.util.accounterror + +import android.content.Context +import com.readrops.api.utils.exceptions.HttpException +import com.readrops.app.R + +class FreshRSSError(context: Context) : AccountError(context) { + + override fun newFeedMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 400 -> context.getString(R.string.feed_already_exists) + else -> httpMessage(exception) + } + } + else -> genericMessage(exception) + } + + override fun updateFeedMessage(exception: Exception): String { + return newFeedMessage(exception) + } + + override fun deleteFeedMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 400 -> context.resources.getString(R.string.feed_doesnt_exist) + else -> httpMessage(exception) + } + } + else -> genericMessage(exception) + } + + override fun newFolderMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 400 -> context.resources.getString(R.string.folder_already_exists) + else -> httpMessage(exception) + } + } + else -> genericMessage(exception) + } + + override fun updateFolderMessage(exception: Exception): String { + return newFolderMessage(exception) + } + + override fun deleteFolderMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 400 -> context.resources.getString(R.string.folder_doesnt_exist) + else -> httpMessage(exception) + } + } + else -> genericMessage(exception) + } + +} \ No newline at end of file diff --git a/app/src/main/java/com/readrops/app/util/accounterror/NextcloudNewsError.kt b/app/src/main/java/com/readrops/app/util/accounterror/NextcloudNewsError.kt new file mode 100644 index 00000000..2f10afc4 --- /dev/null +++ b/app/src/main/java/com/readrops/app/util/accounterror/NextcloudNewsError.kt @@ -0,0 +1,64 @@ +package com.readrops.app.util.accounterror + +import android.content.Context +import com.readrops.api.utils.exceptions.HttpException +import com.readrops.app.R + +class NextcloudNewsError(context: Context) : AccountError(context) { + + override fun newFeedMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 409 -> context.resources.getString(R.string.feed_already_exists) + 422 -> context.getString(R.string.invalid_feed) + else -> httpMessage(exception) + } + } + + else -> genericMessage(exception) + } + + override fun updateFeedMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 404 -> context.resources.getString(R.string.feed_doesnt_exist) + else -> httpMessage(exception) + } + } + + else -> genericMessage(exception) + } + + override fun deleteFeedMessage(exception: Exception): String { + return updateFeedMessage(exception) + } + + override fun newFolderMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 409 -> context.resources.getString(R.string.folder_already_exists) + 422 -> context.resources.getString(R.string.invalid_folder) + else -> httpMessage(exception) + } + } + + else -> genericMessage(exception) + } + + override fun updateFolderMessage(exception: Exception): String = when (exception) { + is HttpException -> { + when (exception.code) { + 404 -> context.resources.getString(R.string.folder_doesnt_exist) + 409 -> context.resources.getString(R.string.folder_already_exists) + 422 -> context.getString(R.string.invalid_folder) + else -> httpMessage(exception) + } + } + + else -> genericMessage(exception) + } + + override fun deleteFolderMessage(exception: Exception): String { + return updateFolderMessage(exception) + } +} \ No newline at end of file diff --git a/app/src/main/res/values-fr/strings.xml b/app/src/main/res/values-fr/strings.xml index be0c9abd..86a50b19 100644 --- a/app/src/main/res/values-fr/strings.xml +++ b/app/src/main/res/values-fr/strings.xml @@ -182,4 +182,7 @@ (%1$s sélectionnés) Ajouter %1$s flux sélectionnés Entrer une URL + Le flux existe déjà + Dossier invalide + Flux invalide \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d1f83319..1b48dee4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -191,4 +191,7 @@ (%1$s selected) Add %1$s selected feeds Enter an URL + Feed already exists + Invalid folder + Invalid feed \ No newline at end of file