Skip to content

refactor(auth-server): remove the canSend feature flag from the mailer - #21023

Open
vbudhram wants to merge 2 commits into
mainfrom
fxa-13034
Open

refactor(auth-server): remove the canSend feature flag from the mailer#21023
vbudhram wants to merge 2 commits into
mainfrom
fxa-13034

Conversation

@vbudhram

@vbudhram vbudhram commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Because

  • canSend was a migration flag for libs/email-sender. Every email goes through the new mailer now, and a rollback is off the table, so the flag and the old-mailer branches behind it are dead weight.
  • Each guarded call site kept an else branch that sent through the old mailer. That branch became unreachable, so it goes with the flag.

This pull request

  • Removes the canSend method from FxaMailer in lib/senders/fxa-mailer.ts.
  • Removes the smtp.fxaMailerDisableSend config block and its SMTP_FXA_MAILER_DISABLE_SEND env entry from config/index.ts.
  • Unwraps 54 guarded call sites across the route handlers, lib/inactive-accounts/index.ts, scripts/verification-reminders.js, and scripts/recorded-future/check-and-reset.ts. The new mailer send stays; the old-mailer else branch goes.
  • Drops the locals, imports, and assignments that only fed the removed branches.
  • Removes the canSend tests and mocks, including the tests that only asserted the disabled or old-mailer path, in fxa-mailer.spec.ts, passkeys.spec.ts, and test/fixtures/fxa-mailer.ts.

Issue that this pull request solves

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

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: lib/senders/fxa-mailer.ts and config/index.ts hold the actual removal. The rest is call-site unwrapping.
  • Suggested review order: mailer, then config, then the routes, then the two scripts, then the specs.
  • Risky or complex parts: the diff must not lose a send. Every changed file keeps the same count of fxaMailer.send* calls as main. unblock-codes.js also loses a db.accountEmails(uid) call, and check-and-reset.ts loses its bouncesFn/sendersFn setup, because nothing but the old mailer used them.

Screenshots (Optional)

Other information (Optional)

Operators should know that emails SMTP_FXA_MAILER_DISABLE_SEND could suppress can no longer be turned off by config. The escape hatch is gone, which is what the ticket asks for.

AC 4 asks for the config value to go from the auth server and the admin server. fxa-admin-server has no occurrence of fxaMailerDisableSend or SMTP_FXA_MAILER_DISABLE_SEND, so there is nothing to remove there.

Two follow-ups I did not do here:

  • 13 route modules no longer use the legacy mailer argument they are constructed with. Dropping that parameter means touching module signatures, call sites, and tests, which is wider than this ticket allows.
  • totp.js line 238 spreads FxaMailerFormat.metricsContext(request) without await, so the metrics fields never reach sendPostChangeTwoStepAuthenticationEmail. It predates this change and sits in the branch I kept. Every other call site awaits it.

Local verification:

  • npx nx lint fxa-auth-server passes.
  • The Jest unit suite reports 3731 passed and 13 failed. All 13 failures reproduce on an unmodified tree. account.spec.ts hits util.isError is not a function under Node 24, and config/index.spec.ts trips on a local snsTopicEndpoint setting. Neither is related to this change.
  • git grep "canSend" returns only canSendToIOS in lib/push.js, which has nothing to do with the mailer.

@vbudhram
vbudhram requested a review from a team as a code owner August 13, 2026 17:34
@vbudhram vbudhram added the auto label Aug 13, 2026
recoveryKeyDeleteAndEmailNotification returned a value from its inner
branches but not when recoveryKeyId was falsy. That was true before this
change too, and it compiled only because the untyped old-mailer fallback
widened the inferred return type to `any`, which suppresses TS7030.
Removing the fallback removed the `any` and exposed the gap.

The caller discards the value, so the branches now just await.

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.

Pull request overview

Removes the obsolete canSend migration flag so auth-server email paths always use FxaMailer.

Changes:

  • Removes the mailer flag and SMTP configuration.
  • Deletes legacy-mailer fallback branches across routes and scripts.
  • Updates affected mocks, fixtures, and tests.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.

Show a summary per file
File Description
config/index.ts Removes obsolete SMTP flag.
lib/inactive-accounts/index.ts Uses FxaMailer directly.
lib/routes/account.ts Removes account email fallbacks.
lib/routes/emails.js Removes secondary-email fallbacks.
lib/routes/ip-profiling.spec.ts Updates mailer fixture setup.
lib/routes/linked-accounts.spec.ts Removes flag mock resets.
lib/routes/linked-accounts.ts Uses modern linked-account email.
lib/routes/mfa.ts Uses modern MFA email.
lib/routes/passkeys.spec.ts Removes flag-path tests.
lib/routes/passkeys.ts Sends passkey emails directly.
lib/routes/password.ts Removes password email fallbacks.
lib/routes/recovery-codes.js Uses modern recovery-code emails.
lib/routes/recovery-key.js Uses modern recovery-key emails.
lib/routes/recovery-phone.ts Uses modern recovery-phone emails.
lib/routes/session.js Removes session email fallbacks.
lib/routes/totp.js Removes TOTP email fallbacks.
lib/routes/unblock-codes.js Uses modern unblock-code email.
lib/routes/unblock-codes.spec.ts Updates fixture documentation.
lib/routes/utils/oauth.js Uses modern device-login email.
lib/routes/utils/signin.js Removes sign-in email fallbacks.
lib/routes/utils/signup.js Uses modern post-verification email.
lib/senders/fxa-mailer.spec.ts Removes canSend tests.
lib/senders/fxa-mailer.ts Removes canSend.
scripts/recorded-future/check-and-reset.ts Removes legacy sender setup.
scripts/verification-reminders.js Sends reminders through FxaMailer.
test/fixtures/fxa-mailer.ts Removes default canSend mock.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

3 participants