Skip to content

Reduce Sentry noise in application password login flow#22846

Merged
nbradbury merged 4 commits into
trunkfrom
fix/sentry-application-password-login-issues
May 13, 2026
Merged

Reduce Sentry noise in application password login flow#22846
nbradbury merged 4 commits into
trunkfrom
fix/sentry-application-password-login-issues

Conversation

@nbradbury
Copy link
Copy Markdown
Contributor

Summary

Fixes four noisy non-fatal Sentry reports from the application password login flow:

All four are user-recoverable failures (network, auth, transient backend errors, cosmetic URL mismatches between callback and stored SiteModel) that were being routed through crashLogging.sendReportWithTag, drowning out genuine bugs in dashboards.

What changed

  • emitError / logAndEmitSiteChangedError gain a reportToSentry flag.
  • fetchSites outer catch classifies the cause: IOException and DiscoveryException (anywhere in the cause chain) skip Sentry.
  • handleSiteChangedError always skips Sentry; the SiteStore error type is preserved in the existing analytics call (site_changed_failed:<TYPE>) and AppLog.
  • Validation outcomes in handleSiteChangedSuccess (site_not_found, no_rows_affected, empty_credentials) skip Sentry. The genuine db_read_exception branch still reports.
  • handleSiteChangedSuccess now reuses ApplicationPasswordLoginHelper.findSiteByUrl (made internal) so cosmetic URL differences (trailing slash, www. prefix, scheme) no longer trigger site_not_found.

Test plan

  • Unit tests pass (:WordPress:testJetpackDebugUnitTest --tests ApplicationPasswordLoginViewModelTest)
  • Added: given onSiteChanged with error, then no Sentry report is sent
  • Added: given site_not_found validation, then no Sentry report is sent
  • Added: given fetchSites with IOException cause, then no Sentry report is sent
  • Added: given onSiteChanged success and site matched via tolerant URL match, then emit success
  • Sanity-check Sentry dashboards 1 week after the next release ships to confirm event-rate drop on the four issues.
  • Walk through application password login on a self-hosted site (happy path) to confirm no behavior regression.

🤖 Generated with Claude Code

Fixes Sentry WORDPRESS-ANDROID-3F8C, WORDPRESS-ANDROID-3F8H,
WORDPRESS-ANDROID-3F8J, and WORDPRESS-ANDROID-3F8P. All four issues
are non-fatal reports from the application password login flow that
fire on user-recoverable failures (network, auth, transient SiteStore
errors, cosmetic URL mismatches), drowning out genuine bugs.

- Add reportToSentry flag to emitError / logAndEmitSiteChangedError.
- Skip Sentry for IOException / DiscoveryException causes in fetchSites.
- Skip Sentry for all OnSiteChanged error events; preserve the SiteStore
  error type in analytics + AppLog instead.
- Skip Sentry for site_not_found / no_rows_affected / empty_credentials
  validation outcomes; keep it for the genuine db_read_exception path.
- Reuse ApplicationPasswordLoginHelper.findSiteByUrl in
  handleSiteChangedSuccess so site_not_found does not fire on cosmetic
  URL differences (trailing slash, www prefix, scheme).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nbradbury nbradbury added the Login label May 8, 2026
@dangermattic
Copy link
Copy Markdown
Collaborator

dangermattic commented May 8, 2026

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented May 8, 2026

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr22846-155f19e
Build Number1488
Application IDorg.wordpress.android.prealpha
Commit155f19e
Installation URL4rqkrv83c9j9g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented May 8, 2026

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr22846-155f19e
Build Number1488
Application IDcom.jetpack.android.prealpha
Commit155f19e
Installation URL7rfhhe2ia5iv8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

Copy link
Copy Markdown
Contributor

@adalpari adalpari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two linked reports, that are mentioning the error A_P: site_not_found, but if you look at the code, that error doesn't exist anymore (the noisy one I changed), so I'm not sure those ones are aplicable.
The other ones look legit.

@nbradbury
Copy link
Copy Markdown
Contributor Author

There are two linked reports, that are mentioning the error A_P: site_not_found, but if you look at the code, that error doesn't exist anymore (the noisy one I changed), so I'm not sure those ones are aplicable.

@adalpari I was confused by this so I asked Claude about it and here is the response:

I think there's been a mix-up between two different report sites — the A_P: site_not_found format you removed in the earlier cleanup is indeed gone, but 3F8J and 3F8P originate from a different exception:

// ApplicationPasswordLoginViewModel.emitError:166
?: Exception("Application password login failed: $errorMessage")

The Sentry titles confirm this — they're Application password login failed: site_not_found and …: site_store_error (no A_P: prefix). When errorMessage happens to be the string site_not_found, the title can look like the old report at a glance.

Both issues are still active:

  • 3F8J — first seen 2026-03-22 (after your cleanup on 2026-03-18), last seen today, 1,705 events / 836 users. I confirmed events firing in release 26.7+1488.
  • 3F8P — first seen 2026-03-23, last seen today, 667 events / 325 users.

So these aren't already-resolved leftovers — they're the same emitError path firing on validation/SiteStore outcomes that this PR is silencing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 37.21%. Comparing base (58080b6) to head (155f19e).
⚠️ Report is 1 commits behind head on trunk.

Files with missing lines Patch % Lines
...ationpassword/ApplicationPasswordLoginViewModel.kt 96.15% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #22846   +/-   ##
=======================================
  Coverage   37.21%   37.21%           
=======================================
  Files        2317     2317           
  Lines      124536   124549   +13     
  Branches    16912    16915    +3     
=======================================
+ Hits        46340    46352   +12     
  Misses      74446    74446           
- Partials     3750     3751    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adalpari
Copy link
Copy Markdown
Contributor

There are two linked reports, that are mentioning the error A_P: site_not_found, but if you look at the code, that error doesn't exist anymore (the noisy one I changed), so I'm not sure those ones are aplicable.

@adalpari I was confused by this so I asked Claude about it and here is the response:

I think there's been a mix-up between two different report sites — the A_P: site_not_found format you removed in the earlier cleanup is indeed gone, but 3F8J and 3F8P originate from a different exception:

// ApplicationPasswordLoginViewModel.emitError:166 ?: Exception("Application password login failed: $errorMessage")

The Sentry titles confirm this — they're Application password login failed: site_not_found and …: site_store_error (no A_P: prefix). When errorMessage happens to be the string site_not_found, the title can look like the old report at a glance.

Both issues are still active:

  • 3F8J — first seen 2026-03-22 (after your cleanup on 2026-03-18), last seen today, 1,705 events / 836 users. I confirmed events firing in release 26.7+1488.
  • 3F8P — first seen 2026-03-23, last seen today, 667 events / 325 users.

So these aren't already-resolved leftovers — they're the same emitError path firing on validation/SiteStore outcomes that this PR is silencing.

Sounds reasonable then.

@nbradbury nbradbury marked this pull request as ready for review May 12, 2026 13:07
@nbradbury nbradbury enabled auto-merge (squash) May 13, 2026 11:12
Copy link
Copy Markdown
Contributor

@adalpari adalpari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@nbradbury nbradbury merged commit 7b62187 into trunk May 13, 2026
21 of 22 checks passed
@nbradbury nbradbury deleted the fix/sentry-application-password-login-issues branch May 13, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants