diff --git a/components/account/ContactEmail.js b/components/account/ContactEmail.js index 58a8639..297c41f 100644 --- a/components/account/ContactEmail.js +++ b/components/account/ContactEmail.js @@ -17,6 +17,18 @@ import { isValidEmailFormat, buildContactEmailUpdate } from "../../lib/contact-e const SEND_ERROR_COPY = { "no-contact-email": "Add a contact email address before requesting a code.", "rate-limited": "Please wait a moment before requesting another code.", + // Sending is paused -- an exhausted daily quota, an unverified sending + // domain, a revoked key. Deliberately vague, and deliberately one message for + // all of them: the cause is operational detail a researcher cannot act on + // differently, and naming it ("we are out of email quota") would be both + // meaningless to them and needlessly revealing. What they CAN act on is the + // only thing said: not now, try later. + // + // The code form is NOT opened for this one (see requestCode) -- there is no + // code out there to enter, and offering the field would be a second, quieter + // lie on top of the first. + "mail-unavailable": + "We can't send verification codes right now. Please try again in a little while.", }; const VERIFY_ERROR_COPY = { "invalid-code": "That code is incorrect. Check it and try again.", diff --git a/components/account/UnverifiedEmailBanner.js b/components/account/UnverifiedEmailBanner.js new file mode 100644 index 0000000..369c163 --- /dev/null +++ b/components/account/UnverifiedEmailBanner.js @@ -0,0 +1,92 @@ +import { Box, Text, VStack, Button, HStack } from "@chakra-ui/react"; +import Link from "next/link"; +import { hasContactEmail } from "../../lib/contact-email"; + +// Shown to researchers whose contact address has never been confirmed. +// +// --------------------------------------------------------------------------- +// WHY *UNVERIFIED* AND NOT *MISSING* +// --------------------------------------------------------------------------- +// +// "No contact email" is very nearly extinct, and deliberately so: +// ContactEmailGate (components/AuthCheck.js) walls off every admin route until +// a usable address exists, so a researcher cannot reach this dashboard without +// one. What the gate does NOT check is whether the address works -- +// hasContactEmail() tests format only, never contactEmailVerified. +// +// That gap is the entire failure mode. A typo passes the gate. So does an +// address the 2026-08 backfill seeded from Firebase Auth, which is real but +// was never confirmed against the person now using the account. And +// upload-failure-notify.ts mails the address regardless of verified status -- +// so the notification goes out, hard-bounces, and is marked terminally failed +// somewhere nobody looks. The researcher's data stopped arriving and the only +// system that could tell them believes it did. +// +// This banner exists because that failure is invisible from the researcher's +// side by construction: the symptom of a notification you cannot receive is +// silence, which is indistinguishable from everything being fine. +// +// --------------------------------------------------------------------------- +// NOT DISMISSIBLE, AND NO FLAG TO MAINTAIN +// --------------------------------------------------------------------------- +// +// Same reasoning as AddSignInMethodBanner: this is not a nag, it is the one +// warning about a silent data-loss path, and it removes itself the moment it is +// acted on. `contactEmailVerified` flipping to true is written server-side by +// verify-contact-email.ts and ONLY there, so the condition is exact and there +// is no dismissal state to store, expire, or reset when the address changes. +// +// --------------------------------------------------------------------------- +// NO SUBSCRIPTION OF ITS OWN +// --------------------------------------------------------------------------- +// +// `userDoc` is the users/{uid} document, passed down from the dashboard's +// single subscription -- the same convention ContactEmail, ProviderConnections +// and SelectAuth follow on the account page. This component used to open its +// own live listener, which made three on one document on /admin (AuthCheck's +// gate, the experiment list's provider check, and this), each with its own +// loading flicker. Undefined means "not loaded yet" and renders nothing, +// exactly as the in-component loading flag did. +export default function UnverifiedEmailBanner({ userDoc }) { + if (!userDoc) return null; + + // Nothing to say to someone who is already reachable. + if (userDoc.contactEmailVerified === true) return null; + + // No address at all is ContactEmailGate's job, not this banner's -- and a + // researcher seeing this dashboard has already been past it. Staying quiet + // here means the two never argue about the same account. + if (!hasContactEmail(userDoc)) return null; + + return ( + + + + + Confirm your email address + + + DataPipe emails you if data stops uploading for one of your + experiments — but {userDoc.contactEmail} has never been confirmed, + so we have no way to know it reaches you. If it does not, that + notification is the one you would never see. + + + + + + + + + ); +} diff --git a/docs/deploy-contact-email.md b/docs/deploy-contact-email.md index c6988af..0fb87ba 100644 --- a/docs/deploy-contact-email.md +++ b/docs/deploy-contact-email.md @@ -397,9 +397,14 @@ Three things about this policy that are worth knowing: `ERROR` that will not be retried. A document in a retryable error state has `delivery.endTime: null` and is therefore *never* eligible for deletion, which is deliberate: the TTL must not reap a mail that is still deliverable. - The flip side is that a document stuck in retryable `ERROR` lives forever. - Those are worth a periodic look (`delivery.state == "ERROR"` and - `delivery.retryable == true`); there is no automatic sweeper. + The flip side used to be that a document stuck in retryable `ERROR` lived + forever, holding an address the TTL could not reach. + `scheduled-mail-retry.ts` is what closes that: every document it looks at + either gets sent or gets a terminal state, so nothing it can see stays + outside the TTL's reach. It sweeps two shapes — a retryable `ERROR`, and a + `PROCESSING` document whose lease has expired, which is what an instance + killed *inside* a send leaves behind. Both need an index; both are in + `firestore.indexes.json`. - **The retention window is seven days, by design.** `mail-delivery.ts` writes `delivery.expireAt = endTime + 7 days` on every terminal outcome (delivered or permanently failed), and the TTL policy above keys on it. @@ -424,6 +429,204 @@ experiments and queue entries. That query is unaffected by delivery: handled (the send resolves, the receipt has nowhere to land, one warning is logged). +## 5. When sending is unavailable: the breaker + +The two things DataPipe mails fail differently, and the difference only matters +when quota runs out: + +| | Verification code | Upload-failure notification | +|---|---|---| +| Nature | **Realtime.** Someone is watching a form. | **Deferrable.** Still true an hour later. | +| Delivered by | The request itself, synchronously | `onmailcreated`, then the sweeper | +| Retried? | **Never** | Yes, `scheduledmailretry` | +| On failure | 503, vague message, cooldown **kept** | Stays queued, swept later | + +**The cooldown is kept on failure, and that is deliberate.** It used to be +cleared, so that a researcher whose code never arrived was not stuck waiting a +minute for a code that does not exist. That is right about the researcher and +wrong about the endpoint: `contactEmailVerifications/{uid}.sentAt` is the only +server-side rate limit on a path that spends a real Resend request, and clearing +it removed the limit from exactly the case that needs one — a signed-in +researcher holding the button while sends fail. What is cleared now is the +`codeHash`, not the record: no code is left standing, the throttle is, and +`verify-contact-email.ts` answers such a record with "request a new code" +instead of spending one of the five attempts on it. + +**`systemStatus/mail` is the breaker.** `mail-delivery.ts` writes it after every +send: the daily quota reading on success, and a shut breaker on any failure that +is not this one message's problem. It is server-only — no `firestore.rules` +match, so it is default-denied to every client, and the account page learns +nothing from it directly. + +**Three reasons it shuts, and they last for different lengths of time:** + +| Cause | Shut until | Why that long | +|---|---|---| +| `daily_quota_exceeded` | next UTC midnight | the daily cap resets there (a guess — see below) | +| `monthly_quota_exceeded` | start of the next UTC month | the 3,000/month cap does **not** reset at midnight. Reopening nightly means probing into a cap with days left to run, and each probe spends one of a queued mail's three attempts — three nights and every queued notification is terminal | +| a revoked key, an unverified domain, missing config (`SYSTEMIC_ERRORS`) | 15 minutes | nothing resets; a human has to act. The pause exists only to stop a loop of failing sends, and to keep the realtime path from minting codes for mail that cannot go anywhere | + +`validation_error` is deliberately **not** in the systemic set: Resend uses it +both for an unverified sending domain and for one malformed recipient address, +and one researcher's typo must not switch verification off for everybody. + +**If you upgrade the plan mid-month**, clear `systemStatus/mail` by hand — the +monthly pause is a wait for a reset that has just stopped applying. + +**The proactive part is a response header.** Resend returns +`x-resend-daily-quota` — the quota *used* today — on ordinary **successful** +responses, so DataPipe learns it is at 94/100 while sending still works rather +than by failing. Verification stops at a ceiling (currently 90 of 100) while +upload-failure notifications keep going to the full limit. That asymmetry is the +point: a researcher waiting on a code can come back later, but a notification +that their data has stopped arriving is the only signal they get. + +The header is documented as free-plan-only, so it disappears on a paid plan. +Absent reads as "no daily cap applies", which is correct — the reserve logic +turns itself off on upgrade instead of needing to be removed. + +**Check what that number actually means before you trust it.** Resend documents +the header's existence, not its semantics, and "used today" and "remaining +today" are the same shape. If it is really the plan *limit*, every reading is +100, the reserve rule is true forever, and verification is off for good — and +because every send rewrites `dailyQuotaObservedAt`, the staleness escape hatch +never fires either. Capture one for yourself: + +``` +curl -sS -D - -o /dev/null -X POST https://api.resend.com/emails \ + -H "Authorization: Bearer $RESEND_API_KEY" \ + -H "Content-Type: application/json" \ + -d '{"from":"DataPipe ", + "to":["you@example.edu"],"subject":"quota header check","text":"."}' \ + | grep -i x-resend +``` + +Send it twice and watch which way the number moves, then compare it with the +Resend dashboard's usage for the day. Two guards stand in the meantime: a +reading outside `0..100` is refused at the write and ignored at the read, and a +refusal on the reserve alone logs at ERROR (§6). + +**When does it reopen?** Resend publishes no daily-reset time, and nothing here +depends on knowing one. `unavailableUntil` is set to the next UTC midnight as a +*ceiling*; what actually reopens sending is the sweeper landing a successful +send. The deferrable path probes, the realtime path only ever reads. If the real +reset is later than midnight UTC the sweeper's next attempt re-arms the breaker; +if it is earlier, the sweeper finds out first. + +**What a researcher sees:** *"We can't send verification codes right now. Please +try again in a little while."* Deliberately vague, and deliberately the same +message for an exhausted quota, an unverified domain and a revoked key — the +cause is operational detail they cannot act on differently. The code-entry form +is not opened, because there is no code out there to enter, and the resend +cooldown is cleared so they can retry the moment it comes back. + +**To force the feature open or shut by hand**, edit +`systemStatus/mail.unavailableUntil` (a Timestamp, or null). Useful for testing +the message, and for shutting off verification during a Resend incident without +a deploy. + +## 6. Alerting: two metrics, no code + +**You cannot email yourself that you are out of email.** Same account, same +quota. Alerting has to be out-of-band, which in practice means Cloud Monitoring +delivering it rather than DataPipe. + +No code is needed — `mail-delivery.ts` already logs everything at error level. +Create log-based metrics on the functions' logs and alert on them: + +| Match | Why | +|---|---| +| `MailConfigMissingError` | **First.** Every notification the deployment sends is being dropped. | +| `daily_quota_exceeded` | Sending has stopped. Reactive — it is already happening. | +| `mail-availability` + `dailyQuotaUsed` above ~80 | **The useful one.** Leading indicator, from the success-response header, while there is still time to act. | +| `MailVerificationUnavailable` | Verification is being refused for everyone. On `quota-reserve` it is also the check on the header's meaning: if this fires all day while sending works, the reading is not what we think it is (§5). | +| `refusing an implausible x-resend-daily-quota` | The reading is out of range — the header is not a count of today's sends. | + +Route them to a channel Google delivers — Slack, PagerDuty, or an email address +that is **not** on the `jspsych.org` sending domain, so an alert about mail +being broken does not depend on mail working. + +Worth watching alongside, though none needs an alert: `scheduled-mail-retry` +lines reporting a non-zero `agedOut` (notifications that expired undelivered), +any accumulation of `delivery.state == "ERROR"` with `retryable == true` (the +sweeper's backlog), and `MailSweepAbandoned` on a document, which means an +instance died inside a send — one is noise, a run of them is a platform problem. + +## 7. Payload retention: the clock now measures the right thing + +`scheduled-upload-retry.ts` deletes queue entries and their Cloud Storage +payloads seven days after **submission**. That is the only thing that deletes +them — there is no GCS lifecycle rule on the bucket — so this sweep is the whole +retention policy. + +Counting from submission is subtly wrong in two ways, and both lose data nobody +meant to lose: + +- **A storage-provider outage.** The entry is still `pending` with retries left, + so it would have uploaded fine on day eight. Deleting it on day seven throws + away data that was never actually lost. +- **A notification that never arrived.** Part of the seven days is spent before + anything goes wrong, and if the notification died (a quota outage, a bounced + address) the window closes without the researcher ever learning there was one. + +`upload-retention.ts` now decides, and the sweep asks it per entry: + +| Condition | Outcome | +|---|---| +| Older than **14 days** from `createdAt` | **delete** — the ceiling wins over everything | +| `status: "pending"` with retries left | retain — the upload may yet succeed | +| `retainUntil` in the future | retain — the researcher has not been told | +| otherwise | delete — unchanged from before | + +`retainUntil` is written by `upload-retention.ts` — which owns this whole rule, +predicate and write together — and `scheduled-mail-retry.ts` is what calls it, +while an upload-failure notification is undelivered. It covers **every +unresolved entry for the experiment**, not just the one that tripped the +episode, and once per experiment per pass however many notifications name it. + +Two things about when it is written: + +- **Even while the breaker is shut.** Extending is a Firestore write that costs + no quota, and an outage is exactly when the data is at risk. +- **Including the pass that gives up on the notification.** Ageing a + notification out is the case where the researcher will never be told at all, + so it must not also be the moment their data quietly goes back on the + original clock. The 14-day ceiling is what bounds this. + +**The deletion sweep pages.** It looks at up to 500 aged entries to find its 50 +deletions, rather than taking the oldest 50 and stopping. Without that, fifty +retained entries at the head of the queue — one experiment behind a dead +provider — meant a pass that deleted nothing at all, for every other experiment +too, until the blockers crossed the 14-day ceiling up to a week later. + +**What deliberately does not extend:** an experiment whose owner has no contact +email. `upload-failure-notify.ts` records `suppressedReason: "no-contact-email"` +and returns before enqueuing any mail, so no mail document exists, so nothing +extends it — it keeps the plain seven days. That is the right answer when there +is nobody to tell, and it falls out of the design rather than being special-cased. + +**The 14-day ceiling is not optional.** Without it, an experiment whose provider +is dead and whose owner never reads their mail would hold research payloads +forever, silently, at DataPipe's cost. + +## 8. The unverified-address warning + +`ContactEmailGate` walls off every admin route until a usable address exists, so +"no contact email" is nearly extinct among active researchers. What the gate +does not check is whether the address **works** — `hasContactEmail()` tests +format only, never `contactEmailVerified`. + +That gap is the failure mode. A typo passes the gate. So does anything the +2026-08 backfill seeded from Firebase Auth. And upload-failure notifications go +to the address regardless of verified status, so the mail bounces and is marked +terminally failed somewhere nobody looks. + +`components/account/UnverifiedEmailBanner.js` renders on the dashboard whenever +`contactEmailVerified` is false and an address exists. Not dismissible, and with +no flag to maintain: `contactEmailVerified` is written server-side by +`verify-contact-email.ts` and only there, so the banner removes itself the moment +it is acted on. + ## Not covered here Firestore index changes (none expected — see the design doc §3.4/§7 on why diff --git a/firestore.indexes.json b/firestore.indexes.json index 3d90964..06c9f01 100644 --- a/firestore.indexes.json +++ b/firestore.indexes.json @@ -67,6 +67,34 @@ "order": "ASCENDING" } ] + }, + { + "collectionGroup": "mail", + "queryScope": "COLLECTION", + "fields": [ + { + "fieldPath": "delivery.state", + "order": "ASCENDING" + }, + { + "fieldPath": "delivery.retryable", + "order": "ASCENDING" + } + ] + }, + { + "collectionGroup": "mail", + "queryScope": "COLLECTION", + "fields": [ + { + "fieldPath": "delivery.state", + "order": "ASCENDING" + }, + { + "fieldPath": "delivery.leaseExpiresAt", + "order": "ASCENDING" + } + ] } ], "fieldOverrides": [ diff --git a/firestore.rules b/firestore.rules index af71c7e..94838af 100644 --- a/firestore.rules +++ b/firestore.rules @@ -84,6 +84,14 @@ service cloud.firestore { // Same for the `mail` collection (functions/src/mail.ts): unmatched, // therefore already closed. Written down so nobody "fixes" the absence // later by granting access. + // + // Same again for `systemStatus/mail` (functions/src/mail-availability.ts), + // which records how much of the Resend daily quota has been used and until + // when sending is paused. A client that could read it would learn nothing + // about any researcher, but it would learn DataPipe's operational posture, + // and the account page does not need it to: the verification endpoint + // answers 503 with `code: "mail-unavailable"` and the UI renders from that. + // Keeping this unmatched keeps a purely operational document off the wire. match /experiments/{experimentId} { function baseFields() { return request.resource.data.keys().hasAll(['active', 'activeBase64', 'activeConditionAssignment', 'id', 'owner', 'title', 'sessions', 'nConditions', 'currentCondition', 'useValidation', 'allowJSON', 'allowCSV', 'requiredFields', 'maxSessions', 'limitSessions']) diff --git a/functions/src/__tests__/mail-availability.test.js b/functions/src/__tests__/mail-availability.test.js new file mode 100644 index 0000000..c847126 --- /dev/null +++ b/functions/src/__tests__/mail-availability.test.js @@ -0,0 +1,659 @@ +/** + * @jest-environment node + */ + +// Pure coverage for the two predicates that decide whether DataPipe sends. +// +// verificationAvailability() may the REALTIME path send right now? +// sweepDecision() may this failed mail be retried right now? +// +// Both are functions of their arguments, and both are the entire safety +// argument of the feature they belong to -- so they are asserted as tables +// here rather than provoked through Firestore, a scheduler and a mail +// transport. The end-to-end behaviour lives in mail-retry-emulator.test.js. +// +// Imports the COMPILED modules (functions/lib/), so `npm --prefix functions run +// build` must run first. Same convention as mail-delivery.test.js, including +// the emulator bootstrap -- these modules reach app.js transitively for `db`, +// and app.js calls initializeApp() with no arguments. Nothing here talks to +// Firestore. + +process.env.FIRESTORE_EMULATOR_HOST = "localhost:8080"; +process.env.GCLOUD_PROJECT = "datapipe-test"; +process.env.TOKEN_ENCRYPTION_KEY ||= "aa".repeat(32); +process.env.FIREBASE_CONFIG = JSON.stringify({ + projectId: "datapipe-test", + storageBucket: "datapipe-test.appspot.com", +}); + +let verificationAvailability; +let deliveryPaused; +let nextUtcMidnight; +let nextUtcMonthStart; +let pauseUntil; +let usableQuotaReading; +let isQuotaReadingStale; +let VERIFICATION_CEILING; +let FREE_PLAN_DAILY_LIMIT; +let SYSTEMIC_PAUSE_MS; + +let sweepDecision; +let pauseKindFor; +let MAX_SWEEP_AGE_MS; +let IDEMPOTENCY_WINDOW_MS; +let MAX_ATTEMPTS; + +let retentionDecision; + +beforeAll(async () => { + ({ + verificationAvailability, + deliveryPaused, + nextUtcMidnight, + nextUtcMonthStart, + pauseUntil, + usableQuotaReading, + isQuotaReadingStale, + VERIFICATION_CEILING, + FREE_PLAN_DAILY_LIMIT, + SYSTEMIC_PAUSE_MS, + } = await import("../../lib/mail-availability.js")); + + ({ sweepDecision, MAX_SWEEP_AGE_MS, IDEMPOTENCY_WINDOW_MS } = await import( + "../../lib/scheduled-mail-retry.js" + )); + + ({ pauseKindFor, MAX_ATTEMPTS } = await import("../../lib/mail-delivery.js")); + + ({ retentionDecision } = await import("../../lib/upload-retention.js")); +}); + +// Stored Timestamps, as the Admin SDK hands them back. +const ts = (ms) => ({ toMillis: () => ms }); + +// A fixed instant well inside a UTC day, so "next midnight" arithmetic is not +// accidentally satisfied by being near a boundary. +const NOON = Date.UTC(2026, 7, 29, 12, 0, 0); // 2026-08-29T12:00:00Z +const MIDNIGHT_AFTER = Date.UTC(2026, 7, 30, 0, 0, 0); + +// --------------------------------------------------------------------------- +// 1. The daily-reset guess, isolated +// --------------------------------------------------------------------------- + +describe("nextUtcMidnight", () => { + test("is the next UTC day boundary, not 24 hours out", () => { + expect(nextUtcMidnight(NOON)).toBe(MIDNIGHT_AFTER); + // One second before midnight -> that midnight is already past, so the next + // one is the following day. The off-by-one that would break the breaker. + expect(nextUtcMidnight(MIDNIGHT_AFTER - 1000)).toBe(MIDNIGHT_AFTER); + expect(nextUtcMidnight(MIDNIGHT_AFTER)).toBe( + Date.UTC(2026, 7, 31, 0, 0, 0) + ); + }); + + test("rolls over month and year boundaries", () => { + expect(nextUtcMidnight(Date.UTC(2026, 7, 31, 23, 0, 0))).toBe( + Date.UTC(2026, 8, 1, 0, 0, 0) + ); + expect(nextUtcMidnight(Date.UTC(2026, 11, 31, 23, 0, 0))).toBe( + Date.UTC(2027, 0, 1, 0, 0, 0) + ); + }); +}); + +describe("nextUtcMonthStart", () => { + test("is the start of the next UTC month, not thirty days out", () => { + expect(nextUtcMonthStart(NOON)).toBe(Date.UTC(2026, 8, 1)); + // The 20th of the month is the motivating case: the monthly cap is hit + // with eleven days still to run, and a daily reset would reopen the + // breaker that night. + expect(nextUtcMonthStart(Date.UTC(2026, 7, 20, 3, 0, 0))).toBe( + Date.UTC(2026, 8, 1) + ); + // December rolls the year. + expect(nextUtcMonthStart(Date.UTC(2026, 11, 31, 23, 59, 59))).toBe( + Date.UTC(2027, 0, 1) + ); + }); +}); + +describe("pauseUntil", () => { + test("a daily cap holds until midnight; a MONTHLY cap holds until the month turns", () => { + // The distinction the breaker used to lack. Treating a monthly exhaustion + // as a daily one reopened sending every midnight into a cap with days left + // to run -- and each probe spent one of a queued mail's three attempts, so + // three nights turned every queued notification terminal. + expect(pauseUntil("daily-quota", NOON)).toBe(MIDNIGHT_AFTER); + expect(pauseUntil("monthly-quota", NOON)).toBe(Date.UTC(2026, 8, 1)); + expect(pauseUntil("monthly-quota", NOON)).toBeGreaterThan( + pauseUntil("daily-quota", NOON) + ); + }); + + test("a systemic failure is a short cooldown, not a wait for a reset", () => { + // Nothing resets: a revoked key or an unverified domain needs a human. The + // pause is only there to stop a loop of failing sends, so it is minutes. + expect(pauseUntil("systemic", NOON)).toBe(NOON + SYSTEMIC_PAUSE_MS); + expect(pauseUntil("systemic", NOON)).toBeLessThan(MIDNIGHT_AFTER); + }); +}); + +describe("pauseKindFor", () => { + test("tells the two quota caps apart, and names the systemic failures", () => { + expect(pauseKindFor("daily_quota_exceeded")).toBe("daily-quota"); + expect(pauseKindFor("monthly_quota_exceeded")).toBe("monthly-quota"); + for (const name of [ + "suspended_api_key", + "missing_api_key", + "restricted_api_key", + "invalid_permission", + "MailConfigMissingError", + ]) { + expect(pauseKindFor(name)).toBe("systemic"); + } + }); + + test("does NOT shut the breaker for a per-message failure", () => { + // validation_error is Resend's name both for an unverified sending domain + // and for one malformed recipient address. One researcher's typo must not + // switch verification off for everybody, so the ambiguous name is left out + // and the unambiguous ones carry the rule. + expect(pauseKindFor("validation_error")).toBeNull(); + expect(pauseKindFor("rate_limit_exceeded")).toBeNull(); + expect(pauseKindFor("application_error")).toBeNull(); + expect(pauseKindFor("ECONNRESET")).toBeNull(); + }); +}); + +describe("usableQuotaReading", () => { + test("refuses a reading that cannot be a count of today's sends", () => { + // The bound exists because the header's MEANING is assumed, not documented. + // If x-resend-daily-quota turns out to be the monthly counter or a plan + // limit, an unbounded reading would sit above the reserve ceiling forever + // and hold verification shut with nothing in the logs saying why. + expect(usableQuotaReading(0)).toBe(0); + expect(usableQuotaReading(94)).toBe(94); + expect(usableQuotaReading(FREE_PLAN_DAILY_LIMIT)).toBe(FREE_PLAN_DAILY_LIMIT); + expect(usableQuotaReading(FREE_PLAN_DAILY_LIMIT + 1)).toBeNull(); + expect(usableQuotaReading(2900)).toBeNull(); + expect(usableQuotaReading(-1)).toBeNull(); + expect(usableQuotaReading("94")).toBeNull(); + expect(usableQuotaReading(Number.NaN)).toBeNull(); + expect(usableQuotaReading(undefined)).toBeNull(); + }); +}); + +describe("isQuotaReadingStale", () => { + test("a reading from earlier today is fresh", () => { + expect(isQuotaReadingStale(NOON - 60_000, NOON)).toBe(false); + }); + + test("a reading from before the last midnight is stale", () => { + // Yesterday's 100/100 must not hold today's verification path shut. + expect(isQuotaReadingStale(NOON - 24 * 60 * 60 * 1000, NOON)).toBe(true); + }); + + test("no reading at all is stale, not fresh", () => { + expect(isQuotaReadingStale(null, NOON)).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// 2. May the realtime path send? +// --------------------------------------------------------------------------- + +describe("verificationAvailability", () => { + test("an absent status document means available", () => { + // Fails OPEN. A missing or unreadable status document must never be the + // reason a researcher cannot verify their address -- the worst case is one + // send that discovers the real state and records it. + expect(verificationAvailability(undefined, NOON).available).toBe(true); + expect(verificationAvailability({}, NOON).available).toBe(true); + }); + + test("a live unavailableUntil closes it", () => { + const result = verificationAvailability( + { unavailableUntil: ts(MIDNIGHT_AFTER) }, + NOON + ); + expect(result.available).toBe(false); + expect(result.reason).toBe("quota-exhausted"); + expect(result.until).toBe(MIDNIGHT_AFTER); + }); + + test("an expired unavailableUntil reopens it without anyone clearing it", () => { + // The breaker has to reopen on its own. If it only ever reopened on a + // successful send, and the realtime path is the only sender, nothing would + // ever send again. + expect( + verificationAvailability({ unavailableUntil: ts(NOON - 1) }, NOON).available + ).toBe(true); + }); + + test("the reserve closes it before the quota is actually gone", () => { + // The whole point of reading x-resend-daily-quota off SUCCESS responses: + // stop the realtime path with headroom left, rather than discovering the + // limit by failing. + const atCeiling = verificationAvailability( + { dailyQuotaUsed: VERIFICATION_CEILING, dailyQuotaObservedAt: ts(NOON - 1000) }, + NOON + ); + expect(atCeiling.available).toBe(false); + expect(atCeiling.reason).toBe("quota-reserve"); + + // ...and one below it is still open. + expect( + verificationAvailability( + { + dailyQuotaUsed: VERIFICATION_CEILING - 1, + dailyQuotaObservedAt: ts(NOON - 1000), + }, + NOON + ).available + ).toBe(true); + }); + + test("the reserve leaves real headroom for upload-failure mail", () => { + // The asymmetry is the design decision, so assert it rather than trusting + // the constants to stay sane: verification must stop with sends to spare, + // because an upload-failure notification is the one nobody can recover + // from missing. + expect(VERIFICATION_CEILING).toBeLessThan(FREE_PLAN_DAILY_LIMIT); + expect(FREE_PLAN_DAILY_LIMIT - VERIFICATION_CEILING).toBeGreaterThanOrEqual(5); + }); + + test("yesterday's reading does not close today", () => { + // Without the staleness check the breaker would latch permanently: a + // reading of 100/100 taken yesterday would keep refusing forever, because + // nothing rewrites it until a send succeeds and no send is attempted. + expect( + verificationAvailability( + { + dailyQuotaUsed: FREE_PLAN_DAILY_LIMIT, + dailyQuotaObservedAt: ts(NOON - 24 * 60 * 60 * 1000), + }, + NOON + ).available + ).toBe(true); + }); + + test("a missing quota reading is not treated as zero", () => { + // On a paid plan Resend stops sending x-resend-daily-quota. Absent must + // read as "no daily cap applies", which is what makes this module turn + // itself off on upgrade instead of needing to be removed. + expect( + verificationAvailability({ dailyQuotaObservedAt: ts(NOON) }, NOON).available + ).toBe(true); + expect( + verificationAvailability( + { dailyQuotaUsed: "lots", dailyQuotaObservedAt: ts(NOON) }, + NOON + ).available + ).toBe(true); + }); +}); + +describe("verificationAvailability, on an implausible reading", () => { + test("an out-of-range reading does not hold verification shut", () => { + // A stored 2,900 is not "today's sends on a free plan" whatever else it + // is -- the monthly counter, say. Acting on it would refuse every + // researcher a code indefinitely, and because each send rewrites + // dailyQuotaObservedAt, the staleness escape hatch would never fire. + expect( + verificationAvailability( + { dailyQuotaUsed: 2900, dailyQuotaObservedAt: ts(NOON - 60_000) }, + NOON + ) + ).toEqual({ available: true }); + }); +}); + +describe("deliveryPaused", () => { + test("tracks unavailableUntil only, and ignores the verification reserve", () => { + // The reserve exists to keep the realtime path off the last few sends. The + // sweeper is the consumer those sends are being reserved FOR, so it must + // not be stopped by them. + expect( + deliveryPaused( + { + dailyQuotaUsed: VERIFICATION_CEILING, + dailyQuotaObservedAt: ts(NOON - 1000), + }, + NOON + ) + ).toBe(false); + + expect(deliveryPaused({ unavailableUntil: ts(MIDNIGHT_AFTER) }, NOON)).toBe(true); + expect(deliveryPaused({ unavailableUntil: ts(NOON - 1) }, NOON)).toBe(false); + expect(deliveryPaused(undefined, NOON)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// 3. May this failed mail be swept? +// --------------------------------------------------------------------------- + +describe("sweepDecision", () => { + const doc = (delivery, extra = {}) => ({ + to: ["researcher@example.edu"], + datapipe: { kind: "upload-failure", owner: "uid-1", ...(extra.datapipe ?? {}) }, + delivery: { state: "ERROR", retryable: true, ...delivery }, + }); + + test("sweeps a refused error at any age inside the age bound", () => { + // Quota is the case the sweeper exists for, and it is provably un-sent: + // Resend refused the request, so there is no message to duplicate. + expect( + sweepDecision( + doc({ + error: { name: "daily_quota_exceeded", message: "quota" }, + lastAttemptAt: ts(NOON - 2 * 60 * 60 * 1000), + }), + NOON + ) + ).toBe("deliver"); + }); + + test("sweeps a refused error even PAST the idempotency window", () => { + // The distinction that makes the sweeper safe. A refusal means Resend + // never accepted anything, so the 24-hour Idempotency-Key window is + // irrelevant -- there is nothing for it to deduplicate. + expect( + sweepDecision( + doc({ + error: { name: "ECONNREFUSED", message: "refused" }, + lastAttemptAt: ts(NOON - 2 * IDEMPOTENCY_WINDOW_MS), + }), + NOON + ) + ).toBe("deliver"); + }); + + test("sweeps a 5xx only INSIDE the idempotency window", () => { + // A 500 may or may not have been accepted. Inside the window the + // Idempotency-Key makes a retry a no-op; outside it, the retry is a coin + // flip on a second copy, so we give up rather than gamble. + const inside = doc({ + error: { name: "application_error", message: "boom" }, + startTime: ts(NOON - 60 * 60 * 1000), + lastAttemptAt: ts(NOON - 60 * 60 * 1000), + }); + expect(sweepDecision(inside, NOON)).toBe("deliver"); + + const outside = doc({ + error: { name: "application_error", message: "boom" }, + startTime: ts(NOON - IDEMPOTENCY_WINDOW_MS - 60_000), + lastAttemptAt: ts(NOON - IDEMPOTENCY_WINDOW_MS - 60_000), + }); + // Terminal, not skipped: it can never become deliverable again -- the key + // has expired and only gets older -- so leaving it "retryable" would keep + // expireAt off the document and hold the address forever, while occupying + // a slot in every future pass. + expect(sweepDecision(outside, NOON)).toBe("age-out"); + }); + + test("measures the idempotency window from startTime, NOT from the last attempt", () => { + // THE BUG THIS PINS. The Idempotency-Key is the document id, and Resend + // expires it 24 hours after its FIRST use. Measuring the window from + // lastAttemptAt slides it forward with every retry: a mail first attempted + // at T0 and retried at T0+20h is "20 hours old" at T0+40h and would be + // sent again on a key that expired at T0+24h -- which Resend treats as a + // new message, and the researcher gets a second copy. + const retriedTwice = doc({ + error: { name: "application_error", message: "boom" }, + startTime: ts(NOON - 40 * 60 * 60 * 1000), + lastAttemptAt: ts(NOON - 20 * 60 * 60 * 1000), + }); + expect(sweepDecision(retriedTwice, NOON)).toBe("age-out"); + + // ...and the same document while the key is genuinely still live. + const stillInside = doc({ + error: { name: "application_error", message: "boom" }, + startTime: ts(NOON - 20 * 60 * 60 * 1000), + lastAttemptAt: ts(NOON - 60 * 60 * 1000), + }); + expect(sweepDecision(stillInside, NOON)).toBe("deliver"); + }); + + test("NEVER sends inline mail, whatever the error says -- it ends it instead", () => { + // A verification code is realtime. Delivering one an hour late is not a + // late success -- it may already have expired, and the researcher has long + // since given up or requested another. Terminal rather than skipped: a + // skipped document keeps `retryable: true`, never gets expireAt, and holds + // its recipient's address outside the TTL's reach for good. + expect( + sweepDecision( + doc( + { + error: { name: "daily_quota_exceeded", message: "quota" }, + startTime: ts(NOON - 60_000), + lastAttemptAt: ts(NOON - 60_000), + }, + { datapipe: { kind: "contact-email-verification", deliverInline: true } } + ), + NOON + ) + ).toBe("age-out"); + }); + + test("recovers a claim that was abandoned mid-send", () => { + // deliverMailDocument's claim writes PROCESSING with `retryable: null` + // BEFORE the send, so an instance killed inside the send leaves a document + // that the retryable-ERROR query cannot see and the TTL cannot reap. An + // expired lease is the proof the claimant is dead. + const stranded = { + to: ["researcher@example.edu"], + datapipe: { kind: "upload-failure", owner: "uid-1" }, + delivery: { + state: "PROCESSING", + retryable: null, + attempts: 1, + startTime: ts(NOON - 30 * 60 * 1000), + leaseExpiresAt: ts(NOON - 60_000), + }, + }; + expect(sweepDecision(stranded, NOON)).toBe("deliver"); + + // A LIVE lease is the one case that is genuinely somebody else's: another + // invocation may be inside the send right now. + expect( + sweepDecision( + { + ...stranded, + delivery: { ...stranded.delivery, leaseExpiresAt: ts(NOON + 60_000) }, + }, + NOON + ) + ).toBe("skip"); + }); + + test("ends a stranded claim rather than resending it once the key has expired", () => { + // Same document, a day and a half later. Nothing knows whether the send + // went out, and the Idempotency-Key that would have made a retry safe is + // gone -- so this is exactly the coin flip the sweeper declines. + expect( + sweepDecision( + { + to: ["researcher@example.edu"], + datapipe: { kind: "upload-failure", owner: "uid-1" }, + delivery: { + state: "PROCESSING", + attempts: 1, + startTime: ts(NOON - IDEMPOTENCY_WINDOW_MS - 60_000), + leaseExpiresAt: ts(NOON - IDEMPOTENCY_WINDOW_MS), + }, + }, + NOON + ) + ).toBe("age-out"); + }); + + test("ends a document that has spent its whole attempt budget", () => { + // Otherwise deliverMailDocument answers "skipped-attempts-exhausted" on + // every pass forever, and the document sits in the query eating the budget + // a deliverable notification needed. + expect( + sweepDecision( + doc({ + error: { name: "daily_quota_exceeded", message: "quota" }, + attempts: MAX_ATTEMPTS, + startTime: ts(NOON - 60_000), + lastAttemptAt: ts(NOON - 60_000), + }), + NOON + ) + ).toBe("age-out"); + }); + + test("ages out anything past the age bound, rather than skipping it", () => { + // Terminal, not skip, and the difference matters: a retryable ERROR never + // gets delivery.expireAt, so skipping would leave the document holding a + // researcher's address outside the TTL policy's reach forever. + expect( + sweepDecision( + doc({ + error: { name: "daily_quota_exceeded", message: "quota" }, + lastAttemptAt: ts(NOON - MAX_SWEEP_AGE_MS - 60_000), + }), + NOON + ) + ).toBe("age-out"); + }); + + test("falls back to startTime when lastAttemptAt is absent", () => { + // Documents written before lastAttemptAt existed still have to age out. + expect( + sweepDecision( + doc({ + error: { name: "daily_quota_exceeded", message: "quota" }, + startTime: ts(NOON - MAX_SWEEP_AGE_MS - 60_000), + }), + NOON + ) + ).toBe("age-out"); + }); + + test("a document with NO usable clock ages out rather than living forever", () => { + // An older deploy's write, a hand edit during an incident, a partially + // applied update. With neither timestamp there is no age at which it would + // ever cross the age bound, so a "skip" here is permanent: no expireAt is + // ever written, the TTL never reaps it, and it holds a researcher's address + // for good -- the precise hole ageing out exists to close. + expect( + sweepDecision( + doc({ error: { name: "application_error", message: "boom" } }), + NOON + ) + ).toBe("age-out"); + }); + + test("ignores anything that is not a live retryable error", () => { + expect(sweepDecision(doc({ state: "SUCCESS", retryable: false }), NOON)).toBe( + "skip" + ); + expect(sweepDecision(doc({ retryable: false }), NOON)).toBe("skip"); + expect(sweepDecision({ to: ["x@example.edu"] }, NOON)).toBe("skip"); + }); + + test("treats an error with no usable name as ambiguous, not as unswept work", () => { + // Not knowing what happened is the definition of ambiguous, and ambiguity + // is what the Idempotency-Key resolves: inside the window a retry is a + // no-op at Resend, so it is safe; outside it, this becomes terminal like + // every other ambiguity. What it must never be is a permanent skip -- that + // is a document nothing ever writes again, in a query nothing ever + // finishes. + expect( + sweepDecision( + doc({ error: {}, startTime: ts(NOON - 1000), lastAttemptAt: ts(NOON - 1000) }), + NOON + ) + ).toBe("deliver"); + expect( + sweepDecision( + doc({ + error: {}, + startTime: ts(NOON - IDEMPOTENCY_WINDOW_MS - 1000), + lastAttemptAt: ts(NOON - IDEMPOTENCY_WINDOW_MS - 1000), + }), + NOON + ) + ).toBe("age-out"); + }); +}); + +// --------------------------------------------------------------------------- +// 4. May this researcher's unuploaded data be destroyed? +// --------------------------------------------------------------------------- + +describe("retentionDecision", () => { + const DAY = 24 * 60 * 60 * 1000; + // The sweep only ever asks about entries already older than seven days. + const entry = (over = {}) => ({ + status: "failed", + retryCount: 5, + maxRetries: 5, + createdAt: ts(NOON - 8 * DAY), + ...over, + }); + + test("deletes an ordinary aged-out entry, exactly as before", () => { + // The unchanged default. Nothing below should make the common case keep + // data longer than it used to. + expect(retentionDecision(entry(), NOON)).toBe("delete"); + }); + + test("retains an entry whose upload is still being retried", () => { + // The storage-provider outage case: this would have uploaded fine on day + // eight, so deleting it on day seven throws away data that was never + // actually lost. + expect( + retentionDecision(entry({ status: "pending", retryCount: 3 }), NOON) + ).toBe("retain"); + }); + + test("deletes a pending entry whose retries are exhausted", () => { + // "Pending" alone is not a reason to keep it -- an entry that has spent its + // whole retry budget is not live work, it is a corpse with a hopeful status. + expect( + retentionDecision(entry({ status: "pending", retryCount: 5 }), NOON) + ).toBe("delete"); + }); + + test("retains an entry the researcher has not been told about", () => { + expect( + retentionDecision(entry({ retainUntil: ts(NOON + 2 * DAY) }), NOON) + ).toBe("retain"); + }); + + test("deletes once the extension itself has expired", () => { + expect( + retentionDecision(entry({ retainUntil: ts(NOON - 1000) }), NOON) + ).toBe("delete"); + }); + + test("the absolute ceiling beats every reason to keep it", () => { + // Without this, an experiment whose provider is dead and whose owner never + // reads their mail would hold research payloads in Cloud Storage forever, + // silently, at DataPipe's cost. Both extension paths are tested against it + // because either one alone would otherwise be unbounded. + const ancient = { createdAt: ts(NOON - 15 * DAY) }; + expect( + retentionDecision( + entry({ ...ancient, status: "pending", retryCount: 0 }), + NOON + ) + ).toBe("delete"); + expect( + retentionDecision( + entry({ ...ancient, retainUntil: ts(NOON + 5 * DAY) }), + NOON + ) + ).toBe("delete"); + }); + + test("a missing or unreadable createdAt does not disable the ceiling check", () => { + // Defensive: a hand-written or partially-migrated entry must not become + // immortal by lacking a field. + expect(retentionDecision({ status: "failed" }, NOON)).toBe("delete"); + }); +}); diff --git a/functions/src/__tests__/mail-delivery.test.js b/functions/src/__tests__/mail-delivery.test.js index 369fa90..a9950a1 100644 --- a/functions/src/__tests__/mail-delivery.test.js +++ b/functions/src/__tests__/mail-delivery.test.js @@ -41,6 +41,8 @@ let LEASE_MS; let MAX_ATTEMPTS; let CONFIG_MISSING_ERROR; let INVALID_DOCUMENT_ERROR; +let quotaFromHeaders; +let resendSender; beforeAll(async () => { ({ @@ -54,6 +56,8 @@ beforeAll(async () => { MAX_ATTEMPTS, CONFIG_MISSING_ERROR, INVALID_DOCUMENT_ERROR, + quotaFromHeaders, + resendSender, } = await import("../../lib/mail-delivery.js")); }); @@ -474,3 +478,115 @@ describe("configuration", () => { expect(classifyMailError(error).retryable).toBe(false); }); }); + +// --------------------------------------------------------------------------- +// 5. The transport, and the two header names the breaker depends on +// --------------------------------------------------------------------------- +// +// WHY THIS IS WORTH ITS OWN SECTION. Everything else in the mail suite mocks +// the sender, which means it hands the delivery path an ALREADY-PARSED +// `dailyQuotaUsed` -- the exact value the transport is supposed to produce. So +// the header names, their casing on a real Headers object, and what +// Number.parseInt does to a value that is not a bare integer were the one part +// of the proactive breaker that no test touched: misspell "x-resend-daily-quota" +// and every send returns no reading, mail-availability.ts never learns a +// number, verification never stops at the reserve, and every other test still +// passes. + +describe("quotaFromHeaders", () => { + test("reads both quota headers, case-insensitively, off a real Headers", () => { + // Headers lower-cases its keys, which is why a real one is used here rather + // than a plain object with a get(). + const headers = new Headers({ + "X-Resend-Daily-Quota": "94", + "x-resend-monthly-quota": "2412", + }); + expect(quotaFromHeaders(headers)).toEqual({ + dailyQuotaUsed: 94, + monthlyQuotaUsed: 2412, + }); + }); + + test("absent headers leave the field out, rather than reporting zero", () => { + // Resend sends the daily header to free-plan accounts only, so absent has + // to read as "no daily cap applies". A 0 here would be a permanent claim + // that nothing has been sent today. + expect(quotaFromHeaders(new Headers())).toEqual({}); + expect(quotaFromHeaders(new Headers({ "x-resend-daily-quota": "0" }))).toEqual({ + dailyQuotaUsed: 0, + }); + }); + + test("an unparseable value is no reading at all", () => { + expect(quotaFromHeaders(new Headers({ "x-resend-daily-quota": "" }))).toEqual({}); + expect( + quotaFromHeaders(new Headers({ "x-resend-daily-quota": "unlimited" })) + ).toEqual({}); + }); +}); + +describe("resendSender", () => { + const okResponse = (body, headers = {}) => + new Response(JSON.stringify(body), { status: 200, headers }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test("sends the id and the idempotency key, and returns the quota reading", async () => { + const fetchMock = jest + .spyOn(globalThis, "fetch") + .mockResolvedValue( + okResponse({ id: "resend-1" }, { "x-resend-daily-quota": "94" }) + ); + + const result = await resendSender(CONFIG)( + { from: CONFIG.from, to: ["researcher@example.edu"], subject: "s", text: "t" }, + { timeoutMs: 1000, idempotencyKey: "mail-doc-1" } + ); + + expect(result).toEqual({ id: "resend-1", dailyQuotaUsed: 94 }); + + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toBe("https://api.resend.com/emails"); + // The document id as the Idempotency-Key is what makes an ambiguous + // timeout safe to retry at all (see AMBIGUOUS_ERRORS). + expect(init.headers["Idempotency-Key"]).toBe("mail-doc-1"); + expect(init.headers.Authorization).toBe(`Bearer ${CONFIG.apiKey}`); + }); + + test("a 2xx with an unreadable body is still a send, and still a reading", async () => { + // Losing the message id costs an audit trail, not a delivery -- and the + // headers are read before the body precisely so a bad body cannot cost the + // quota reading too. + jest.spyOn(globalThis, "fetch").mockResolvedValue( + new Response("not json", { + status: 200, + headers: { "x-resend-daily-quota": "12" }, + }) + ); + + await expect( + resendSender(CONFIG)( + { from: CONFIG.from, to: ["researcher@example.edu"], subject: "s", text: "t" }, + { timeoutMs: 1000, idempotencyKey: "mail-doc-2" } + ) + ).resolves.toEqual({ dailyQuotaUsed: 12 }); + }); + + test("a refusal throws with Resend's own name, so the taxonomy can classify it", async () => { + jest.spyOn(globalThis, "fetch").mockResolvedValue( + new Response( + JSON.stringify({ name: "daily_quota_exceeded", message: "over quota" }), + { status: 429 } + ) + ); + + await expect( + resendSender(CONFIG)( + { from: CONFIG.from, to: ["researcher@example.edu"], subject: "s", text: "t" }, + { timeoutMs: 1000, idempotencyKey: "mail-doc-3" } + ) + ).rejects.toMatchObject({ name: "daily_quota_exceeded", status: 429 }); + }); +}); diff --git a/functions/src/__tests__/mail-retry-emulator.test.js b/functions/src/__tests__/mail-retry-emulator.test.js new file mode 100644 index 0000000..846c56e --- /dev/null +++ b/functions/src/__tests__/mail-retry-emulator.test.js @@ -0,0 +1,765 @@ +/** + * @jest-environment node + */ + +// The quota breaker and the retry sweeper, against the Firestore emulator. +// +// Harness conventions are mail-delivery-emulator.test.js's, for the same +// reasons: emulator env at module scope before any import that reaches app.js, +// a NAMED admin app, a dynamic import of the COMPILED module from +// functions/lib/, and scoped cleanup via a registrar of created refs rather +// than a collection-wide wipe. +// +// THREE SEAMS ARE INJECTED HERE, AND ONLY THE FIRST IS ABOUT CONVENIENCE. +// +// _setMailSenderForTests the transport, so nothing reaches api.resend.com. +// _setMailStatusDocForTests the breaker document. +// _setMailCollectionForTests the mail collection. +// +// The last two are about the same hazard, which is SHARED SINGLETONS in an +// emulator that suites run against in parallel. +// +// The breaker lives at `systemStatus/mail`. Half the assertions below require +// it to be SHUT, and a shut breaker makes every other suite that sends mail +// fail -- including contact-email-verify-emulator.test.js, which drives the +// real deployed endpoint over HTTP and would start getting 503s. +// +// The `mail` COLLECTION is the other one, and it took a review to notice. +// sweepRetryableMail is a QUERY over the whole collection, so under +// `--maxWorkers=2` this suite would pick up mail-delivery-emulator.test.js's +// fixtures, deliver them through THIS suite's injected transport, and rewrite +// them mid-assertion -- while its own exact-count assertions (`retained` is 3, +// `agedOut` is 1, the sender was called once) failed for reasons that had +// nothing to do with the code under test. Both are suites failing in a +// different place each run, which is the worst kind of bug to chase. + +process.env.FIRESTORE_EMULATOR_HOST = "localhost:8080"; +process.env.GCLOUD_PROJECT = "datapipe-test"; +process.env.TOKEN_ENCRYPTION_KEY ||= "aa".repeat(32); +process.env.FIREBASE_CONFIG = JSON.stringify({ + projectId: "datapipe-test", + storageBucket: "datapipe-test.appspot.com", +}); + +// cleanupOldEntries deletes the payload before the queue entry, so the sweep +// has to have somewhere to send that delete. 404 from the emulator is fine -- +// the delete is wrapped precisely because the object may already be gone. +process.env.FIREBASE_STORAGE_EMULATOR_HOST = "localhost:9199"; + +process.env.RESEND_API_KEY = "re_test_not_a_real_key"; +process.env.MAIL_FROM = "DataPipe (test) "; + +import { initializeApp, getApp } from "firebase-admin/app"; +import { getFirestore, Timestamp } from "firebase-admin/firestore"; +import { randomUUID } from "crypto"; + +// Block 4 imports the compiled scheduled-upload-retry.js, which pulls in every +// provider adapter, each importing ESM-only "node-fetch" at module scope -- +// which Jest's CJS transform cannot parse. Stubbed exactly as +// payload-encryption-emulator.test.js and upload-queue.test.js do. Nothing here +// reaches a provider; the deletion sweep only touches Firestore and Storage. +jest.mock("node-fetch", () => ({ + __esModule: true, + default: jest.fn(), +})); + +jest.setTimeout(30000); + +let db; +let deliverMailDocument; +let _setMailSenderForTests; +let sweepRetryableMail; +let _setMailStatusDocForTests; +let _setMailCollectionForTests; +let cleanupOldEntries; +let MAX_SWEEP_AGE_MS; +let MAIL_RETENTION_MS; +let IDEMPOTENCY_WINDOW_MS; +let SWEEP_ABANDONED_ERROR; +let RETENTION_GRACE_MS; + +// This suite's private breaker document, and its private mail collection. +const STATUS_DOC_ID = `mail-test-${randomUUID()}`; +const MAIL_COLLECTION_ID = `mail-test-${randomUUID()}`; + +beforeAll(async () => { + let app; + try { + app = getApp("mail-retry-test"); + } catch { + app = initializeApp({ projectId: "datapipe-test" }, "mail-retry-test"); + } + db = getFirestore(app); + + ({ deliverMailDocument, _setMailSenderForTests, MAIL_RETENTION_MS } = await import( + "../../lib/mail-delivery.js" + )); + ({ + sweepRetryableMail, + MAX_SWEEP_AGE_MS, + IDEMPOTENCY_WINDOW_MS, + SWEEP_ABANDONED_ERROR, + } = await import("../../lib/scheduled-mail-retry.js")); + ({ _setMailStatusDocForTests } = await import("../../lib/mail-availability.js")); + ({ _setMailCollectionForTests } = await import("../../lib/mail.js")); + ({ cleanupOldEntries } = await import("../../lib/scheduled-upload-retry.js")); + ({ RETENTION_GRACE_MS } = await import("../../lib/upload-retention.js")); + + _setMailStatusDocForTests(STATUS_DOC_ID); + _setMailCollectionForTests(MAIL_COLLECTION_ID); +}); + +afterAll(async () => { + _setMailStatusDocForTests(null); + _setMailCollectionForTests(null); + await db.collection("systemStatus").doc(STATUS_DOC_ID).delete().catch(() => {}); +}); + +const created = []; +const RECIPIENT = "researcher@example.edu"; + +async function seedMail({ delivery, inline = false, kind = "upload-failure" } = {}) { + const ref = db.collection(MAIL_COLLECTION_ID).doc(); + created.push(ref); + await ref.set({ + to: [RECIPIENT], + message: { + subject: "DataPipe couldn't upload data for Working Memory Span", + text: "The file is not lost.", + html: "

The file is not lost.

", + }, + datapipe: { + kind, + owner: `mr-user-${randomUUID()}`, + experimentID: `mr-exp-${randomUUID()}`, + queuedAt: Timestamp.now(), + ...(inline ? { deliverInline: true } : {}), + }, + ...(delivery ? { delivery } : {}), + }); + return { ref, id: ref.id }; +} + +// One owner for every queue entry this suite creates. uploadQueue is shared +// with every other suite, and cleanupOldEntries below sweeps it BY AGE with no +// other filter -- so the deletion tests are scoped to this owner the same way +// retryPendingUploads' ownerScope seam scopes the retry tests. +const OWNER_ID = `mr-user-${randomUUID()}`; + +async function seedQueueEntry( + experimentID, + { status = "failed", ageMs = 8 * 24 * 60 * 60 * 1000, ...rest } = {} +) { + const ref = db.collection("uploadQueue").doc(); + created.push(ref); + await ref.set({ + experimentID, + owner: OWNER_ID, + status, + retryCount: 5, + maxRetries: 5, + storagePath: `pending-data/${experimentID}/subject-1.json`, + createdAt: Timestamp.fromMillis(Date.now() - ageMs), + ...rest, + }); + return ref; +} + +const statusRef = () => db.collection("systemStatus").doc(STATUS_DOC_ID); +const deliveryOf = async (ref) => (await ref.get()).data()?.delivery; + +function sendingOk(id = "resend-ok", headers = {}) { + return jest.fn().mockResolvedValue({ id, ...headers }); +} + +function sendingError(name, extra = {}) { + return jest.fn().mockRejectedValue( + Object.assign(new Error(`${name} happened`), { name }, extra) + ); +} + +// A retryable failure already on the document, as finish() would have left it. +function failedDelivery(name, agoMs = 60_000, attempts = 1) { + const at = Timestamp.fromMillis(Date.now() - agoMs); + return { + state: "ERROR", + retryable: true, + attempts, + error: { name, message: `${name} happened` }, + startTime: at, + lastAttemptAt: at, + leaseExpiresAt: null, + endTime: null, + }; +} + +let errorSpy; +let warnSpy; +let logSpy; + +beforeEach(() => { + errorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + logSpy = jest.spyOn(console, "log").mockImplementation(() => {}); +}); + +afterEach(async () => { + _setMailSenderForTests(null); + jest.restoreAllMocks(); + await statusRef().delete().catch(() => {}); + const batch = db.batch(); + while (created.length) batch.delete(created.pop()); + await batch.commit(); +}); + +// --------------------------------------------------------------------------- +// 1. The breaker gets written by delivery +// --------------------------------------------------------------------------- + +describe("recording quota state", () => { + test("a successful send records the daily quota reading from the header", async () => { + // The proactive signal. x-resend-daily-quota rides on SUCCESS responses, so + // the breaker learns we are at 94/100 while sending still works. + _setMailSenderForTests(sendingOk("resend-1", { dailyQuotaUsed: 94 })); + const { id } = await seedMail(); + + expect(await deliverMailDocument(id)).toBe("sent"); + + const status = (await statusRef().get()).data(); + expect(status.dailyQuotaUsed).toBe(94); + expect(status.dailyQuotaObservedAt.toMillis()).toBeGreaterThan(0); + expect(status.unavailableUntil).toBeNull(); + }); + + test("a quota failure shuts the breaker until the next UTC midnight", async () => { + _setMailSenderForTests(sendingError("daily_quota_exceeded", { status: 429 })); + const { id } = await seedMail(); + + expect(await deliverMailDocument(id)).toBe("retryable-error"); + + const status = (await statusRef().get()).data(); + expect(status.reason).toBe("daily_quota_exceeded"); + expect(status.unavailableUntil.toMillis()).toBeGreaterThan(Date.now()); + // Pinned at the limit: a stale lower reading from earlier in the day must + // not keep claiming the verification reserve is untouched. + expect(status.dailyQuotaUsed).toBe(100); + }); + + test("a MONTHLY cap shuts sending until the month turns, not until midnight", async () => { + // The two free-plan caps reset on different clocks. Treated as a daily + // one, a monthly exhaustion reopens the breaker at midnight, the sweeper + // probes into a cap with days left to run, and each probe spends one of a + // queued mail's three attempts -- three nights and every queued + // notification is terminal. + _setMailSenderForTests(sendingError("monthly_quota_exceeded", { status: 429 })); + const { id } = await seedMail(); + + expect(await deliverMailDocument(id)).toBe("retryable-error"); + + const status = (await statusRef().get()).data(); + expect(status.reason).toBe("monthly_quota_exceeded"); + const nextMidnight = Date.UTC( + new Date().getUTCFullYear(), + new Date().getUTCMonth(), + new Date().getUTCDate() + 1 + ); + expect(status.unavailableUntil.toMillis()).toBeGreaterThanOrEqual(nextMidnight); + // ...and the DAILY counter is left alone. A monthly cap says nothing about + // today's sends, so pinning it at 100 would go on blocking verification on + // the reserve rule after the daily counter had reset. + expect(status.dailyQuotaUsed).toBeUndefined(); + }); + + test("a revoked key shuts the breaker too, briefly -- quota is not the only way to be unable to send", async () => { + // Without this the breaker only ever shut on quota, so a revoked key left + // verificationAvailability answering "available" indefinitely: every click + // minted a code, wrote a mail document and spent a real Resend request, and + // nothing anywhere counted them. + _setMailSenderForTests(sendingError("suspended_api_key", { status: 403 })); + const { id } = await seedMail(); + + expect(await deliverMailDocument(id)).toBe("terminal-error"); + + const status = (await statusRef().get()).data(); + expect(status.reason).toBe("suspended_api_key"); + expect(status.unavailableUntil.toMillis()).toBeGreaterThan(Date.now()); + // Minutes, not "until the quota resets": nothing resets, so the pause is + // only there to stop a loop of failing sends until a human sees the logs. + expect(status.unavailableUntil.toMillis()).toBeLessThan(Date.now() + 60 * 60 * 1000); + }); + + test("a per-message failure does NOT shut sending for everybody", async () => { + // validation_error is Resend's name both for an unverified sending domain + // and for one malformed recipient address. One researcher's typo must not + // switch verification off for every other researcher. + _setMailSenderForTests(sendingError("validation_error", { status: 422 })); + const { id } = await seedMail(); + + expect(await deliverMailDocument(id)).toBe("terminal-error"); + expect((await statusRef().get()).exists).toBe(false); + }); + + test("a later success reopens the breaker -- this is the half-open close", async () => { + _setMailSenderForTests(sendingError("daily_quota_exceeded", { status: 429 })); + const first = await seedMail(); + await deliverMailDocument(first.id); + expect((await statusRef().get()).data().unavailableUntil).not.toBeNull(); + + _setMailSenderForTests(sendingOk("resend-2", { dailyQuotaUsed: 3 })); + const second = await seedMail(); + expect(await deliverMailDocument(second.id)).toBe("sent"); + + const status = (await statusRef().get()).data(); + expect(status.unavailableUntil).toBeNull(); + expect(status.dailyQuotaUsed).toBe(3); + }); + + test("a send with no quota header leaves no reading, rather than recording zero", async () => { + // Resend omits the header on paid plans. Absent must read as "no daily cap + // applies", not as "0 used" and certainly not as a stale 0 that survives. + _setMailSenderForTests(sendingOk("resend-3")); + const { id } = await seedMail(); + await deliverMailDocument(id); + + const status = (await statusRef().get()).data(); + expect(status.dailyQuotaUsed).toBeUndefined(); + expect(status.unavailableUntil).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// 2. Inline mail is terminal, because nothing will ever retry it +// --------------------------------------------------------------------------- + +describe("inline mail", () => { + test("a retryable-class error is TERMINAL when the mail is delivered inline", async () => { + // A verification code is realtime: its sender is holding an HTTP request + // open and the sweeper skips it forever. Calling it "retryable" would be a + // lie that also costs something -- a retryable error is never given + // delivery.expireAt, so the document would sit outside the TTL's reach + // holding an address indefinitely. + _setMailSenderForTests(sendingError("daily_quota_exceeded", { status: 429 })); + const { ref, id } = await seedMail({ + inline: true, + kind: "contact-email-verification", + }); + + expect(await deliverMailDocument(id)).toBe("terminal-error"); + + const delivery = await deliveryOf(ref); + expect(delivery.state).toBe("ERROR"); + expect(delivery.retryable).toBe(false); + // Terminal means the TTL can reap it, which is the whole point. + expect(delivery.endTime.toMillis()).toBeGreaterThan(0); + expect(delivery.expireAt.toMillis()).toBeGreaterThan(Date.now()); + }); + + test("the same error on QUEUED mail stays retryable", async () => { + // The contrast that proves the branch is about inline-ness, not about the + // error. + _setMailSenderForTests(sendingError("daily_quota_exceeded", { status: 429 })); + const { ref, id } = await seedMail(); + + expect(await deliverMailDocument(id)).toBe("retryable-error"); + const delivery = await deliveryOf(ref); + expect(delivery.retryable).toBe(true); + expect(delivery.endTime).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// 3. The sweep +// --------------------------------------------------------------------------- + +describe("sweepRetryableMail", () => { + test("SENDS nothing while the breaker is shut, and burns no attempts", async () => { + // The most important send-side assertion in this file. Sweeping into an + // exhausted quota fails every document and spends one of its three + // MAX_ATTEMPTS doing it -- so a day-long outage would exhaust the retry + // budget of every queued mail and turn all of them terminal, which is the + // exact opposite of what the sweeper is for. + const send = sendingOk(); + _setMailSenderForTests(send); + const { ref } = await seedMail({ delivery: failedDelivery("daily_quota_exceeded") }); + await statusRef().set({ + unavailableUntil: Timestamp.fromMillis(Date.now() + 60 * 60 * 1000), + reason: "daily_quota_exceeded", + }); + + const report = await sweepRetryableMail(); + + expect(report.paused).toBe(true); + expect(send).not.toHaveBeenCalled(); + // Untouched: attempts did not creep up by being looked at. + expect((await deliveryOf(ref)).attempts).toBe(1); + }); + + test("delivers a quota-failed mail once the breaker has expired", async () => { + const send = sendingOk("resend-swept", { dailyQuotaUsed: 5 }); + _setMailSenderForTests(send); + const { ref } = await seedMail({ delivery: failedDelivery("daily_quota_exceeded") }); + // Yesterday's breaker, already expired. + await statusRef().set({ + unavailableUntil: Timestamp.fromMillis(Date.now() - 1000), + }); + + const report = await sweepRetryableMail(); + + expect(report.paused).toBe(false); + expect(report.delivered).toBeGreaterThanOrEqual(1); + expect(send).toHaveBeenCalled(); + + const delivery = await deliveryOf(ref); + expect(delivery.state).toBe("SUCCESS"); + expect(delivery.attempts).toBe(2); + expect(delivery.info.transport).toBe("resend"); + // ...and the successful probe reopened the realtime path. + expect((await statusRef().get()).data().unavailableUntil).toBeNull(); + }); + + test("never SENDS inline mail -- it ends it, so the TTL can have the address", async () => { + // Belt and braces: inline failures are already marked terminal, so this + // document should not exist. If one ever does -- a hand edit, an older + // deploy's write -- the sweeper still must not resurrect a verification + // code that expired hours ago. Ending it rather than skipping it is what + // stops it sitting in the query forever with no expireAt, holding a + // researcher's address outside the TTL policy's reach. + const send = sendingOk(); + _setMailSenderForTests(send); + const { ref } = await seedMail({ + inline: true, + kind: "contact-email-verification", + delivery: failedDelivery("daily_quota_exceeded"), + }); + + const report = await sweepRetryableMail(); + + expect(send).not.toHaveBeenCalled(); + expect(report.agedOut).toBe(1); + const delivery = await deliveryOf(ref); + expect(delivery.state).toBe("ERROR"); + expect(delivery.retryable).toBe(false); + expect(delivery.expireAt.toMillis()).toBeGreaterThan(Date.now()); + }); + + test("recovers a claim that was abandoned mid-send", async () => { + // THE HOLE THIS CLOSES. The claim transaction rewrites the document to + // PROCESSING with `retryable: null` BEFORE the send, so an instance + // preempted (or rolled by a deploy) inside the send left a document that + // the retryable-ERROR query could not see and the TTL could not reap -- + // holding a researcher's address indefinitely, which is the exact failure + // the sweeper exists to prevent. + const send = sendingOk("resend-recovered", { dailyQuotaUsed: 7 }); + _setMailSenderForTests(send); + const at = Timestamp.fromMillis(Date.now() - 30 * 60 * 1000); + const { ref } = await seedMail({ + delivery: { + state: "PROCESSING", + attempts: 1, + retryable: null, + startTime: at, + // Expired: LEASE_MS is five minutes, so its claimant is provably dead. + leaseExpiresAt: Timestamp.fromMillis(Date.now() - 60_000), + endTime: null, + }, + }); + + const report = await sweepRetryableMail(); + + expect(report.delivered).toBe(1); + const delivery = await deliveryOf(ref); + expect(delivery.state).toBe("SUCCESS"); + expect(delivery.attempts).toBe(2); + }); + + test("leaves a claim alone while its lease is still live", async () => { + // The other half: an invocation may be inside the send right now, and two + // senders on one document is the double-send the whole claim machinery + // exists to prevent. + const send = sendingOk(); + _setMailSenderForTests(send); + const { ref } = await seedMail({ + delivery: { + state: "PROCESSING", + attempts: 1, + startTime: Timestamp.now(), + leaseExpiresAt: Timestamp.fromMillis(Date.now() + 4 * 60 * 1000), + }, + }); + + const report = await sweepRetryableMail(); + + expect(send).not.toHaveBeenCalled(); + expect(report.agedOut).toBe(0); + expect((await deliveryOf(ref)).state).toBe("PROCESSING"); + }); + + test("ends an abandoned claim once the idempotency key has expired", async () => { + // Nothing knows whether the send went out, and the key that would have made + // a retry safe is gone -- so this is the coin flip the sweeper declines. + // Terminal, and recorded as such, rather than left as a PROCESSING document + // that the next pass would look at and leave again. + const send = sendingOk(); + _setMailSenderForTests(send); + const old = Timestamp.fromMillis(Date.now() - IDEMPOTENCY_WINDOW_MS - 60_000); + const { ref } = await seedMail({ + delivery: { + state: "PROCESSING", + attempts: 1, + startTime: old, + leaseExpiresAt: old, + }, + }); + + const report = await sweepRetryableMail(); + + expect(send).not.toHaveBeenCalled(); + expect(report.agedOut).toBe(1); + const delivery = await deliveryOf(ref); + expect(delivery.state).toBe("ERROR"); + expect(delivery.retryable).toBe(false); + expect(delivery.error.name).toBe(SWEEP_ABANDONED_ERROR); + expect(delivery.expireAt.toMillis()).toBeGreaterThan(Date.now()); + }); + + test("ages out a mail nobody delivered in time, and lets the TTL have it", async () => { + // Terminal rather than skipped, deliberately: a retryable ERROR never gets + // delivery.expireAt, so skipping would leave this document holding a + // researcher's address outside the TTL policy's reach forever. + const send = sendingOk(); + _setMailSenderForTests(send); + const { ref } = await seedMail({ + delivery: failedDelivery("daily_quota_exceeded", MAX_SWEEP_AGE_MS + 60_000), + }); + + const report = await sweepRetryableMail(); + + expect(report.agedOut).toBe(1); + expect(send).not.toHaveBeenCalled(); + + const delivery = await deliveryOf(ref); + expect(delivery.retryable).toBe(false); + expect(delivery.endTime.toMillis()).toBeGreaterThan(0); + expect(delivery.expireAt.toMillis()).toBeCloseTo( + delivery.endTime.toMillis() + MAIL_RETENTION_MS, + -4 + ); + }); + + test("stops the pass when a send trips the breaker mid-sweep", async () => { + // Without this the remaining documents each fail and each spend an attempt, + // converting one quota outage into an exhausted retry budget. + const send = sendingError("daily_quota_exceeded", { status: 429 }); + _setMailSenderForTests(send); + await seedMail({ delivery: failedDelivery("daily_quota_exceeded") }); + await seedMail({ delivery: failedDelivery("daily_quota_exceeded") }); + await seedMail({ delivery: failedDelivery("daily_quota_exceeded") }); + + const report = await sweepRetryableMail(); + + expect(report.paused).toBe(true); + // One attempt discovers the quota; the rest of the pass stands down. + expect(send).toHaveBeenCalledTimes(1); + }); + + test("holds the data back EVEN WHILE PAUSED -- the case that needs it most", async () => { + // Retention is a Firestore write, not a send: it costs no quota, so an + // exhausted quota is no reason to skip it. The opposite, in fact. A quota + // outage is exactly when a researcher's unuploaded data is ageing towards + // deletion behind a notification that never arrived, so putting this after + // the breaker check would switch the protection off in the only situation + // that needs it. + const send = sendingOk(); + _setMailSenderForTests(send); + const experimentID = `mr-exp-${randomUUID()}`; + const entry = await seedQueueEntry(experimentID); + const { ref } = await seedMail({ delivery: failedDelivery("daily_quota_exceeded") }); + await ref.update({ "datapipe.experimentID": experimentID }); + await statusRef().set({ + unavailableUntil: Timestamp.fromMillis(Date.now() + 60 * 60 * 1000), + }); + + const report = await sweepRetryableMail(); + + expect(report.paused).toBe(true); + expect(send).not.toHaveBeenCalled(); + expect(report.retained).toBe(1); + const retainUntil = (await entry.get()).data().retainUntil; + expect(retainUntil.toMillis()).toBeGreaterThan(Date.now()); + }); + + test("holds back EVERY unresolved entry for the experiment, not just the one that tripped it", async () => { + // A notification is per EPISODE, and an episode belongs to an experiment, + // not to one file. datapipe.queueDocId records only the entry that tripped + // it -- extending just that one would leave the rest of the episode's data + // expiring on schedule, which is the original bug in miniature. + _setMailSenderForTests(sendingOk()); + const experimentID = `mr-exp-${randomUUID()}`; + const entries = [ + await seedQueueEntry(experimentID), + await seedQueueEntry(experimentID), + await seedQueueEntry(experimentID, { status: "pending" }), + ]; + const { ref } = await seedMail({ delivery: failedDelivery("daily_quota_exceeded") }); + await ref.update({ "datapipe.experimentID": experimentID }); + + const report = await sweepRetryableMail(); + + expect(report.retained).toBe(3); + for (const entry of entries) { + expect((await entry.get()).data().retainUntil.toMillis()).toBeGreaterThan( + Date.now() + ); + } + }); + + test("holds the data back on the pass that GIVES UP on the notification", async () => { + // The case that matters most, and the one this used to miss. Retention was + // only extended for mail the sweeper was about to try again, so the moment + // a notification became undeliverable the extensions stopped -- and the + // data it was about went back on the original clock and was deleted, with + // the researcher never having been told anything at all. Giving up on + // telling them is not a reason to shorten their window; it is the reason + // they need it. + _setMailSenderForTests(sendingOk()); + const experimentID = `mr-exp-${randomUUID()}`; + const entry = await seedQueueEntry(experimentID); + const { ref } = await seedMail({ + delivery: failedDelivery("daily_quota_exceeded", MAX_SWEEP_AGE_MS + 60_000), + }); + await ref.update({ "datapipe.experimentID": experimentID }); + + const report = await sweepRetryableMail(); + + expect(report.agedOut).toBe(1); + expect(report.retained).toBe(1); + expect((await entry.get()).data().retainUntil.toMillis()).toBeGreaterThan( + Date.now() + ); + }); + + test("extends an experiment ONCE a pass, however many notifications name it", async () => { + // Two failure episodes for one experiment are two mail documents. Running + // the query and the batch twice would double the writes and count the same + // entries twice in the report -- and the sweep runs every ten minutes for + // as long as the outage lasts, so "twice" is really "tens of thousands of + // redundant writes a day" against a 20,000/day free tier. + _setMailSenderForTests(sendingOk()); + const experimentID = `mr-exp-${randomUUID()}`; + const entry = await seedQueueEntry(experimentID); + for (const _ of [1, 2]) { + const { ref } = await seedMail({ + delivery: failedDelivery("daily_quota_exceeded"), + }); + await ref.update({ "datapipe.experimentID": experimentID }); + } + + const report = await sweepRetryableMail(); + + expect(report.scanned).toBe(2); + expect(report.retained).toBe(1); + expect((await entry.get()).data().retainUntil.toMillis()).toBeGreaterThan( + Date.now() + ); + }); + + test("does not rewrite a retainUntil that is already most of the way out", async () => { + // The same reasoning one level down: a pass every ten minutes must not + // rewrite the same field to nearly the same value each time. The stored + // value can never be closer than half the grace window to expiring, which + // is days of slack on a ten-minute sweep. + _setMailSenderForTests(sendingOk()); + const experimentID = `mr-exp-${randomUUID()}`; + const alreadyExtended = Timestamp.fromMillis( + Date.now() + RETENTION_GRACE_MS - 60_000 + ); + const entry = await seedQueueEntry(experimentID, { + retainUntil: alreadyExtended, + }); + const { ref } = await seedMail({ + delivery: failedDelivery("daily_quota_exceeded"), + }); + await ref.update({ "datapipe.experimentID": experimentID }); + + // Still reported as held back -- it IS held back; it just did not need a + // write to stay that way. + expect((await sweepRetryableMail()).retained).toBe(1); + expect((await entry.get()).data().retainUntil.toMillis()).toBe( + alreadyExtended.toMillis() + ); + }); + + test("holds nothing back for a verification code -- there is no data behind it", async () => { + _setMailSenderForTests(sendingOk()); + const experimentID = `mr-exp-${randomUUID()}`; + const entry = await seedQueueEntry(experimentID); + const { ref } = await seedMail({ + kind: "contact-email-verification", + delivery: failedDelivery("daily_quota_exceeded"), + }); + await ref.update({ "datapipe.experimentID": experimentID }); + + const report = await sweepRetryableMail(); + + expect(report.retained).toBe(0); + expect((await entry.get()).data().retainUntil).toBeUndefined(); + }); + + test("an empty mail collection is a quiet no-op", async () => { + const send = sendingOk(); + _setMailSenderForTests(send); + + const report = await sweepRetryableMail(); + + expect(send).not.toHaveBeenCalled(); + expect(report.delivered).toBe(0); + expect(report.agedOut).toBe(0); + }); +}); + +// --------------------------------------------------------------------------- +// 4. The other end of the story: the sweep that actually deletes +// --------------------------------------------------------------------------- + +describe("cleanupOldEntries", () => { + test("deletes past a queue whose head is all retained entries", async () => { + // THE STARVATION THIS FIXES. The query finds entries by age, ascending, + // and a retained entry is not removed from the result set by being looked + // at -- so with a single limit(50) on the query, fifty retained entries at + // the head of the queue meant a pass that deleted nothing, forever. One + // experiment stuck behind a dead storage provider could stop every OTHER + // experiment's payloads from ever being deleted, until the blockers finally + // crossed the 14-day ceiling up to a week later. + const blocked = `mr-exp-${randomUUID()}`; + const deletable = `mr-exp-${randomUUID()}`; + + // 55 older entries that must be kept: still pending, with retries left. + const batch = db.batch(); + for (let i = 0; i < 55; i += 1) { + const ref = db.collection("uploadQueue").doc(); + created.push(ref); + batch.set(ref, { + experimentID: blocked, + owner: OWNER_ID, + status: "pending", + retryCount: 0, + maxRetries: 5, + storagePath: `pending-data/${blocked}/subject-${i}.json`, + createdAt: Timestamp.fromMillis(Date.now() - 10 * 24 * 60 * 60 * 1000), + }); + } + await batch.commit(); + + // ...and behind them, younger but still aged out, five that may go. + const doomed = []; + for (let i = 0; i < 5; i += 1) { + doomed.push(await seedQueueEntry(deletable)); + } + + await cleanupOldEntries(OWNER_ID); + + for (const entry of doomed) { + expect((await entry.get()).exists).toBe(false); + } + }); +}); diff --git a/functions/src/index.ts b/functions/src/index.ts index 416eeea..75adf20 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -18,6 +18,11 @@ import { onUploadFailure } from "./upload-failure-notify.js"; // denied production access); mail.ts's document contract is unchanged through // both swaps, so nothing on the write side moved. import { onMailCreated } from "./mail-delivery.js"; +// Re-drives mail whose failure has since stopped being true (a quota that has +// rolled over, a blip that has passed). onMailCreated cannot: an +// onDocumentCreated trigger does not re-fire on updates, so before this existed +// a `retryable` ERROR was retried by nobody. +import { scheduledMailRetry } from "./scheduled-mail-retry.js"; // The verification round trip (plan §2.2, §5 package P3): a resend-capable // send + a hash-checked verify, both bearer-token onRequest endpoints in the // same shape as deleteAccount / apiQueueStatus below. @@ -53,6 +58,7 @@ export { onUploadQueueChanged as onuploadqueuechanged, onUploadFailure as onuploadfailure, onMailCreated as onmailcreated, + scheduledMailRetry as scheduledmailretry, sendContactEmailVerification as sendcontactemailverification, verifyContactEmail as verifycontactemail, apiQueueStatus as apiqueuestatus, diff --git a/functions/src/mail-availability.ts b/functions/src/mail-availability.ts new file mode 100644 index 0000000..6fd92af --- /dev/null +++ b/functions/src/mail-availability.ts @@ -0,0 +1,342 @@ +// Is DataPipe able to send mail right now, and if not, until when? +// +// --------------------------------------------------------------------------- +// WHY THIS EXISTS +// --------------------------------------------------------------------------- +// +// The two things DataPipe mails are not the same kind of thing, and the +// difference only shows up when quota runs out: +// +// contact-email verification REALTIME. A researcher clicked a button and is +// watching for a 6-digit code that expires in 24 +// hours. A code delivered tomorrow is not a late +// success, it is a failure with extra steps. +// Never retried. +// upload-failure notification DEFERRABLE. "Your data stopped arriving" is +// just as true an hour later. Retried by +// scheduled-mail-retry.ts. +// +// So the realtime path needs to know, BEFORE it does anything, whether a send +// can succeed -- otherwise it mints a code, arms its own resend cooldown, tells +// the researcher to check their inbox, and only then discovers that nothing can +// be sent. That is the state this module exists to prevent, and it is why the +// check has to happen ahead of the send rather than being inferred from it. +// +// --------------------------------------------------------------------------- +// WHERE THE NUMBER COMES FROM +// --------------------------------------------------------------------------- +// +// Resend returns `x-resend-daily-quota` -- the quota USED so far today -- on +// ordinary successful responses, not only on 429s. That is the whole reason +// this can be proactive: we learn we are at 94/100 while sending is still +// working, instead of finding out by failing. +// +// The rate-limit headers (`ratelimit-reset`, `retry-after`) are NOT useful +// here. They describe the per-second request limit (10/s per team), so +// `ratelimit-reset` counts down seconds to the next second. Resend documents no +// header or endpoint for when the DAILY quota rolls over, so nothing here may +// depend on knowing it -- see "the reset time is a guess" below. +// +// `x-resend-daily-quota` is documented as "only sent to free plan users". Its +// absence therefore means the daily cap does not apply, which is exactly right: +// on a paid plan this module quietly stops tripping instead of needing to be +// removed. +// +// THAT READING IS AN ASSUMPTION, AND IT IS LOAD-BEARING. Resend documents the +// header's existence, not its semantics, and "used today" and "remaining today" +// are the same shape. If it is really the plan LIMIT, every reading is 100, the +// reserve rule below is true forever, and verification is off for good -- and +// because each send rewrites dailyQuotaObservedAt, the staleness escape hatch +// never fires either. Three things bound that: +// +// 1. A reading outside [0, FREE_PLAN_DAILY_LIMIT] is refused at the write +// (recordSendOutcome) and ignored at the read. That catches a header that +// turns out to be the monthly counter, or a remaining-quota value on a +// paid plan. +// 2. Refusing a verification on the reserve alone logs at ERROR with a stable +// token, so the condition is alertable rather than silent +// (docs/deploy-contact-email.md §6). +// 3. The runbook says how to capture a real response header and check the +// number against the Resend dashboard (§5). Do that before trusting it. + +import { Timestamp } from "firebase-admin/firestore"; +import { db } from "./app.js"; + +export const STATUS_COLLECTION = "systemStatus"; +export const MAIL_STATUS_DOC = "mail"; + +// Resend's free plan: 3,000/month with a 100/day ceiling. Only used to derive +// the reserve below -- nothing here fails closed if the real limit differs, +// because an actual `daily_quota_exceeded` sets the breaker regardless. +export const FREE_PLAN_DAILY_LIMIT = 100; + +// Sends held back from the REALTIME path. +// +// Verification stops at the ceiling; upload-failure notifications keep going to +// the full limit. The asymmetry is deliberate and it is the important design +// decision in this file: a researcher waiting on a verification code can come +// back in an hour, but an upload-failure notification is the only signal that a +// researcher's data has stopped arriving. If anything gets the last ten sends +// of the day, it should be the one nobody can recover from missing. +export const VERIFICATION_RESERVE = 10; +export const VERIFICATION_CEILING = FREE_PLAN_DAILY_LIMIT - VERIFICATION_RESERVE; + +export interface MailStatus { + dailyQuotaUsed?: unknown; + dailyQuotaObservedAt?: unknown; + unavailableUntil?: unknown; + reason?: unknown; +} + +export type UnavailableReason = "quota-exhausted" | "quota-reserve"; + +export type Availability = + | { available: true } + | { available: false; reason: UnavailableReason; until: number | null }; + +function millisOrNull(value: unknown): number | null { + if (!value || typeof (value as { toMillis?: unknown }).toMillis !== "function") { + return null; + } + return (value as { toMillis: () => number }).toMillis(); +} + +/** + * Midnight UTC after the given instant. + * + * THE RESET TIME IS A GUESS, and this is where the guess lives. Resend does not + * publish when a daily quota rolls over; UTC midnight is the conventional + * answer and is almost certainly right, but nothing here is allowed to DEPEND + * on it being right. It is used only as a ceiling on how long the breaker stays + * shut. What actually reopens sending early is a successful send discovered by + * scheduled-mail-retry.ts -- the deferrable path probes, the realtime path only + * ever reads. If the real reset is an hour later than this, the sweeper's next + * attempt fails and re-arms the breaker; if it is earlier, the sweeper finds + * out and clears it. Either way the guess costs nothing. + */ +export function nextUtcMidnight(nowMs: number): number { + const d = new Date(nowMs); + return Date.UTC(d.getUTCFullYear(), d.getUTCMonth(), d.getUTCDate() + 1); +} + +/** + * Start of the UTC month after the given instant. + * + * The free plan has TWO caps -- 100/day and 3,000/month -- and they reset on + * different clocks. Reusing the daily reset for a monthly exhaustion is not a + * small error: hit the monthly cap on the 20th and the breaker reopens at + * midnight, the sweeper probes into a cap that has eleven days left to run, + * fails, and spends one of each queued document's MAX_ATTEMPTS doing it. Three + * nights of that and every queued notification is terminal -- which is the + * exact retry-budget exhaustion the breaker exists to prevent. + * + * Same guess-status as nextUtcMidnight, and the same escape hatch: an operator + * who upgrades the plan mid-month clears `systemStatus/mail` by hand rather + * than waiting this out (docs/deploy-contact-email.md §5). + */ +export function nextUtcMonthStart(nowMs: number): number { + const d = new Date(nowMs); + return Date.UTC(d.getUTCFullYear(), d.getUTCMonth() + 1, 1); +} + +// Why sending stopped, and therefore how long it stays stopped. +// +// daily-quota resets at the next UTC midnight. +// monthly-quota resets at the start of the next UTC month. +// systemic nothing about the account is exhausted; the deployment +// cannot send at all (a revoked key, an unverified domain, +// missing configuration). No reset time exists, so this is a +// short cooldown and nothing more -- see SYSTEMIC_PAUSE_MS. +export type PauseKind = "daily-quota" | "monthly-quota" | "systemic"; + +// How long a systemic failure holds sending shut. +// +// Short on purpose. A systemic failure needs a human, and this cannot wait for +// one -- but it must not let a loop of failing sends run either. Fifteen +// minutes bounds a revoked key to ~4 wasted Resend requests an hour no matter +// how many researchers press the button, and it is long enough that the +// sweeper (every 10 minutes) skips at most two passes if the diagnosis was +// wrong. +export const SYSTEMIC_PAUSE_MS = 15 * 60 * 1000; + +/** When may sending be tried again, given why it stopped? */ +export function pauseUntil(kind: PauseKind, nowMs: number): number { + if (kind === "daily-quota") return nextUtcMidnight(nowMs); + if (kind === "monthly-quota") return nextUtcMonthStart(nowMs); + return nowMs + SYSTEMIC_PAUSE_MS; +} + +/** + * A stored daily-quota reading, or null if there isn't a usable one. + * + * The bound is the point. `dailyQuotaUsed` comes from a response header whose + * semantics are assumed rather than documented (see the header), and a reading + * that cannot be what we think it is must not be allowed to hold the + * verification path shut forever. Anything outside [0, FREE_PLAN_DAILY_LIMIT] + * is not a count of today's sends on a free plan, whatever else it is. + */ +export function usableQuotaReading(value: unknown): number | null { + if (typeof value !== "number" || !Number.isFinite(value)) return null; + if (value < 0 || value > FREE_PLAN_DAILY_LIMIT) return null; + return value; +} + +/** + * Has a quota reading aged out? + * + * A reading taken before the most recent UTC midnight describes yesterday's + * usage and must not hold today's verification path shut. This is the same + * guess as above, in the opposite direction, and it fails SAFE: treating a + * still-valid reading as stale merely lets a send through, and that send + * returns a fresh number. + */ +export function isQuotaReadingStale(observedAtMs: number | null, nowMs: number): boolean { + if (observedAtMs === null) return true; + return nextUtcMidnight(observedAtMs) <= nowMs; +} + +/** + * May the REALTIME path send right now? + * + * Pure, and exported for it: this is the predicate a researcher's account page + * ultimately renders, and it needs no Firestore to assert. + */ +export function verificationAvailability( + status: MailStatus | undefined, + nowMs: number +): Availability { + const until = millisOrNull(status?.unavailableUntil); + if (until !== null && until > nowMs) { + return { available: false, reason: "quota-exhausted", until }; + } + + const used = usableQuotaReading(status?.dailyQuotaUsed); + const observedAt = millisOrNull(status?.dailyQuotaObservedAt); + if ( + used !== null && + !isQuotaReadingStale(observedAt, nowMs) && + used >= VERIFICATION_CEILING + ) { + return { + available: false, + reason: "quota-reserve", + until: observedAt === null ? null : nextUtcMidnight(observedAt), + }; + } + + return { available: true }; +} + +/** + * Is delivery paused outright? Used by scheduled-mail-retry.ts to decide + * whether to sweep at all. + * + * Deliberately does NOT consider the verification reserve: the reserve exists + * to keep the realtime path off the last few sends, and the sweeper is the + * consumer those sends are being reserved FOR. + */ +export function deliveryPaused(status: MailStatus | undefined, nowMs: number): boolean { + const until = millisOrNull(status?.unavailableUntil); + return until !== null && until > nowMs; +} + +let statusDocId = MAIL_STATUS_DOC; + +/** + * Test seam: point the breaker at a different document. + * + * `systemStatus/mail` is a SINGLETON, and the emulator is shared by suites + * running in parallel. A test that trips the breaker on the real document would + * make every concurrently-running suite that sends mail fail -- intermittently, + * in a different place each run, which is the worst kind of failure to chase. + * Pointing this suite at its own document is what keeps that from happening. + * Pass null to restore. + */ +export function _setMailStatusDocForTests(docId: string | null): void { + statusDocId = docId ?? MAIL_STATUS_DOC; +} + +function statusRef(): FirebaseFirestore.DocumentReference { + return db.collection(STATUS_COLLECTION).doc(statusDocId); +} + +/** + * Read the breaker. Never throws: a status read that fails must not be the + * reason a researcher cannot verify their address, so an unreadable document is + * treated as "no reason to believe anything is wrong". + */ +export async function readMailStatus(): Promise { + try { + const snap = await statusRef().get(); + return snap.exists ? (snap.data() as MailStatus) : undefined; + } catch (error) { + console.error( + "mail-availability: could not read mail status, assuming available:", + error instanceof Error ? error.message : "Unknown error" + ); + return undefined; + } +} + +/** + * Record the outcome of a send. + * + * Never throws, and never fails a delivery: this is bookkeeping alongside the + * real work, and a mail that was actually sent must not be reported as failed + * because a status write lost a race. + */ +export async function recordSendOutcome(outcome: { + dailyQuotaUsed?: number; + pause?: PauseKind; + errorName?: string; +}): Promise { + const now = Date.now(); + const updates: Record = { updatedAt: Timestamp.fromMillis(now) }; + + if (outcome.pause) { + updates.unavailableUntil = Timestamp.fromMillis(pauseUntil(outcome.pause, now)); + updates.reason = outcome.errorName ?? outcome.pause; + if (outcome.pause === "daily-quota") { + // Pin the counter at the limit. Without this a stale, lower reading from + // earlier in the day would keep saying the reserve is untouched. + // + // ONLY for the daily cap. A monthly exhaustion says nothing about + // today's counter, and pinning it there would block verification on the + // reserve rule for a reason that has nothing to do with the reserve -- + // and would go on doing so after the daily counter resets. The monthly + // pause above already holds everything shut for as long as it needs to. + updates.dailyQuotaUsed = FREE_PLAN_DAILY_LIMIT; + updates.dailyQuotaObservedAt = Timestamp.fromMillis(now); + } + } else { + // A send got through, so whatever the breaker believed is now out of date. + // This is the half-open close: the sweeper probes, and its success is what + // reopens the realtime path. + updates.unavailableUntil = null; + updates.reason = null; + if (outcome.dailyQuotaUsed !== undefined) { + const used = usableQuotaReading(outcome.dailyQuotaUsed); + if (used === null) { + // Loud, because the alternative is silent: an implausible reading + // stored here would hold the verification path shut on the reserve + // rule with nothing in the logs saying why. See the header -- this is + // what a wrong guess about the header's meaning looks like. + console.error( + `mail-availability: refusing an implausible x-resend-daily-quota reading (${outcome.dailyQuotaUsed}); expected 0..${FREE_PLAN_DAILY_LIMIT}. Check what the header actually means before trusting it.` + ); + } else { + updates.dailyQuotaUsed = used; + updates.dailyQuotaObservedAt = Timestamp.fromMillis(now); + } + } + } + + try { + await statusRef().set(updates, { merge: true }); + } catch (error) { + console.error( + "mail-availability: could not record send outcome:", + error instanceof Error ? error.message : "Unknown error" + ); + } +} diff --git a/functions/src/mail-delivery.ts b/functions/src/mail-delivery.ts index 07d5ffb..cbd64ce 100644 --- a/functions/src/mail-delivery.ts +++ b/functions/src/mail-delivery.ts @@ -73,7 +73,9 @@ import { onDocumentCreated } from "firebase-functions/v2/firestore"; import { Timestamp } from "firebase-admin/firestore"; import { randomUUID } from "crypto"; import { db } from "./app.js"; -import { MAIL_COLLECTION } from "./mail.js"; +import { MAIL_COLLECTION, mailCollection } from "./mail.js"; +import { recordSendOutcome } from "./mail-availability.js"; +import type { PauseKind } from "./mail-availability.js"; // --------------------------------------------------------------------------- // Timings. The relationship between these three numbers is load-bearing. @@ -399,6 +401,73 @@ const AMBIGUOUS_ERRORS = new Set([ "UND_ERR_SOCKET", ]); +// Quota exhaustion, as opposed to the per-second rate limit. These are what +// trip the breaker in mail-availability.ts, because they are the ones that stay +// true for the rest of the day -- or, for the monthly cap, the rest of the +// month, which is why they are told apart rather than lumped together. +export const QUOTA_ERRORS = new Set(["daily_quota_exceeded", "monthly_quota_exceeded"]); + +// Nothing is exhausted; this deployment cannot send AT ALL until a human +// changes something. A revoked or rescoped key, an unverified sending domain, +// missing configuration, a wrong endpoint. +// +// These trip the breaker too, and the reason is the realtime path. Without it +// the breaker only ever shuts on quota, so a revoked key leaves +// verificationAvailability answering "available" forever: every click mints a +// code, writes a document, spends a real Resend request and fails -- with no +// server-side ceiling on how often. Tripping here bounds that to one request +// per SYSTEMIC_PAUSE_MS no matter how many researchers are pressing the button. +// +// WHAT IS DELIBERATELY NOT HERE: `validation_error`. Resend uses it both for +// "your sending domain is unverified" (systemic) and for "this recipient +// address is malformed" (one researcher's typo). One researcher's typo must +// not switch verification off for everybody, so the ambiguous name stays out +// and the unambiguous ones carry the rule. +export const SYSTEMIC_ERRORS = new Set([ + "missing_api_key", + "restricted_api_key", + "suspended_api_key", + "invalid_permission", + "not_found", + "method_not_allowed", + CONFIG_MISSING_ERROR, +]); + +/** + * How long should this failure stop DataPipe sending, if at all? + * + * Pure, and exported for it: "how long is the breaker shut" is a decision worth + * asserting as a table rather than provoking through a transport. + */ +export function pauseKindFor(errorName: string): PauseKind | null { + if (errorName === "daily_quota_exceeded") return "daily-quota"; + if (errorName === "monthly_quota_exceeded") return "monthly-quota"; + if (SYSTEMIC_ERRORS.has(errorName)) return "systemic"; + return null; +} + +// Errors where Resend provably did NOT send: it refused the request, or we +// never reached it. Safe to retry at ANY age, because there is no message to +// duplicate. +// +// This set is the sweeper's licence. Everything else that is merely `retryable` +// is only safe to retry inside the 24-hour window in which Resend still honours +// the Idempotency-Key -- past that, a 5xx that might have been accepted becomes +// a coin flip on a second copy. See scheduled-mail-retry.ts. +export const REFUSED_ERRORS = new Set([ + "daily_quota_exceeded", + "monthly_quota_exceeded", + "rate_limit_exceeded", + "concurrent_idempotent_requests", + "resource_locked", + "ENOTFOUND", + "ECONNREFUSED", + "EAI_AGAIN", + "EHOSTUNREACH", + "ENETUNREACH", + "UND_ERR_CONNECT_TIMEOUT", +]); + /** * A non-2xx answer from Resend, or an unusable one. Carries the HTTP status so * classification can fall back on it when the body named nothing familiar. @@ -513,6 +582,23 @@ function millisOrZero(value: unknown): number { return (value as { toMillis: () => number }).toMillis(); } +/** + * Is a PROCESSING claim's lease dead? + * + * The single source of truth for "the claimant is provably gone". Exported + * because scheduled-mail-retry.ts's sweepDecision asks the same question about + * the same field for the opposite reason -- claimDecision asks so it may take + * the claim, the sweep asks so it may recover the document -- and two + * independent copies of the lease rule is exactly how one of them silently + * stops matching LEASE_MS. + */ +export function leaseIsExpired( + delivery: { leaseExpiresAt?: unknown } | undefined, + nowMs: number +): boolean { + return millisOrZero(delivery?.leaseExpiresAt) <= nowMs; +} + function attemptsOf(delivery: DeliveryRecord | undefined): number { const n = delivery?.attempts; return typeof n === "number" && Number.isFinite(n) && n > 0 ? n : 0; @@ -538,7 +624,7 @@ export function claimDecision( if (state === "PROCESSING") { // Someone holds the claim and may be inside the send right now. - if (millisOrZero(delivery?.leaseExpiresAt) > nowMs) return "skip-in-flight"; + if (!leaseIsExpired(delivery, nowMs)) return "skip-in-flight"; // Lease expired: the claimant is provably dead (see LEASE_MS). Recoverable. } else if (state === "ERROR") { if (delivery?.retryable !== true) return "skip-terminal"; @@ -563,6 +649,13 @@ export interface SendResult { // Resend's `id` for the accepted message. Recorded as // delivery.info.messageId, which is the extension's field name. id?: string; + // `x-resend-daily-quota` -- the quota USED today, read off the SUCCESS + // response. This is what makes the breaker proactive instead of reactive: + // we learn we are at 94/100 while sending still works. Absent on paid plans + // (Resend only sends it to free-plan accounts), which correctly reads as + // "no daily cap applies". + dailyQuotaUsed?: number; + monthlyQuotaUsed?: number; } export interface SendOptions { @@ -580,6 +673,34 @@ export type MailSender = ( const RESEND_ENDPOINT = "https://api.resend.com/emails"; +/** A response header as a number, or undefined if absent or unparseable. */ +function headerInt(headers: Headers, name: string): number | undefined { + const raw = headers.get(name); + if (raw === null) return undefined; + const n = Number.parseInt(raw, 10); + return Number.isFinite(n) ? n : undefined; +} + +// The quota headers, named ONCE each. +// +// Exported because these two string literals are the entire proactive half of +// the breaker and nothing else can tell you they are wrong: misspell one and +// every send returns no reading, mail-availability.ts never learns a number, +// verificationAvailability answers "available" forever, and every test still +// passes. Taking a plain Headers makes that assertable without a network call +// (mail-delivery.test.js). +export function quotaFromHeaders(headers: Headers): { + dailyQuotaUsed?: number; + monthlyQuotaUsed?: number; +} { + const daily = headerInt(headers, "x-resend-daily-quota"); + const monthly = headerInt(headers, "x-resend-monthly-quota"); + return { + ...(daily === undefined ? {} : { dailyQuotaUsed: daily }), + ...(monthly === undefined ? {} : { monthlyQuotaUsed: monthly }), + }; +} + let injectedSender: MailSender | null = null; /** @@ -604,7 +725,7 @@ export function _setMailSenderForTests(sender: MailSender | null): void { * apidata (index.ts imports every module in this codebase). There is now no * mail dependency to keep off it. */ -function resendSender(config: MailConfig): MailSender { +export function resendSender(config: MailConfig): MailSender { return async (input, { timeoutMs, idempotencyKey }) => { const response = await fetch(RESEND_ENDPOINT, { method: "POST", @@ -648,14 +769,22 @@ function resendSender(config: MailConfig): MailSender { throw new MailTransportError(name, message, response.status); } + // Quota headers ride on the SUCCESS response, which is the whole point -- + // see mail-availability.ts. Read before the body so a malformed body cannot + // cost us the reading. + const quota = quotaFromHeaders(response.headers); + // 2xx means Resend accepted it. An unreadable body after that point costs // us the message id, which is a worse audit trail -- not a failed send, so // it must not throw. delivery.info.messageId simply lands null. try { const body = (await response.json()) as { id?: unknown }; - return { id: typeof body?.id === "string" ? body.id : undefined }; + return { + ...quota, + id: typeof body?.id === "string" ? body.id : undefined, + }; } catch { - return {}; + return quota; } }; } @@ -685,6 +814,20 @@ const SKIP_OUTCOMES: Record, DeliveryOutcome> = "skip-attempts-exhausted": "skipped-attempts-exhausted", }; +/** + * Is this mail delivered inline by its caller, rather than by the trigger? + * + * Set by send-contact-email-verification.ts. Two consequences, both here and + * both deliberate: onMailCreated leaves the document alone (so the trigger and + * the inline caller cannot race for the claim, which would leave the caller + * unable to report an outcome), and a failure is terminal rather than + * retryable. + */ +export function isInline(data: FirebaseFirestore.DocumentData | undefined): boolean { + const meta = data?.datapipe as { deliverInline?: unknown } | undefined; + return meta?.deliverInline === true; +} + interface Claim { data: FirebaseFirestore.DocumentData; attempts: number; @@ -736,7 +879,7 @@ async function writeIfStillOurs( * real trigger deliveries out of the emulator. */ export async function deliverMailDocument(docId: string): Promise { - const ref = db.collection(MAIL_COLLECTION).doc(docId); + const ref = mailCollection().doc(docId); const leaseOwner = randomUUID(); // ---------------- CLAIM ------------------------------------------------- @@ -798,6 +941,17 @@ export async function deliverMailDocument(docId: string): Promise= MAX_ATTEMPTS; - const retryable = classified.retryable && !exhausted; + // An inline send is a realtime one: its caller is holding an HTTP request + // open and will report the outcome to a researcher who is watching. Nothing + // will ever retry it -- scheduled-mail-retry.ts skips these on purpose -- + // so calling it "retryable" would be a lie that also costs something real: + // a retryable error is never given delivery.expireAt, so the document would + // sit outside the TTL's reach holding an address forever. + const retryable = classified.retryable && !exhausted && !isInline(claim.data); // No recipient in this line: the docId is the handle, and the neighbours // (upload-failure-notify.ts, send-contact-email-verification.ts) log ids // and uids, never addresses. @@ -866,6 +1037,17 @@ export async function deliverMailDocument(docId: string): Promise ${outcome}`); diff --git a/functions/src/mail.ts b/functions/src/mail.ts index 2ee3376..c0182da 100644 --- a/functions/src/mail.ts +++ b/functions/src/mail.ts @@ -50,6 +50,33 @@ import { db } from "./app.js"; export const MAIL_COLLECTION = "mail"; +let mailCollectionName: string = MAIL_COLLECTION; + +/** + * The mail collection. Everything that reads or writes mail goes through this + * rather than naming the collection itself. + * + * WHY THE INDIRECTION EXISTS: scheduled-mail-retry.ts's sweep is a QUERY over + * the whole collection, and the emulator is shared by suites running in + * parallel. A suite that seeds a retryable failure and then sweeps would pick + * up another suite's fixtures, deliver them through ITS injected transport, and + * rewrite them mid-assertion -- while its own exact-count assertions failed for + * reasons that had nothing to do with the code under test. Isolating the status + * document (mail-availability.ts's _setMailStatusDocForTests) was never enough + * on its own, because the collection is the other shared singleton. + * + * The onMailCreated trigger still binds to MAIL_COLLECTION itself: the trigger + * path is deploy-time configuration, not something a test may move. + */ +export function mailCollection(): FirebaseFirestore.CollectionReference { + return db.collection(mailCollectionName); +} + +/** Test seam: point every mail read and write at another collection. */ +export function _setMailCollectionForTests(name: string | null): void { + mailCollectionName = name ?? MAIL_COLLECTION; +} + export interface MailMessage { // Single recipient. The extension accepts a string or an array; we always // write an array, so the shape is uniform for tests and for purge queries. @@ -91,7 +118,7 @@ function mailDocument({ to, subject, text, html, meta }: MailInput) { // (a transaction may not allocate ids mid-flight), so this is separate from // enqueueMail. export function newMailRef(): FirebaseFirestore.DocumentReference { - return db.collection(MAIL_COLLECTION).doc(); + return mailCollection().doc(); } // Transactional enqueue. The mail document is created as part of the caller's diff --git a/functions/src/purge-user-data.ts b/functions/src/purge-user-data.ts index 718fcfb..5e6ffc7 100644 --- a/functions/src/purge-user-data.ts +++ b/functions/src/purge-user-data.ts @@ -1,5 +1,5 @@ import { db, storage } from "./app.js"; -import { MAIL_COLLECTION } from "./mail.js"; +import { mailCollection } from "./mail.js"; // Everything that belongs to one researcher, removed in one pass. // @@ -132,8 +132,7 @@ export async function purgeUserData(uid: string): Promise { // them. Deleting these is why account deletion must not leave an address // behind: an undelivered or already-processed mail document is still a // record of where DataPipe last tried to reach this person. - const queuedMail = await db - .collection(MAIL_COLLECTION) + const queuedMail = await mailCollection() .where("datapipe.owner", "==", uid) .get(); counts.mailDocuments = await deleteInBatches( diff --git a/functions/src/scheduled-mail-retry.ts b/functions/src/scheduled-mail-retry.ts new file mode 100644 index 0000000..9002fa2 --- /dev/null +++ b/functions/src/scheduled-mail-retry.ts @@ -0,0 +1,435 @@ +// Re-drive mail that failed for a reason that has since stopped being true. +// +// --------------------------------------------------------------------------- +// THE GAP THIS CLOSES +// --------------------------------------------------------------------------- +// +// mail-delivery.ts's onDocumentCreated trigger does not re-fire when a document +// is UPDATED, so a `retryable` ERROR was, until this file existed, retried by +// nobody. It sat marked "still deliverable" forever and nothing ever delivered +// it. +// +// That is worse than it sounds, because of what happens upstream. +// upload-failure-notify.ts writes `uploadFailure.notifiedAt` in the SAME +// transaction that enqueues the mail, and `lastNotifiedAt` is a 24-hour floor +// ACROSS episodes. So a mail that dies on quota leaves an episode armed as "we +// told them": the researcher is never told their data stopped arriving, nothing +// re-notifies, and the experiment document positively asserts that they were +// informed. The only dissent is a mail document nobody reads. +// +// --------------------------------------------------------------------------- +// EVERY DOCUMENT THIS FINDS LEAVES BY A DOOR +// --------------------------------------------------------------------------- +// +// The sweep's queries are unordered `limit()`s, so any document the pass can +// look at without changing is a document it will look at again next pass, and +// forever -- occupying the budget that a deliverable notification needed. So +// the decision table has no permanent "skip" in it. Every outcome either sends +// (leaving SUCCESS or a fresh error), or writes a TERMINAL state that takes the +// document out of both queries. The only skips left are documents that another +// invocation is actively working on, which resolve themselves within LEASE_MS. +// +// Terminal matters for more than starvation. A retryable ERROR is never given +// `delivery.expireAt`, so it sits outside the TTL policy's reach holding a +// researcher's address indefinitely (docs/deploy-contact-email.md §4). Ageing +// one out is how that address finally gets deleted. +// +// --------------------------------------------------------------------------- +// WHAT IT DELIBERATELY DOES NOT SEND +// --------------------------------------------------------------------------- +// +// 1. INLINE MAIL (contact-email verification). Never. A verification code is +// realtime -- it expires in 24 hours and its recipient is watching a form +// right now -- so a code delivered an hour late is not a late success, it is +// a confusing failure. Those are marked terminal at failure time +// (mail-delivery.ts's isInline), so they should never match the queries +// here; one that somehow does is aged out rather than skipped, because a +// skipped one would sit there holding an address forever. +// +// 2. AMBIGUOUS ERRORS, past the idempotency window. mail-delivery.ts makes +// timeouts retryable ONLY because Resend honours an Idempotency-Key, and it +// honours it for 24 hours FROM ITS FIRST USE. A sweeper is by nature a late +// retry -- quota resets daily -- so retrying a maybe-delivered send past +// that window is a coin flip on a second copy of a notification whose whole +// value is arriving once. REFUSED_ERRORS (the request was refused, or never +// reached Resend at all) carry no such risk and are swept at any age; +// everything else is swept only inside the window. +// +// 3. ANYTHING, while the breaker is shut. If mail-availability.ts says sending +// is paused, sending would fail every document AND burn one of its three +// MAX_ATTEMPTS doing it -- so an outage would exhaust the retry budget of +// every queued mail and turn all of them terminal, which is the precise +// opposite of this file's purpose. Checking the breaker first is what makes +// the sweep free to run often. The two passes that are only WRITES -- +// retention and ageing out -- run ahead of that check, because neither +// spends any quota and both matter most during an outage. + +import { onSchedule } from "firebase-functions/v2/scheduler"; +import { Timestamp } from "firebase-admin/firestore"; +import { mailCollection } from "./mail.js"; +import { + deliverMailDocument, + isInline, + leaseIsExpired, + REFUSED_ERRORS, + MAIL_RETENTION_MS, + MAX_ATTEMPTS, +} from "./mail-delivery.js"; +import { deliveryPaused, readMailStatus } from "./mail-availability.js"; +import { extendRetentionForExperiment } from "./upload-retention.js"; + +// How long Resend honours an Idempotency-Key, measured FROM ITS FIRST USE. The +// bound on retrying anything that is not provably un-sent. Raising this without +// checking Resend's docs would silently reintroduce double-sends. +export const IDEMPOTENCY_WINDOW_MS = 24 * 60 * 60 * 1000; + +// Past this, a notification has stopped being worth sending. Three days is +// chosen against what the mail SAYS: "your uploads are failing" is still true +// and still actionable a day or two later, but a researcher who has not noticed +// in three days is better served by the dashboard than by an email about a +// failure episode that has probably long since drained. +export const MAX_SWEEP_AGE_MS = 3 * 24 * 60 * 60 * 1000; + +// One pass' budget, per query. Small on purpose: the steady state is zero +// documents, the bad case is a quota outage that queued a few dozen, and a +// sweep that tried to drain hundreds would run into Resend's per-second rate +// limit and convert a recoverable backlog into a burned retry budget. +export const SWEEP_LIMIT = 25; + +// What a document that was abandoned mid-send is recorded as. It has no error +// of its own -- the claim cleared the previous one -- and "we do not know what +// happened to this" is worth saying out loud in the audit trail rather than +// leaving a terminal document with an empty `error`. +export const SWEEP_ABANDONED_ERROR = "MailSweepAbandoned"; + +export interface SweepReport { + scanned: number; + delivered: number; + failed: number; + skipped: number; + agedOut: number; + paused: boolean; + // Queue entries whose deletion was pushed back because the researcher has + // not been told about them yet. + retained: number; +} + +function millisOrZero(value: unknown): number { + if (!value || typeof (value as { toMillis?: unknown }).toMillis !== "function") { + return 0; + } + return (value as { toMillis: () => number }).toMillis(); +} + +/** + * May this document be retried right now? + * + * Pure, and exported for it -- this predicate is the entire safety argument of + * the file, and it should be assertable as a table rather than provoked through + * Firestore and a mail transport. + * + * "age-out" means TERMINAL: stop trying, and let the TTL have the address. + * "skip" is reserved for documents that are somebody else's right now. + */ +export function sweepDecision( + data: FirebaseFirestore.DocumentData, + nowMs: number +): "deliver" | "age-out" | "skip" { + const delivery = (data.delivery ?? {}) as Record; + + if (delivery.state === "PROCESSING") { + // Someone holds the claim and may be inside the send right now. Same rule + // as claimDecision's, and deliberately the same function: see leaseIsExpired. + if (!leaseIsExpired(delivery, nowMs)) return "skip"; + // Lease expired: the claimant is provably dead (mail-delivery.ts's + // LEASE_MS). Nothing else would ever look at this document again -- the + // claim rewrote it to PROCESSING with retryable null, which is outside the + // retryable-ERROR query that used to be the only one here, and outside the + // TTL too. That is how a preempted instance or a mid-deploy roll used to + // strand a researcher's address permanently. + } else if (delivery.state !== "ERROR" || delivery.retryable !== true) { + return "skip"; + } + + // Inline mail is realtime and nothing may re-send it late (see the header). + // Terminal rather than skipped: a verification code that failed is finished + // work, and leaving it "retryable" keeps expireAt off the document. + if (isInline(data)) return "age-out"; + + // Out of attempts. mail-delivery.ts marks these terminal itself, so this is + // for the document it never got to write -- and without it, deliverMailDocument + // would answer "skipped-attempts-exhausted" on every pass forever. + const attempts = delivery.attempts; + if (typeof attempts === "number" && attempts >= MAX_ATTEMPTS) return "age-out"; + + // TWO CLOCKS, AND THEY MEASURE DIFFERENT THINGS. + // + // startTime when delivery FIRST began, and therefore when this + // document's Idempotency-Key (its own id) was first used. + // Never moves. This is what the 24-hour window is measured + // from -- measuring it from the last attempt slides the + // window forward with every retry, so a document retried at + // +20h and again at +40h would be sent on a key that expired + // at +24h, and Resend would treat it as a new message. + // lastAttemptAt when anything last happened. Age-out is measured from + // this, so a mail that has been retried into this morning is + // young however long ago it was first written. + const startedAt = millisOrZero(delivery.startTime); + const lastAttempt = millisOrZero(delivery.lastAttemptAt); + + // A document with neither is undatable, and an undatable document must not + // become immortal by lacking a field: with no clock to measure, there is no + // age at which it would ever age out, and it would hold its address forever. + if (startedAt === 0 && lastAttempt === 0) return "age-out"; + + if (nowMs - Math.max(startedAt, lastAttempt) > MAX_SWEEP_AGE_MS) return "age-out"; + + // Provably un-sent: retry at any age, because there is no message to + // duplicate and so nothing for the Idempotency-Key to do. + const name = (delivery.error as { name?: unknown } | undefined)?.name; + if (typeof name === "string" && REFUSED_ERRORS.has(name)) return "deliver"; + + // Everything else -- a 5xx that may or may not have been accepted, an error + // this deploy has no name for, or a claim abandoned mid-send -- is ambiguous, + // and ambiguity is exactly what the Idempotency-Key resolves. Inside the + // window a retry is a no-op at Resend; outside it, the key has expired and a + // retry is a coin flip on a second copy, so we stop rather than gamble. + if (nowMs - (startedAt || lastAttempt) <= IDEMPOTENCY_WINDOW_MS) return "deliver"; + return "age-out"; +} + +/** + * The experiment whose data this notification is about, if it is about any. + * + * Verification codes have no data behind them to keep. + */ +function retentionTargetOf(mailData: FirebaseFirestore.DocumentData): string | null { + const meta = (mailData.datapipe ?? {}) as Record; + if (meta.kind !== "upload-failure") return null; + return typeof meta.experimentID === "string" ? meta.experimentID : null; +} + +/** The terminal write. Takes the document out of both sweep queries. */ +function ageOutUpdates( + data: FirebaseFirestore.DocumentData, + nowMs: number +): Record { + const delivery = (data.delivery ?? {}) as Record; + const at = Timestamp.fromMillis(nowMs); + const updates: Record = { + // ERROR and not PROCESSING, so a document abandoned mid-send cannot be + // matched by the stranded-claim query again next pass. + "delivery.state": "ERROR", + "delivery.retryable": false, + "delivery.leaseExpiresAt": null, + "delivery.endTime": at, + // The field the TTL policy keys on. Writing it is the whole point of + // ageing out rather than skipping. + "delivery.expireAt": Timestamp.fromMillis(nowMs + MAIL_RETENTION_MS), + }; + // Never overwrite a real failure with a generic one: the recorded error is + // usually the only account of why this mail never arrived. + if (!delivery.error || typeof delivery.error !== "object") { + updates["delivery.error"] = { + name: SWEEP_ABANDONED_ERROR, + message: "Delivery was abandoned in flight and could not safely be retried.", + }; + } + return updates; +} + +/** + * One sweep. + * + * Exported as the test seam this codebase already uses for scheduled work + * (scheduled-upload-retry.ts's retryPendingUploads, scheduled-pending- + * recovery.ts's recoverPendingUploads): tests drive it in-process rather than + * trying to provoke a real scheduler tick out of the emulator. + */ +export async function sweepRetryableMail(nowMs = Date.now()): Promise { + const report: SweepReport = { + scanned: 0, + delivered: 0, + failed: 0, + skipped: 0, + agedOut: 0, + paused: false, + retained: 0, + }; + + // TWO QUERIES, BECAUSE THERE ARE TWO WAYS TO BE STUCK. + // + // The first is a failure nobody retried. The second is a claim nobody + // finished: deliverMailDocument's claim transaction rewrites the document to + // PROCESSING with `retryable: null` BEFORE the send, so an instance that dies + // in the send leaves a document that the retryable-ERROR query cannot see and + // the TTL cannot reap. claimDecision already knows how to recover an expired + // lease; until now, nothing ever asked it to. + const [failed, stranded] = await Promise.all([ + mailCollection() + .where("delivery.state", "==", "ERROR") + .where("delivery.retryable", "==", true) + .limit(SWEEP_LIMIT) + .get(), + mailCollection() + .where("delivery.state", "==", "PROCESSING") + .where("delivery.leaseExpiresAt", "<=", Timestamp.fromMillis(nowMs)) + .limit(SWEEP_LIMIT) + .get(), + ]); + + const docs = [...failed.docs, ...stranded.docs]; + report.scanned = docs.length; + + // Decided once, so the retention pass, the age-out pass and the send pass + // cannot disagree about which documents are still live. + const decisions = docs.map((doc) => [doc, sweepDecision(doc.data(), nowMs)] as const); + + // ------------------------------------------------------------------------- + // RETENTION FIRST, AND DELIBERATELY BEFORE THE BREAKER. + // ------------------------------------------------------------------------- + // + // Holding a queue entry back is a Firestore write, not a send: it costs no + // quota, so nothing about an exhausted quota is a reason to skip it. The + // opposite, in fact -- a quota outage is precisely when a researcher's data + // is quietly ageing towards deletion behind a notification that never + // arrived. Putting this after the breaker check would switch the protection + // off in the only situation that needs it. + // + // It covers the documents about to be AGED OUT as well as the ones about to + // be sent, and that is the more important half: giving up on a notification + // is the case where the researcher will never be told at all, and it must not + // also be the moment their data quietly goes back on the original clock. The + // 14-day ceiling in upload-retention.ts is what bounds this. + // + // Once per EXPERIMENT, not once per document: two failure episodes for the + // same experiment produce two mail documents, and running the same query and + // the same batch twice in one pass would double the writes and double-count + // them in the report. + // + // Concurrently, because the experiments are disjoint: each call is its own + // query and its own batch over a different experiment's entries, so awaiting + // them one after another only adds round-trips to a scheduled pass. + const extended = new Set(); + for (const [doc, decision] of decisions) { + if (decision === "skip") continue; + const experimentID = retentionTargetOf(doc.data()); + if (experimentID) extended.add(experimentID); + } + await Promise.all( + [...extended].map(async (experimentID) => { + try { + report.retained += await extendRetentionForExperiment(experimentID, nowMs); + } catch (error) { + // Never fatal, and per experiment: failing to extend one costs that + // researcher time, but throwing would cost every other one the sweep. + console.error( + `scheduled-mail-retry: could not extend retention for ${experimentID}:`, + error instanceof Error ? error.message : "Unknown error" + ); + } + }) + ); + + // ------------------------------------------------------------------------- + // AGE-OUT, ALSO BEFORE THE BREAKER, AND FOR THE SAME REASON. + // ------------------------------------------------------------------------- + // + // Giving up is a write, not a send. Leaving it behind the breaker would mean + // a month-long monthly-quota outage held every abandoned document -- and the + // researcher address in each one -- for the whole outage, which is the exact + // retention hole this pass exists to close. + // + // Concurrently: these are independent single-document writes, and a backlog + // that has just aged out can be the whole of both queries -- fifty serialized + // round-trips for writes that have nothing to say to each other. Each keeps + // its own error handling rather than becoming one batch, because a batch is + // atomic and one vanished document would take the other forty-nine with it. + await Promise.all( + decisions + .filter(([, decision]) => decision === "age-out") + .map(async ([doc]) => { + try { + await doc.ref.update(ageOutUpdates(doc.data(), nowMs)); + report.agedOut += 1; + console.log( + `scheduled-mail-retry: ${doc.id} aged out undelivered, marked terminal` + ); + } catch (error) { + // Not fatal, and the likeliest cause is benign: purge-user-data.ts + // deleting the researcher's mail out from under the pass, which is an + // expected event rather than a fault. Throwing here would cost the + // rest of the sweep -- including deliveries -- for a document that no + // longer needs anything done to it. + console.error( + `scheduled-mail-retry: could not age out ${doc.id}:`, + error instanceof Error ? error.message : "Unknown error" + ); + } + }) + ); + + // The breaker. See the header: sending into a shut breaker fails every + // document and spends a MAX_ATTEMPTS on each failure. + const status = await readMailStatus(); + if (deliveryPaused(status, nowMs)) { + report.paused = true; + report.skipped += decisions.filter(([, d]) => d === "skip").length; + console.log( + "scheduled-mail-retry: delivery is paused, sending nothing this pass" + ); + return report; + } + + for (const [doc, decision] of decisions) { + if (decision !== "deliver") { + if (decision === "skip") report.skipped += 1; + continue; + } + + const outcome = await deliverMailDocument(doc.id); + if (outcome === "sent") { + report.delivered += 1; + } else { + report.failed += 1; + } + console.log(`scheduled-mail-retry: ${doc.id} -> ${outcome}`); + + // A failure mid-sweep may mean the rest of this pass would fail too, and + // each failure costs a retry attempt. deliverMailDocument has already + // tripped the breaker if the cause was one that shuts it; honour that + // immediately rather than after 24 more wasted attempts. + if (outcome === "retryable-error" || outcome === "terminal-error") { + const fresh = await readMailStatus(); + if (deliveryPaused(fresh, nowMs)) { + report.paused = true; + console.log( + "scheduled-mail-retry: sending was paused mid-sweep, stopping this pass" + ); + break; + } + } + } + + // A delivery during this pass already cleared the breaker via + // recordSendOutcome. Nothing to do here -- stated so nobody adds a redundant + // clear that would reopen the realtime path on a pass that sent nothing. + return report; +} + +// Every 10 minutes. Cheap by construction: the steady state is two indexed +// queries returning nothing, and a shut breaker still costs only those two plus +// one document read. +// +// No `retry` config. A failed sweep is a logged line and the next pass tries +// again ten minutes later, which is what a sweeper is for -- replaying a failed +// sweep would stack passes on top of each other for no benefit. +export const scheduledMailRetry = onSchedule( + { schedule: "*/10 * * * *", memory: "256MiB" }, + async () => { + const report = await sweepRetryableMail(); + if (report.scanned > 0 || report.paused) { + console.log(`scheduled-mail-retry: ${JSON.stringify(report)}`); + } + } +); diff --git a/functions/src/scheduled-upload-retry.ts b/functions/src/scheduled-upload-retry.ts index cd9d597..f0d1f17 100644 --- a/functions/src/scheduled-upload-retry.ts +++ b/functions/src/scheduled-upload-retry.ts @@ -9,6 +9,7 @@ import { ExperimentData, UserData } from "./interfaces.js"; import { isFastRetry } from "./queue-upload.js"; import { isCompactionInFlight, COMPACTION_HOLD_REASON } from "./compaction-gate.js"; import { decryptPayload } from "./payload-crypto.js"; +import { retentionDecision } from "./upload-retention.js"; const SEVEN_DAYS_MS = 7 * 24 * 60 * 60 * 1000; const MAX_BACKOFF_MS = 24 * 60 * 60 * 1000; // 24 hours (slow tier cap, unchanged) @@ -404,30 +405,91 @@ async function handleRetryFailure( }); } -async function cleanupOldEntries() { +// One pass' deletion budget, unchanged. The sweep runs every five minutes, so +// this is 14,400 entries a day -- far more than the steady state, and small +// enough that no single pass is a surprise. +const CLEANUP_DELETE_LIMIT = 50; + +// How many aged entries one pass will LOOK at. +// +// The two numbers are different now, and that is the fix. The query finds +// entries by age, ascending -- the inequality on `createdAt` forces that order +// -- but whether one may actually be destroyed is a separate question +// (upload-retention.ts), and an entry that must be retained is not removed from +// the result set by being looked at. So with a single `limit(50)` on the query, +// fifty retained entries at the head of the queue meant a pass that deleted +// nothing, forever: one experiment stuck behind a dead storage provider could +// stop every OTHER experiment's payloads from ever being deleted, until the +// blockers crossed the 14-day ceiling up to a week later. Paging past them with +// a cursor is what keeps the retention rule from becoming a deletion outage. +const CLEANUP_SCAN_LIMIT = 500; + +// Page size for that scan. +const CLEANUP_PAGE_SIZE = 100; + +/** + * Delete aged-out queue entries and the payloads behind them. + * + * Exported for the same reason retryPendingUploads is, and with the same seam: + * `ownerScope` defaults to production behaviour (every entry) and lets a test + * confine an age-based sweep of a collection every suite shares to its own + * fixtures. + */ +export async function cleanupOldEntries(ownerScope?: string) { const cutoff = Timestamp.fromMillis(Date.now() - SEVEN_DAYS_MS); + const bucket = storage.bucket(); + const now = Date.now(); + + let deleted = 0; + let retained = 0; + let scanned = 0; + let cursor: FirebaseFirestore.QueryDocumentSnapshot | null = null; + + while (deleted < CLEANUP_DELETE_LIMIT && scanned < CLEANUP_SCAN_LIMIT) { + // orderBy is explicit rather than left implicit: the inequality already + // imposes it, and startAfter() below depends on it. + let query = db + .collection("uploadQueue") + .where("createdAt", "<=", cutoff) + .orderBy("createdAt") + .limit(CLEANUP_PAGE_SIZE); + if (cursor) { + query = query.startAfter(cursor); + } - const oldEntries = await db - .collection("uploadQueue") - .where("createdAt", "<=", cutoff) - .limit(50) - .get(); + const page = await query.get(); + if (page.empty) break; + cursor = page.docs[page.docs.length - 1]; + + for (const doc of page.docs) { + if (deleted >= CLEANUP_DELETE_LIMIT) break; + scanned += 1; + const data = doc.data(); + if (ownerScope && data.owner !== ownerScope) continue; + + // The query above finds entries by AGE. Whether one may actually be + // destroyed is a separate question, and not one an age answers on its own + // -- see upload-retention.ts. + if (retentionDecision(data, now) === "retain") { + retained += 1; + continue; + } - if (oldEntries.empty) { - return; + try { + await bucket.file(data.storagePath).delete(); + } catch { + // File may already be deleted + } + await doc.ref.delete(); + deleted += 1; + } + + if (page.size < CLEANUP_PAGE_SIZE) break; } - console.log(`Cleaning up ${oldEntries.size} old queue entries.`); + if (deleted === 0 && retained === 0) return; - const bucket = storage.bucket(); - - for (const doc of oldEntries.docs) { - const data = doc.data(); - try { - await bucket.file(data.storagePath).delete(); - } catch { - // File may already be deleted - } - await doc.ref.delete(); - } + console.log( + `Cleanup: ${deleted} old queue entries deleted, ${retained} retained (still retrying, or the researcher has not been notified yet).` + ); } diff --git a/functions/src/send-contact-email-verification.ts b/functions/src/send-contact-email-verification.ts index be44cd8..e0a3baf 100644 --- a/functions/src/send-contact-email-verification.ts +++ b/functions/src/send-contact-email-verification.ts @@ -2,6 +2,8 @@ import { onRequest } from "firebase-functions/v2/https"; import { randomInt, createHash } from "crypto"; import { db, auth } from "./app.js"; import { contactEmailRecipient, sendMail } from "./mail.js"; +import { deliverMailDocument } from "./mail-delivery.js"; +import { readMailStatus, verificationAvailability } from "./mail-availability.js"; import { ACCOUNT_URL } from "./email/upload-failure-copy.js"; // Send (or resend) a 6-digit code that confirms users/{uid}.contactEmail. @@ -34,13 +36,19 @@ const CODE_SPACE = 10 ** CODE_DIGITS; // 1_000_000 possible codes // plan §2.2: "24-hour expiry, 5 attempts." export const EXPIRY_MS = 24 * 60 * 60 * 1000; -// Floor between two sends to the same account. The plan does not pin a -// number beyond "rate-limited" and putting `sentAt` in the +// Floor between two ATTEMPTS to send to the same account. The plan does not pin +// a number beyond "rate-limited" and putting `sentAt` in the // contactEmailVerifications schema for exactly this purpose; one minute is // enough to stop a resend-mail-bomb loop (an impatient double-click, or // someone hammering the endpoint) without making a researcher who mistyped // and wants a fresh code wait anywhere near as long as the 24-hour code // expiry itself. +// +// ATTEMPTS, not deliveries, and that distinction is the rate limit. This +// endpoint spends a real Resend request every time it gets past this check, so +// a path that reaches the send is a path that has to be throttled whatever the +// send then does. See the failure branch below, which used to delete the very +// record this reads. export const RESEND_COOLDOWN_MS = 60 * 1000; export const VERIFICATIONS_COLLECTION = "contactEmailVerifications"; @@ -143,6 +151,41 @@ export const sendContactEmailVerification = onRequest( return; } + // --------------------------------------------------------------------- + // CAN WE SEND AT ALL? ASKED BEFORE ANYTHING IS SPENT. + // --------------------------------------------------------------------- + // + // Deliberately ahead of minting a code, writing the verification record, + // and arming the resend cooldown. Ask afterwards and a researcher gets + // told to check their inbox, gets a 60-second cooldown on requesting + // another, and never receives anything -- the failure invisible on both + // ends. Asking here also means a researcher clicking the button + // repeatedly during a quota outage costs zero Resend requests, which is + // the difference between a paused feature and a feature that makes the + // outage worse. + // + // The message is vague on purpose. "Quota" is operational detail that + // means nothing to a researcher, and one honest sentence covers every + // terminal cause -- exhausted quota, an unverified domain, a revoked key + // -- none of which they can act on differently anyway. + const availability = verificationAvailability(await readMailStatus(), Date.now()); + if (!availability.available) { + // ERROR, not WARN, and with a stable token: "verification is refused + // for everybody" is a condition an operator wants to hear about, + // especially in the `quota-reserve` case, where the only evidence that + // the daily-quota header means what this code thinks it means is that + // this line is NOT firing all day (docs/deploy-contact-email.md §6). + console.error( + `send-contact-email-verification: MailVerificationUnavailable (${availability.reason}) -- refused for ${uid}` + ); + res.status(503).json({ + error: + "We can't send verification codes right now. Please try again in a little while.", + code: "mail-unavailable", + }); + return; + } + const existing = await verificationRef(uid).get(); const sentAt = existing.exists ? (existing.data()?.sentAt as number | undefined) @@ -175,14 +218,108 @@ export const sendContactEmailVerification = onRequest( // there is nothing else to keep consistent with the send -- this is a // direct response to a request the researcher just made, which is // exactly the case mail.ts's sendMail doc comment names. - await sendMail({ + const mailRef = await sendMail({ to: recipient, subject, text, html, - meta: { kind: "contact-email-verification", owner: uid }, + meta: { + kind: "contact-email-verification", + owner: uid, + // Delivered below, synchronously, by THIS request -- not by the + // onmailcreated trigger, which stands down for inline mail. The + // document is still written first, so the audit trail, the + // purge-user-data handle and the TTL all work exactly as they do for + // queued mail; only who performs the send changes. + deliverInline: true, + }, }); + // --------------------------------------------------------------------- + // THE EMULATOR NEVER SENDS, SO IT NEVER REPORTS A SEND FAILURE EITHER. + // --------------------------------------------------------------------- + // + // onMailCreated returns before doing anything at all when + // FUNCTIONS_EMULATOR is set, and the inline path has to make the same + // promise -- otherwise it becomes the one route by which a local test run + // could mail a real person, which is precisely the hole that gate exists + // to close. + // + // It also has to answer 200. The mail document is written either way, and + // contact-email-verify-emulator.test.js recovers the code from it exactly + // as it did when the trigger owned delivery; answering 503 here would + // fail every emulator-backed verification test on a send that was never + // going to be attempted. + // + // Coverage does not suffer: the inline delivery path below is exercised + // in-process, against the emulator, with an injected transport, by + // mail-retry-emulator.test.js. + if (process.env.FUNCTIONS_EMULATOR === "true") { + console.log( + `send-contact-email-verification: emulator instance, leaving mail ${mailRef.id} unsent` + ); + res.status(200).json({ success: true }); + return; + } + + // --------------------------------------------------------------------- + // AWAITED, BECAUSE SOMEBODY IS WATCHING A FORM. + // --------------------------------------------------------------------- + // + // The queue's asynchrony is right for a failure notification and wrong + // here: a verification code that arrives tomorrow is not a late success, + // it is a code that expired (EXPIRY_MS) before it landed. So this request + // does not return until the send has actually happened or actually + // failed. deliverMailDocument never throws -- every expected failure is a + // returned outcome and a field on the document. + const outcome = await deliverMailDocument(mailRef.id); + if (outcome !== "sent") { + // --------------------------------------------------------------------- + // THE RECORD STAYS. IT IS THE RATE LIMIT. + // --------------------------------------------------------------------- + // + // This branch used to delete it, so that a researcher whose code never + // arrived would not be stuck behind a cooldown for a code that does not + // exist. That reasoning is right about the researcher and wrong about + // the endpoint: `sentAt` is the ONLY server-side throttle on this path, + // and deleting it here removed the throttle from exactly the case that + // needs one. A send that fails still costs a Resend request and still + // writes a mail document, so a signed-in researcher holding the button + // -- or a loop on one valid ID token -- drove both without any bound at + // all, and the pre-send breaker did not cover it either, because it only + // shut on quota errors. + // + // Two changes make that safe rather than merely throttled: the breaker + // now also shuts on a systemic failure (mail-delivery.ts's + // SYSTEMIC_ERRORS), so a revoked key or an unverified domain stops + // costing a request per click within one attempt; and the code itself is + // cleared here, so what survives is a cooldown and not a secret nobody + // received. The researcher waits at most RESEND_COOLDOWN_MS, which is the + // same minute they would wait after a send that worked. + try { + await verificationRef(uid).update({ + codeHash: null, + deliveryFailedAt: now, + }); + } catch (cleanupError) { + // Non-fatal. The code is undeliverable either way and expires in 24 + // hours; what matters is that `sentAt` is still standing. + console.error( + `send-contact-email-verification: could not clear the unsent code for ${uid}:`, + cleanupError instanceof Error ? cleanupError.message : "Unknown error" + ); + } + console.error( + `send-contact-email-verification: delivery for ${uid} ended as ${outcome}, mail ${mailRef.id}` + ); + res.status(503).json({ + error: + "We couldn't send a verification code right now. Please try again in a little while.", + code: "mail-unavailable", + }); + return; + } + res.status(200).json({ success: true }); } catch (error) { console.error( diff --git a/functions/src/upload-failure-notify.ts b/functions/src/upload-failure-notify.ts index d37e127..ba302a3 100644 --- a/functions/src/upload-failure-notify.ts +++ b/functions/src/upload-failure-notify.ts @@ -47,6 +47,7 @@ import { contactEmailRecipient, enqueueMail, newMailRef } from "./mail.js"; import type { MailMeta } from "./mail.js"; import { buildUploadFailureEmail } from "./email/upload-failure-copy.js"; import type { UploadFailureState } from "./interfaces.js"; +import { unresolvedQueueEntriesQuery } from "./upload-retention.js"; // One mail per experiment per 24 hours, maximum, no matter how often the queue // flaps clear->fail. A study that breaks every hour for a week produces seven @@ -54,20 +55,6 @@ import type { UploadFailureState } from "./interfaces.js"; // transactions below cannot close -- see clearIfDrained. export const RATE_LIMIT_MS = 24 * 60 * 60 * 1000; -// "Not resolved yet, from the researcher's point of view." Deliberately the -// predicate the dashboard already uses (pages/admin/[experiment_id].js:46-53 -// and api-queue-status.ts:143-144), INCLUDING `failed`, and deliberately not -// finalization.ts's, which excludes `failed` so that a permanently dead file -// cannot block sealing forever. -// -// Including `failed` is what stops the notification re-arming while a dead -// file is still sitting there: that file is an unresolved problem the -// researcher has not dealt with, and re-arming would mail them again about a -// queue that never actually got better. The episode closes when the last -// unresolved entry either completes or is swept away by cleanupOldEntries at -// seven days. -export const UNRESOLVED_STATUSES = ["pending", "processing", "failed"]; - // The Admin SDK default is 5, which is not enough here. The motivating case is // the ordinary one: a metadataActive submission produces a raw file, a main // CSV and one sidecar per extracted column, so a single participant can put @@ -337,14 +324,9 @@ async function clearIfDrained(experimentID: string): Promise // limit(1) rather than count(): a plain query inside a transaction is // universally supported by the Admin SDK and the emulator, costs one // document read, and answers the only question being asked -- "is anything - // still unresolved?". Index-wise this is `experimentID ==` + `status in`, - // which the existing composite (experimentID, status, providerErrorCode) - // serves as a prefix, and finalization.ts already runs the same shape. - const stillUnresolved = db - .collection("uploadQueue") - .where("experimentID", "==", experimentID) - .where("status", "in", UNRESOLVED_STATUSES) - .limit(1); + // still unresolved?". The shape, and the composite index it rests on, are + // upload-retention.ts's -- which owns what "unresolved" means. + const stillUnresolved = unresolvedQueueEntriesQuery(experimentID).limit(1); return db.runTransaction(async (tx) => { // ---------------- ALL READS FIRST (Firestore transaction law) ---------- diff --git a/functions/src/upload-retention.ts b/functions/src/upload-retention.ts new file mode 100644 index 0000000..95d55e4 --- /dev/null +++ b/functions/src/upload-retention.ts @@ -0,0 +1,221 @@ +// Whether a researcher's unuploaded data may be destroyed yet. +// +// Its own module, and a pure function, for two reasons. It decides the least +// reversible thing in this codebase -- deleting research data the researcher +// has not got back -- so it should be assertable as a table rather than +// provoked through a scheduler. And scheduled-upload-retry.ts, where the +// deletion actually happens, imports the whole provider stack; a test that +// wanted only this predicate would have to load all of it. +// +// --------------------------------------------------------------------------- +// WHY THE PLAIN AGE TEST WAS NOT ENOUGH +// --------------------------------------------------------------------------- +// +// The cleanup sweep used to delete on `createdAt <= now - 7d` alone -- no check +// on status, and no check on whether anyone had been told. Two ways that loses +// data nobody meant to lose: +// +// 1. A STORAGE PROVIDER OUTAGE. The entry is still `pending` with retries +// left, so it would have uploaded fine on day eight. Deleting it on day +// seven throws away data that was never actually lost. +// +// 2. A NOTIFICATION THAT NEVER ARRIVED. The seven days are counted from +// SUBMISSION, so part of the window is already spent before anything goes +// wrong -- and if the notification died (a Resend quota outage, say) the +// researcher's window closes without them ever learning there was one. +// +// `retainUntil` is written at the bottom of this file, and +// scheduled-mail-retry.ts is what calls it: while an upload-failure +// notification for the experiment is undelivered -- including the pass that +// finally gives up on it, so that abandoning a notification does not also +// quietly shorten the window for the data it was about. +// +// NOTE WHAT DOES NOT WRITE IT. An experiment whose owner has no contact email +// never produces a mail document at all -- upload-failure-notify.ts records +// `suppressedReason: "no-contact-email"` and returns before enqueuing -- so it +// is never extended and keeps the plain seven days. That is the right answer +// when there is nobody to tell, and it falls out of the design rather than +// being special-cased. + +import { Timestamp } from "firebase-admin/firestore"; +import { db } from "./app.js"; + +// The ceiling on everything below. An entry is deleted once it reaches this +// age no matter what else is still true about it. +// +// The extensions exist so a researcher gets a fair chance to act on data that +// has not uploaded. This exists so that chance cannot become permanent storage +// of research payloads DataPipe was never able to deliver: an experiment whose +// provider is dead and whose owner never reads their mail would otherwise +// accumulate forever, silently, at DataPipe's cost. Fourteen days is the plain +// seven a researcher was always promised, plus another seven to absorb an +// outage or a missed notification. +export const ABSOLUTE_MAX_RETENTION_MS = 14 * 24 * 60 * 60 * 1000; + +function millisOrZero(value: unknown): number { + if (!value || typeof (value as { toMillis?: unknown }).toMillis !== "function") { + return 0; + } + return (value as { toMillis: () => number }).toMillis(); +} + +/** + * Is this aged-out queue entry safe to delete? + * + * Only ever asked about entries the sweep has already found by age, so + * "delete" here means "the seven days are up AND none of the reasons to keep it + * apply". + */ +export function retentionDecision( + data: FirebaseFirestore.DocumentData, + nowMs: number +): "delete" | "retain" { + const createdAt = millisOrZero(data.createdAt); + + // The ceiling wins over every reason to keep it. Checked FIRST, and checked + // even when createdAt is missing or unreadable -- an entry must not become + // immortal by lacking a field. + if (createdAt === 0 || nowMs - createdAt >= ABSOLUTE_MAX_RETENTION_MS) { + return "delete"; + } + + // Still live work. The upload itself may yet succeed, which makes this data + // not merely un-notified but not actually lost. "Pending" alone is not + // enough: an entry that has spent its whole retry budget is not live work, + // it is a corpse with a hopeful status. + const retryCount = typeof data.retryCount === "number" ? data.retryCount : 0; + const maxRetries = typeof data.maxRetries === "number" ? data.maxRetries : 0; + if (data.status === "pending" && retryCount < maxRetries) { + return "retain"; + } + + // The researcher has not been told yet, and still might be. + if (millisOrZero(data.retainUntil) > nowMs) { + return "retain"; + } + + return "delete"; +} + +// --------------------------------------------------------------------------- +// Holding data back while the researcher has not been told about it. +// --------------------------------------------------------------------------- +// +// The WRITE lives here, next to the predicate that reads it, rather than in +// scheduled-mail-retry.ts where it started. The rule "may this data be +// destroyed, and what holds it back" is one decision; splitting it across the +// mail sweeper, this module and the deletion sweep meant three files in two +// subsystems had to be read together to answer a single question, and the mail +// subsystem had to know the uploadQueue's schema to do it. +// +// The mail sweeper now says only the thing it actually knows -- "this +// experiment's researcher has not been told yet" -- and this module decides +// what that means for their data. +// +// WHAT WAS CONSIDERED AND NOT DONE: inverting the dependency completely, so +// that cleanupOldEntries asks "is a notification for this experiment still +// undelivered?" at the moment of deletion and nothing is written ahead of time. +// It is the tidier shape, but it makes the least reversible operation in the +// codebase depend on a query that can fail, and a failing query there deletes +// data. `retainUntil` fails the other way: a write that does not happen costs +// the researcher an extension, not their data. + + +// "Not resolved yet, from the researcher's point of view." Deliberately the +// predicate the dashboard already uses (pages/admin/[experiment_id].js:46-53 +// and api-queue-status.ts:143-144), INCLUDING `failed`, and deliberately not +// finalization.ts's, which excludes `failed` so that a permanently dead file +// cannot block sealing forever. +// +// Including `failed` is what stops the upload-failure notification re-arming +// while a dead file is still sitting there: that file is an unresolved problem +// the researcher has not dealt with, and re-arming would mail them again about +// a queue that never actually got better. The episode closes when the last +// unresolved entry either completes or is swept away by cleanupOldEntries. +export const UNRESOLVED_STATUSES = ["pending", "processing", "failed"]; + +/** + * Every uploadQueue entry for one experiment that the researcher would still + * call unresolved. + * + * One builder rather than two, because both callers rest on the same index + * argument: `experimentID ==` plus `status in` is served as a prefix by the + * existing (experimentID, status, providerErrorCode) composite. The caller adds + * its own limit -- upload-failure-notify.ts only asks whether anything is left, + * this module has to touch all of them. + */ +export function unresolvedQueueEntriesQuery( + experimentID: string +): FirebaseFirestore.Query { + return db + .collection("uploadQueue") + .where("experimentID", "==", experimentID) + .where("status", "in", UNRESOLVED_STATUSES); +} + +// How much longer a researcher's unuploaded data is kept while the +// notification about it is still undelivered. +// +// Seven days is not arbitrary: it is the SAME window the researcher would have +// had if the notification had worked. scheduled-upload-retry.ts's sweep counts +// from `createdAt` -- submission -- so by the time a notification fails, part +// of that window is already spent on a problem the researcher could not have +// known about. This gives it back, and keeps giving it back until they are +// actually told. The absolute ceiling is ABSOLUTE_MAX_RETENTION_MS above, and +// the deletion itself is scheduled-upload-retry.ts's, so there is exactly one +// place that can decide data is gone. +export const RETENTION_GRACE_MS = 7 * 24 * 60 * 60 * 1000; + +// Unresolved entries touched per undelivered notification. An episode is +// usually a handful of files; the cap is there so one pathological experiment +// cannot make a sweep unbounded. +export const RETENTION_BATCH = 100; + +// Don't rewrite a `retainUntil` that is already most of the way out. +// +// The extension is driven by the mail sweeper, which runs every ten minutes for +// as long as a notification stays undelivered -- so without this, a day-long +// outage with a couple of dozen queued notifications rewrites the same field +// tens of thousands of times to the same effect. Firestore's free tier is +// 20,000 writes a day. Half the grace window is the threshold because the +// consequence of skipping a write is bounded by it: the stored value cannot be +// closer than RETENTION_GRACE_MS/2 to expiring, which is three and a half days +// of slack on a sweep that runs every ten minutes. +export const RETENTION_REWRITE_FLOOR_MS = RETENTION_GRACE_MS / 2; + +/** + * Hold back the data an undelivered notification is ABOUT. + * + * The notification is per EPISODE, and an episode belongs to an experiment, not + * to one file -- `datapipe.queueDocId` merely records the entry that tripped + * it. So the extension has to cover every unresolved entry for the experiment; + * extending only the one that tripped it would leave the other nineteen failed + * files in the same episode expiring on schedule, which is the original bug in + * miniature. + * + * Returns how many entries are being held back (whether or not this call was + * the one that had to write them). + */ +export async function extendRetentionForExperiment( + experimentID: string, + nowMs: number +): Promise { + const entries = await unresolvedQueueEntriesQuery(experimentID) + .limit(RETENTION_BATCH) + .get(); + if (entries.empty) return 0; + + const floor = nowMs + RETENTION_REWRITE_FLOOR_MS; + const stale = entries.docs.filter( + (entry) => millisOrZero(entry.get("retainUntil")) < floor + ); + if (stale.length > 0) { + const retainUntil = Timestamp.fromMillis(nowMs + RETENTION_GRACE_MS); + const batch = db.batch(); + for (const entry of stale) { + batch.update(entry.ref, { retainUntil }); + } + await batch.commit(); + } + return entries.size; +} diff --git a/functions/src/verify-contact-email.ts b/functions/src/verify-contact-email.ts index 7f835e8..32969fa 100644 --- a/functions/src/verify-contact-email.ts +++ b/functions/src/verify-contact-email.ts @@ -42,6 +42,18 @@ function verificationRef(uid: string): FirebaseFirestore.DocumentReference { return db.collection(VERIFICATIONS_COLLECTION).doc(uid); } +// One answer, two ways to arrive at it: no record at all, and a record whose +// code was cleared because it could not be delivered. The researcher's next +// move is the same either way, so the wording is defined once rather than kept +// in step by hand. +const NO_CODE_REQUESTED = { + status: 400, + body: { + error: "Request a new verification code.", + code: "no-code-requested", + }, +}; + export const verifyContactEmail = onRequest({ cors: true }, async (req, res) => { if (req.method !== "POST") { res.status(405).json({ error: "Method not allowed" }); @@ -104,16 +116,22 @@ export const verifyContactEmail = onRequest({ cors: true }, async (req, res) => } if (!vSnap.exists) { - return { - status: 400, - body: { - error: "Request a new verification code.", - code: "no-code-requested", - }, - }; + return NO_CODE_REQUESTED; } const v = vSnap.data()!; + + // A record whose code was never delivered. send-contact-email- + // verification.ts clears `codeHash` and keeps the rest of the record -- + // the record is that endpoint's rate limit, so it may not be deleted, but + // there is no code out there to enter. Answering "request a new one" is + // both true and the same answer a missing record gets above; falling + // through would spend one of the five attempts on a code that does not + // exist. + if (typeof v.codeHash !== "string") { + return NO_CODE_REQUESTED; + } + const expiresAt = typeof v.expiresAt === "number" ? v.expiresAt : 0; if (Date.now() > expiresAt) { return { diff --git a/pages/admin/index.js b/pages/admin/index.js index c5bd488..a5ee2c0 100644 --- a/pages/admin/index.js +++ b/pages/admin/index.js @@ -18,6 +18,7 @@ import { } from "@chakra-ui/react"; import { Trash2 } from "lucide-react"; import AddSignInMethodBanner from "../../components/account/AddSignInMethodBanner"; +import UnverifiedEmailBanner from "../../components/account/UnverifiedEmailBanner"; import OsfSunsetNotice from "../../components/OsfSunsetNotice"; import { isLegacyOsfExperiment } from "../../lib/osf-sunset"; import { STORAGE_PROVIDERS } from "../../lib/provider-config"; @@ -30,19 +31,41 @@ import ConfirmDialog from "../../components/ui/ConfirmDialog"; export default function AdminPage({}) { return ( - {/* DESIGN.md §4 allows exactly two measures: 560px for a single-subject - settings column, 1100px for dashboard and marketing pages. This was - 960px, one of three different widths across the three dashboard - routes (960 / 540 / 1200). */} - - - - + ); } -function ExperimentList() { +// Inside AuthCheck rather than in AdminPage itself, because everything here +// reads `auth.currentUser` and AuthCheck is what guarantees there is one. +// +// ONE subscription to users/{uid} for this page, passed down -- the convention +// pages/admin/account.js already follows (ProviderConnections, ContactEmail, +// OAuthTokenStatus and SelectAuth all take `data` as a prop and none of them +// subscribes). Two components here read that document for different reasons, +// and before this they opened a listener each: two live listeners on one +// document, two independent loading flickers, and two chances for them to +// render disagreeing states for a frame. +function Dashboard() { + const user = auth.currentUser; + const [userDoc] = useDocumentData( + user?.uid ? doc(db, "users", user.uid) : null + ); + + return ( + /* DESIGN.md §4 allows exactly two measures: 560px for a single-subject + settings column, 1100px for dashboard and marketing pages. This was + 960px, one of three different widths across the three dashboard + routes (960 / 540 / 1200). */ + + + + + + ); +} + +function ExperimentList({ userDoc }) { const user = auth.currentUser; const experiments = collection(db, `experiments`); const q = query(experiments, where("owner", "==", user.uid)); @@ -54,9 +77,6 @@ function ExperimentList() { // step, not a maintenance screen), and firestore.rules enforces it. Offering // "Create your first experiment" to someone who will be refused is the dead // end the critique traces persona Jordan into. - const [userDoc] = useDocumentData( - user?.uid ? doc(db, "users", user.uid) : null - ); const hasProvider = Object.values(STORAGE_PROVIDERS).some((p) => p.isConnected(userDoc) );