Fix app-password login aborting when apiRootUrl was recovered - #23132
Merged
Conversation
The recovered apiRootUrl was dropped between the helper and the ViewModel, causing valid logins to fail with empty_fetch_params and file it as a Sentry bug. Carry the effective login on SiteNotFound and stop reporting the user-recoverable empty_fetch_params condition to Sentry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Generated by 🚫 Danger |
The empty_fetch_params branch is only reachable via an empty-string siteUrl: storeApplicationPasswordCredentialsFrom rejects an empty apiRootUrl as BadData before it can return SiteNotFound, but only guards siteUrl against null, so an empty string slips through to fetchSites. Update the test fixture accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
adalpari
marked this pull request as ready for review
July 22, 2026 11:32
Contributor
|
|
Contributor
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes application-password logins that were silently aborting with
empty_fetch_params,which was also being reported to Sentry as if it were a crash.
Root cause (functional bug):
fetchSitesis only reached from theSiteNotFoundbranch, which means the credentials already passed the non-empty validation inside
storeApplicationPasswordCredentialsFrom. The empty param was theapiRootUrl: when itwas missing from the callback (cache miss — e.g. the process was killed between discovery
and the auth callback), the helper recovered it via fallback discovery into a local
effectiveUrlLogin. ButStoreCredentialsResult.SiteNotFoundcarried no data, so theViewModel fell back to the original
urlLoginwhoseapiRootUrlwas still empty →fetchSitessaw an empty param and aborted an otherwise-valid login.Sentry noise: the
empty_fetch_paramsfailure built a syntheticException("Application password login failed: empty_fetch_params")purely to file aSentry report. It's a user-recoverable data condition (still logged + tracked via
analytics), not a bug — consistent with how
site_store_errorand the validation casesare already handled in this file.
Changes
ApplicationPasswordLoginHelper:SiteNotFoundis nowdata class SiteNotFound(val urlLogin: UriLogin)and returns the effective login withthe recovered
apiRootUrl.ApplicationPasswordLoginViewModel: theSiteNotFoundbranch fetches with therecovered login; the
empty_fetch_paramserror now passesreportToSentry = false.SiteNotFoundstubs and added regression coverage for therecovered-
apiRootUrlpropagation (helper + ViewModel) and forempty_fetch_paramsnot hitting Sentry.
Testing instructions
application-password authentication that requires REST API discovery.
empty_fetch_paramserror appears🤖 Generated with Claude Code