From ba27dace55758e1587207387749fa36f39f91f31 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Tue, 17 Mar 2026 11:50:23 -0600 Subject: [PATCH 01/10] Add diagnostic error handling for application password login failures Surface detailed error messages when application password login fails, instead of showing a generic toast or silently crashing. Catches exceptions in SiteStore encryption/decryption paths and threads error details through to the UI toast. Co-Authored-By: Claude Opus 4.6 --- .../ApplicationPasswordLoginActivity.kt | 12 +- .../ApplicationPasswordLoginViewModel.kt | 118 ++++++++++++------ .../android/fluxc/store/SiteStore.kt | 29 +++-- 3 files changed, 106 insertions(+), 53 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt index 061f1eafc195..6c933f11ace0 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt @@ -67,13 +67,13 @@ class ApplicationPasswordLoginActivity: BaseAppCompatActivity() { ) intent.setData(null) } else { - ToastUtils.showToast( - this, - getString( - R.string.application_password_credentials_storing_error, - navigationActionData.siteUrl - ) + val detail = navigationActionData.errorMessage + val baseMessage = getString( + R.string.application_password_credentials_storing_error, + navigationActionData.siteUrl ) + val message = if (detail != null) "$baseMessage\n$detail" else baseMessage + ToastUtils.showToast(this, message) } // Check if we're in a share flow - if so, just finish and return to LoginActivity diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index 698c2bd9028b..0c4c13ac19c6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -60,15 +60,11 @@ class ApplicationPasswordLoginViewModel @Inject constructor( fun setupSite(rawData: String) { viewModelScope.launch { if (rawData.isEmpty()) { - appLogWrapper.e(AppLog.T.MAIN, "A_P: Cannot store credentials: rawData is empty") - _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = false, - siteUrl = "", - oldSitesIDs = oldSitesIDs, - isError = true - ) + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: Cannot store credentials: rawData is empty" ) + emitError(siteUrl = "", errorMessage = "Callback data was empty") return@launch } val urlLogin = applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData) @@ -117,7 +113,13 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ", siteUrl isEmpty=${siteUrl.isEmpty()}" + ", apiRootUrl isEmpty=${apiRootUrl.isEmpty()}" ) - emitErrorFetching(siteUrl) + emitError( + siteUrl = siteUrl, + errorMessage = "Missing login data — " + + "username empty: ${username.isEmpty()}, " + + "password empty: ${password.isEmpty()}, " + + "apiRootUrl empty: ${apiRootUrl.isEmpty()}" + ) } else { val xmlRpcEndpoint = selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(siteUrl) @@ -137,56 +139,93 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.API, "A_P: Error fetching sites: ${e.stackTraceToString()}" ) - emitErrorFetching(siteUrl) + emitError(siteUrl = siteUrl, errorMessage = e.message) } } - private suspend fun emitErrorFetching(siteUrl: String) = _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = false, - siteUrl = siteUrl, - oldSitesIDs = oldSitesIDs, - isError = true + private suspend fun emitError(siteUrl: String, errorMessage: String? = null) = + _onFinishedEvent.emit( + NavigationActionData( + showSiteSelector = false, + siteUrl = siteUrl, + oldSitesIDs = oldSitesIDs, + isError = true, + errorMessage = errorMessage + ) ) - ) + @Suppress("TooGenericExceptionCaught") @SuppressWarnings("unused") @Subscribe(threadMode = ThreadMode.BACKGROUND) fun onSiteChanged(event: OnSiteChanged) { viewModelScope.launch { val currentNormalizedUrl = UrlUtils.normalizeUrl(currentUrlLogin?.siteUrl) - val site = siteStore.sites.firstOrNull { UrlUtils.normalizeUrl(it.url) == currentNormalizedUrl } - if (event.rowsAffected < 1 || site == null || applicationPasswordLoginHelper.siteHasBadCredentials(site)) { + + if (event.isError) { + val error = event.error appLogWrapper.e( AppLog.T.MAIN, - "A_P: onSiteChanged failed" + - " for: ${currentUrlLogin?.siteUrl}" + - " - rowsAffected=${event.rowsAffected}" + - ", siteFound=${site != null}" + - ", badCredentials=${ - site?.let { - applicationPasswordLoginHelper - .siteHasBadCredentials(it) - } - }" + "A_P: onSiteChanged failed: " + + "SiteStore error ${error?.type}: ${error?.message}" ) - _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = false, - siteUrl = currentUrlLogin?.siteUrl, - oldSitesIDs = oldSitesIDs, - isError = true - ) + emitError( + siteUrl = currentUrlLogin?.siteUrl.orEmpty(), + errorMessage = "SiteStore error: " + + "${error?.type} — ${error?.message}" + ) + return@launch + } + + val site = try { + siteStore.sites.firstOrNull { + UrlUtils.normalizeUrl(it.url) == currentNormalizedUrl + } + } catch (e: Exception) { + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: onSiteChanged failed: " + + "exception reading sites from DB: " + + e.stackTraceToString() + ) + emitError( + siteUrl = currentUrlLogin?.siteUrl.orEmpty(), + errorMessage = "Failed to read sites: ${e.message}" + ) + return@launch + } + + val errorMessage = when { + event.rowsAffected < 1 -> { + "No rows affected (rowsAffected=${event.rowsAffected})" + } + site == null -> { + "Site not found for URL: $currentNormalizedUrl" + } + applicationPasswordLoginHelper.siteHasBadCredentials(site) -> { + "Credentials are empty for site: $currentNormalizedUrl" + } + else -> null + } + + if (errorMessage != null) { + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: onSiteChanged failed: $errorMessage" + ) + emitError( + siteUrl = currentUrlLogin?.siteUrl.orEmpty(), + errorMessage = errorMessage ) } else { + val nonNullSite = site!! _onFinishedEvent.emit( NavigationActionData( showSiteSelector = siteStore.hasSite() && - oldSitesIDs?.contains(site.id) != true, // null or false + oldSitesIDs?.contains(nonNullSite.id) != true, siteUrl = currentUrlLogin?.siteUrl, oldSitesIDs = oldSitesIDs, isError = false, - newSiteLocalId = site.id + newSiteLocalId = nonNullSite.id ) ) } @@ -198,6 +237,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( val siteUrl: String?, val oldSitesIDs: ArrayList?, val isError: Boolean, - val newSiteLocalId: Int? = null + val newSiteLocalId: Int? = null, + val errorMessage: String? = null ) } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index 514c23868790..3638ac49d88c 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -1480,18 +1480,24 @@ open class SiteStore @Inject constructor( } } + @Suppress("TooGenericExceptionCaught") suspend fun fetchSitesXmlRpcFromApplicationPassword( payload: RefreshSitesXMLRPCApplicationPasswordCredentialsPayload ): OnSiteChanged { return coroutineEngine.withDefaultContext(T.API, this, "Fetch sites") { - updateSites( - siteXMLRPCClient.fetchSitesFromApplicationPassword( - payload.url, - payload.apiRootUrl, - payload.username, - payload.password + try { + updateSites( + siteXMLRPCClient.fetchSitesFromApplicationPassword( + payload.url, + payload.apiRootUrl, + payload.username, + payload.password + ) ) - ) + } catch (e: Exception) { + AppLog.e(T.API, "Failed to fetch/store sites: ${e.message}", e) + OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, e.message)) + } } } @@ -1547,7 +1553,7 @@ open class SiteStore @Inject constructor( } } - @Suppress("SwallowedException") + @Suppress("SwallowedException", "TooGenericExceptionCaught") private fun updateApplicationPassword(siteModel: SiteModel): OnSiteChanged { return try { val siteFromDB = getSiteByLocalId(siteModel.id) @@ -1569,6 +1575,13 @@ open class SiteStore @Inject constructor( OnSiteChanged(siteSqlUtils.insertOrUpdateSite(siteToStore)) } catch (e: DuplicateSiteException) { OnSiteChanged(SiteError(DUPLICATE_SITE)) + } catch (e: Exception) { + AppLog.e( + T.DB, + "Failed to update application password: ${e.message}", + e + ) + OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, e.message)) } } From 3f65a6750fc1fbabbbc0d7ced8048082b7e0d241 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Tue, 17 Mar 2026 11:56:06 -0600 Subject: [PATCH 02/10] Send Sentry report on application password login failure Every error path in the login flow now sends a crash report via CrashLogging so we get visibility into failures in the wild. Co-Authored-By: Claude Opus 4.6 --- .../ApplicationPasswordLoginViewModel.kt | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index 0c4c13ac19c6..6a3170c1e063 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -21,6 +21,8 @@ import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.UriLogin import org.wordpress.android.util.AppLog import org.wordpress.android.util.UrlUtils +import org.wordpress.android.util.crashlogging.sendReportWithTag +import com.automattic.android.tracks.crashlogging.CrashLogging import javax.inject.Inject import javax.inject.Named @@ -32,6 +34,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( private val selfHostedEndpointFinder: SelfHostedEndpointFinder, private val siteStore: SiteStore, private val appLogWrapper: AppLogWrapper, + private val crashLogging: CrashLogging, ) : ViewModel() { private val _onFinishedEvent = MutableSharedFlow() /** @@ -139,11 +142,18 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.API, "A_P: Error fetching sites: ${e.stackTraceToString()}" ) - emitError(siteUrl = siteUrl, errorMessage = e.message) + emitError(siteUrl = siteUrl, errorMessage = e.message, cause = e) } } - private suspend fun emitError(siteUrl: String, errorMessage: String? = null) = + private suspend fun emitError( + siteUrl: String, + errorMessage: String? = null, + cause: Throwable? = null + ) { + val exception = cause + ?: Exception("Application password login failed: $errorMessage") + crashLogging.sendReportWithTag(exception, AppLog.T.MAIN) _onFinishedEvent.emit( NavigationActionData( showSiteSelector = false, @@ -153,6 +163,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( errorMessage = errorMessage ) ) + } @Suppress("TooGenericExceptionCaught") @SuppressWarnings("unused") @@ -189,7 +200,8 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) emitError( siteUrl = currentUrlLogin?.siteUrl.orEmpty(), - errorMessage = "Failed to read sites: ${e.message}" + errorMessage = "Failed to read sites: ${e.message}", + cause = e ) return@launch } From 37ee5c971c4065ce57aa6417ae17e08f323a6419 Mon Sep 17 00:00:00 2001 From: adalpari Date: Tue, 17 Mar 2026 19:41:25 +0100 Subject: [PATCH 03/10] Add analytics tracking and crash logging for app password storing failures Re-applies the reverted changes from #22694 that add trackStoringFailed() calls with specific reason codes (empty_raw_data, empty_fetch_params, fetch_sites_exception, site_changed_failed, bad_data, site_not_found) and CrashLogging reports for better debugging. Co-Authored-By: Claude Opus 4.6 --- .../ApplicationPasswordLoginViewModel.kt | 19 +++++++++++++++ .../login/ApplicationPasswordLoginHelper.kt | 24 ++++++++++++++++++- .../ApplicationPasswordLoginHelperTest.kt | 7 +++++- .../android/analytics/AnalyticsTracker.java | 3 ++- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index 6a3170c1e063..2d6a3ae96d98 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -67,6 +67,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.MAIN, "A_P: Cannot store credentials: rawData is empty" ) + applicationPasswordLoginHelper.trackStoringFailed("", "empty_raw_data") emitError(siteUrl = "", errorMessage = "Callback data was empty") return@launch } @@ -116,6 +117,9 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ", siteUrl isEmpty=${siteUrl.isEmpty()}" + ", apiRootUrl isEmpty=${apiRootUrl.isEmpty()}" ) + applicationPasswordLoginHelper.trackStoringFailed( + siteUrl, "empty_fetch_params" + ) emitError( siteUrl = siteUrl, errorMessage = "Missing login data — " + @@ -142,6 +146,9 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.API, "A_P: Error fetching sites: ${e.stackTraceToString()}" ) + applicationPasswordLoginHelper.trackStoringFailed( + siteUrl, "fetch_sites_exception" + ) emitError(siteUrl = siteUrl, errorMessage = e.message, cause = e) } } @@ -179,6 +186,10 @@ class ApplicationPasswordLoginViewModel @Inject constructor( "A_P: onSiteChanged failed: " + "SiteStore error ${error?.type}: ${error?.message}" ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) emitError( siteUrl = currentUrlLogin?.siteUrl.orEmpty(), errorMessage = "SiteStore error: " + @@ -198,6 +209,10 @@ class ApplicationPasswordLoginViewModel @Inject constructor( "exception reading sites from DB: " + e.stackTraceToString() ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) emitError( siteUrl = currentUrlLogin?.siteUrl.orEmpty(), errorMessage = "Failed to read sites: ${e.message}", @@ -224,6 +239,10 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.MAIN, "A_P: onSiteChanged failed: $errorMessage" ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) emitError( siteUrl = currentUrlLogin?.siteUrl.orEmpty(), errorMessage = errorMessage diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt index 043d995ed239..e1ade62d6e13 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt @@ -2,6 +2,7 @@ package org.wordpress.android.ui.accounts.login import android.content.Context import androidx.core.net.toUri +import com.automattic.android.tracks.crashlogging.CrashLogging import org.wordpress.android.R import org.wordpress.android.util.DeviceUtils import kotlinx.coroutines.CoroutineDispatcher @@ -17,6 +18,7 @@ import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.util.AppLog import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.UrlUtils +import org.wordpress.android.util.crashlogging.sendReportWithTag import rs.wordpress.api.kotlin.ApiDiscoveryResult import rs.wordpress.api.kotlin.WpLoginClient import uniffi.wp_api.applicationPasswordsUrl @@ -25,6 +27,7 @@ import javax.inject.Named private const val URL_TAG = "url" private const val SUCCESS_TAG = "success" +private const val REASON_TAG = "reason" class ApplicationPasswordLoginHelper @Inject constructor( @param:Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, @@ -35,7 +38,8 @@ class ApplicationPasswordLoginHelper @Inject constructor( private val wpLoginClient: WpLoginClient, private val appLogWrapper: AppLogWrapper, private val apiRootUrlCache: ApiRootUrlCache, - private val discoverSuccessWrapper: DiscoverSuccessWrapper + private val discoverSuccessWrapper: DiscoverSuccessWrapper, + private val crashLogging: CrashLogging ) { private var processedAppPasswordData: String? = null @@ -103,6 +107,7 @@ class ApplicationPasswordLoginHelper @Inject constructor( ", alreadyProcessed=" + "${urlLogin.siteUrl == processedAppPasswordData}" ) + trackStoringFailed(urlLogin.siteUrl, "bad_data") return false } @@ -132,11 +137,28 @@ class ApplicationPasswordLoginHelper @Inject constructor( " - site not found in store" + " (${siteStore.sites.size} sites available)" ) + trackStoringFailed(urlLogin.siteUrl, "site_not_found") false } } } + fun trackStoringFailed(siteUrl: String?, reason: String) { + val properties: MutableMap = HashMap() + properties[URL_TAG] = siteUrl + properties[REASON_TAG] = reason + AnalyticsTracker.track( + Stat.APPLICATION_PASSWORD_STORING_FAILED, + properties + ) + crashLogging.sendReportWithTag( + exception = Exception( + "A_P: storing failed for $siteUrl - $reason" + ), + tag = AppLog.T.DB + ) + } + private fun trackSuccessful(siteUrl: String) { val properties: MutableMap = HashMap() properties[URL_TAG] = siteUrl diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt index 5a501c0ba910..f57fe9381820 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt @@ -5,6 +5,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test +import com.automattic.android.tracks.crashlogging.CrashLogging import org.mockito.Mock import org.mockito.Mockito.mock import org.mockito.MockitoAnnotations @@ -67,6 +68,9 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -81,7 +85,8 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { wpLoginClient, appLogWrapper, apiRootUrlCache, - discoverSuccessWrapper + discoverSuccessWrapper, + crashLogging ) } diff --git a/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java b/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java index 95d0188fb386..518c849ac958 100644 --- a/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java +++ b/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java @@ -1094,7 +1094,8 @@ public enum Stat { BACKGROUND_REST_AUTODISCOVERY_FAILED, WP_ANDROID_APPLICATION_PASSWORD_LOGIN, JP_ANDROID_APPLICATION_PASSWORD_LOGIN, - APPLICATION_PASSWORD_SET_OFF; + APPLICATION_PASSWORD_SET_OFF, + APPLICATION_PASSWORD_STORING_FAILED; /* * Please set the event name in the enum only if the new Stat's name in lower case does not match it. * In that case you also need to add the event in the `AnalyticsTrackerNosaraTest.specialNames` map. From 18be9c344049a77a07536afcb6e99ec404a51ae9 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 15:55:26 +0100 Subject: [PATCH 04/10] Address code review: hide internal errors from toast, fix non-null assertion, add tests - Show only user-friendly message in toast, keep detailed errors in logs/crash reports - Replace site!! with safe ?: return@launch - Fix import ordering (com.* before org.*) - Add TODO comments on broad catch blocks to narrow once root cause identified - Add CrashLogging mock and 5 new tests for error branches (SiteStore error, no rows affected, bad credentials, DB exception, crash report verification) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ApplicationPasswordLoginActivity.kt | 12 +- .../ApplicationPasswordLoginViewModel.kt | 8 +- .../ApplicationPasswordLoginViewModelTest.kt | 211 +++++++++++++++--- .../android/fluxc/store/SiteStore.kt | 4 + 4 files changed, 196 insertions(+), 39 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt index 6c933f11ace0..061f1eafc195 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginActivity.kt @@ -67,13 +67,13 @@ class ApplicationPasswordLoginActivity: BaseAppCompatActivity() { ) intent.setData(null) } else { - val detail = navigationActionData.errorMessage - val baseMessage = getString( - R.string.application_password_credentials_storing_error, - navigationActionData.siteUrl + ToastUtils.showToast( + this, + getString( + R.string.application_password_credentials_storing_error, + navigationActionData.siteUrl + ) ) - val message = if (detail != null) "$baseMessage\n$detail" else baseMessage - ToastUtils.showToast(this, message) } // Check if we're in a share flow - if so, just finish and return to LoginActivity diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index 2d6a3ae96d98..736889c312f5 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -21,8 +21,8 @@ import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.UriLogin import org.wordpress.android.util.AppLog import org.wordpress.android.util.UrlUtils -import org.wordpress.android.util.crashlogging.sendReportWithTag import com.automattic.android.tracks.crashlogging.CrashLogging +import org.wordpress.android.util.crashlogging.sendReportWithTag import javax.inject.Inject import javax.inject.Named @@ -248,15 +248,15 @@ class ApplicationPasswordLoginViewModel @Inject constructor( errorMessage = errorMessage ) } else { - val nonNullSite = site!! + val resolvedSite = site ?: return@launch _onFinishedEvent.emit( NavigationActionData( showSiteSelector = siteStore.hasSite() && - oldSitesIDs?.contains(nonNullSite.id) != true, + oldSitesIDs?.contains(resolvedSite.id) != true, siteUrl = currentUrlLogin?.siteUrl, oldSitesIDs = oldSitesIDs, isError = false, - newSiteLocalId = nonNullSite.id + newSiteLocalId = resolvedSite.id ) ) } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt index 8ee972d40a1f..f40636f3bc4a 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt @@ -1,25 +1,27 @@ package org.wordpress.android.ui.accounts.applicationpassword import app.cash.turbine.test +import com.automattic.android.tracks.crashlogging.CrashLogging import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test import org.mockito.Mock import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.eq import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest -import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper -import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder -import org.wordpress.android.fluxc.store.SiteStore -import org.mockito.kotlin.any -import org.mockito.kotlin.eq import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder +import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper import kotlin.test.assertEquals +import kotlin.test.assertTrue @ExperimentalCoroutinesApi @Suppress("MaxLineLength") @@ -39,6 +41,9 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { @Mock lateinit var appLogWrapper: AppLogWrapper + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var viewModel: ApplicationPasswordLoginViewModel private val rawData = "url=callback?site_url=https://example.com&user_login=user&password=pass" @@ -63,7 +68,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { applicationPasswordLoginHelper, selfHostedEndpointFinder, siteStore, - appLogWrapper + appLogWrapper, + crashLogging ) whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData)).thenReturn(urlLogin) } @@ -76,7 +82,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showSiteSelector = false, siteUrl = "", oldSitesIDs = null, - isError = true + isError = true, + errorMessage = "Callback data was empty" ) // When @@ -102,7 +109,11 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showSiteSelector = false, siteUrl = "", oldSitesIDs = null, - isError = true + isError = true, + errorMessage = "Missing login data — " + + "username empty: true, " + + "password empty: true, " + + "apiRootUrl empty: true" ) whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(malformedRawData)) .thenReturn( @@ -129,7 +140,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showSiteSelector = false, siteUrl = urlLogin.siteUrl, oldSitesIDs = null, - isError = true + isError = true, + errorMessage = null ) whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))) .thenReturn(false) @@ -152,31 +164,32 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { runTest { // Given val xmlRpcEndpoint = "https://example.com/xmlrpc.php" - val expectedResult = ApplicationPasswordLoginViewModel.NavigationActionData( - showSiteSelector = false, - siteUrl = urlLogin.siteUrl, - oldSitesIDs = null, - isError = true - ) - whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))) - .thenReturn(false) - whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)) - .thenReturn(xmlRpcEndpoint) + whenever( + applicationPasswordLoginHelper + .storeApplicationPasswordCredentialsFrom(eq(urlLogin)) + ).thenReturn(false) + whenever( + selfHostedEndpointFinder + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!) + ).thenReturn(xmlRpcEndpoint) // When viewModel.onFinishedEvent.test { viewModel.setupSite(rawData) // Mock onSiteChanged event viewModel.onSiteChanged( - SiteStore.OnSiteChanged( - rowsAffected = 1, - ) + SiteStore.OnSiteChanged(rowsAffected = 1) ) // Then val finishedEvent = awaitItem() - assertEquals(expectedResult, finishedEvent) - verify(selfHostedEndpointFinder, times(1)).verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) + assertTrue(finishedEvent.isError) + assertTrue( + finishedEvent.errorMessage + ?.contains("Site not found") == true + ) + verify(selfHostedEndpointFinder, times(1)) + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) verify(siteStore, times(1)).sites cancelAndIgnoreRemainingEvents() } @@ -272,10 +285,14 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { ) whenever(siteStore.hasSite()).thenReturn(true) whenever(siteStore.sites).thenReturn(listOf(testSite)) - whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))) - .thenReturn(false) - whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)) - .thenReturn(xmlRpcEndpoint) + whenever( + applicationPasswordLoginHelper + .storeApplicationPasswordCredentialsFrom(eq(urlLogin)) + ).thenReturn(false) + whenever( + selfHostedEndpointFinder + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!) + ).thenReturn(xmlRpcEndpoint) // When viewModel.onFinishedEvent.test { @@ -291,8 +308,144 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { // Then val finishedEvent = awaitItem() assertEquals(expectedResult, finishedEvent) - verify(selfHostedEndpointFinder, times(1)).verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) + verify(selfHostedEndpointFinder, times(1)) + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `given onSiteChanged with error, then emit error with SiteStore details`() = + runTest { + // Given + setupFetchSitesFlow() + val siteError = SiteStore.SiteError( + SiteStore.SiteErrorType.GENERIC_ERROR, "encryption failed" + ) + val errorEvent = SiteStore.OnSiteChanged(0, siteError) + + // When + viewModel.onFinishedEvent.test { + viewModel.setupSite(rawData) + viewModel.onSiteChanged(errorEvent) + + // Then + val result = awaitItem() + assertTrue(result.isError) + assertEquals( + "SiteStore error: GENERIC_ERROR — encryption failed", + result.errorMessage + ) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `given onSiteChanged with no rows affected, then emit error`() = + runTest { + // Given + setupFetchSitesFlow() + + // When + viewModel.onFinishedEvent.test { + viewModel.setupSite(rawData) + viewModel.onSiteChanged( + SiteStore.OnSiteChanged(rowsAffected = 0) + ) + + // Then + val result = awaitItem() + assertTrue(result.isError) + assertTrue( + result.errorMessage?.contains("No rows affected") == true + ) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `given onSiteChanged with bad credentials, then emit error`() = + runTest { + // Given + setupFetchSitesFlow() + whenever(siteStore.sites).thenReturn(listOf(testSite)) + whenever( + applicationPasswordLoginHelper.siteHasBadCredentials(any()) + ).thenReturn(true) + + // When + viewModel.onFinishedEvent.test { + viewModel.setupSite(rawData) + viewModel.onSiteChanged( + SiteStore.OnSiteChanged( + rowsAffected = 1, + updatedSites = listOf(testSite) + ) + ) + + // Then + val result = awaitItem() + assertTrue(result.isError) + assertTrue( + result.errorMessage + ?.contains("Credentials are empty") == true + ) cancelAndIgnoreRemainingEvents() } } + + @Test + fun `given onSiteChanged with DB exception, then emit error`() = + runTest { + // Given + setupFetchSitesFlow() + whenever(siteStore.sites) + .thenThrow(RuntimeException("DB corrupted")) + + // When + viewModel.onFinishedEvent.test { + viewModel.setupSite(rawData) + viewModel.onSiteChanged( + SiteStore.OnSiteChanged(rowsAffected = 1) + ) + + // Then + val result = awaitItem() + assertTrue(result.isError) + assertEquals( + "Failed to read sites: DB corrupted", + result.errorMessage + ) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `given error emitted, then crash report is sent`() = runTest { + // Given & When + viewModel.onFinishedEvent.test { + viewModel.setupSite("") + + // Then + awaitItem() + verify(crashLogging).sendReport( + exception = any(), + tags = eq(mapOf("tag" to "MAIN")), + message = eq(null) + ) + cancelAndIgnoreRemainingEvents() + } + } + + private suspend fun setupFetchSitesFlow() { + val xmlRpcEndpoint = "https://example.com/xmlrpc.php" + whenever( + applicationPasswordLoginHelper + .storeApplicationPasswordCredentialsFrom(eq(urlLogin)) + ).thenReturn(false) + whenever( + selfHostedEndpointFinder + .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!) + ).thenReturn(xmlRpcEndpoint) + } } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index 3638ac49d88c..ce549ecb0c5f 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -1495,6 +1495,8 @@ open class SiteStore @Inject constructor( ) ) } catch (e: Exception) { + // TODO: Narrow to specific exception type once Android 16 + // KeyStore root cause is identified (CMM-1949) AppLog.e(T.API, "Failed to fetch/store sites: ${e.message}", e) OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, e.message)) } @@ -1576,6 +1578,8 @@ open class SiteStore @Inject constructor( } catch (e: DuplicateSiteException) { OnSiteChanged(SiteError(DUPLICATE_SITE)) } catch (e: Exception) { + // TODO: Narrow to specific exception type once Android 16 + // KeyStore root cause is identified (CMM-1949) AppLog.e( T.DB, "Failed to update application password: ${e.message}", From a22e7fa62c3fdd44361d3e7ff3fc27568e2dbffe Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 16:14:00 +0100 Subject: [PATCH 05/10] Refactor onSiteChanged to fix LongMethod detekt finding, remove TODOs Split onSiteChanged into smaller focused methods to stay under the 60-line detekt limit: handleSiteChangedError, handleSiteChangedSuccess, validateSiteChanged, and logAndEmitSiteChangedError. Also remove TODO comments from SiteStore catch blocks. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ApplicationPasswordLoginViewModel.kt | 167 ++++++++++-------- .../android/fluxc/store/SiteStore.kt | 4 - 2 files changed, 90 insertions(+), 81 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index 736889c312f5..f501ca2fe056 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -11,6 +11,7 @@ import org.greenrobot.eventbus.Subscribe import org.greenrobot.eventbus.ThreadMode import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.generated.SiteActionBuilder +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.discovery.SelfHostedEndpointFinder import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.fluxc.store.SiteStore.OnSiteChanged @@ -172,97 +173,109 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) } - @Suppress("TooGenericExceptionCaught") @SuppressWarnings("unused") @Subscribe(threadMode = ThreadMode.BACKGROUND) fun onSiteChanged(event: OnSiteChanged) { viewModelScope.launch { - val currentNormalizedUrl = UrlUtils.normalizeUrl(currentUrlLogin?.siteUrl) - if (event.isError) { - val error = event.error - appLogWrapper.e( - AppLog.T.MAIN, - "A_P: onSiteChanged failed: " + - "SiteStore error ${error?.type}: ${error?.message}" - ) - applicationPasswordLoginHelper.trackStoringFailed( - currentUrlLogin?.siteUrl, - "site_changed_failed" - ) - emitError( - siteUrl = currentUrlLogin?.siteUrl.orEmpty(), - errorMessage = "SiteStore error: " + - "${error?.type} — ${error?.message}" - ) - return@launch + handleSiteChangedError(event) + } else { + handleSiteChangedSuccess(event) } + } + } - val site = try { - siteStore.sites.firstOrNull { - UrlUtils.normalizeUrl(it.url) == currentNormalizedUrl - } - } catch (e: Exception) { - appLogWrapper.e( - AppLog.T.MAIN, - "A_P: onSiteChanged failed: " + - "exception reading sites from DB: " + - e.stackTraceToString() - ) - applicationPasswordLoginHelper.trackStoringFailed( - currentUrlLogin?.siteUrl, - "site_changed_failed" - ) - emitError( - siteUrl = currentUrlLogin?.siteUrl.orEmpty(), - errorMessage = "Failed to read sites: ${e.message}", - cause = e - ) - return@launch - } + private suspend fun handleSiteChangedError(event: OnSiteChanged) { + val error = event.error + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: onSiteChanged failed: " + + "SiteStore error ${error?.type}: ${error?.message}" + ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) + emitError( + siteUrl = currentUrlLogin?.siteUrl.orEmpty(), + errorMessage = "SiteStore error: " + + "${error?.type} — ${error?.message}" + ) + } - val errorMessage = when { - event.rowsAffected < 1 -> { - "No rows affected (rowsAffected=${event.rowsAffected})" - } - site == null -> { - "Site not found for URL: $currentNormalizedUrl" - } - applicationPasswordLoginHelper.siteHasBadCredentials(site) -> { - "Credentials are empty for site: $currentNormalizedUrl" - } - else -> null + @Suppress("TooGenericExceptionCaught") + private suspend fun handleSiteChangedSuccess(event: OnSiteChanged) { + val normalizedUrl = + UrlUtils.normalizeUrl(currentUrlLogin?.siteUrl) + + val site = try { + siteStore.sites.firstOrNull { + UrlUtils.normalizeUrl(it.url) == normalizedUrl } + } catch (e: Exception) { + logAndEmitSiteChangedError( + "exception reading sites from DB: " + + e.stackTraceToString(), + "Failed to read sites: ${e.message}", + e + ) + return + } - if (errorMessage != null) { - appLogWrapper.e( - AppLog.T.MAIN, - "A_P: onSiteChanged failed: $errorMessage" - ) - applicationPasswordLoginHelper.trackStoringFailed( - currentUrlLogin?.siteUrl, - "site_changed_failed" - ) - emitError( - siteUrl = currentUrlLogin?.siteUrl.orEmpty(), - errorMessage = errorMessage - ) - } else { - val resolvedSite = site ?: return@launch - _onFinishedEvent.emit( - NavigationActionData( - showSiteSelector = siteStore.hasSite() && - oldSitesIDs?.contains(resolvedSite.id) != true, - siteUrl = currentUrlLogin?.siteUrl, - oldSitesIDs = oldSitesIDs, - isError = false, - newSiteLocalId = resolvedSite.id - ) + val errorMessage = validateSiteChanged( + event, site, normalizedUrl + ) + if (errorMessage != null) { + logAndEmitSiteChangedError(errorMessage, errorMessage) + } else { + val resolvedSite = site ?: return + _onFinishedEvent.emit( + NavigationActionData( + showSiteSelector = siteStore.hasSite() && + oldSitesIDs?.contains(resolvedSite.id) != true, + siteUrl = currentUrlLogin?.siteUrl, + oldSitesIDs = oldSitesIDs, + isError = false, + newSiteLocalId = resolvedSite.id ) - } + ) } } + private fun validateSiteChanged( + event: OnSiteChanged, + site: SiteModel?, + normalizedUrl: String? + ): String? = when { + event.rowsAffected < 1 -> + "No rows affected (rowsAffected=${event.rowsAffected})" + site == null -> + "Site not found for URL: $normalizedUrl" + applicationPasswordLoginHelper.siteHasBadCredentials(site) -> + "Credentials are empty for site: $normalizedUrl" + else -> null + } + + private suspend fun logAndEmitSiteChangedError( + logMessage: String, + errorMessage: String, + cause: Throwable? = null + ) { + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: onSiteChanged failed: $logMessage" + ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) + emitError( + siteUrl = currentUrlLogin?.siteUrl.orEmpty(), + errorMessage = errorMessage, + cause = cause + ) + } + data class NavigationActionData( val showSiteSelector: Boolean, val siteUrl: String?, diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index ce549ecb0c5f..3638ac49d88c 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -1495,8 +1495,6 @@ open class SiteStore @Inject constructor( ) ) } catch (e: Exception) { - // TODO: Narrow to specific exception type once Android 16 - // KeyStore root cause is identified (CMM-1949) AppLog.e(T.API, "Failed to fetch/store sites: ${e.message}", e) OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, e.message)) } @@ -1578,8 +1576,6 @@ open class SiteStore @Inject constructor( } catch (e: DuplicateSiteException) { OnSiteChanged(SiteError(DUPLICATE_SITE)) } catch (e: Exception) { - // TODO: Narrow to specific exception type once Android 16 - // KeyStore root cause is identified (CMM-1949) AppLog.e( T.DB, "Failed to update application password: ${e.message}", From 5100b8f64d5d2562c53ead8752002ebfe0ec9254 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 16:49:34 +0100 Subject: [PATCH 06/10] Fix double Sentry reporting, unreported storeCredentials exception, and error message leaks - Remove crashLogging from ApplicationPasswordLoginHelper.trackStoringFailed so only the ViewModel's emitError sends Sentry reports with full context - Add trackStoringFailed and crashLogging.sendReportWithTag to the storeCredentials catch block so KeyStore failures are tracked - Replace technical error messages in NavigationActionData.errorMessage with opaque error codes (e.g. site_store_error, no_rows_affected) - Use e.message ?: e.javaClass.simpleName in SiteStore catch blocks as fallback for exceptions without a message Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ApplicationPasswordLoginViewModel.kt | 65 +++++++++++-------- .../login/ApplicationPasswordLoginHelper.kt | 11 +--- .../ApplicationPasswordLoginViewModelTest.kt | 31 +++------ .../ApplicationPasswordLoginHelperTest.kt | 7 +- .../android/fluxc/store/SiteStore.kt | 10 +-- 5 files changed, 55 insertions(+), 69 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index f501ca2fe056..1eea9ba4bd4b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -69,7 +69,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( "A_P: Cannot store credentials: rawData is empty" ) applicationPasswordLoginHelper.trackStoringFailed("", "empty_raw_data") - emitError(siteUrl = "", errorMessage = "Callback data was empty") + emitError(siteUrl = "", errorMessage = "empty_raw_data") return@launch } val urlLogin = applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData) @@ -97,6 +97,10 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.DB, "A_P: Error storing credentials: ${e.stackTraceToString()}" ) + applicationPasswordLoginHelper.trackStoringFailed( + urlLogin.siteUrl, "store_credentials_exception" + ) + crashLogging.sendReportWithTag(e, AppLog.T.DB) false } } @@ -123,10 +127,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) emitError( siteUrl = siteUrl, - errorMessage = "Missing login data — " + - "username empty: ${username.isEmpty()}, " + - "password empty: ${password.isEmpty()}, " + - "apiRootUrl empty: ${apiRootUrl.isEmpty()}" + errorMessage = "empty_fetch_params" ) } else { val xmlRpcEndpoint = @@ -198,8 +199,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) emitError( siteUrl = currentUrlLogin?.siteUrl.orEmpty(), - errorMessage = "SiteStore error: " + - "${error?.type} — ${error?.message}" + errorMessage = "site_store_error" ) } @@ -214,19 +214,20 @@ class ApplicationPasswordLoginViewModel @Inject constructor( } } catch (e: Exception) { logAndEmitSiteChangedError( - "exception reading sites from DB: " + + logMessage = "exception reading sites from DB: " + e.stackTraceToString(), - "Failed to read sites: ${e.message}", - e + errorCode = "db_read_exception", + cause = e ) return } - val errorMessage = validateSiteChanged( - event, site, normalizedUrl - ) - if (errorMessage != null) { - logAndEmitSiteChangedError(errorMessage, errorMessage) + val validationError = validateSiteChanged(event, site) + if (validationError != null) { + logAndEmitSiteChangedError( + logMessage = validationError.logMessage, + errorCode = validationError.errorCode + ) } else { val resolvedSite = site ?: return _onFinishedEvent.emit( @@ -244,21 +245,33 @@ class ApplicationPasswordLoginViewModel @Inject constructor( private fun validateSiteChanged( event: OnSiteChanged, - site: SiteModel?, - normalizedUrl: String? - ): String? = when { - event.rowsAffected < 1 -> - "No rows affected (rowsAffected=${event.rowsAffected})" - site == null -> - "Site not found for URL: $normalizedUrl" - applicationPasswordLoginHelper.siteHasBadCredentials(site) -> - "Credentials are empty for site: $normalizedUrl" + site: SiteModel? + ): SiteChangedValidationError? = when { + event.rowsAffected < 1 -> SiteChangedValidationError( + logMessage = "No rows affected " + + "(rowsAffected=${event.rowsAffected})", + errorCode = "no_rows_affected" + ) + site == null -> SiteChangedValidationError( + logMessage = "Site not found after update", + errorCode = "site_not_found" + ) + applicationPasswordLoginHelper + .siteHasBadCredentials(site) -> SiteChangedValidationError( + logMessage = "Credentials are empty after store", + errorCode = "empty_credentials" + ) else -> null } + private data class SiteChangedValidationError( + val logMessage: String, + val errorCode: String + ) + private suspend fun logAndEmitSiteChangedError( logMessage: String, - errorMessage: String, + errorCode: String, cause: Throwable? = null ) { appLogWrapper.e( @@ -271,7 +284,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) emitError( siteUrl = currentUrlLogin?.siteUrl.orEmpty(), - errorMessage = errorMessage, + errorMessage = errorCode, cause = cause ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt index e1ade62d6e13..b97efd353030 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt @@ -2,7 +2,6 @@ package org.wordpress.android.ui.accounts.login import android.content.Context import androidx.core.net.toUri -import com.automattic.android.tracks.crashlogging.CrashLogging import org.wordpress.android.R import org.wordpress.android.util.DeviceUtils import kotlinx.coroutines.CoroutineDispatcher @@ -18,7 +17,6 @@ import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.util.AppLog import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.UrlUtils -import org.wordpress.android.util.crashlogging.sendReportWithTag import rs.wordpress.api.kotlin.ApiDiscoveryResult import rs.wordpress.api.kotlin.WpLoginClient import uniffi.wp_api.applicationPasswordsUrl @@ -38,8 +36,7 @@ class ApplicationPasswordLoginHelper @Inject constructor( private val wpLoginClient: WpLoginClient, private val appLogWrapper: AppLogWrapper, private val apiRootUrlCache: ApiRootUrlCache, - private val discoverSuccessWrapper: DiscoverSuccessWrapper, - private val crashLogging: CrashLogging + private val discoverSuccessWrapper: DiscoverSuccessWrapper ) { private var processedAppPasswordData: String? = null @@ -151,12 +148,6 @@ class ApplicationPasswordLoginHelper @Inject constructor( Stat.APPLICATION_PASSWORD_STORING_FAILED, properties ) - crashLogging.sendReportWithTag( - exception = Exception( - "A_P: storing failed for $siteUrl - $reason" - ), - tag = AppLog.T.DB - ) } private fun trackSuccessful(siteUrl: String) { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt index f40636f3bc4a..d01b72db1aaf 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt @@ -83,7 +83,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { siteUrl = "", oldSitesIDs = null, isError = true, - errorMessage = "Callback data was empty" + errorMessage = "empty_raw_data" ) // When @@ -110,10 +110,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { siteUrl = "", oldSitesIDs = null, isError = true, - errorMessage = "Missing login data — " + - "username empty: true, " + - "password empty: true, " + - "apiRootUrl empty: true" + errorMessage = "empty_fetch_params" ) whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(malformedRawData)) .thenReturn( @@ -184,9 +181,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { // Then val finishedEvent = awaitItem() assertTrue(finishedEvent.isError) - assertTrue( - finishedEvent.errorMessage - ?.contains("Site not found") == true + assertEquals( + "site_not_found", finishedEvent.errorMessage ) verify(selfHostedEndpointFinder, times(1)) .verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) @@ -332,10 +328,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { // Then val result = awaitItem() assertTrue(result.isError) - assertEquals( - "SiteStore error: GENERIC_ERROR — encryption failed", - result.errorMessage - ) + assertEquals("site_store_error", result.errorMessage) cancelAndIgnoreRemainingEvents() } } @@ -356,9 +349,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { // Then val result = awaitItem() assertTrue(result.isError) - assertTrue( - result.errorMessage?.contains("No rows affected") == true - ) + assertEquals("no_rows_affected", result.errorMessage) cancelAndIgnoreRemainingEvents() } } @@ -386,10 +377,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { // Then val result = awaitItem() assertTrue(result.isError) - assertTrue( - result.errorMessage - ?.contains("Credentials are empty") == true - ) + assertEquals("empty_credentials", result.errorMessage) cancelAndIgnoreRemainingEvents() } } @@ -412,10 +400,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { // Then val result = awaitItem() assertTrue(result.isError) - assertEquals( - "Failed to read sites: DB corrupted", - result.errorMessage - ) + assertEquals("db_read_exception", result.errorMessage) cancelAndIgnoreRemainingEvents() } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt index f57fe9381820..5a501c0ba910 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt @@ -5,7 +5,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test -import com.automattic.android.tracks.crashlogging.CrashLogging import org.mockito.Mock import org.mockito.Mockito.mock import org.mockito.MockitoAnnotations @@ -68,9 +67,6 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache - @Mock - lateinit var crashLogging: CrashLogging - private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -85,8 +81,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { wpLoginClient, appLogWrapper, apiRootUrlCache, - discoverSuccessWrapper, - crashLogging + discoverSuccessWrapper ) } diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt index 3638ac49d88c..6133c0bf61a6 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt @@ -1495,8 +1495,9 @@ open class SiteStore @Inject constructor( ) ) } catch (e: Exception) { - AppLog.e(T.API, "Failed to fetch/store sites: ${e.message}", e) - OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, e.message)) + val errorMsg = e.message ?: e.javaClass.simpleName + AppLog.e(T.API, "Failed to fetch/store sites: $errorMsg", e) + OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, errorMsg)) } } } @@ -1576,12 +1577,13 @@ open class SiteStore @Inject constructor( } catch (e: DuplicateSiteException) { OnSiteChanged(SiteError(DUPLICATE_SITE)) } catch (e: Exception) { + val errorMsg = e.message ?: e.javaClass.simpleName AppLog.e( T.DB, - "Failed to update application password: ${e.message}", + "Failed to update application password: $errorMsg", e ) - OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, e.message)) + OnSiteChanged(SiteError(SiteErrorType.GENERIC_ERROR, errorMsg)) } } From 034bc143b5ae357117090b561ad73d423cc8006f Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 17:23:09 +0100 Subject: [PATCH 07/10] Restore Sentry reports in helper for storeCredentials false-return paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add crashLogging.sendReportWithTag calls in storeApplicationPasswordCredentialsFrom for the bad_data and site_not_found paths, which return false without throwing. These are not duplicates of the ViewModel's emitError reports — they cover failures where execution continues to fetchSites and the ViewModel never calls emitError. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../login/ApplicationPasswordLoginHelper.kt | 31 ++++++++++++++++++- .../ApplicationPasswordLoginHelperTest.kt | 9 ++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt index b97efd353030..92d8590d4449 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt @@ -2,6 +2,7 @@ package org.wordpress.android.ui.accounts.login import android.content.Context import androidx.core.net.toUri +import com.automattic.android.tracks.crashlogging.CrashLogging import org.wordpress.android.R import org.wordpress.android.util.DeviceUtils import kotlinx.coroutines.CoroutineDispatcher @@ -17,6 +18,7 @@ import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.util.AppLog import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.UrlUtils +import org.wordpress.android.util.crashlogging.sendReportWithTag import rs.wordpress.api.kotlin.ApiDiscoveryResult import rs.wordpress.api.kotlin.WpLoginClient import uniffi.wp_api.applicationPasswordsUrl @@ -36,7 +38,8 @@ class ApplicationPasswordLoginHelper @Inject constructor( private val wpLoginClient: WpLoginClient, private val appLogWrapper: AppLogWrapper, private val apiRootUrlCache: ApiRootUrlCache, - private val discoverSuccessWrapper: DiscoverSuccessWrapper + private val discoverSuccessWrapper: DiscoverSuccessWrapper, + private val crashLogging: CrashLogging ) { private var processedAppPasswordData: String? = null @@ -105,6 +108,22 @@ class ApplicationPasswordLoginHelper @Inject constructor( "${urlLogin.siteUrl == processedAppPasswordData}" ) trackStoringFailed(urlLogin.siteUrl, "bad_data") + crashLogging.sendReportWithTag( + Exception( + "A_P: bad_data for ${urlLogin.siteUrl}" + + " — apiRootUrl isNull=" + + "${urlLogin.apiRootUrl == null}" + + ", user isEmpty=" + + "${urlLogin.user.isNullOrEmpty()}" + + ", password isEmpty=" + + "${urlLogin.password.isNullOrEmpty()}" + + ", siteUrl isNull=" + + "${urlLogin.siteUrl == null}" + + ", alreadyProcessed=" + + "${urlLogin.siteUrl == processedAppPasswordData}" + ), + AppLog.T.DB + ) return false } @@ -135,6 +154,16 @@ class ApplicationPasswordLoginHelper @Inject constructor( " (${siteStore.sites.size} sites available)" ) trackStoringFailed(urlLogin.siteUrl, "site_not_found") + crashLogging.sendReportWithTag( + Exception( + "A_P: site_not_found for " + + "${urlLogin.siteUrl}" + + " (normalized: $normalizedUrl)" + + " — ${siteStore.sites.size} sites" + + " available" + ), + AppLog.T.DB + ) false } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt index 5a501c0ba910..7440e2c1896e 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt @@ -1,6 +1,7 @@ package org.wordpress.android.ui.accounts.login import android.content.Context +import com.automattic.android.tracks.crashlogging.CrashLogging import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Before @@ -67,6 +68,9 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -81,7 +85,8 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { wpLoginClient, appLogWrapper, apiRootUrlCache, - discoverSuccessWrapper + discoverSuccessWrapper, + crashLogging ) } @@ -154,7 +159,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { val result = applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(testUriLogin) assertFalse(result) - verify(siteStore, times(2)).sites + verify(siteStore, times(3)).sites verify(dispatcherWrapper, times(0)).updateApplicationPassword(any()) verify(dispatcherWrapper, times(0)).removeApplicationPassword(any()) } From c864f66847292a73b71e6d17dbc0dd7a375483ee Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 17:42:57 +0100 Subject: [PATCH 08/10] Add commented-out forced error for testing Sentry reporting path TODO: Remove before merging. Uncomment the throw line to verify that the storeCredentials error path sends analytics and Sentry reports. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../applicationpassword/ApplicationPasswordLoginViewModel.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index 1eea9ba4bd4b..d5919a3e9913 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -91,6 +91,9 @@ class ApplicationPasswordLoginViewModel @Inject constructor( @Suppress("TooGenericExceptionCaught") private suspend fun storeCredentials(urlLogin: UriLogin): Boolean = withContext(ioDispatcher) { try { + // TODO: Remove this line before merging — used to force + // the error path for testing Sentry/analytics reporting + // throw RuntimeException("Forced error for Sentry testing") applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(urlLogin) } catch (e: Exception) { appLogWrapper.e( From 1228b52e4135b7a916679b3ffa95abe0404021c0 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 17:56:32 +0100 Subject: [PATCH 09/10] Refactor helper to fix LongMethod detekt finding Extract logAndReportBadData and logAndReportSiteNotFound from storeApplicationPasswordCredentialsFrom to bring it under the 60-line detekt limit. Also extract reportStoringFailedToSentry to deduplicate Sentry exception construction. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../login/ApplicationPasswordLoginHelper.kt | 98 ++++++++++--------- .../ApplicationPasswordLoginHelperTest.kt | 2 +- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt index 92d8590d4449..53e37f8faa23 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt @@ -89,41 +89,16 @@ class ApplicationPasswordLoginHelper @Inject constructor( } @Suppress("ComplexCondition") - suspend fun storeApplicationPasswordCredentialsFrom(urlLogin: UriLogin): Boolean { + suspend fun storeApplicationPasswordCredentialsFrom( + urlLogin: UriLogin + ): Boolean { if (urlLogin.apiRootUrl == null || urlLogin.user.isNullOrEmpty() || urlLogin.password.isNullOrEmpty() || urlLogin.siteUrl == null || urlLogin.siteUrl == processedAppPasswordData - ) { - appLogWrapper.e( - AppLog.T.DB, - "A_P: Cannot save application password credentials" + - " for: ${urlLogin.siteUrl}" + - " - apiRootUrl isNull=${urlLogin.apiRootUrl == null}" + - ", user isEmpty=${urlLogin.user.isNullOrEmpty()}" + - ", password isEmpty=${urlLogin.password.isNullOrEmpty()}" + - ", siteUrl isNull=${urlLogin.siteUrl == null}" + - ", alreadyProcessed=" + - "${urlLogin.siteUrl == processedAppPasswordData}" - ) - trackStoringFailed(urlLogin.siteUrl, "bad_data") - crashLogging.sendReportWithTag( - Exception( - "A_P: bad_data for ${urlLogin.siteUrl}" + - " — apiRootUrl isNull=" + - "${urlLogin.apiRootUrl == null}" + - ", user isEmpty=" + - "${urlLogin.user.isNullOrEmpty()}" + - ", password isEmpty=" + - "${urlLogin.password.isNullOrEmpty()}" + - ", siteUrl isNull=" + - "${urlLogin.siteUrl == null}" + - ", alreadyProcessed=" + - "${urlLogin.siteUrl == processedAppPasswordData}" - ), - AppLog.T.DB - ) + ) { + logAndReportBadData(urlLogin) return false } @@ -145,24 +120,8 @@ class ApplicationPasswordLoginHelper @Inject constructor( processedAppPasswordData = urlLogin.siteUrl // Save locally to avoid duplicated calls true } else { - appLogWrapper.e( - AppLog.T.DB, - "A_P: Cannot save application password" + - " credentials for: ${urlLogin.siteUrl}" + - " (normalized: $normalizedUrl)" + - " - site not found in store" + - " (${siteStore.sites.size} sites available)" - ) - trackStoringFailed(urlLogin.siteUrl, "site_not_found") - crashLogging.sendReportWithTag( - Exception( - "A_P: site_not_found for " + - "${urlLogin.siteUrl}" + - " (normalized: $normalizedUrl)" + - " — ${siteStore.sites.size} sites" + - " available" - ), - AppLog.T.DB + logAndReportSiteNotFound( + urlLogin.siteUrl, normalizedUrl ) false } @@ -179,6 +138,49 @@ class ApplicationPasswordLoginHelper @Inject constructor( ) } + private fun reportStoringFailedToSentry( + reason: String, + detail: String + ) { + crashLogging.sendReportWithTag( + Exception("A_P: $reason — $detail"), + AppLog.T.DB + ) + } + + private fun logAndReportBadData(urlLogin: UriLogin) { + val detail = + "apiRootUrl isNull=${urlLogin.apiRootUrl == null}" + + ", user isEmpty=${urlLogin.user.isNullOrEmpty()}" + + ", password isEmpty=" + + "${urlLogin.password.isNullOrEmpty()}" + + ", siteUrl isNull=${urlLogin.siteUrl == null}" + + ", alreadyProcessed=" + + "${urlLogin.siteUrl == processedAppPasswordData}" + appLogWrapper.e( + AppLog.T.DB, + "A_P: Cannot save credentials" + + " for: ${urlLogin.siteUrl} - $detail" + ) + trackStoringFailed(urlLogin.siteUrl, "bad_data") + reportStoringFailedToSentry("bad_data", detail) + } + + private fun logAndReportSiteNotFound( + siteUrl: String?, + normalizedUrl: String? + ) { + val detail = "$siteUrl (normalized: $normalizedUrl)" + + " — ${siteStore.sites.size} sites available" + appLogWrapper.e( + AppLog.T.DB, + "A_P: Cannot save credentials" + + " - site not found: $detail" + ) + trackStoringFailed(siteUrl, "site_not_found") + reportStoringFailedToSentry("site_not_found", detail) + } + private fun trackSuccessful(siteUrl: String) { val properties: MutableMap = HashMap() properties[URL_TAG] = siteUrl diff --git a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt index 7440e2c1896e..dae94fde2290 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt @@ -159,7 +159,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { val result = applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(testUriLogin) assertFalse(result) - verify(siteStore, times(3)).sites + verify(siteStore, times(2)).sites verify(dispatcherWrapper, times(0)).updateApplicationPassword(any()) verify(dispatcherWrapper, times(0)).removeApplicationPassword(any()) } From f36dbbd2d98b6772e9a55849c26be8f8911bf6ac Mon Sep 17 00:00:00 2001 From: adalpari Date: Thu, 19 Mar 2026 09:02:09 +0100 Subject: [PATCH 10/10] Remove commented-out forced error used for testing Sentry reporting Co-Authored-By: Claude Opus 4.6 (1M context) --- .../applicationpassword/ApplicationPasswordLoginViewModel.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt index d5919a3e9913..1eea9ba4bd4b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt @@ -91,9 +91,6 @@ class ApplicationPasswordLoginViewModel @Inject constructor( @Suppress("TooGenericExceptionCaught") private suspend fun storeCredentials(urlLogin: UriLogin): Boolean = withContext(ioDispatcher) { try { - // TODO: Remove this line before merging — used to force - // the error path for testing Sentry/analytics reporting - // throw RuntimeException("Forced error for Sentry testing") applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(urlLogin) } catch (e: Exception) { appLogWrapper.e(