feat(email): real SMTP delivery with durable idempotent send tracking - #24
Merged
ameeribro4-sudo merged 2 commits intoAug 20, 2026
Conversation
Replace the simulated email path with a config-driven Nodemailer
transport (SMTP_HOST/PORT/SECURE/USER/PASSWORD, EMAIL_FROM) that
throws on transport errors so Bull backoff retries, render email jobs
through a template registry (welcome, trade-completed, test) with
{{key}} interpolation, and guard delivery with an atomic Redis
SET NX EX marker keyed by emailId so retries and duplicate enqueues
never double-send while genuinely failed sends are re-sent.
snowrugar-beep
force-pushed
the
feat/issue-17-real-idempotent-email
branch
from
August 20, 2026 15:52
13be43c to
36d33ad
Compare
6 tasks
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
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.
Summary
Closes #17
Replaces the simulated email path in
EmailJobProcessorwith real, config-driven SMTP delivery via Nodemailer, real template rendering, and durable idempotent send tracking. The single most important design decision is the atomic RedisSET ... NX EXsend-marker keyed byemailId: the claim is taken before sending and released only on failure, so a Bull retry or a duplicate enqueue of an already-sent email short-circuits (never double-sends), while a job whose SMTP call genuinely failed releases the claim and is re-sent by the queue's existing backoff. This is the same claim/release pattern used for the swap processor, kept consistent across the queue layer.Why
The processor never sent anything:
sendEmailsimulated a 500ms delay,isEmailSentalways returnedfalse,markEmailAsSentonly logged, andrenderTemplatereturned a hardcoded string. Because the idempotency check always reported "not sent", a Bull retry (the queue is configured withattempts: 3and exponential backoff) after a transient SMTP failure would re-send an email the user already received — and, worse, every job's email was effectively "already sent" yet would be re-sent. A naivenodemailer.sendMail()drop-in would inherit exactly that double-send trap, so durable per-email tracking was a required part of the feature, not an optional extra. The claim-before-send ordering matters: checking first and marking after leaves a window where two workers both see "not sent" and both send; the atomicNXclaim closes it.What was built
src/queue/processors/email.processor.ts(rewritten; tests insrc/queue/processors/email.processor.spec.ts):email.processor.tsConfigService(SMTP_HOST,SMTP_PORT,SMTP_SECURE,SMTP_USER,SMTP_PASSWORD,EMAIL_FROM), sent withto,subject, renderedhtml, andattachments. Transport errors propagate out ofprocessEmail(rethrown), so Bull's configured backoff retries. Idempotency:claimSend(emailId)doesSET email:sent:{emailId} 1 EX 2592000 NXvia the existingRedisPoolService; on failurereleaseSendDELs the marker. Renders throughrenderEmailTemplate. The transport interface is explicitly typed (SmtpTransport) because the project declaresnodemaileras an ambient module with no@typespackage — a narrow, honest typing of only the surface used rather than spreadingany.email.processor.spec.ts(new)NX/EX: one transport call with the correct recipient/subject/from/rendered body for a first send; zero transport calls for a duplicateemailId; SMTP failure throws and releases the claim so the retry re-sends (two calls total); welcome/trade-completed/test template resolution; unknown-template fallback; HTML-escaped context.src/notifications/templates/email.templates.ts(new):email.templates.tswelcome,trade-completed,testtemplates plusGENERIC_TEMPLATEfallback, andrenderEmailTemplate(name, context)with{{key}}interpolation (the same placeholder syntax the i18n notification templates use). Values are HTML-escaped to prevent template injection. Templates are TS constants rather than loose.htmlfiles so they survivenest build(which emits only TS;src/loose files would not reachdist/). Unknown template names fall back to the generic template so a misconfigured job produces a usable email instead of failing forever.Integration changes outside
src/queue/processors/README.md— addsSMTP_SECUREandEMAIL_FROMto the required env var block (they were only in.env.example) and documents template resolution rules and the idempotent-delivery marker under the configuration section.Deliberately separate from
NotificationsModule.EmailService: that service swallows transport errors and returnsfalsefor graceful degradation — which would defeat Bull retries — so the queue path owns its own throwing transport. The split is documented in the processor docstring.Acceptance criteria coverage
sendEmailinvokes the real SMTP transport with the configured host/port/from and the job's recipients, subject, and rendered body. (email.processor.spec.ts— "sends a real transport call with recipient, subject, from, and rendered body" asserts the exactsendMailpayload includingfrom: 'noreply@peerx.com'and the rendered HTML; transport is built fromConfigServiceSMTP_* values)email.processor.spec.ts— "throws on SMTP failure and releases the claim so a retry re-sends" rejects with the transport error; the processor rethrows out ofprocessEmail, which Bull treats as a retryable failure)emailIdis skipped, not re-sent. (email.processor.spec.ts— "skips a duplicate emailId with zero transport calls"; atomicSET NX EXclaim held across the retry)emailId. (email.processor.spec.ts— first test assertstoHaveBeenCalledTimes(1); duplicate test asserts zero additional calls)welcomeandtrade-completedtemplates. (email.processor.spec.ts— "renders the welcome template with context" and "renders the trade-completed template with context", plus the test template)SMTP_*andEMAIL_FROMvariables and the template resolution rules are documented in the README configuration section. (README — env block updated withSMTP_SECURE/EMAIL_FROM; "Email template resolution" and "Idempotent email delivery" notes)Test plan
npm run build— succeedsnpx jest src/queue/processors/email.processor.spec.ts— 8/8 passing (8 new tests)npm run test— 457/510 passing vs 449/502 on base; failing suites identical to base except one pre-existing flakyadvanced-analyticsspec (a "deterministic anomalyScore" test that asserts on random-bounded values and fails intermittently even in isolation on both base and this branch — unrelated to the email path); zero new failuresnpx eslinton changed source — 0 issues on all three new/changed filesnpx madge --circular --extensions ts src/— same 3 pre-existing cycles, none involving queue or notificationsEnv vars / Notes
Templates: names
welcome,trade-completed, andtestresolve fromsrc/notifications/templates/email.templates.ts;{{key}}placeholders are HTML-escaped; unknown names fall back to a generic template. Idempotency: an atomic RedisSET email:sent:{emailId} 1 EX 2592000 NXmarker (30-day TTL, matching DLQ retention) is claimed before send and released only on failure; Redis must be reachable for the guard to work (the queue already requires Redis). Jobs withoutto/subject/templatefail fast withInvalid email data. This PR does not change the notification processor, SMS/push delivery, or the queue's retry policy (all out of scope per the issue).