Fix digest job race: events received between fetch and delete are dropped unsent #8

Merged

matt merged 1 commit from mailship-091-event-arrival-race-in-digest-job-events--614 into main

2026-09-14 17:12:11 +00:00

hudson commented

2026-09-14 17:09:07 +00:00

Collaborator

mailship-091

Summary: Fix two bugs in the digest cron worker: (1) a race condition where events arriving between the fetch and the delete are deleted without being emailed, and (2) a stale closure that prevents last_digest_at from advancing, causing duplicate sends on every tick.

Changes:

src/database.ts — Added deleteEventsByIds(subscriptionId, eventIds) which deletes only the exact events that were sent, identified by their primary-key IDs. This replaces the timestamp-based deleteEventsForSubscription(sub.id, since) that previously deleted every row with received_at > since — including events that arrived during the slow digest send (profile loads, MJML render, SMTP).

src/worker/email.ts — Two changes. First, runJob now collects the IDs of the events it fetched and calls deleteEventsByIds with those IDs instead of the timestamp-based delete. Second, createJob now re-fetches the subscription from the database on each cron tick before calling runJob, so sub.last_digest_at reflects the latest value instead of the stale value captured when the job was first registered.

test/event-arrival-race.test.js — New test that simulates the race: inserts Event A, fetches it, inserts Event B (the late arrival), deletes by the fetched IDs, and asserts that Event B survives. Previously this test failed because the timestamp-based delete removed Event B too.

How to test: Run pnpm run build && node test/event-arrival-race.test.js with DATA_DIR set to a temporary directory. All 7 assertions should pass, confirming that a late-arriving event is no longer deleted.

mailship-091 **Summary:** Fix two bugs in the digest cron worker: (1) a race condition where events arriving between the fetch and the delete are deleted without being emailed, and (2) a stale closure that prevents `last_digest_at` from advancing, causing duplicate sends on every tick. **Changes:** `src/database.ts` — Added `deleteEventsByIds(subscriptionId, eventIds)` which deletes only the exact events that were sent, identified by their primary-key IDs. This replaces the timestamp-based `deleteEventsForSubscription(sub.id, since)` that previously deleted every row with `received_at > since` — including events that arrived during the slow digest send (profile loads, MJML render, SMTP). `src/worker/email.ts` — Two changes. First, `runJob` now collects the IDs of the events it fetched and calls `deleteEventsByIds` with those IDs instead of the timestamp-based delete. Second, `createJob` now re-fetches the subscription from the database on each cron tick before calling `runJob`, so `sub.last_digest_at` reflects the latest value instead of the stale value captured when the job was first registered. `test/event-arrival-race.test.js` — New test that simulates the race: inserts Event A, fetches it, inserts Event B (the late arrival), deletes by the fetched IDs, and asserts that Event B survives. Previously this test failed because the timestamp-based delete removed Event B too. **How to test:** Run `pnpm run build && node test/event-arrival-race.test.js` with `DATA_DIR` set to a temporary directory. All 7 assertions should pass, confirming that a late-arriving event is no longer deleted.

hudson added 1 commit 2026-09-14 17:09:08 +00:00

fix digest job race: delete sent events by ID, not timestamp 737f949f9b

Bug: runJob computed since = sub.last_digest_at, fetched events with
received_at > since, sent the digest (slow), then deleted ALL events
with received_at > since. Any /notify event that arrived between the
fetch and the delete was also received_at > since, so it was deleted
without ever being sent in a digest.

Two changes:

1. Delete by exact event IDs (src/database.ts, src/worker/email.ts):
   Added deleteEventsByIds(subscriptionId, eventIds) which deletes
   only the events that were actually fetched + sent. The old
   timestamp-based delete is retained but no longer called from runJob.

2. Re-fetch subscription on each cron tick (src/worker/email.ts):
   createJob's closure captured the original sub, so sub.last_digest_at
   stayed stale in memory. Every subsequent tick recomputed since from
   the old value, re-fetching and re-sending duplicate events. Now each
   tick re-fetches the subscription from the DB via getSubscriptionById
   before calling runJob.

Fixes bead mailship-091