From 651df37a9086b5f0f13c6594e401ca695ecab332 Mon Sep 17 00:00:00 2001 From: dadachi Date: Wed, 14 Oct 2026 18:27:35 +0900 Subject: [PATCH] Localize user-facing CodedError messages; log developer details What the user sees is now in the app language, with the iOS wording; the error codes are unchanged: - 2002 with no response (IOException) or an error status without a message: "Something went wrong while communicating with the server. Please try again later." - 2002 for a response the app could not process, and 1001: "An unexpected error occurred." - 2001 and devise's message: the server's message as is, without " [Status: nnn]" or "Processing error: ". The raw detail and HTTP status stay in Throwable.message and are logged with the code where the error is made (ApiException.logged(), tag ApiError). ErrorMessages resolves the strings with the application Context installed in Application.onCreate, since most errors are made without a Context. AGENTS.md: user-facing error text is localized; developer details go to logs; NATIVEAPPTEMPLATE-nnnn codes stay untranslated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01K5hZEFqXhBsUQwSWsgR6eH --- AGENTS.md | 5 +- CHANGELOG.md | 3 + .../NativeAppTemplateApplication.kt | 3 + .../common/errors/ApiException.kt | 41 +++++++++- .../common/errors/AppError.kt | 11 ++- .../common/errors/ErrorMessages.kt | 33 ++++++++ .../data/login/LoginRepositoryImpl.kt | 3 +- .../network/ApiResponseExtensions.kt | 30 ++++++- app/src/main/res/values-ja/strings.xml | 2 + app/src/main/res/values/strings.xml | 2 + .../common/errors/CodedErrorTest.kt | 77 +++++++++++++++--- .../data/RepositoryRequestsTest.kt | 9 ++- .../data/login/LoginRepositoryImplTest.kt | 11 ++- .../network/ApiResponseExtensionsTest.kt | 78 ++++++++++++++++++- .../testing/util/TestErrors.kt | 6 +- .../SignInEmailAndPasswordViewModelTest.kt | 24 ++++-- .../settings/ShopkeeperEditViewModelTest.kt | 13 +++- 17 files changed, 312 insertions(+), 39 deletions(-) create mode 100644 app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ErrorMessages.kt diff --git a/AGENTS.md b/AGENTS.md index 0dd3553..39861ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,6 +57,8 @@ All errors should use the `CodedError` interface. Error codes use the `NATIVEAPP - New error types must implement `CodedError` - Use `codedDescription` (not `message` or `localizedMessage`) in all user-facing error messages — this prepends `[NATIVEAPPTEMPLATE-XXXX]` for `CodedError` types +- `errorDescription` is what the user sees, in the app language: the server's message as is (`2001`, and `2002` with devise_token_auth's message), "Something went wrong while communicating with the server. Please try again later." (`error_communication_failed`) for no response or an error status without a message, and "An unexpected error occurred." (`error_unexpected`) for internal errors (a response the app could not process, `AppError.Unexpected`). Errors made without a Context get these strings from `ErrorMessages` (installed in `Application.onCreate`) +- Developer details go to `Throwable.message` and the log, with the code, where the error is made: `ApiException.logged()` (`adb logcat -s ApiError`) ## Testing @@ -102,5 +104,6 @@ The UI is in English (`values/strings.xml`) and Japanese (`values-ja/strings.xml - Use the terms of the API's `config/locales/ja.yml`: Shop = 店舗, Item Tag = アイテムタグ, idled / completed = 未完了 / 完了. Reuse the paid edition's Japanese for strings both editions share, so they read the same. Polite です/ます for sentences, no 。 on buttons and labels, full-width 「」（）：. - nativeapptemplate-agent renames 店舗 and アイテムタグ as plain substrings, so write them in full: never ショップ / 店 / お店, never タグ or アイテム alone. `StringResourcesTranslationTest` fails otherwise. The Free edition has no organizations, so no string may mention 組織. `app_name` is `translatable="false"` so the English rename covers the product name. - Don't build sentences from fragments: Japanese word order differs. Use one string with `%1$s` arguments; for links inside a sentence use `linkedFormat()` (`ui/common/LinkedFormat.kt`). Counts use `` (Japanese needs only `other`). -- Keep untranslated: product names, URLs, `NATIVEAPPTEMPLATE-XXXX` codes and `CodedError` descriptions, time zone names, logs, the API's error messages (the server localizes them), and what the user typed. +- User-facing error text is localized (string resources, as on iOS); developer details (raw messages, HTTP statuses) go to the log, never on screen. See Error Handling. +- Keep untranslated: product names, URLs, `NATIVEAPPTEMPLATE-XXXX` codes, time zone names, logs, the API's error messages (the server localizes them), and what the user typed. - There is no `localeFilters`/`resourceConfigurations` in Gradle; adding one without `ja` would strip the Japanese (`JapaneseResourcesPackagedTest` would fail). diff --git a/CHANGELOG.md b/CHANGELOG.md index 9863bbe..d16060a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - `HardcodedUiStringsTest` scans each file as a whole, so it also catches a literal on the line after `Text(`, and `updateMessage("...")` / `?: "..."` fallbacks - Move agent instructions from `CLAUDE.md` to `AGENTS.md` so Claude Code, Codex, and other agents share one file. `CLAUDE.md` now only imports it (`@AGENTS.md`) +### Fixed +- Error messages are in the app language and say what the user can do, with the same wording as NativeAppTemplate-Free-iOS; the error codes are unchanged. A request that got no answer (no network, a timeout) or an error status without a message (e.g. a 502 page) shows `[NATIVEAPPTEMPLATE-2002]` "Something went wrong while communicating with the server. Please try again later." (サーバーとの通信中に問題が発生しました。しばらくしてからもう一度お試しください。) instead of the English system text (e.g. "Processing error: Unable to resolve host ..." or "Processing error: HTTP 502"). A response the app could not process (`2002`) and `1001` show "An unexpected error occurred." (予期しないエラーが発生しました。). The server's own messages are shown as is, without the developer suffix " [Status: 422]" (`2001`) or the "Processing error: " prefix (sign-in's "Invalid login credentials"). The details are logged with the code instead (`adb logcat -s ApiError`) + ## [3.2.6] - 2026-05-24 ### Fixed diff --git a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/NativeAppTemplateApplication.kt b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/NativeAppTemplateApplication.kt index e006ae9..78fbe2e 100644 --- a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/NativeAppTemplateApplication.kt +++ b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/NativeAppTemplateApplication.kt @@ -1,6 +1,7 @@ package com.nativeapptemplate.nativeapptemplatefree import android.app.Application +import com.nativeapptemplate.nativeapptemplatefree.common.errors.ErrorMessages import com.nativeapptemplate.nativeapptemplatefree.utils.ProfileVerifierLogger import dagger.hilt.android.HiltAndroidApp import javax.inject.Inject @@ -11,6 +12,8 @@ class NativeAppTemplateApplication : Application() { lateinit var profileVerifierLogger: ProfileVerifierLogger override fun onCreate() { + // Before anything that may make an error (super.onCreate() runs Hilt's injection). + ErrorMessages.install(this) super.onCreate() profileVerifierLogger() } diff --git a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ApiException.kt b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ApiException.kt index 1295eeb..eb2cfa4 100644 --- a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ApiException.kt +++ b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ApiException.kt @@ -1,5 +1,7 @@ package com.nativeapptemplate.nativeapptemplatefree.common.errors +import android.util.Log + /** * @property httpStatusCode The HTTP status of the failed response, or null when the request * never got a response (e.g. no network) or the status is unknown. @@ -20,15 +22,50 @@ sealed class ApiException( httpStatusCode: Int? = null, ) : ApiException("$apiMessage [Status: $code]", httpStatusCode = httpStatusCode) { override val errorCode: String = "NATIVEAPPTEMPLATE-2001" - override val errorDescription: String = "$apiMessage [Status: $code]" + + /** The API's message as is (the server localizes it); the status is only in [message], for the log. */ + override val errorDescription: String = apiMessage + } + + /** What the user sees for an [UnprocessableError]. */ + sealed interface UserMessage { + /** The server's own message (devise_token_auth's), already localized: shown as is. */ + data class Server(val text: String) : UserMessage + + /** No usable answer: no network, a timeout, or an error status without a message. */ + data object CommunicationFailure : UserMessage + + /** A response the app could not process: a bug, nothing the user can act on. */ + data object Unexpected : UserMessage } + /** + * No response, an error response that could not be deserialized, or a response the app could not + * process. [rawMessage] is the developer detail, in [message] for the log; the user sees [userMessage]. + */ class UnprocessableError( val rawMessage: String, cause: Throwable? = null, httpStatusCode: Int? = null, + val userMessage: UserMessage = UserMessage.Unexpected, ) : ApiException("Not processable error($rawMessage).", cause, httpStatusCode) { override val errorCode: String = "NATIVEAPPTEMPLATE-2002" - override val errorDescription: String = "Processing error: $rawMessage" + override val errorDescription: String + get() = when (userMessage) { + is UserMessage.Server -> userMessage.text + UserMessage.CommunicationFailure -> ErrorMessages.communicationFailed + UserMessage.Unexpected -> ErrorMessages.unexpected + } } } + +/** + * Logs this error's code and developer detail (the raw message, the HTTP status), which the user + * never sees (AGENTS.md, Error Handling). Call it where the error is made. Returns this error. + */ +fun E.logged(): E { + Log.w(API_ERROR_LOG_TAG, "[$errorCode] $message (HTTP ${httpStatusCode ?: "no response"})", cause) + return this +} + +private const val API_ERROR_LOG_TAG = "ApiError" diff --git a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/AppError.kt b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/AppError.kt index ed2f5ff..0bd5820 100644 --- a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/AppError.kt +++ b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/AppError.kt @@ -2,11 +2,14 @@ package com.nativeapptemplate.nativeapptemplatefree.common.errors sealed class AppError( override val errorCode: String, - override val errorDescription: String, -) : Exception(errorDescription), CodedError { + message: String, +) : Exception(message), CodedError { + /** The user sees the generic [ErrorMessages.unexpected]; [detail] is only in [message], for the log. */ class Unexpected(detail: String? = null) : AppError( errorCode = "NATIVEAPPTEMPLATE-1001", - errorDescription = "Unexpected error" + if (detail != null) ": $detail" else "", - ) + message = "Unexpected error" + if (detail != null) ": $detail" else "", + ) { + override val errorDescription: String get() = ErrorMessages.unexpected + } } diff --git a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ErrorMessages.kt b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ErrorMessages.kt new file mode 100644 index 0000000..4029f70 --- /dev/null +++ b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/ErrorMessages.kt @@ -0,0 +1,33 @@ +package com.nativeapptemplate.nativeapptemplatefree.common.errors + +import android.content.Context +import androidx.annotation.StringRes +import com.nativeapptemplate.nativeapptemplatefree.R + +/** + * The user-facing messages of [CodedError]s that are not the server's own: string resources in the + * app language (values/ and values-ja/), worded as on iOS. + * + * Most errors are made where there is no Context (the network layer), so the application installs + * its Context here once, first thing in `Application.onCreate`. A message is looked up when it is + * read, so it follows the app language. Developer details (raw messages, HTTP statuses) are never + * part of them: they are logged where the error is made, and kept in the exception's message. + */ +object ErrorMessages { + @Volatile + private var context: Context? = null + + fun install(context: Context) { + this.context = context.applicationContext + } + + /** No usable answer from the server: no network, a timeout, or an error status without a message. */ + val communicationFailed: String get() = string(R.string.error_communication_failed) + + /** An internal error (a response the app could not process, a bug): nothing the user can act on. */ + val unexpected: String get() = string(R.string.error_unexpected) + + private fun string(@StringRes id: Int): String = checkNotNull(context) { + "ErrorMessages.install was not called (Application.onCreate)" + }.getString(id) +} diff --git a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImpl.kt b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImpl.kt index 8dba178..418db39 100644 --- a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImpl.kt +++ b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImpl.kt @@ -2,6 +2,7 @@ package com.nativeapptemplate.nativeapptemplatefree.data.login import androidx.annotation.VisibleForTesting import com.nativeapptemplate.nativeapptemplatefree.common.errors.ApiException +import com.nativeapptemplate.nativeapptemplatefree.common.errors.logged import com.nativeapptemplate.nativeapptemplatefree.datastore.NativeAppTemplatePreferencesDataSource import com.nativeapptemplate.nativeapptemplatefree.model.* import com.nativeapptemplate.nativeapptemplatefree.model.LoggedInShopkeeper @@ -148,7 +149,7 @@ private fun LoggedInShopkeeper.requireCompleteSession(): LoggedInShopkeeper { .map { (name, _) -> name } if (missing.isNotEmpty()) { - throw ApiException.UnprocessableError(rawMessage = "Incomplete sign-in response: missing ${missing.joinToString()}") + throw ApiException.UnprocessableError(rawMessage = "Incomplete sign-in response: missing ${missing.joinToString()}").logged() } return this } diff --git a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensions.kt b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensions.kt index e3ff6ad..4ed7265 100644 --- a/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensions.kt +++ b/app/src/main/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensions.kt @@ -1,6 +1,7 @@ package com.nativeapptemplate.nativeapptemplatefree.network import com.nativeapptemplate.nativeapptemplatefree.common.errors.ApiException +import com.nativeapptemplate.nativeapptemplatefree.common.errors.logged import com.nativeapptemplate.nativeapptemplatefree.model.NativeAppTemplateApiError import com.skydoves.sandwich.ApiResponse import com.skydoves.sandwich.message @@ -88,7 +89,7 @@ internal fun throwApiErrorFor( errorMessage: String, ): Nothing { val httpError = response as? ApiResponse.Failure.Error - ?: throw ApiException.UnprocessableError(rawMessage = errorMessage) + ?: throw noResponseError(response, errorMessage) val httpStatusCode = httpError.statusCode.code // The body can be read only once. @@ -112,13 +113,34 @@ internal fun throwApiErrorFor( code = nativeAppTemplateApiError.code, apiMessage = nativeAppTemplateApiError.message, httpStatusCode = httpStatusCode, - ) + ).logged() } + // devise_token_auth's message is the server's, already localized; without one, the user can only retry. + val deviseMessage = errorBody?.let(::deviseErrorMessage) throw ApiException.UnprocessableError( - rawMessage = errorBody?.let(::deviseErrorMessage) ?: "HTTP $httpStatusCode", + rawMessage = deviseMessage ?: "HTTP $httpStatusCode", httpStatusCode = httpStatusCode, - ) + userMessage = deviseMessage?.let { ApiException.UserMessage.Server(it) } + ?: ApiException.UserMessage.CommunicationFailure, + ).logged() +} + +/** + * No HTTP response. An [IOException] (no network, a timeout) is a communication failure; anything + * else (e.g. a body the converter could not decode) is a response the app could not process. + */ +private fun noResponseError(response: ApiResponse<*>, errorMessage: String): ApiException.UnprocessableError { + val cause = (response as? ApiResponse.Failure.Exception)?.throwable + return ApiException.UnprocessableError( + rawMessage = errorMessage, + cause = cause, + userMessage = if (cause is IOException) { + ApiException.UserMessage.CommunicationFailure + } else { + ApiException.UserMessage.Unexpected + }, + ).logged() } /** diff --git a/app/src/main/res/values-ja/strings.xml b/app/src/main/res/values-ja/strings.xml index b9522da..a96fa88 100644 --- a/app/src/main/res/values-ja/strings.xml +++ b/app/src/main/res/values-ja/strings.xml @@ -188,4 +188,6 @@ %1$d文字以上で入力してください。 + サーバーとの通信中に問題が発生しました。しばらくしてからもう一度お試しください。 + 予期しないエラーが発生しました。 diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 62980b4..d266638 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -185,4 +185,6 @@ %1$d character minimum. %1$d characters minimum. + Something went wrong while communicating with the server. Please try again later. + An unexpected error occurred. diff --git a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/CodedErrorTest.kt b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/CodedErrorTest.kt index cc9bc69..4813c2a 100644 --- a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/CodedErrorTest.kt +++ b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/common/errors/CodedErrorTest.kt @@ -2,7 +2,12 @@ package com.nativeapptemplate.nativeapptemplatefree.common.errors import org.junit.Assert.assertEquals import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +/** Robolectric for the string resources of the messages (values/ and values-ja/). */ +@RunWith(RobolectricTestRunner::class) class CodedErrorTest { @Test @@ -11,10 +16,13 @@ class CodedErrorTest { assertEquals("NATIVEAPPTEMPLATE-2001", error.errorCode) } + // The user sees the API's error_message as is (the server localizes it); the HTTP status is a + // developer detail, kept in the exception's message for the log (AGENTS.md, Error Handling). @Test - fun apiError_formattedDescription_includesCodeAndMessage() { + fun apiError_formattedDescription_isTheServerMessageWithoutTheStatus() { val error = ApiException.ApiError(code = 422, apiMessage = "Validation failed") - assertEquals("[NATIVEAPPTEMPLATE-2001] Validation failed [Status: 422]", error.formattedDescription) + assertEquals("[NATIVEAPPTEMPLATE-2001] Validation failed", error.formattedDescription) + assertEquals("Validation failed [Status: 422]", error.message) } @Test @@ -23,29 +31,80 @@ class CodedErrorTest { assertEquals("NATIVEAPPTEMPLATE-2002", error.errorCode) } + // A response the app could not process is a bug, not something the user can act on: the generic + // values/strings.xml error_unexpected, "An unexpected error occurred." (the iOS wording). The raw + // detail stays in the exception's message, for the log. @Test - fun unprocessableError_formattedDescription_includesCodeAndMessage() { - val error = ApiException.UnprocessableError(rawMessage = "timeout") - assertEquals("[NATIVEAPPTEMPLATE-2002] Processing error: timeout", error.formattedDescription) + fun unprocessableError_byDefault_showsTheUnexpectedErrorMessage() { + val error = ApiException.UnprocessableError(rawMessage = "Incomplete account data") + assertEquals("[NATIVEAPPTEMPLATE-2002] An unexpected error occurred.", error.formattedDescription) + assertEquals("Not processable error(Incomplete account data).", error.message) + } + + // No usable answer from the server: values/strings.xml error_communication_failed. + @Test + fun unprocessableError_communicationFailure_showsTheCommunicationMessage() { + val error = ApiException.UnprocessableError( + rawMessage = "Unable to resolve host", + userMessage = ApiException.UserMessage.CommunicationFailure, + ) + assertEquals( + "[NATIVEAPPTEMPLATE-2002] Something went wrong while communicating with the server. Please try again later.", + error.formattedDescription, + ) + } + + // values-ja/strings.xml error_communication_failed, the wording shared with iOS. + @Test + @Config(qualifiers = "ja") + fun unprocessableError_communicationFailure_isInJapanese() { + val error = ApiException.UnprocessableError( + rawMessage = "Unable to resolve host", + userMessage = ApiException.UserMessage.CommunicationFailure, + ) + assertEquals( + "[NATIVEAPPTEMPLATE-2002] サーバーとの通信中に問題が発生しました。しばらくしてからもう一度お試しください。", + error.formattedDescription, + ) + } + + // The server's own message (devise_token_auth's), already localized, shown as is. + @Test + fun unprocessableError_serverMessage_showsItAsIs() { + val error = ApiException.UnprocessableError( + rawMessage = "Invalid login credentials. Please try again.", + httpStatusCode = 401, + userMessage = ApiException.UserMessage.Server("Invalid login credentials. Please try again."), + ) + assertEquals("[NATIVEAPPTEMPLATE-2002] Invalid login credentials. Please try again.", error.formattedDescription) } @Test fun appError_unexpected_hasCorrectCode() { val error = AppError.Unexpected() assertEquals("NATIVEAPPTEMPLATE-1001", error.errorCode) - assertEquals("[NATIVEAPPTEMPLATE-1001] Unexpected error", error.formattedDescription) + assertEquals("[NATIVEAPPTEMPLATE-1001] An unexpected error occurred.", error.formattedDescription) } + // The detail is for developers: in the exception's message (the log), not on screen. @Test - fun appError_unexpected_withDetail_includesDetail() { + fun appError_unexpected_withDetail_keepsTheDetailOutOfTheDescription() { val error = AppError.Unexpected(detail = "null pointer") - assertEquals("[NATIVEAPPTEMPLATE-1001] Unexpected error: null pointer", error.formattedDescription) + assertEquals("[NATIVEAPPTEMPLATE-1001] An unexpected error occurred.", error.formattedDescription) + assertEquals("Unexpected error: null pointer", error.message) + } + + // values-ja/strings.xml error_unexpected, the wording shared with iOS. + @Test + @Config(qualifiers = "ja") + fun appError_unexpected_isInJapanese() { + assertEquals("[NATIVEAPPTEMPLATE-1001] 予期しないエラーが発生しました。", AppError.Unexpected().formattedDescription) } @Test fun codedDescription_forCodedError_returnsFormattedDescription() { val error: Throwable = ApiException.ApiError(code = 500, apiMessage = "Server error") - assertEquals("[NATIVEAPPTEMPLATE-2001] Server error [Status: 500]", error.codedDescription) + assertEquals("[NATIVEAPPTEMPLATE-2001] Server error", error.codedDescription) } @Test diff --git a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/RepositoryRequestsTest.kt b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/RepositoryRequestsTest.kt index 50fb871..2a25f42 100644 --- a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/RepositoryRequestsTest.kt +++ b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/RepositoryRequestsTest.kt @@ -41,6 +41,8 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner import retrofit2.Retrofit import java.net.InetAddress import java.net.InetSocketAddress @@ -55,6 +57,8 @@ import kotlin.test.assertFailsWith * `GET {account_id}/api/v1/shopkeeper/shops/{shop_id}/item_tags`); the account id is the one saved * at sign-in (UserPreferences.accountId). */ +// Robolectric for android.util.Log: API errors are logged where they are made. +@RunWith(RobolectricTestRunner::class) class RepositoryRequestsTest { private class Received(val method: String, val pathAndQuery: String, val headers: Map, val body: String) @@ -361,8 +365,9 @@ class RepositoryRequestsTest { assertEquals("Name has already been taken", exception.apiMessage) assertEquals(422, exception.httpStatusCode) - // ApiException.ApiError: "[NATIVEAPPTEMPLATE-2001] " + "$apiMessage [Status: $code]". - assertEquals("[NATIVEAPPTEMPLATE-2001] Name has already been taken [Status: 422]", exception.formattedDescription) + // ApiException.ApiError: the user sees "[NATIVEAPPTEMPLATE-2001] $apiMessage"; the status is + // only in the message, for the log. + assertEquals("[NATIVEAPPTEMPLATE-2001] Name has already been taken", exception.formattedDescription) } @Test diff --git a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImplTest.kt b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImplTest.kt index 64cf3c7..5f8f44d 100644 --- a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImplTest.kt +++ b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/data/login/LoginRepositoryImplTest.kt @@ -3,6 +3,7 @@ package com.nativeapptemplate.nativeapptemplatefree.data.login import androidx.datastore.core.DataStore import com.nativeapptemplate.nativeapptemplatefree.UserPreferences import com.nativeapptemplate.nativeapptemplatefree.common.errors.ApiException +import com.nativeapptemplate.nativeapptemplatefree.common.errors.codedDescription import com.nativeapptemplate.nativeapptemplatefree.datastore.NativeAppTemplatePreferencesDataSource import com.nativeapptemplate.nativeapptemplatefree.datastoreTest.InMemoryDataStore import com.nativeapptemplate.nativeapptemplatefree.model.Attributes @@ -20,9 +21,13 @@ import okhttp3.MediaType.Companion.toMediaType import okhttp3.ResponseBody.Companion.toResponseBody import org.junit.Assert.assertEquals import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner import retrofit2.Response import kotlin.test.assertFailsWith +/** Robolectric for android.util.Log (errors are logged where they are made) and the string resources. */ +@RunWith(RobolectricTestRunner::class) class LoginRepositoryImplTest { /** Every value written to the store, in order: one entry per DataStore write. */ @@ -62,9 +67,13 @@ class LoginRepositoryImplTest { fun login_withoutAToken_failsTheFlowWithACodedError() = runTest { // Thrown inside the flow so the ViewModel's .catch shows it, instead of crashing when the // missing value is later dereferenced while saving the session. - assertFailsWith { + val exception = assertFailsWith { repository(sessionWith(token = null)).login(Login(email = "john@example.com", password = "password")).first() } + // A response missing what the app needs is an internal error: values/strings.xml error_unexpected. + // What is missing is a developer detail, in the message for the log. + assertEquals("[NATIVEAPPTEMPLATE-2002] An unexpected error occurred.", exception.codedDescription) + assertEquals("Not processable error(Incomplete sign-in response: missing token).", exception.message) } @Test diff --git a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensionsTest.kt b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensionsTest.kt index 4ce4f02..e468ee1 100644 --- a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensionsTest.kt +++ b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/network/ApiResponseExtensionsTest.kt @@ -1,6 +1,7 @@ package com.nativeapptemplate.nativeapptemplatefree.network import com.nativeapptemplate.nativeapptemplatefree.common.errors.ApiException +import com.nativeapptemplate.nativeapptemplatefree.common.errors.codedDescription import com.skydoves.sandwich.ApiResponse import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flow @@ -12,10 +13,14 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner import retrofit2.Response import kotlin.test.assertFailsWith import kotlin.test.assertIs +/** Robolectric for the string resources of the messages and for android.util.Log. */ +@RunWith(RobolectricTestRunner::class) class ApiResponseExtensionsTest { @Test @@ -186,7 +191,7 @@ class ApiResponseExtensionsTest { @Test fun throwApiError_apiErrorBody_takesTheCodeFromTheBodyAndTheStatusFromTheResponse() { - // The two can differ; ApiError.code (shown as "[Status: code]") is the body's. + // The two can differ; ApiError.code (logged as "[Status: code]") is the body's. val response = httpError(403, """{"code":1001,"error_message":"Shop limit reached"}""") val exception = assertFailsWith { @@ -195,7 +200,74 @@ class ApiResponseExtensionsTest { assertEquals(1001, exception.code) assertEquals(403, exception.httpStatusCode) - // ApiException.ApiError: "[NATIVEAPPTEMPLATE-2001] " + "$apiMessage [Status: $code]". - assertEquals("[NATIVEAPPTEMPLATE-2001] Shop limit reached [Status: 1001]", exception.formattedDescription) + // ApiException.ApiError: the user sees "[NATIVEAPPTEMPLATE-2001] $apiMessage"; the status is in + // the message, for the log. + assertEquals("[NATIVEAPPTEMPLATE-2001] Shop limit reached", exception.formattedDescription) + assertEquals("Shop limit reached [Status: 1001]", exception.message) + } + + // What the user sees (codedDescription). The generic messages are values/strings.xml's + // error_communication_failed and error_unexpected, the wording shared with iOS. + + private val communicationFailed = + "[NATIVEAPPTEMPLATE-2002] Something went wrong while communicating with the server. Please try again later." + + @Test + fun noResponse_showsTheCommunicationMessage() { + // No network or a timeout: Sandwich wraps OkHttp's IOException. + val response: ApiResponse = ApiResponse.Failure.Exception(java.net.UnknownHostException("Unable to resolve host")) + + val exception = assertFailsWith { + throwApiError(response, "Unable to resolve host") + } + + assertEquals(communicationFailed, exception.codedDescription) + // The detail stays in the exception's message, for the log. + assertEquals("Not processable error(Unable to resolve host).", exception.message) + } + + @Test + fun errorStatusWithoutAMessage_showsTheCommunicationMessage() = runTest { + val response = httpError(502, "Bad Gateway") + + val exception = assertFailsWith { + flow { emitApiResponse(response) }.first() + } + + assertEquals(communicationFailed, exception.codedDescription) + } + + @Test + fun responseTheAppCannotProcess_showsTheUnexpectedErrorMessage() { + // Not an IOException: e.g. a success body the converter could not decode. + val response: ApiResponse = ApiResponse.Failure.Exception(IllegalArgumentException("Unexpected JSON token")) + + val exception = assertFailsWith { + throwApiError(response, "Unexpected JSON token") + } + + assertEquals("[NATIVEAPPTEMPLATE-2002] An unexpected error occurred.", exception.codedDescription) + } + + @Test + fun deviseErrorBody_showsTheServerMessageAsIs() = runTest { + val response = httpError(401, """{"success":false,"errors":["Invalid login credentials. Please try again."]}""") + + val exception = assertFailsWith { + flow { emitApiResponse(response) }.first() + } + + assertEquals("[NATIVEAPPTEMPLATE-2002] Invalid login credentials. Please try again.", exception.codedDescription) + } + + @Test + fun apiErrorBody_showsTheServerMessageWithoutTheStatus() { + val response = httpError(500, """{"code":500,"error_message":"Something broke on our side."}""") + + val exception = assertFailsWith { + throwApiError(response, "server error") + } + + assertEquals("[NATIVEAPPTEMPLATE-2001] Something broke on our side.", exception.codedDescription) } } diff --git a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/testing/util/TestErrors.kt b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/testing/util/TestErrors.kt index f81a0cf..d744111 100644 --- a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/testing/util/TestErrors.kt +++ b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/testing/util/TestErrors.kt @@ -7,7 +7,7 @@ val testApiError = ApiException.ApiError(code = 422, apiMessage = "Name has alre /** * What a screen must show for [testApiError]: CodedError.codedDescription is "[] " - * (AGENTS.md, Error Handling), and a failed request is NATIVEAPPTEMPLATE-2001 with the description - * " [Status: ]" (ApiException.ApiError). + * (AGENTS.md, Error Handling). A failed request is NATIVEAPPTEMPLATE-2001 whose description is the API's + * message as is (the server localizes it); the HTTP status goes to the log, not the screen. */ -const val TEST_API_ERROR_MESSAGE = "[NATIVEAPPTEMPLATE-2001] Name has already been taken [Status: 422]" +const val TEST_API_ERROR_MESSAGE = "[NATIVEAPPTEMPLATE-2001] Name has already been taken" diff --git a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/app_root/SignInEmailAndPasswordViewModelTest.kt b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/app_root/SignInEmailAndPasswordViewModelTest.kt index 9525104..b390a8c 100644 --- a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/app_root/SignInEmailAndPasswordViewModelTest.kt +++ b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/app_root/SignInEmailAndPasswordViewModelTest.kt @@ -106,15 +106,23 @@ class SignInEmailAndPasswordViewModelTest { viewModel.updateEmail(testInputLoggedInShopkeeper.getEmail()!!) viewModel.updatePassword(testInputPassword) loginRepository.sendLoggedInShopkeeper(testInputLoggedInShopkeeper) - val error = ApiException.UnprocessableError(rawMessage = "Bad Gateway", httpStatusCode = 502) + // What throwApiError makes of a 502 without the API's error body. + val error = ApiException.UnprocessableError( + rawMessage = "HTTP 502", + httpStatusCode = 502, + userMessage = ApiException.UserMessage.CommunicationFailure, + ) loginRepository.failPermissions(error) viewModel.login() assertFalse(viewModel.uiState.value.isLoading) // The user-facing message is the coded description (AGENTS.md: CodedError system): - // ApiException.UnprocessableError = "[NATIVEAPPTEMPLATE-2002] Processing error: $rawMessage". - assertEquals("[NATIVEAPPTEMPLATE-2002] Processing error: Bad Gateway", viewModel.uiState.value.message) + // values/strings.xml error_communication_failed, after the code (AGENTS.md, Error Handling). + assertEquals( + "[NATIVEAPPTEMPLATE-2002] Something went wrong while communicating with the server. Please try again later.", + viewModel.uiState.value.message, + ) // A server error is not a rejected session; permissions are fetched again on the next resume. assertEquals(0, loginRepository.logoutCallCount) } @@ -126,15 +134,19 @@ class SignInEmailAndPasswordViewModelTest { viewModel.updatePassword(testInputPassword) // What throwApiError makes of devise_token_auth's 401 {"errors": ["Invalid login credentials. ..."]}. loginRepository.failLogin( - ApiException.UnprocessableError(rawMessage = "Invalid login credentials. Please try again.", httpStatusCode = 401), + ApiException.UnprocessableError( + rawMessage = "Invalid login credentials. Please try again.", + httpStatusCode = 401, + userMessage = ApiException.UserMessage.Server("Invalid login credentials. Please try again."), + ), ) viewModel.login() val uiStateValue = viewModel.uiState.value assertFalse(uiStateValue.isLoading) - // ApiException.UnprocessableError = "[NATIVEAPPTEMPLATE-2002] Processing error: $rawMessage". - assertEquals("[NATIVEAPPTEMPLATE-2002] Processing error: Invalid login credentials. Please try again.", uiStateValue.message) + // The server's message as is, after the code (AGENTS.md, Error Handling). + assertEquals("[NATIVEAPPTEMPLATE-2002] Invalid login credentials. Please try again.", uiStateValue.message) // The previous session (if any) is left alone, and a rejected sign-in is not a sign-out. assertEquals(0, loginRepository.clearSessionCallCount) assertEquals(0, loginRepository.logoutCallCount) diff --git a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/settings/ShopkeeperEditViewModelTest.kt b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/settings/ShopkeeperEditViewModelTest.kt index eedef4a..48780ee 100644 --- a/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/settings/ShopkeeperEditViewModelTest.kt +++ b/app/src/test/kotlin/com/nativeapptemplate/nativeapptemplatefree/ui/settings/ShopkeeperEditViewModelTest.kt @@ -277,7 +277,11 @@ class ShopkeeperEditViewModelTest { ), ) viewModel.reload() - val error = ApiException.UnprocessableError(rawMessage = "Unable to resolve host") + // What throwApiError makes of a request without a response (no network). + val error = ApiException.UnprocessableError( + rawMessage = "Unable to resolve host", + userMessage = ApiException.UserMessage.CommunicationFailure, + ) signUpRepository.failDeleteAccount(error) viewModel.deleteShopkeeper() @@ -286,8 +290,11 @@ class ShopkeeperEditViewModelTest { assertEquals(0, loginRepository.clearSessionCallCount) assertFalse(viewModel.uiState.value.isLoading) assertFalse(viewModel.uiState.value.isDeleted) - // ApiException.UnprocessableError = "[NATIVEAPPTEMPLATE-2002] Processing error: $rawMessage". - assertEquals("[NATIVEAPPTEMPLATE-2002] Processing error: Unable to resolve host", viewModel.uiState.value.message) + // values/strings.xml error_communication_failed, after the code (AGENTS.md, Error Handling). + assertEquals( + "[NATIVEAPPTEMPLATE-2002] Something went wrong while communicating with the server. Please try again later.", + viewModel.uiState.value.message, + ) } @Test