Migrate transactional email from Bento to AutoSend#1
Open
multiplehats wants to merge 1 commit into
Open
Conversation
Replace @bentonow/bento-node-sdk with autosendjs. Swap the email service to AutoSend's structured from-address and res.success checks, switch to single-key (AUTOSEND_API_KEY) init, and delete the Bento wrapper. No event tracking was on Bento (analytics run via @stacksee/analytics), so nothing was dropped.
📝 WalkthroughWalkthroughThis pull request migrates the application's email delivery backend from Bento to Autosend. It introduces a new ChangesEmail provider migration: Bento → Autosend
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/features/email/server/email.service.ts`:
- Around line 65-71: The current sendEmail path silently returns { skipped: true
} when autosendClient.isEnabled() is false; change it to fail fast outside
explicit local-preview/dev flows by throwing an error (e.g., throw new
Error('Autosend provider not configured') ) instead of returning skipped when
not in a local preview mode; keep the existing skip behavior only when an
explicit local-preview flag or NODE_ENV==='development' (or an isLocalPreview
config) is present. Update the branch that checks autosendClient.isEnabled()
(and the analogous branch at lines ~116-122) to throw in production/non-local
modes so callers of sendEmail() (such as auth.ts awaiting sendEmail()) cannot
silently drop emails, and additionally add a startup validation that verifies
autosendClient is configured to fail-fast during init if the app is expected to
send real emails.
In `@src/lib/features/email/server/index.ts`:
- Around line 15-23: The parseEmailAddress function may return { email: '' } for
whitespace-only inputs; normalize and trim the incoming value first and also
trim the regex capture groups, then if the resulting email string is empty
return the provided fallback. Update parseEmailAddress (and any analogous
parsing used for defaultFrom/defaultReplyTo) to run value = value?.trim() ||
undefined before matching, use trimmed captures for name and email, and if the
parsed email after trimming is falsy, return fallback instead of { email: '' }.
In `@src/lib/server/autosend.ts`:
- Around line 93-99: The current batch send uses Promise.all which rejects on
the first failed send and hides partial successes; replace
Promise.all([...this.sendTransactionalEmail...]) with Promise.allSettled so each
email's outcome is captured, then map the settled results to a per-email status
object (include original email id/address, status: "fulfilled" or "rejected",
value or reason) and return that array instead of throwing; update operation.end
to record succeeded count (count of fulfilled results) and ensure callers can
inspect per-email results to implement idempotent retries; reference
this.sendTransactionalEmail and the batch-sending block in autosend.ts when
making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47603c44-749f-4186-aba0-2407b7e736fa
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.env.examplepackage.jsonsrc/lib/features/email/server/email.service.tssrc/lib/features/email/server/index.tssrc/lib/server/autosend.tssrc/lib/server/bento.tssrc/routes/api/footprint/ingest/+server.tssrc/routes/email-preview/[...email]/+page.server.ts
💤 Files with no reviewable changes (1)
- src/lib/server/bento.ts
Comment on lines
+65
to
71
| if (!this.autosendClient.isEnabled()) { | ||
| this.logger.warn('Autosend disabled; skipping sendEmail', { | ||
| subject: options.subject, | ||
| to: options.to | ||
| }); | ||
| operation.end({ skipped: true }); | ||
| return { skipped: true }; |
There was a problem hiding this comment.
Don't silently skip sends when Autosend is misconfigured outside local preview/dev flows.
These branches turn a required provider misconfiguration into { skipped: true }. src/lib/server/auth.ts:101-128 awaits emailService.sendEmail() but never checks skipped, so password-reset mail can be dropped while the auth operation still records success. Throw here in non-dev (or fail fast during startup) and keep the skip path only for explicitly local preview scenarios.
Also applies to: 116-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/features/email/server/email.service.ts` around lines 65 - 71, The
current sendEmail path silently returns { skipped: true } when
autosendClient.isEnabled() is false; change it to fail fast outside explicit
local-preview/dev flows by throwing an error (e.g., throw new Error('Autosend
provider not configured') ) instead of returning skipped when not in a local
preview mode; keep the existing skip behavior only when an explicit
local-preview flag or NODE_ENV==='development' (or an isLocalPreview config) is
present. Update the branch that checks autosendClient.isEnabled() (and the
analogous branch at lines ~116-122) to throw in production/non-local modes so
callers of sendEmail() (such as auth.ts awaiting sendEmail()) cannot silently
drop emails, and additionally add a startup validation that verifies
autosendClient is configured to fail-fast during init if the app is expected to
send real emails.
Comment on lines
+15
to
+23
| function parseEmailAddress(value: string | undefined, fallback: EmailAddress): EmailAddress { | ||
| if (!value) return fallback; | ||
| const match = value.match(/^\s*(.*?)\s*<\s*([^>]+)\s*>\s*$/); | ||
| if (match) { | ||
| const [, name, email] = match; | ||
| return name ? { email, name } : { email }; | ||
| } | ||
| return { email: value.trim() }; | ||
| } |
There was a problem hiding this comment.
Harden env email parsing to avoid invalid defaults reaching Autosend
At Line 16 and Line 22, whitespace-only env values can produce { email: '' } instead of using fallback. This can break all sends when defaultFrom/defaultReplyTo are built from misconfigured env vars. Normalize first, trim captures, and fall back if parsed email is empty.
Suggested patch
function parseEmailAddress(value: string | undefined, fallback: EmailAddress): EmailAddress {
- if (!value) return fallback;
- const match = value.match(/^\s*(.*?)\s*<\s*([^>]+)\s*>\s*$/);
+ const normalized = value?.trim();
+ if (!normalized) return fallback;
+ const match = normalized.match(/^\s*(.*?)\s*<\s*([^>]+)\s*>\s*$/);
if (match) {
- const [, name, email] = match;
- return name ? { email, name } : { email };
+ const [, rawName, rawEmail] = match;
+ const email = rawEmail.trim();
+ if (!email) return fallback;
+ const name = rawName.trim();
+ return name ? { email, name } : { email };
}
- return { email: value.trim() };
+ return normalized ? { email: normalized } : fallback;
}Also applies to: 25-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/features/email/server/index.ts` around lines 15 - 23, The
parseEmailAddress function may return { email: '' } for whitespace-only inputs;
normalize and trim the incoming value first and also trim the regex capture
groups, then if the resulting email string is empty return the provided
fallback. Update parseEmailAddress (and any analogous parsing used for
defaultFrom/defaultReplyTo) to run value = value?.trim() || undefined before
matching, use trimmed captures for name and email, and if the parsed email after
trimming is falsy, return fallback instead of { email: '' }.
Comment on lines
+93
to
+99
| try { | ||
| // Autosend has no multi-template batch endpoint; send each individually. | ||
| const responses = await Promise.all( | ||
| emails.map((email) => this.sendTransactionalEmail(email)) | ||
| ); | ||
| operation.end({ count: responses.length }); | ||
| return responses; |
There was a problem hiding this comment.
Return per-email batch results instead of failing the whole batch on first rejection.
Promise.all() makes this non-atomic: some emails can already be delivered before one send rejects, but callers only get a thrown batch error. Retrying the batch can then double-send the subset that succeeded on the first attempt. Please surface partial successes/failures (Promise.allSettled + per-email status) or add an idempotent retry strategy here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/server/autosend.ts` around lines 93 - 99, The current batch send uses
Promise.all which rejects on the first failed send and hides partial successes;
replace Promise.all([...this.sendTransactionalEmail...]) with Promise.allSettled
so each email's outcome is captured, then map the settled results to a per-email
status object (include original email id/address, status: "fulfilled" or
"rejected", value or reason) and return that array instead of throwing; update
operation.end to record succeeded count (count of fulfilled results) and ensure
callers can inspect per-email results to implement idempotent retries; reference
this.sendTransactionalEmail and the batch-sending block in autosend.ts when
making the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Migrating all apps off Bento (
@bentonow/bento-node-sdk) onto AutoSend (autosendjs) for transactional email. This is the EmitKit app.What changed (9 files, +192 / -209)
src/lib/server/bento.tsAnalyticswrappersrc/lib/server/autosend.tsAutosendClientwrapper: structuredfrom/replyTo, throws on!res.success, batch viaPromise.allsrc/lib/features/email/server/email.service.tsbentoClient→autosendClient;defaultFrom/defaultReplyTo/from/replyToretypedstring→EmailAddress;html_body→html;tonow{ email }src/lib/features/email/server/index.tsif (AUTOSEND_API_KEY)), env rename,parseEmailAddresshelper to normalize"Name <email>"strings into the structured objectsrc/routes/email-preview/[...email]/+page.server.tssrc/routes/api/footprint/ingest/+server.ts@stacksee/analytics, untouched.env.examplepackage.json/pnpm-lock.yaml-@bentonow/bento-node-sdk,+autosendjs@^1.0.3API / field mapping
to: stringto: { email }from: 'Name <email>'from: { email, name }html_bodyhtmltransactional: trueBatch.sendTransactionalEmails({ emails })Promise.all(emails.map(send))bento.V1.Subscribers.createSubscriberclient.contacts.upsert(...)Gotchas handled
from, not RFC-5322 string. Passing"Name <email>"to AutoSend returns success-shaped but never sends. The wrapper +parseEmailAddressenforce{ email, name }.res.successis checked.client.emails.send()does not throw on API-level failures; the wrapper inspects the result and throws so failures surface.replyTonow actually sends — AutoSend supports it; the old Bento code logged-and-ignored it.Runtime / await discipline
SvelteKit on Vercel (serverless) —
@sveltejs/adapter-vercel(nodejs20.x),vercel.json. Better Auth callbacks (sendResetPassword,sendVerificationEmail,emailOTP.sendVerificationOTP) alreadyawaitthrough the email service inside try/catch that swallows errors — correct for "check your inbox" flows. No auth changes needed.Event tracking
None on Bento. Analytics run via
@stacksee/analytics(which declares Bento as an optional transitive dep — that's why it may still appear in the lockfile; it is not our direct dependency). Nothing dropped.Remote env changes required before deploy (Vercel)
BENTO_PUBLISHABLE_KEY,BENTO_SECRET_KEY,BENTO_SITE_UUID,BENTO_DEFAULT_FROM,BENTO_REPLY_TOAUTOSEND_API_KEY(required); optionalAUTOSEND_DEFAULT_FROM,AUTOSEND_REPLY_TO.envis plaintext (not dotenvx) — no encryption stepTest plan
pnpm run check— 0 errors, 0 warningsres.successruntime gotchas. Trigger signup / OTP / password reset and confirm delivery.AUTOSEND_API_KEYlocally + in Vercel before testingAUTOSEND_DEFAULT_FROM(defaults toEmitKit <noreply@emitkit.com>) is verified