Skip to content

feat: add internal SMTP failover support#11761

Draft
TorstenDittmann wants to merge 2 commits into1.9.xfrom
feat-internal-smtp-failover
Draft

feat: add internal SMTP failover support#11761
TorstenDittmann wants to merge 2 commits into1.9.xfrom
feat-internal-smtp-failover

Conversation

@TorstenDittmann
Copy link
Copy Markdown
Contributor

Summary

  • add internal SMTP failover support by wrapping Appwrite's env-configured SMTP client with utopia-php/messaging multi-adapter failover and introducing a secondary SMTP env configuration
  • centralize internal SMTP setup under Appwrite\SMTP, update the mail worker and doctor task to use the shared client, and keep project-level custom SMTP using direct adapter construction
  • align account recovery, email verification, magic URL, email OTP, team invitations, and email MFA checks with the shared primary SMTP enablement logic while ignoring secondary SMTP settings unless a primary host is configured

Validation

  • ran ./vendor/bin/pint --test --config pint.json on the touched PHP files
  • ran php -l on the touched PHP files
  • smoke tested that a single primary SMTP config returns the shared SMTP client, dual SMTP config enables failover, and secondary-only config is ignored

Use utopia-php/messaging's multi-adapter failover support for Appwrite's internal SMTP client so deployments can define a fallback SMTP transport without changing project-level SMTP behavior.

Add secondary SMTP environment variables, centralize internal SMTP setup under Appwrite\SMTP, and update internal SMTP availability checks so failover configuration stays consistent across the mail worker, doctor task, account flows, MFA, and team invitations while ignoring secondary settings unless a primary host is configured.
@TorstenDittmann TorstenDittmann marked this pull request as draft April 2, 2026 14:40
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR introduces internal SMTP failover by wrapping Appwrite's env-configured SMTP client in utopia-php/messaging's Messenger multi-adapter and adding a secondary SMTP env-variable set (_APP_SMTP_*_SECONDARY). It centralises all internal SMTP logic under Appwrite\SMTP\{SMTP,Client}, updates the mail worker and doctor task to use the shared client, and aligns every SMTP-enabled guard in the account/teams/MFA controllers to call SMTP\SMTP::isEnabled() instead of reading the env var directly.

  • The previous round's concerns (dynamic Messenger instantiation, redundant createAttachment wrapper) have both been addressed in this latest commit.
  • The utopia-php/messaging dependency is still pinned to an unreleased feature-branch commit (dev-feat-multiple-adapters-failover#c1c2409... aliased as 0.22.99); this was acknowledged in a prior thread and is blocked on an upstream tagged release before the PR can safely target 1.9.x.
  • Two minor P2 style findings in SMTP.php: the getEnvConfig return type (array<string, int|string>) is slightly too wide and causes PHPStan warnings when its values are passed to string-typed parameters in buildAdapter; and buildAdapter itself is a private pass-through with no meaningful abstraction that could simply be inlined.

Confidence Score: 5/5

  • Code logic is correct and safe to merge once the upstream utopia-php/messaging dependency is cut as a stable release (already tracked in a prior thread).
  • All new findings in this pass are P2 style suggestions. The two previously flagged P1 issues (dynamic instantiation, createAttachment wrapper) are resolved in this commit. The outstanding dev-branch composer.json constraint is a known blocker already captured in earlier review threads — not a new finding.
  • composer.json (dev-branch dependency) and src/Appwrite/SMTP/SMTP.php (loose array type / redundant buildAdapter).

Important Files Changed

Filename Overview
src/Appwrite/SMTP/SMTP.php New factory class centralising internal SMTP setup; env-var naming/suffix logic is correct and the failover path is wired properly, but the loose `array<string, int
src/Appwrite/SMTP/Client.php Thin Closure-based wrapper with a single send() method; correct and minimal. Works via duck-typing with EmailAdapter at every call-site.
src/Appwrite/Platform/Workers/Mails.php Correctly delegates internal SMTP to the new SMTP\SMTP::isEnabled() / registry path and custom project SMTP to SMTP\SMTP::createAdapter(); previous createAttachment wrapper and inline new SMTP(...) cleaned up.
src/Appwrite/Platform/Tasks/Doctor.php Guards the SMTP connectivity test with SMTP\SMTP::isEnabled() before touching the registry, preventing an uncaught InvalidArgumentException when SMTP is not configured.
app/init/registers.php Replaces inline new SMTP(...) construction in the smtp registry factory with SMTP\SMTP::createService(); lazy evaluation is preserved so the factory only fires on first access.
composer.json utopia-php/messaging still pinned to an unmerged feature-branch commit aliased as 0.22.99; acknowledged in previous threads — must be updated to a stable release before this targets production.

Reviews (2): Last reviewed commit: "style: address SMTP review feedback" | Re-trigger Greptile

Comment thread composer.json
Comment thread src/Appwrite/SMTP/SMTP.php Outdated
Comment thread src/Appwrite/Platform/Workers/Mails.php Outdated
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 2, 2026

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit 31f2369 - 3 flaky tests
Test Retries Total Time Details
UsageTest::testVectorsDBStats 1 10.27s Logs
LegacyCustomClientTest::testAttributeResponseModels 1 243.74s Logs
LegacyCustomServerTest::testCreateIndexes 1 244.34s Logs
Commit 53ca7d0 - 2 flaky tests
Test Retries Total Time Details
UsageTest::testVectorsDBStats 1 10.07s Logs
TablesDBTransactionsCustomServerTest::testCreateDocument 1 240.59s Logs

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 2, 2026

✨ Benchmark results

  • Requests per second: 1,574
  • Requests with 200 status code: 283,296
  • P99 latency: 0.112179537

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,574 1,247
200 283,296 224,503
P99 0.112179537 0.169254402

@TorstenDittmann
Copy link
Copy Markdown
Contributor Author

@greptileai take another look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant