fix(mailer): normalize provider response to nodemailer format#3375
Conversation
- GET /api/admin/readiness (admin-only) returns startup checklist as JSON - validateSaaSReadiness() prints status table at boot (skipped in test) - 7 categories: Config, Security, Auth, Mail, Billing, Analytics, Monitoring - Smart dependencies: Resend configured → skip Nodemailer warning - Non-blocking: warnings only, never prevents startup
…tion - Add proper JSDoc with @returns on logConfiguration - Fix isSet JSDoc to match implementation (non-empty string check) - JWT check now catches missing/empty secret, not just default value - Remove mail provider name from response to avoid leaking placeholders
… cache isSet - Add @returns {void} to readiness controller JSDoc - Use can('manage', 'Readiness') matching standard CASL pattern - Log readiness check failures at boot instead of swallowing - Cache isSet(config.domain) to avoid duplicate calls
Cover all getReadinessStatus branches (ok/warning paths for each check category) and the readiness controller error handler to improve patch and project coverage.
- Wrap config mutation tests with try/finally to prevent state leakage - Strengthen 401 assertion with status check - Clarify isSet JSDoc wording
Pre-clean regular-readiness@test.com before creating it to prevent unique index failures on re-runs, hoist to suite scope, and delete in afterAll.
Resend returns { id } while Nodemailer returns { accepted, rejected }.
Normalize in sendMail() so controllers checking mail?.accepted.length
work correctly with any provider. Also add attachment support to both
providers and mailer index.
Closes #3373
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3375 +/- ##
==========================================
+ Coverage 84.60% 84.84% +0.24%
==========================================
Files 110 110
Lines 2767 2811 +44
Branches 769 789 +20
==========================================
+ Hits 2341 2385 +44
Misses 337 337
Partials 89 89 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves mail delivery consistency by normalizing provider responses to Nodemailer’s { accepted, rejected } shape and adds attachment support across mail providers. It also introduces a new admin-only “readiness” endpoint plus startup logging to surface configuration readiness signals.
Changes:
- Normalize
sendMail()results so non-Nodemailer providers (e.g., Resend) still return{ accepted, rejected }for downstream controllers. - Add attachment support for both Resend (base64 encoding) and Nodemailer providers.
- Add admin-only readiness checks endpoint (
/api/admin/readiness) and log a readiness summary during server startup.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/home/tests/home.integration.tests.js | Adds integration coverage for readiness auth + response shape and scenarios. |
| modules/home/services/home.service.js | Implements readiness checks derived from config + mailer configuration. |
| modules/home/routes/home.route.js | Registers new admin readiness route with JWT auth + CASL policy. |
| modules/home/policies/home.policy.js | Grants admins CASL ability to manage/read the readiness subject. |
| modules/home/controllers/home.controller.js | Adds readiness controller action returning the readiness checks. |
| lib/middlewares/policy.js | Maps /api/admin/readiness to the Readiness CASL subject type. |
| lib/helpers/mailer/tests/provider.resend.unit.tests.js | Adds unit test coverage for attachment encoding in Resend provider. |
| lib/helpers/mailer/tests/mailer.unit.tests.js | Adds unit tests for response normalization + attachment forwarding. |
| lib/helpers/mailer/provider.resend.js | Implements attachment base64 encoding for Resend payloads. |
| lib/helpers/mailer/provider.nodemailer.js | Adds attachment forwarding into Nodemailer sendMail() payload. |
| lib/helpers/mailer/index.js | Forwards attachments and normalizes provider send results to {accepted,rejected}. |
| lib/app.js | Logs readiness summary at startup via dynamic import of HomeService. |
Summary
{ id }while Nodemailer returns{ accepted, rejected }— controllers checkingmail?.accepted.lengthalways got 0 with Resend, returning 400 "Failure sending email"sendMail(), normalize response: ifresult.acceptedis not an array, spread result and injectaccepted: [mail.to], rejected: []Test plan
mailer.unit.tests.js— covers normalization for non-array response and pass-through when already normalizedprovider.resend.unit.tests.js— covers attachment encodingmigrations.integration.tests.jsis unrelatedCloses #3373