Skip to content

fix(auth,settings): keep account data usable on subscription errors - #21358

Open
fxa-agent[bot] wants to merge 1 commit into
mainfrom
fxa-14647
Open

fxa-agent[bot] wants to merge 1 commit into
mainfrom
fxa-14647

Conversation

@fxa-agent

@fxa-agent fxa-agent Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Because

  • GET /v1/account returned a 5xx when the Stripe, Google Play or App Store read failed, so one subscriptions error broke the whole account response.
  • Settings ignored a rejected account fetch and showed default values, such as 2FA off and no recovery key. This looked like lost account data.
  • The Security section showed a password "Created" date of 1969 or 1970 when passwordCreated was 0 or missing.

This pull request

  • Changes the subscriptions catch in the GET /v1/account handler (account.ts). It now returns 200 with subscriptions: [], logs Account.get.subscriptions.error with log.error, and sends the error to Sentry with reportSentryError. The response schema does not change.
  • Resets all three lists (Stripe, Google Play, App Store) on any failure. So a Play or App Store failure after a Stripe success also returns empty subscriptions. This is by design.
  • Keeps UNKNOWN_SUBSCRIPTION_CUSTOMER silent, with no log and no Sentry report.
  • Makes useAccountData set its error when the account fetch is rejected for a reason other than an invalid token. Settings then shows AppErrorDialog ("Something went wrong") instead of default values. Profile and attached-clients failures keep their soft handling.
  • Hides the "Created" date in the Security password row when passwordCreated is 0 or missing.
  • Adds the story "Password set, no created date", the page-object getter errorLoadingApp, and the functional test accountFetchError.spec.ts.

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-14647

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: the subscriptions catch in account.ts and fetchAccountData in useAccountData/index.ts.
  • Suggested review order: account.ts, useAccountData/index.ts, Security/index.tsx, then the tests.
  • Risky or complex parts: a Play or App Store failure also drops the Stripe subscriptions that loaded correctly.
  • One reviewer call: is it correct to reset all three lists on any failure, instead of only the list that failed?

Screenshots (Optional)

Storybook, Security section with a password and no created date:

Security password row with no created date

Storybook, Security section with a created date, for comparison:

Security password row with a created date

Other information (Optional)

I ran these checks through /fxa-verify --run:

  • Functional, packages/functional-tests/tests/settings/accountFetchError.spec.ts on the local stack: 2 passed.
    • "settings shows the error dialog when the account fetch fails": page.route forces /v1/account to return 500.
    • "password row shows no created date without passwordCreatedAt": the test removes the field from the real response.
  • Unit, fxa-auth-server/lib/routes/account.spec.ts: 143 passed, 0 failed. The /account block includes a Stripe failure, a Play failure after a Stripe success, and a silent unknown customer.
  • Unit, fxa-settings useAccountData/index.test.ts and Security/index.test.tsx: 9 passed, 0 failed.
  • Lint: eslint is clean on every changed file, through /fxa-verify.

Not observable on the local stack: the subscriptions failure path. Stripe, Google Play and App Store are off locally. The route unit tests cover this path, and CI runs the full suites.

@fxa-agent
fxa-agent Bot requested a review from a team as a code owner September 30, 2026 19:50
@fxa-agent fxa-agent Bot added the auto label Sep 30, 2026
@vbudhram
vbudhram requested a lite review from Copilot October 1, 2026 21:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Add App Store failure coverage and prevent duplicate Sentry reporting for account outages.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Improves account and subscription error handling so Settings avoids misleading defaults and invalid password dates.

Changes:

  • Returns empty subscriptions on provider failures, with reporting for unexpected errors.
  • Shows an error dialog for account-fetch failures.
  • Hides missing password creation dates and adds test coverage.
File Summary
packages/​fxa-settings/​src/​lib/​hooks/​useAccountData/​index.ts Propagates account-fetch errors.
packages/​fxa-settings/​src/​lib/​hooks/​useAccountData/​index.test.ts Tests account and profile failure handling.
packages/​fxa-settings/​src/​components/​Settings/​Security/​index.tsx Hides missing password dates.
packages/​fxa-settings/​src/​components/​Settings/​Security/​index.test.tsx Tests password date rendering.
packages/​fxa-settings/​src/​components/​Settings/​Security/​index.stories.tsx Adds the missing-date story.
packages/​fxa-auth-server/​lib/​routes/​account.ts Handles subscription read failures.
packages/​fxa-auth-server/​lib/​routes/​account.spec.ts Tests subscription failure behavior.
packages/​functional-tests/​tests/​settings/​accountFetchError.spec.ts Adds functional error coverage.
packages/​functional-tests/​pages/​settings/​index.ts Adds the error-dialog page-object getter.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +268 to +271
Sentry.captureMessage(
`Failed to fetch account: ${accountResult.reason}`
);
throw accountResult.reason;
## Because

- GET /v1/account returned a 5xx when the Stripe, Google Play or App Store read failed. One subscriptions error broke the whole account response.
- Settings ignored a rejected account fetch and showed default values, such as 2FA off and no recovery key. This looked like lost account data.
- The Security section showed a password "Created" date of 1969 or 1970 when `passwordCreated` was 0 or missing.

## This pull request

- Changes the subscriptions `catch` in the GET /v1/account handler (`account.ts`). It now returns 200 with `subscriptions: []`, logs `Account.get.subscriptions.error` with `log.error`, and reports the error with `reportSentryError`. The response schema does not change.
- Resets all three lists (Stripe, Google Play, App Store) on any failure. A Play or App Store failure after a Stripe success also returns empty subscriptions. This is by design.
- Keeps `UNKNOWN_SUBSCRIPTION_CUSTOMER` silent, with no log and no Sentry report.
- Makes `useAccountData` set its `error` when the account fetch fails for a reason other than an invalid token. Settings then shows `AppErrorDialog` ("Something went wrong"), not default values. Profile and attached-clients failures keep their soft handling.
- Removes the `Sentry.captureMessage` call for a failed account fetch in `useAccountData`. `SettingsError` already reports that error with `Sentry.captureException`, so the hook sent a second event.
- Hides the "Created" date in the Security password row when `passwordCreated` is 0 or missing.
- Adds the story "Password set, no created date", the page-object getter `errorLoadingApp`, and the functional test `accountFetchError.spec.ts`.

## Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-14647

This branch has not been deployed

No deployments
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.

2 participants