From b116e8847f260f9e75aa337d83c67071511f6cab Mon Sep 17 00:00:00 2001 From: adalpari Date: Mon, 16 Mar 2026 16:03:43 +0100 Subject: [PATCH 01/18] Improve application password storing error logs for debugging Add detailed field-level diagnostics to all error paths in the application password login flow so developers can identify the root cause from logs alone. Co-Authored-By: Claude Opus 4.6 --- .../ApplicationPasswordLoginViewModel.kt | 37 +++++++++++++++---- .../login/ApplicationPasswordLoginHelper.kt | 18 ++++++++- ...licationPasswordAutoAuthDialogViewModel.kt | 20 ++++++++-- 3 files changed, 63 insertions(+), 12 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 b529272367c4..afaf730ef3cb 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 @@ -62,7 +62,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( fun setupSite(rawData: String) { viewModelScope.launch { if (rawData.isEmpty()) { - appLogWrapper.e(AppLog.T.MAIN, "Cannot store credentials: rawData is empty") + appLogWrapper.e(AppLog.T.MAIN, "A_P: Cannot store credentials: rawData is empty") _onFinishedEvent.emit( NavigationActionData( showSiteSelector = false, @@ -95,7 +95,10 @@ class ApplicationPasswordLoginViewModel @Inject constructor( try { applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(urlLogin) } catch (e: Exception) { - appLogWrapper.e(AppLog.T.DB, "Error storing credentials: ${e.stackTraceToString()}") + appLogWrapper.e( + AppLog.T.DB, + "A_P: Error storing credentials: ${e.stackTraceToString()}" + ) false } } @@ -109,9 +112,14 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) = withContext(ioDispatcher) { try { if (username.isEmpty() || password.isEmpty() || siteUrl.isEmpty() || apiRootUrl.isEmpty()) { - appLogWrapper.e(AppLog.T.MAIN, "Cannot fetch sites for credential storing: " + - "Username: $username, Password: ${password.isEmpty()}, SiteUrl: $siteUrl, " + - "API Root URL: $apiRootUrl") + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: Cannot fetch sites for credential storing" + + " - username isEmpty=${username.isEmpty()}" + + ", password isEmpty=${password.isEmpty()}" + + ", siteUrl isEmpty=${siteUrl.isEmpty()}" + + ", apiRootUrl isEmpty=${apiRootUrl.isEmpty()}" + ) emitErrorFetching(siteUrl) } else { val xmlRpcEndpoint = @@ -128,7 +136,10 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) } } catch (e: Exception) { - appLogWrapper.e(AppLog.T.API, "Error fetching sites: ${e.stackTraceToString()}") + appLogWrapper.e( + AppLog.T.API, + "A_P: Error fetching sites: ${e.stackTraceToString()}" + ) emitErrorFetching(siteUrl) } } @@ -150,7 +161,19 @@ class ApplicationPasswordLoginViewModel @Inject constructor( 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)) { - appLogWrapper.e(AppLog.T.MAIN, "Site not found or credentials are empty.") + appLogWrapper.e( + AppLog.T.MAIN, + "A_P: onSiteChanged failed" + + " for: ${currentUrlLogin?.siteUrl}" + + " - rowsAffected=${event.rowsAffected}" + + ", siteFound=${site != null}" + + ", badCredentials=${ + site?.let { + applicationPasswordLoginHelper + .siteHasBadCredentials(it) + } + }" + ) _onFinishedEvent.emit( NavigationActionData( showSiteSelector = false, 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 1b9f4d39680d..5ae1edadc16d 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 @@ -91,7 +91,14 @@ class ApplicationPasswordLoginHelper @Inject constructor( ) { appLogWrapper.e( AppLog.T.DB, - "A_P: Cannot save application password credentials for: ${urlLogin.siteUrl} - bad data" + "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}" ) return false } @@ -114,9 +121,16 @@ class ApplicationPasswordLoginHelper @Inject constructor( processedAppPasswordData = urlLogin.siteUrl // Save locally to avoid duplicated calls true } else { + val availableSiteUrls = siteStore.sites.map { + UrlUtils.normalizeUrl(it.url) + } appLogWrapper.e( AppLog.T.DB, - "A_P: Cannot save application password credentials for: ${urlLogin.siteUrl} - null site" + "A_P: Cannot save application password" + + " credentials for: ${urlLogin.siteUrl}" + + " (normalized: $normalizedUrl)" + + " - site not found in store." + + " Available sites: $availableSiteUrls" ) false } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt index 39689966d8f9..3a52ba063b5d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt @@ -89,12 +89,22 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( } else -> { - appLogWrapper.e(AppLog.T.API, "Error creating application password") + appLogWrapper.e( + AppLog.T.API, + "A_P: Error creating application password" + + " for: ${site.url}" + + " - response: $response" + ) fallbackToManualLogin(site.url) } } } catch (e: Exception) { - appLogWrapper.e(AppLog.T.API, "Exception creating application password: ${e.message}") + appLogWrapper.e( + AppLog.T.API, + "A_P: Exception creating application password" + + " for: ${site.url}" + + " - ${e.message}" + ) fallbackToManualLogin(site.url) } finally { _isLoading.value = false @@ -115,7 +125,11 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( val authUrl = applicationPasswordLoginHelper.getAuthorizationUrlComplete(siteUrl) _navigationEvent.emit(NavigationEvent.FallbackToManualLogin(authUrl)) } catch (e: Exception) { - appLogWrapper.e(AppLog.T.API, "Failed to get authorization URL: ${e.message}") + appLogWrapper.e( + AppLog.T.API, + "A_P: Failed to get authorization URL" + + " for: $siteUrl - ${e.message}" + ) _navigationEvent.emit(NavigationEvent.Error) } } From 79cabe67454c7f9d483cfae5dd24b39931027447 Mon Sep 17 00:00:00 2001 From: adalpari Date: Mon, 16 Mar 2026 16:12:18 +0100 Subject: [PATCH 02/18] Avoid logging full response object to prevent potential data leak Log only the response class name instead of the full object, since WpRequestResult variants could contain sensitive API response data. Co-Authored-By: Claude Opus 4.6 --- .../ApplicationPasswordAutoAuthDialogViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt index 3a52ba063b5d..582fbe032c9a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt @@ -93,7 +93,7 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( AppLog.T.API, "A_P: Error creating application password" + " for: ${site.url}" + - " - response: $response" + " - response type: ${response::class.simpleName}" ) fallbackToManualLogin(site.url) } From 0fdd1a2b04fbcd69dc454975372076397281237f Mon Sep 17 00:00:00 2001 From: adalpari Date: Mon, 16 Mar 2026 17:15:25 +0100 Subject: [PATCH 03/18] Extract log helper to fix detekt LongMethod violation The createApplicationPassword function exceeded the 60-line limit after adding detailed error logging. Extract a logCreationError helper to keep the method concise. Co-Authored-By: Claude Opus 4.6 --- ...licationPasswordAutoAuthDialogViewModel.kt | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt index 582fbe032c9a..36afdc5fe87e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/ApplicationPasswordAutoAuthDialogViewModel.kt @@ -89,22 +89,12 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( } else -> { - appLogWrapper.e( - AppLog.T.API, - "A_P: Error creating application password" + - " for: ${site.url}" + - " - response type: ${response::class.simpleName}" - ) + logCreationError(site.url, "response type: ${response::class.simpleName}") fallbackToManualLogin(site.url) } } } catch (e: Exception) { - appLogWrapper.e( - AppLog.T.API, - "A_P: Exception creating application password" + - " for: ${site.url}" + - " - ${e.message}" - ) + logCreationError(site.url, e.message.orEmpty()) fallbackToManualLogin(site.url) } finally { _isLoading.value = false @@ -112,6 +102,13 @@ class ApplicationPasswordAutoAuthDialogViewModel @Inject constructor( } } + private fun logCreationError(siteUrl: String, detail: String) { + appLogWrapper.e( + AppLog.T.API, + "A_P: Error creating application password for: $siteUrl - $detail" + ) + } + @Suppress("TooGenericExceptionCaught") private fun enableApplicationPasswordIfNecessary() { if (!experimentalFeatures.isEnabled(Feature.EXPERIMENTAL_APPLICATION_PASSWORD_FEATURE)) { From bd3937e174c72ad19843d959c5f7bc53a34a2c55 Mon Sep 17 00:00:00 2001 From: adalpari Date: Tue, 17 Mar 2026 09:25:43 +0100 Subject: [PATCH 04/18] Fix test expecting single siteStore.sites call The new logging in the "site not found" branch reads siteStore.sites a second time to log available URLs. Update the test verification to expect two calls. Co-Authored-By: Claude Opus 4.6 --- .../ui/accounts/login/ApplicationPasswordLoginHelperTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9c4a55e708a8..84a085d8440a 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 @@ -150,7 +150,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { val result = applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(testUriLogin) assertFalse(result) - verify(siteStore).sites + verify(siteStore, times(2)).sites verify(dispatcherWrapper, times(0)).updateApplicationPassword(any()) verify(dispatcherWrapper, times(0)).removeApplicationPassword(any()) } From 83c2831879695d5606f88497353699a846a0872f Mon Sep 17 00:00:00 2001 From: adalpari Date: Tue, 17 Mar 2026 12:10:14 +0100 Subject: [PATCH 05/18] Add Tracks and Sentry events for application password storing failures Fire APPLICATION_PASSWORD_STORING_FAILED Tracks event and a Sentry report whenever the credential storing flow fails, so we can monitor failure rates and reasons in production. Also replace available site URL logging with just a count to avoid PII exposure. Co-Authored-By: Claude Opus 4.6 --- .../ApplicationPasswordLoginViewModel.kt | 11 +++++++ .../login/ApplicationPasswordLoginHelper.kt | 31 +++++++++++++++---- .../ApplicationPasswordLoginHelperTest.kt | 7 ++++- .../android/analytics/AnalyticsTracker.java | 3 +- 4 files changed, 44 insertions(+), 8 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 afaf730ef3cb..ababe0104ac0 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 @@ -63,6 +63,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( viewModelScope.launch { if (rawData.isEmpty()) { appLogWrapper.e(AppLog.T.MAIN, "A_P: Cannot store credentials: rawData is empty") + applicationPasswordLoginHelper.trackStoringFailed("", "empty_raw_data") _onFinishedEvent.emit( NavigationActionData( showSiteSelector = false, @@ -120,6 +121,9 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ", siteUrl isEmpty=${siteUrl.isEmpty()}" + ", apiRootUrl isEmpty=${apiRootUrl.isEmpty()}" ) + applicationPasswordLoginHelper.trackStoringFailed( + siteUrl, "empty_fetch_params" + ) emitErrorFetching(siteUrl) } else { val xmlRpcEndpoint = @@ -140,6 +144,9 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.API, "A_P: Error fetching sites: ${e.stackTraceToString()}" ) + applicationPasswordLoginHelper.trackStoringFailed( + siteUrl, "fetch_sites_exception" + ) emitErrorFetching(siteUrl) } } @@ -174,6 +181,10 @@ class ApplicationPasswordLoginViewModel @Inject constructor( } }" ) + applicationPasswordLoginHelper.trackStoringFailed( + currentUrlLogin?.siteUrl, + "site_changed_failed" + ) _onFinishedEvent.emit( NavigationActionData( showSiteSelector = false, 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 5ae1edadc16d..982736eecf69 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 @@ -1,6 +1,7 @@ package org.wordpress.android.ui.accounts.login import androidx.core.net.toUri +import com.automattic.android.tracks.crashlogging.CrashLogging import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext import org.wordpress.android.analytics.AnalyticsTracker @@ -14,6 +15,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 @@ -22,6 +24,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, @@ -32,7 +35,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 @@ -100,6 +104,7 @@ class ApplicationPasswordLoginHelper @Inject constructor( ", alreadyProcessed=" + "${urlLogin.siteUrl == processedAppPasswordData}" ) + trackStoringFailed(urlLogin.siteUrl, "bad_data") return false } @@ -121,22 +126,36 @@ class ApplicationPasswordLoginHelper @Inject constructor( processedAppPasswordData = urlLogin.siteUrl // Save locally to avoid duplicated calls true } else { - val availableSiteUrls = siteStore.sites.map { - UrlUtils.normalizeUrl(it.url) - } appLogWrapper.e( AppLog.T.DB, "A_P: Cannot save application password" + " credentials for: ${urlLogin.siteUrl}" + " (normalized: $normalizedUrl)" + - " - site not found in store." + - " Available sites: $availableSiteUrls" + " - 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 84a085d8440a..5f0a9a704f95 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 @@ -4,6 +4,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 @@ -63,6 +64,9 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -77,7 +81,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 6ecaef5b2a47..a9a16a2fbf16 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 @@ -1125,7 +1125,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 b6934d17714de15745eb9fdf1bbb0295f793dbb4 Mon Sep 17 00:00:00 2001 From: adalpari Date: Tue, 17 Mar 2026 12:10:57 +0100 Subject: [PATCH 06/18] Revert "Add Tracks and Sentry events for application password storing failures" This reverts commit 90319f7217d11739429598921ab02b8d5e265181. --- .../ApplicationPasswordLoginViewModel.kt | 11 ------- .../login/ApplicationPasswordLoginHelper.kt | 31 ++++--------------- .../ApplicationPasswordLoginHelperTest.kt | 7 +---- .../android/analytics/AnalyticsTracker.java | 3 +- 4 files changed, 8 insertions(+), 44 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 ababe0104ac0..afaf730ef3cb 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 @@ -63,7 +63,6 @@ class ApplicationPasswordLoginViewModel @Inject constructor( viewModelScope.launch { if (rawData.isEmpty()) { appLogWrapper.e(AppLog.T.MAIN, "A_P: Cannot store credentials: rawData is empty") - applicationPasswordLoginHelper.trackStoringFailed("", "empty_raw_data") _onFinishedEvent.emit( NavigationActionData( showSiteSelector = false, @@ -121,9 +120,6 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ", siteUrl isEmpty=${siteUrl.isEmpty()}" + ", apiRootUrl isEmpty=${apiRootUrl.isEmpty()}" ) - applicationPasswordLoginHelper.trackStoringFailed( - siteUrl, "empty_fetch_params" - ) emitErrorFetching(siteUrl) } else { val xmlRpcEndpoint = @@ -144,9 +140,6 @@ class ApplicationPasswordLoginViewModel @Inject constructor( AppLog.T.API, "A_P: Error fetching sites: ${e.stackTraceToString()}" ) - applicationPasswordLoginHelper.trackStoringFailed( - siteUrl, "fetch_sites_exception" - ) emitErrorFetching(siteUrl) } } @@ -181,10 +174,6 @@ class ApplicationPasswordLoginViewModel @Inject constructor( } }" ) - applicationPasswordLoginHelper.trackStoringFailed( - currentUrlLogin?.siteUrl, - "site_changed_failed" - ) _onFinishedEvent.emit( NavigationActionData( showSiteSelector = false, 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 982736eecf69..5ae1edadc16d 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 @@ -1,7 +1,6 @@ package org.wordpress.android.ui.accounts.login import androidx.core.net.toUri -import com.automattic.android.tracks.crashlogging.CrashLogging import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext import org.wordpress.android.analytics.AnalyticsTracker @@ -15,7 +14,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 @@ -24,7 +22,6 @@ 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,8 +32,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 @@ -104,7 +100,6 @@ class ApplicationPasswordLoginHelper @Inject constructor( ", alreadyProcessed=" + "${urlLogin.siteUrl == processedAppPasswordData}" ) - trackStoringFailed(urlLogin.siteUrl, "bad_data") return false } @@ -126,36 +121,22 @@ class ApplicationPasswordLoginHelper @Inject constructor( processedAppPasswordData = urlLogin.siteUrl // Save locally to avoid duplicated calls true } else { + val availableSiteUrls = siteStore.sites.map { + UrlUtils.normalizeUrl(it.url) + } 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)" + " - site not found in store." + + " Available sites: $availableSiteUrls" ) - 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 5f0a9a704f95..84a085d8440a 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 @@ -4,7 +4,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 @@ -64,9 +63,6 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache - @Mock - lateinit var crashLogging: CrashLogging - private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -81,8 +77,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { wpLoginClient, appLogWrapper, apiRootUrlCache, - discoverSuccessWrapper, - crashLogging + discoverSuccessWrapper ) } 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 a9a16a2fbf16..6ecaef5b2a47 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 @@ -1125,8 +1125,7 @@ public enum Stat { BACKGROUND_REST_AUTODISCOVERY_FAILED, WP_ANDROID_APPLICATION_PASSWORD_LOGIN, JP_ANDROID_APPLICATION_PASSWORD_LOGIN, - APPLICATION_PASSWORD_SET_OFF, - APPLICATION_PASSWORD_STORING_FAILED; + APPLICATION_PASSWORD_SET_OFF; /* * 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 2b003be94a8f54a10bb2106920608c15c0d43ede Mon Sep 17 00:00:00 2001 From: adalpari Date: Tue, 17 Mar 2026 12:12:12 +0100 Subject: [PATCH 07/18] Avoid logging user site URLs to prevent PII exposure Replace the full list of available site URLs with just a count in the "site not found" error log, since site URLs may reveal personal domains or business names. Co-Authored-By: Claude Opus 4.6 --- .../ui/accounts/login/ApplicationPasswordLoginHelper.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 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 5ae1edadc16d..b728c1fe8f6f 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 @@ -121,16 +121,13 @@ class ApplicationPasswordLoginHelper @Inject constructor( processedAppPasswordData = urlLogin.siteUrl // Save locally to avoid duplicated calls true } else { - val availableSiteUrls = siteStore.sites.map { - UrlUtils.normalizeUrl(it.url) - } appLogWrapper.e( AppLog.T.DB, "A_P: Cannot save application password" + " credentials for: ${urlLogin.siteUrl}" + " (normalized: $normalizedUrl)" + - " - site not found in store." + - " Available sites: $availableSiteUrls" + " - site not found in store" + + " (${siteStore.sites.size} sites available)" ) false } From 9d44e1985650289a80c6b52f4a5f420228718ec0 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 08/18] 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 | 122 ++++++++++++------ .../android/fluxc/store/SiteStore.kt | 29 +++-- 3 files changed, 107 insertions(+), 56 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 63bd1383f272..d217180570bc 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 @@ -64,13 +64,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) } if (navigationActionData.isError) { 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 afaf730ef3cb..fa6d597c758b 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 @@ -62,16 +62,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, - showPostSignupInterstitial = 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) @@ -120,7 +115,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) @@ -140,60 +141,96 @@ 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, - showPostSignupInterstitial = false, - siteUrl = siteUrl, - oldSitesIDs = oldSitesIDs, - isError = true + private suspend fun emitError(siteUrl: String, errorMessage: String? = null) = + _onFinishedEvent.emit( + NavigationActionData( + showSiteSelector = false, + showPostSignupInterstitial = 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, - showPostSignupInterstitial = 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, showPostSignupInterstitial = !siteStore.hasSite() && appPrefsWrapper.shouldShowPostSignupInterstitial, siteUrl = currentUrlLogin?.siteUrl, oldSitesIDs = oldSitesIDs, isError = false, - newSiteLocalId = site.id + newSiteLocalId = nonNullSite.id ) ) } @@ -206,6 +243,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 52461380e2fdd741d0b2a8834d70d13b3fdd3f01 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 09/18] 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 fa6d597c758b..b92a242526cf 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 @@ -22,6 +22,8 @@ import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.Ur import org.wordpress.android.ui.prefs.AppPrefsWrapper 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 @@ -34,6 +36,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( private val siteStore: SiteStore, private val appPrefsWrapper: AppPrefsWrapper, private val appLogWrapper: AppLogWrapper, + private val crashLogging: CrashLogging, ) : ViewModel() { private val _onFinishedEvent = MutableSharedFlow() /** @@ -141,11 +144,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, @@ -156,6 +166,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( errorMessage = errorMessage ) ) + } @Suppress("TooGenericExceptionCaught") @SuppressWarnings("unused") @@ -192,7 +203,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 1b647c4e0b733b03c0dbefb8a4207a441a94635a Mon Sep 17 00:00:00 2001 From: adalpari Date: Tue, 17 Mar 2026 19:41:25 +0100 Subject: [PATCH 10/18] 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 | 26 ++++++++++++++++++- .../ApplicationPasswordLoginHelperTest.kt | 7 ++++- .../android/analytics/AnalyticsTracker.java | 3 ++- 4 files changed, 52 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 b92a242526cf..2633e1499d24 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,6 +69,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 } @@ -118,6 +119,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 — " + @@ -144,6 +148,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) } } @@ -182,6 +189,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: " + @@ -201,6 +212,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}", @@ -227,6 +242,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 b728c1fe8f6f..7a0968a66723 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 @@ -1,6 +1,9 @@ package org.wordpress.android.ui.accounts.login 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 import kotlinx.coroutines.withContext import org.wordpress.android.analytics.AnalyticsTracker @@ -14,6 +17,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 @@ -22,6 +26,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, @@ -32,7 +37,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 @@ -100,6 +106,7 @@ class ApplicationPasswordLoginHelper @Inject constructor( ", alreadyProcessed=" + "${urlLogin.siteUrl == processedAppPasswordData}" ) + trackStoringFailed(urlLogin.siteUrl, "bad_data") return false } @@ -129,11 +136,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 84a085d8440a..5f0a9a704f95 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 @@ -4,6 +4,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 @@ -63,6 +64,9 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -77,7 +81,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 6ecaef5b2a47..a9a16a2fbf16 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 @@ -1125,7 +1125,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 dd53d6132bd1ce1781266fc56ca8cef20867e2f7 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 15:55:26 +0100 Subject: [PATCH 11/18] 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 | 206 ++++++++++++++---- .../android/fluxc/store/SiteStore.kt | 4 + 4 files changed, 175 insertions(+), 55 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 d217180570bc..63bd1383f272 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 @@ -64,13 +64,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) } if (navigationActionData.isError) { 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 2633e1499d24..c77c3a25c6b3 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 @@ -22,8 +22,8 @@ import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.Ur import org.wordpress.android.ui.prefs.AppPrefsWrapper 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 @@ -251,17 +251,17 @@ 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, showPostSignupInterstitial = !siteStore.hasSite() && appPrefsWrapper.shouldShowPostSignupInterstitial, 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 2636207cd207..86f91633ea64 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,26 +1,28 @@ 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 org.wordpress.android.ui.prefs.AppPrefsWrapper import kotlin.test.assertEquals +import kotlin.test.assertTrue @ExperimentalCoroutinesApi @Suppress("MaxLineLength") @@ -43,6 +45,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() { selfHostedEndpointFinder, siteStore, appPrefsWrapper, - appLogWrapper + appLogWrapper, + crashLogging ) whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData)).thenReturn(urlLogin) } @@ -77,7 +83,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showPostSignupInterstitial = false, siteUrl = "", oldSitesIDs = null, - isError = true + isError = true, + errorMessage = "Callback data was empty" ) // When @@ -104,7 +111,11 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showPostSignupInterstitial = 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( @@ -132,7 +143,8 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { showPostSignupInterstitial = false, siteUrl = urlLogin.siteUrl, oldSitesIDs = null, - isError = true + isError = true, + errorMessage = null ) whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))).thenReturn(false) whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(any())).thenThrow(RuntimeException()) @@ -154,31 +166,32 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { runTest { // Given val xmlRpcEndpoint = "https://example.com/xmlrpc.php" - val expectedResult = ApplicationPasswordLoginViewModel.NavigationActionData( - showSiteSelector = false, - showPostSignupInterstitial = 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() } @@ -275,9 +288,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 { @@ -293,34 +311,74 @@ 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 intent rawData, when setup site and not able to store credentials but store fetch, then emit ok with no interstitial by preferences`() = + fun `given onSiteChanged with error, then emit error with SiteStore details`() = runTest { // Given - val xmlRpcEndpoint = "https://example.com/xmlrpc.php" - val expectedResult = ApplicationPasswordLoginViewModel.NavigationActionData( - showSiteSelector = false, - showPostSignupInterstitial = false, - siteUrl = urlLogin.siteUrl, - oldSitesIDs = null, - isError = false, - newSiteLocalId = testSite.id + 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(appPrefsWrapper.shouldShowPostSignupInterstitial).thenReturn(false) - whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin))).thenReturn(false) - whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)) - .thenReturn(xmlRpcEndpoint) + whenever( + applicationPasswordLoginHelper.siteHasBadCredentials(any()) + ).thenReturn(true) // When viewModel.onFinishedEvent.test { viewModel.setupSite(rawData) - // Mock onSiteChanged event viewModel.onSiteChanged( SiteStore.OnSiteChanged( rowsAffected = 1, @@ -329,10 +387,68 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { ) // Then - val finishedEvent = awaitItem() - assertEquals(expectedResult, finishedEvent) - verify(selfHostedEndpointFinder, times(1)).verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl) + 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 fc25020a66848656a074e84a9a54cac1a4105ed6 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 16:14:00 +0100 Subject: [PATCH 12/18] 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 | 171 ++++++++++-------- .../android/fluxc/store/SiteStore.kt | 4 - 2 files changed, 92 insertions(+), 83 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 c77c3a25c6b3..14596047c09a 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 @@ -175,99 +176,111 @@ 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, - showPostSignupInterstitial = !siteStore.hasSite() - && appPrefsWrapper.shouldShowPostSignupInterstitial, - 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, + showPostSignupInterstitial = !siteStore.hasSite() + && appPrefsWrapper.shouldShowPostSignupInterstitial, + 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 showPostSignupInterstitial: Boolean, 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 e9c6c9de0b11fd04dbef679367b1d9a96b202820 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 16:49:34 +0100 Subject: [PATCH 13/18] 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 14596047c09a..5c724c3b9a5b 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 @@ -71,7 +71,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) @@ -99,6 +99,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 } } @@ -125,10 +129,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 = @@ -201,8 +202,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor( ) emitError( siteUrl = currentUrlLogin?.siteUrl.orEmpty(), - errorMessage = "SiteStore error: " + - "${error?.type} — ${error?.message}" + errorMessage = "site_store_error" ) } @@ -217,19 +217,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( @@ -249,21 +250,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( @@ -276,7 +289,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 7a0968a66723..270a0d501421 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 @@ -1,7 +1,6 @@ package org.wordpress.android.ui.accounts.login 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,7 +16,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 @@ -37,8 +35,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 @@ -150,12 +147,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 86f91633ea64..a4a249e5ccee 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 @@ -84,7 +84,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() { siteUrl = "", oldSitesIDs = null, isError = true, - errorMessage = "Callback data was empty" + errorMessage = "empty_raw_data" ) // When @@ -112,10 +112,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( @@ -186,9 +183,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) @@ -335,10 +331,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() } } @@ -359,9 +352,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() } } @@ -389,10 +380,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() } } @@ -415,10 +403,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 5f0a9a704f95..84a085d8440a 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 @@ -4,7 +4,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 @@ -64,9 +63,6 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache - @Mock - lateinit var crashLogging: CrashLogging - private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -81,8 +77,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 fbdfd15d97ade7c94da0f72e36e5d0d93609934a Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 17:23:09 +0100 Subject: [PATCH 14/18] 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 | 10 ++++-- 2 files changed, 38 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 270a0d501421..5936a3a44273 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 @@ -1,6 +1,7 @@ package org.wordpress.android.ui.accounts.login 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 @@ -16,6 +17,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 @@ -35,7 +37,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 @@ -104,6 +107,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 } @@ -134,6 +153,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 84a085d8440a..c7075213785b 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,5 +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 @@ -63,6 +65,9 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { @Mock lateinit var apiRootUrlCache: ApiRootUrlCache + @Mock + lateinit var crashLogging: CrashLogging + private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper @Before @@ -77,7 +82,8 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() { wpLoginClient, appLogWrapper, apiRootUrlCache, - discoverSuccessWrapper + discoverSuccessWrapper, + crashLogging ) } @@ -150,7 +156,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 97659f7320ee7001ad59ac8aec51e14137f99190 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 17:42:57 +0100 Subject: [PATCH 15/18] 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 5c724c3b9a5b..1eb154bc691d 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 @@ -93,6 +93,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 b13ba51affe58e87bfdd786a77cf08eb5bb3da01 Mon Sep 17 00:00:00 2001 From: adalpari Date: Wed, 18 Mar 2026 17:56:32 +0100 Subject: [PATCH 16/18] 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 5936a3a44273..62dd1afa3419 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 @@ -88,41 +88,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 } @@ -144,24 +119,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 } @@ -178,6 +137,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 c7075213785b..10eb8caea016 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 @@ -156,7 +156,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 7e6d75384c80ab4b1805feb52ca60b92edcb7277 Mon Sep 17 00:00:00 2001 From: adalpari Date: Thu, 19 Mar 2026 09:02:09 +0100 Subject: [PATCH 17/18] 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 1eb154bc691d..5c724c3b9a5b 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 @@ -93,9 +93,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( From a046bd1f2c35ccba1353fcfb406cf57300e089be Mon Sep 17 00:00:00 2001 From: adalpari Date: Thu, 19 Mar 2026 10:51:08 +0100 Subject: [PATCH 18/18] Fix unused imports and missing null annotation from code checks Co-Authored-By: Claude Opus 4.6 --- .../android/ui/accounts/login/ApplicationPasswordLoginHelper.kt | 2 -- .../ui/accounts/login/ApplicationPasswordLoginHelperTest.kt | 1 - .../java/org/wordpress/android/analytics/AnalyticsTracker.java | 2 +- 3 files changed, 1 insertion(+), 4 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 62dd1afa3419..d48b57889a49 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,8 +2,6 @@ package org.wordpress.android.ui.accounts.login 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 import kotlinx.coroutines.withContext import org.wordpress.android.analytics.AnalyticsTracker 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 10eb8caea016..b877ea62cfdc 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,5 @@ 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 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 a9a16a2fbf16..6b050b6c96d1 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 @@ -1134,7 +1134,7 @@ public enum Stat { private String mEventName; - Stat(String eventName) { + Stat(@Nullable String eventName) { this.mEventName = eventName; }