Skip to content

fix(automation): Mark survey reminders sent after sending emails, not before - #63

Merged
tylermenezes merged 1 commit into
mainfrom
detail/bug-fix/fix-automation-mark-survey-reminders-sent-after-se-78ee94
Sep 18, 2026
Merged

tylermenezes merged 1 commit into
mainfrom
detail/bug-fix/fix-automation-mark-survey-reminders-sent-after-se-78ee94

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Bug

emailDueSurveysReminder (a 5-minute cron task) emails mentors/students a "due soon" reminder when a SurveyOccurence becomes visible and an "OVERDUE" reminder when it passes dueAt, tracking each via sentVisibleReminder/sentOverdueReminder so each fires at most once.

For each selected occurrence the task persisted the flag before the per-recipient email.sendMail loop, and the loop had no try/catch. When sendMail rejected for one recipient (network error, SMTP 5xx, bad address), the throw aborted the rest of that occurrence's cohort and the rest of the batch, while the flag was already true. For the overdue arm this is permanent loss: once sentOverdueReminder is set, the occurrence matches neither branch of the selection query and nothing resets the flag, so recipients ordered after the failing one never receive the final [Action Required] notice.

Fix

In src/automation/tasks/emailDueSurveysReminder.ts, per occurrence:

  • Wrapped each email.sendMail in a try/catch (tracking sentAtLeastOne / sentAll), so one failing recipient no longer aborts the rest of the cohort or the rest of the batch.
  • Moved the flag update to after the send loop, with a per-arm predicate:
    • Overdue arm: mark only when all sends succeed, so a transient failure leaves the flag false and the occurrence is re-selected on the next tick (self-heals; accepts duplicate emails to already-delivered recipients as a bounded tradeoff).
    • Visible arm: mark when at least one send succeeds (or there are no targets), so a transient blip does not re-send the "due soon" email every 5 minutes across the whole visible window — the visible arm is already self-healing via the overdue run.

Zero-target occurrences are still marked on both arms (avoids re-query churn). The query, subject line, and template selection are unchanged.

Testing

  • Added an offline unit test (emailDueSurveysReminder.test.ts, colocated and excluded from the cron loader) stubbing the Prisma client, nodemailer transport, and template loader — no DB, SMTP, or env required. It covers: a failing recipient not aborting the cohort on both arms; the overdue arm not marking on partial failure and recovering on the next tick (the durable-loss case); the visible arm marking on any success to avoid window duplicates; zero-target marking on both arms; a failing occurrence not aborting the rest of the batch; and the global "flag update always happens after sends" invariant.
  • Routine checks pass: npx tsc --skipLibCheck --noEmit (incl. the new test file), the new unit tests under both tsx and ts-node, and the pre-existing syncAlumniInteractions.test.ts regression suite.
  • End-to-end (not versioned): brought up a real Postgres in Docker and an in-process smtp-server stub scriptable to reject specific recipients with 550 5.1.1, seeded an active event with one visible and one overdue occurrence and three students, then ran the task across simulated ticks. Verified: the happy path sends 6 emails on tick 1 and 0 on tick 2; an overdue transient failure for one recipient leaves sentOverdueReminder=false and the recipient receives the overdue reminder on the next tick once the failure clears (no permanent loss); a visible-arm failure marks on any success and does not re-send during the window; a failing recipient in one occurrence does not abort the other occurrence in the batch; a permanently bad address keeps the occurrence unmarked and retrying while deliverable recipients still receive the reminder each tick. Reverting the fix reproduces the bug — the throw aborts the run and sentOverdueReminder=true is persisted though the recipient never received the email — confirming the E2E is a valid regression test.
  • Lint (npx eslint) could not be run: the repo's @typescript-eslint/parser@3.10.1 is incompatible with typescript@5.2.2 and fails to parse every .ts file (including untouched ones) with a DeprecationError: 'originalKeywordKind'. This is a pre-existing toolchain issue independent of this change, and lint is not part of the package's verification commands.

Automatic Fixes PRs can be configured here.

@detail-app
detail-app Bot requested a review from tylermenezes September 18, 2026 02:55
@tylermenezes
tylermenezes merged commit 9557388 into main Sep 18, 2026
1 check passed
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