From 2e500128ddf85fd16de24419f608e7a1cff31e46 Mon Sep 17 00:00:00 2001 From: Lasta Apps Date: Fri, 25 Oct 2024 21:40:00 +0300 Subject: [PATCH] refactor: Improved error reporting, StravnikWallet error reporting --- .idea/vcs.xml | 1 - .../api/agata/api/AndroidAgataCtuWalletApi.kt | 2 +- .../menza/api/agata/api/StravnikWalletApi.kt | 12 +++- .../api/agata/test/StravnikWalletTest.kt | 55 +++++++++++++++++++ .../features/other/ui/dialog/ReportDialog.kt | 8 ++- .../settings/ui/screens/SettingsScreen.kt | 2 +- .../cz/lastaapps/menza/ui/util/HandleError.kt | 10 +++- .../kotlin/cz/lastaapps/core/ui/ErrorText.kt | 2 +- .../lastaapps/core/domain/error/ApiError.kt | 3 +- .../core/domain/error/DomainError.kt | 3 + gradle/libs.versions.toml | 2 + .../lastaapps/plugin/jvm/JvmAppConvention.kt | 1 + .../multiplatform/KMPLibraryConvention.kt | 1 + 13 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 api/agata/src/jvmTest/kotlin/cz/lastaapps/api/agata/test/StravnikWalletTest.kt diff --git a/.idea/vcs.xml b/.idea/vcs.xml index d9452c65..94a25f7f 100644 --- a/.idea/vcs.xml +++ b/.idea/vcs.xml @@ -2,6 +2,5 @@ - \ No newline at end of file diff --git a/api/agata/src/androidMain/kotlin/cz/lastaapps/menza/api/agata/api/AndroidAgataCtuWalletApi.kt b/api/agata/src/androidMain/kotlin/cz/lastaapps/menza/api/agata/api/AndroidAgataCtuWalletApi.kt index 5bb53489..e57ab4fb 100644 --- a/api/agata/src/androidMain/kotlin/cz/lastaapps/menza/api/agata/api/AndroidAgataCtuWalletApi.kt +++ b/api/agata/src/androidMain/kotlin/cz/lastaapps/menza/api/agata/api/AndroidAgataCtuWalletApi.kt @@ -188,6 +188,6 @@ internal class AndroidAgataCtuWalletApi( .toFloatOrNull() .bind() } - }?.right() ?: WalletError.TotallyBroken.left() + }?.right() ?: WalletError.TotallyBroken().left() }.flatten() } diff --git a/api/agata/src/commonMain/kotlin/cz/lastaapps/menza/api/agata/api/StravnikWalletApi.kt b/api/agata/src/commonMain/kotlin/cz/lastaapps/menza/api/agata/api/StravnikWalletApi.kt index 9d915d1d..278665df 100644 --- a/api/agata/src/commonMain/kotlin/cz/lastaapps/menza/api/agata/api/StravnikWalletApi.kt +++ b/api/agata/src/commonMain/kotlin/cz/lastaapps/menza/api/agata/api/StravnikWalletApi.kt @@ -139,7 +139,7 @@ internal class StravnikWalletApiImpl( } delay(42.milliseconds) } - raise(ApiError.WalletError.TotallyBroken) + raise(ApiError.WalletError.TotallyBroken("Failed after all the iterations")) } } @@ -185,7 +185,10 @@ internal class StravnikWalletApiImpl( { log.e(it) { "Finding regex failed" } log.e { "The error body was:\n$this" } - return ApiError.WalletError.TotallyBroken.left() + return ApiError.WalletError.TotallyBroken( + "Rexex did not match:\n" + + this@processBody + ).left() }, ) }?.replace(',', '.') @@ -193,5 +196,8 @@ internal class StravnikWalletApiImpl( ?.replace(" ", "") ?.toFloatOrNull() ?.right() - ?: ApiError.WalletError.TotallyBroken.left() + ?: ApiError.WalletError.TotallyBroken( + "Failed to parse number:\n" + + this@processBody + ).left() } diff --git a/api/agata/src/jvmTest/kotlin/cz/lastaapps/api/agata/test/StravnikWalletTest.kt b/api/agata/src/jvmTest/kotlin/cz/lastaapps/api/agata/test/StravnikWalletTest.kt new file mode 100644 index 00000000..29f6dbaf --- /dev/null +++ b/api/agata/src/jvmTest/kotlin/cz/lastaapps/api/agata/test/StravnikWalletTest.kt @@ -0,0 +1,55 @@ +/* + * Copyright 2024, Petr Laštovička as Lasta apps, All rights reserved + * + * This file is part of Menza. + * + * Menza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Menza is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Menza. If not, see . + */ + +package cz.lastaapps.api.agata.test + +import arrow.core.Either.Right +import cz.lastaapps.menza.api.agata.api.CafeteriaApiImpl +import cz.lastaapps.menza.api.agata.api.StravnikWalletApiImpl +import cz.lastaapps.menza.api.agata.data.createAgataClient +import cz.lastaapps.menza.api.agata.data.model.AgataBEConfig +import cz.lastaapps.menza.api.agata.data.model.dto.DishTypeDto +import cz.lastaapps.menza.api.agata.data.model.dto.ServingPlaceDto +import cz.lastaapps.menza.api.agata.data.model.dto.SubsystemDto +import io.kotest.assertions.arrow.core.shouldBeRight +import io.kotest.core.spec.style.StringSpec +import io.kotest.matchers.types.shouldBeInstanceOf +import io.ktor.client.HttpClient +import io.ktor.client.plugins.logging.LogLevel.BODY +import io.ktor.client.plugins.logging.Logging + +class StravnikWalletTest : StringSpec( + { + fun api() = StravnikWalletApiImpl( + HttpClient { + install(Logging) { + level = BODY + } + }, + ) + + "getBalance" { + val username = "" + val password = "" + api() + .getBalance(username, password) + .shouldBeRight() + } + }, +) diff --git a/app/src/main/kotlin/cz/lastaapps/menza/features/other/ui/dialog/ReportDialog.kt b/app/src/main/kotlin/cz/lastaapps/menza/features/other/ui/dialog/ReportDialog.kt index b0718a3d..f03504c2 100644 --- a/app/src/main/kotlin/cz/lastaapps/menza/features/other/ui/dialog/ReportDialog.kt +++ b/app/src/main/kotlin/cz/lastaapps/menza/features/other/ui/dialog/ReportDialog.kt @@ -240,7 +240,9 @@ private fun ReportDialog( fun sendReport( context: Context, mode: ReportMode, - throwable: Throwable? = null, + errorText: String, + extraMessage: String?, + throwable: Throwable?, ) { val text = """ @@ -251,6 +253,10 @@ fun sendReport( | |"Internal app problem" |${LocalDateTime.now().format(DateTimeFormatter.ISO_DATE_TIME)} + |${errorText} + | + |${extraMessage ?: ""} + | |${throwable?.message ?: "Unknown error message"} |${throwable?.stackTraceToString() ?: ""} """.trimMargin() diff --git a/app/src/main/kotlin/cz/lastaapps/menza/features/settings/ui/screens/SettingsScreen.kt b/app/src/main/kotlin/cz/lastaapps/menza/features/settings/ui/screens/SettingsScreen.kt index 392fccea..5ec503ee 100644 --- a/app/src/main/kotlin/cz/lastaapps/menza/features/settings/ui/screens/SettingsScreen.kt +++ b/app/src/main/kotlin/cz/lastaapps/menza/features/settings/ui/screens/SettingsScreen.kt @@ -329,7 +329,7 @@ private fun ReportButton(modifier: Modifier = Modifier) { val context = LocalContext.current ReportDialog(shown, false, { shown = false }) { - sendReport(context, it) + sendReport(context, it, "User feedback", null, null) shown = false } } diff --git a/app/src/main/kotlin/cz/lastaapps/menza/ui/util/HandleError.kt b/app/src/main/kotlin/cz/lastaapps/menza/ui/util/HandleError.kt index e166a9d7..1ee9acb6 100644 --- a/app/src/main/kotlin/cz/lastaapps/menza/ui/util/HandleError.kt +++ b/app/src/main/kotlin/cz/lastaapps/menza/ui/util/HandleError.kt @@ -85,12 +85,20 @@ fun HandleError( } toReport?.let { errorToReport -> + val errorClass = errorToReport::class.qualifiedName + val errorText = errorToReport.text() ReportDialog( shown = true, reportsCrash = true, onDismissRequest = { toReport = null }, ) { - sendReport(context, it, errorToReport.throwable) + sendReport( + context, + it, + "$errorText (class: $errorClass)", + errorToReport.extraMessage, + errorToReport.throwable, + ) } } } diff --git a/core/src/androidMain/kotlin/cz/lastaapps/core/ui/ErrorText.kt b/core/src/androidMain/kotlin/cz/lastaapps/core/ui/ErrorText.kt index 0c3f71c0..7abb4b09 100644 --- a/core/src/androidMain/kotlin/cz/lastaapps/core/ui/ErrorText.kt +++ b/core/src/androidMain/kotlin/cz/lastaapps/core/ui/ErrorText.kt @@ -117,7 +117,7 @@ val ApiError.text: AppText is WalletError -> when (this) { - TotallyBroken -> E(R.string.error_wallet_login_failed_critical) + is TotallyBroken -> E(R.string.error_wallet_login_failed_critical) InvalidCredentials -> E(R.string.error_wallet_login_failed_credentials) } } diff --git a/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/ApiError.kt b/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/ApiError.kt index 5a1c8081..289ed3f6 100644 --- a/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/ApiError.kt +++ b/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/ApiError.kt @@ -37,7 +37,8 @@ sealed interface ApiError : DomainError.Logic { } sealed interface WalletError : ApiError { - data object TotallyBroken : WalletError + @JvmInline + value class TotallyBroken(override val extraMessage: String? = null) : WalletError data object InvalidCredentials : WalletError } diff --git a/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/DomainError.kt b/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/DomainError.kt index b450aced..43a125a5 100644 --- a/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/DomainError.kt +++ b/core/src/commonMain/kotlin/cz/lastaapps/core/domain/error/DomainError.kt @@ -25,6 +25,9 @@ sealed interface DomainError { val throwable: Throwable? get() = null + val extraMessage: String? + get() = null + sealed interface Runtime : DomainError sealed interface Logic : DomainError diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 9682bff1..0394f0f9 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -20,6 +20,7 @@ kermit = "2.0.4" koin = "4.0.0" koin-annotations = "1.0.3" kotest = "5.9.1" +kotest-arrow = "1.4.0" kotlin = "2.0.20" # @keep kotlin-api = "2.0" @@ -129,6 +130,7 @@ koin-ktorServer = { module = "io.insert-koin:koin-ktor", version.ref = "koin" } koin-ktorServer-logger = { module = "io.insert-koin:koin-logger-slf4j", version.ref = "koin" } koin-test-core = { module = "io.insert-koin:koin-test", version.ref = "koin" } koin-test-jUnit5 = { module = "io.insert-koin:koin-test-junit5", version.ref = "koin" } +kotest-arrow = { module = "io.kotest.extensions:kotest-assertions-arrow", version.ref = "kotest-arrow" } kotest-assertion = { module = "io.kotest:kotest-assertions-core", version.ref = "kotest" } kotest-jUnit5runner = { module = "io.kotest:kotest-runner-junit5", version.ref = "kotest" } kotest-property = { module = "io.kotest:kotest-property", version.ref = "kotest" } diff --git a/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/jvm/JvmAppConvention.kt b/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/jvm/JvmAppConvention.kt index 5fd028d2..4f700db8 100644 --- a/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/jvm/JvmAppConvention.kt +++ b/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/jvm/JvmAppConvention.kt @@ -75,6 +75,7 @@ class JvmAppConvention : testImplementation(libs.kotlin.test.annotation) testImplementation(libs.kotlin.test.common) testImplementation(libs.kotlin.test.core) + implementation(libs.kotest.arrow) testImplementation(libs.kotest.assertion) testImplementation(libs.kotlinx.coroutines.test) testImplementation(libs.kotest.jUnit5runner) diff --git a/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/multiplatform/KMPLibraryConvention.kt b/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/multiplatform/KMPLibraryConvention.kt index 8b7ea612..a1bb8f34 100644 --- a/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/multiplatform/KMPLibraryConvention.kt +++ b/gradle/plugins/convention/src/main/kotlin/cz/lastaapps/plugin/multiplatform/KMPLibraryConvention.kt @@ -96,6 +96,7 @@ class KMPLibraryConvention : // implementation(libs.kotlin.test.common) // implementation(libs.kotlin.test.core) // implementation(libs.kotlin.test.jUnit5) + implementation(libs.kotest.arrow) implementation(libs.kotest.assertion) implementation(libs.kotlinx.coroutines.test) // implementation(libs.koin.test.jUnit5)