diff --git a/.github/workflows/firebase-deploy-test.yml b/.github/workflows/firebase-deploy-test.yml index 2420c23..6b9cea8 100644 --- a/.github/workflows/firebase-deploy-test.yml +++ b/.github/workflows/firebase-deploy-test.yml @@ -91,22 +91,25 @@ jobs: # redirect uri and ZENODO_ENV are non-secret and live in # functions/.env.datapipe-test. echo "ZENODO_CLIENT_SECRET=${{ secrets.TEST_ZENODO_CLIENT_SECRET }}" >> .env - # Amazon SES for the deployed test site (functions/src/mail-delivery.ts). - # Leave these three repo secrets UNSET to keep the test site from - # mailing real people: the onmailcreated trigger then records a - # terminal MailConfigMissingError on each mail document and sends - # nothing, which is visible rather than silent. Set them only against - # an SES account still in the sandbox, whose verified identities are - # addresses you own. See docs/deploy-contact-email.md §2. - echo "SES_REGION=${{ secrets.TEST_SES_REGION }}" >> .env - echo "SES_ACCESS_KEY_ID=${{ secrets.TEST_SES_ACCESS_KEY_ID }}" >> .env - echo "SES_SECRET_ACCESS_KEY=${{ secrets.TEST_SES_SECRET_ACCESS_KEY }}" >> .env + # Resend for the deployed test site (functions/src/mail-delivery.ts). + # SET this one. The test site is the only place mail DELIVERY is + # exercised before production -- the emulator short-circuits before + # sending and the unit suites mock the transport -- so it is where a + # mail change is proved to pass DKIM/SPF and reach an inbox. Use a + # SEPARATE sending-only key from prod, on the same Resend account + # (the From domain is the same, so it has to be the same account). + # Unset is still safe rather than silent: the onmailcreated trigger + # records a terminal MailConfigMissingError on each mail document and + # sends nothing. Reputation and daily quota are shared with prod -- + # see docs/deploy-contact-email.md §2(d). + echo "RESEND_API_KEY=${{ secrets.TEST_RESEND_API_KEY }}" >> .env # Not secret, so literals here, the same way REDIRECT_URI above is. - # Same sender as production: SES verifies the DOMAIN (jspsych.org), - # not the deployment, and datapipe-test.web.app is not a verified - # identity -- sending from it was rejected outright, which is why - # contact-email codes never arrived on the test site. Only the - # display name differs, so test mail is recognisable in an inbox. + # Same sender as production: Resend verifies the DOMAIN + # (jspsych.org), not the deployment, and datapipe-test.web.app is not + # a verified domain -- sending from it is a 403 validation_error, + # which is why contact-email codes never arrived on the test site. + # Only the display name differs, so test mail is recognisable in an + # inbox. echo "MAIL_FROM=DataPipe (test) " >> .env echo "MAIL_REPLY_TO=datapipe@jspsych.org" >> .env - name: Install dependencies and build functions diff --git a/.github/workflows/firebase-deploy.yml b/.github/workflows/firebase-deploy.yml index ae0155c..1cd2ebd 100644 --- a/.github/workflows/firebase-deploy.yml +++ b/.github/workflows/firebase-deploy.yml @@ -67,17 +67,15 @@ jobs: echo "REDIRECT_URI=https://pipe.jspsych.org/oauth2/callback" >> .env echo "TOKEN_ENCRYPTION_KEY=${{ secrets.FIRESTORE_KEY_PRODUCTION }}" >> .env echo "NEXT_PUBLIC_OSF_ENV=" >> .env - # Amazon SES. functions/src/mail-delivery.ts delivers everything - # mail.ts queues into the `mail` collection; without these three the + # Resend. functions/src/mail-delivery.ts delivers everything mail.ts + # queues into the `mail` collection; without this key the # onmailcreated trigger writes a terminal MailConfigMissingError on - # every notification and sends nothing. The IAM user behind the keys - # is scoped to ses:SendEmail only -- see docs/deploy-contact-email.md - # §2, which also covers domain verification and sandbox exit. - echo "SES_REGION=${{ secrets.PROD_SES_REGION }}" >> .env - echo "SES_ACCESS_KEY_ID=${{ secrets.PROD_SES_ACCESS_KEY_ID }}" >> .env - echo "SES_SECRET_ACCESS_KEY=${{ secrets.PROD_SES_SECRET_ACCESS_KEY }}" >> .env + # every notification and sends nothing. Scope the key to Sending + # access only -- see docs/deploy-contact-email.md §2, which also + # covers domain verification. + echo "RESEND_API_KEY=${{ secrets.PROD_RESEND_API_KEY }}" >> .env # Not secret, so literals here, the same way REDIRECT_URI above is. - # MAIL_FROM must be an address on the SES-verified sending domain. + # MAIL_FROM must be an address on a domain verified in Resend. echo "MAIL_FROM=DataPipe " >> .env echo "MAIL_REPLY_TO=datapipe@jspsych.org" >> .env - name: Install dependencies and build functions diff --git a/docs/deploy-contact-email.md b/docs/deploy-contact-email.md index cc35d5e..97b045c 100644 --- a/docs/deploy-contact-email.md +++ b/docs/deploy-contact-email.md @@ -32,134 +32,130 @@ Both failure modes are silent-to-the-researcher permission errors, not crashes, which is exactly the kind of bug that sits undetected until a support email arrives. Deploy rules first. -## 2. Amazon SES setup - -> **This section replaced the Trigger Email extension.** The plan was -> `firebase/firestore-send-email`; that platform is deprecated, so delivery now -> lives in this repo — `functions/src/mail-delivery.ts`, a Firestore -> `onDocumentCreated("mail/{id}")` trigger deployed as `onmailcreated`, sending -> through the Amazon SES v2 API directly (no SMTP, no nodemailer). +## 2. Resend setup + +> **This section has been rewritten twice.** The original plan was the Firebase +> Trigger Email extension (`firebase/firestore-send-email`); that platform is +> deprecated, so delivery moved in-repo to `functions/src/mail-delivery.ts`, a +> Firestore `onDocumentCreated("mail/{id}")` trigger deployed as `onmailcreated`. +> That version sent through Amazon SES. **AWS denied the SES production-access +> request**, which would have left every send restricted to verified recipient +> addresses — i.e. failing for every real researcher — so the transport is now +> **Resend**, over its JSON HTTP API. +> +> **Nothing on the write side changed, either time.** `functions/src/mail.ts` +> still owns the document shape, and `mail-delivery.ts` still writes the +> extension's outcome fields (`delivery.state`, `delivery.attempts`, +> `delivery.startTime`, `delivery.endTime`, `delivery.error`, +> `delivery.info.messageId`), so §4's TTL policy and every test that reads the +> `mail` collection are unaffected. `delivery.info.transport` now reads +> `"resend"` rather than `"ses"`. > -> **Nothing on the write side changed.** `functions/src/mail.ts` still owns the -> document shape, and `mail-delivery.ts` still writes the extension's outcome -> fields (`delivery.state`, `delivery.attempts`, `delivery.startTime`, -> `delivery.endTime`, `delivery.error`, `delivery.info.messageId`), so §4's TTL -> policy and every test that reads the `mail` collection are unaffected. What -> changed is that there is no longer an extension to install, and no -> `SMTP_CONNECTION_URI` — the credentials are ordinary function env vars, -> plumbed exactly like `TOKEN_ENCRYPTION_KEY` already is. - -### (a) Verify the sending domain, and publish its DKIM records - -In the SES console → **Verified identities** → *Create identity* → **Domain**, -enter the sending domain (`jspsych.org` — the org domain, which covers every -subdomain, so `pipe.jspsych.org` needs no separate identity), and leave **Easy DKIM** on -(RSA_2048). SES then hands back **three CNAME records**: +> **What got simpler.** One secret instead of three; no region to keep in sync +> with a verified identity; no IAM user, no long-lived AWS access key, and no +> request signing that a stray newline can break. The `@aws-sdk/client-sesv2` +> dependency is gone — delivery is one `fetch` to one endpoint, so there is no +> SDK on the cold-start path to keep off it either. +> +> **If you are migrating an already-deployed SES setup**, see (f) at the end +> for what to tear down. -``` -._domainkey.jspsych.org CNAME .dkim.amazonses.com -._domainkey.jspsych.org CNAME .dkim.amazonses.com -._domainkey.jspsych.org CNAME .dkim.amazonses.com -``` +### (a) Verify the sending domain, and publish the DNS records + +Resend dashboard → **Domains** → *Add Domain* → `jspsych.org`, and pick the +region you want to send from. + +Resend then shows the exact records to publish — **the dashboard is the +authority on the values, which are per-account; do not copy them from here.** +The shape is three things: -Publish all three in DNS. Verification usually completes within an hour; -the identity's status must read **Verified** before anything is sent. +- a **DKIM** `TXT` record on a `resend._domainkey` host, +- an **SPF** `TXT` record on a `send` subdomain, and +- an **MX** record on that same `send` subdomain, which is the custom + Return-Path that makes SPF align with your From domain. -Two optional-but-recommended records while you have DNS open: +Publish all of them and wait for the domain to read **Verified** in the +dashboard. Until it does, every send fails with a `403 validation_error` +("domain is not verified"), which `mail-delivery.ts` classifies as +**permanent** — the mail is not retried, it is marked terminally failed. -- **A custom MAIL FROM domain** (e.g. `mail.jspsych.org`), which needs an - MX record pointing at `feedback-smtp..amazonses.com` and a TXT - record `"v=spf1 include:amazonses.com ~all"`. This is what makes SPF align - with the From domain; without it, SES sends SPF-aligned to - `amazonses.com` and only DKIM carries alignment. -- **A DMARC record** on the org domain: `_dmarc.jspsych.org TXT - "v=DMARC1; p=none; rua=mailto:"`. Start at `p=none`; - it reports without rejecting. +Two notes on scope: + +- **A verified root domain does not automatically cover subdomains in Resend**, + unlike an SES identity. This does not matter here — `MAIL_FROM` is an address + *at* `jspsych.org`, not at `pipe.jspsych.org` — but it is worth knowing before + someone changes the From address to a subdomain and watches every mail fail. +- **DMARC** is worth adding while DNS is open: `_dmarc.jspsych.org TXT + "v=DMARC1; p=none; rua=mailto:"`. Start at `p=none`; it + reports without rejecting. **The spam warning still stands, and it is the reason this step is first.** -Without DKIM (and ideally SPF alignment) in place, a meaningful share of these +Without DKIM and SPF alignment in place, a meaningful share of these notifications will land in spam or be silently dropped — and because this feature exists specifically to reach a researcher whose data has stopped arriving, a notification nobody sees is functionally the same as no notification at all. This needs DNS access to `jspsych.org`. Confirm the -records are in place (or in progress) before flipping this on in prod, and -leave the SES secrets unset in the interim rather than sending unauthenticated -mail from a new address — an unset secret is a loud, recorded failure (see (d)), -not a silent one. - -### (b) Leave the SES sandbox - -A new SES account is in the **sandbox**: it can only send *to* verified -addresses, and is capped at 200 messages/day. Every notification to a real -researcher would be rejected with `MessageRejected` ("Email address is not -verified"), which `mail-delivery.ts` classifies as **permanent** — the mail is -not retried, it is marked terminally failed. So a production deploy that skips -this step does not degrade gracefully; it fails every send. - -SES console → **Account dashboard** → *Request production access*. The request -asks for the use case; the honest answer is short and is what gets approved: -transactional-only mail, to addresses the recipient entered themselves on their -own account page, one notification per experiment per 24 hours maximum -(`RATE_LIMIT_MS` in `upload-failure-notify.ts`), plus verification codes the -recipient just asked for. No marketing, no lists, no purchased addresses. -Mention that bounces and complaints are visible because the sending volume is -tiny. Turnaround is typically one business day. - -Sanity-check the granted **sending quota** afterwards. DataPipe's steady-state -volume is minuscule, but the burst case is real: an outage at a storage -provider can put many experiments into a failure episode at once. - -### (c) An IAM user scoped to `ses:SendEmail`, and nothing else - -Cloud Functions has no way to assume an AWS role, so this is a long-lived -access key. Keep it worth as little as possible: - -```json -{ - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "SendOnlyFromDataPipeIdentity", - "Effect": "Allow", - "Action": "ses:SendEmail", - "Resource": "arn:aws:ses:::identity/jspsych.org", - "Condition": { - "StringEquals": { - "ses:FromAddress": "datapipe-notifications@jspsych.org" - } - } - } - ] -} -``` - -Create the IAM user with **no console access**, attach only this policy, and -create one access key. `ses:SendEmail` covers the v2 `SendEmail` call this code -makes; `ses:SendRawEmail` is *not* needed (nothing here sends raw MIME), and -neither is any `ses:Get*`/`ses:List*` — the function never reads SES state. -The `Resource` and the `ses:FromAddress` condition are what stop a leaked key -from being used to send as anything other than DataPipe. - -### (d) Three repo secrets, two literals, and the workflows that write them +records are in place before flipping this on in prod, and leave `RESEND_API_KEY` +unset in the interim rather than sending from an unverified domain — an unset +secret is a loud, recorded failure (see (d)), not a silent one. + +### (b) What replaces the SES sandbox + +There is no sandbox to exit, which is the entire reason for the migration. Two +limits take its place, and both behave differently from the SES one: + +- **Before the domain is verified**, a Resend account can only send to the + account owner's own address. Same failure shape as the SES sandbox, same + terminal classification — and the fix is (a), not a support ticket. +- **After verification, the plan caps apply.** The free plan is **3,000 + emails/month with a 100/day ceiling**; Pro is $20/month for 50,000. DataPipe's + steady-state volume is far below either — verification codes are one per + address change (with a resend cooldown in + `send-contact-email-verification.ts`), and failure notifications are capped at + one per experiment per 24 hours (`RATE_LIMIT_MS` in + `upload-failure-notify.ts`). + + **The burst case is the one to size for, not the steady state.** An outage at + a storage provider puts many experiments into a failure episode at once, and + 100/day is reachable there. When it is reached, Resend answers + `429 daily_quota_exceeded`, which `mail-delivery.ts` classifies as retryable — + but nothing retries a retryable error on a timer (see the file header), so in + practice those notifications are undelivered until someone re-drives them. + That is precisely the moment these mails matter most, so + **`daily_quota_exceeded` in the logs is the signal to move to a paid plan.** + It is the second line worth alerting on, after `MailConfigMissingError`. + +### (c) An API key scoped to sending, and to this domain + +Resend dashboard → **API keys** → *Create API key*: + +- **Permission: sending access**, not full access. A sending-only key cannot + read the account, list domains, or create other keys. +- **Domain: `jspsych.org`.** The optional domain restriction is the Resend + equivalent of the `ses:FromAddress` IAM condition it replaces, and it is what + stops a leaked key from being used to send as anything other than DataPipe. + +The key is shown **once**. Put it straight into the repo secret in (d); if it is +lost, delete it and make another rather than leaving an unaccounted-for key +active. + +### (d) Two repo secrets, two literals, and the workflows that write them The credentials reach the functions as ordinary environment variables in `functions/.env`, written at deploy time from GitHub repo secrets — the same -mechanism `TOKEN_ENCRYPTION_KEY` already uses. There is no Secret Manager -entry and no extension config to keep in sync. +mechanism `TOKEN_ENCRYPTION_KEY` already uses. There is no Secret Manager entry +and no extension config to keep in sync. | Function env var | Secret? | Production value from | Test value from | |---|---|---|---| -| `SES_REGION` | yes (repo secret) | `PROD_SES_REGION` | `TEST_SES_REGION` | -| `SES_ACCESS_KEY_ID` | yes (repo secret) | `PROD_SES_ACCESS_KEY_ID` | `TEST_SES_ACCESS_KEY_ID` | -| `SES_SECRET_ACCESS_KEY` | yes (repo secret) | `PROD_SES_SECRET_ACCESS_KEY` | `TEST_SES_SECRET_ACCESS_KEY` | -| `MAIL_FROM` | no — literal in the workflow | `DataPipe ` | `DataPipe (test) ` — same verified address, display name marks it as test | +| `RESEND_API_KEY` | yes (repo secret) | `PROD_RESEND_API_KEY` | `TEST_RESEND_API_KEY` | +| `MAIL_FROM` | no — literal in the workflow | `DataPipe ` | `DataPipe (test) ` — same verified domain, display name marks it as test | | `MAIL_REPLY_TO` | no — literal in the workflow | `datapipe@jspsych.org` | `datapipe@jspsych.org` | -So: **three new repo secrets per environment** (region, access key id, secret -access key), six in total, named in the repo's existing -`PROD_*`/`TEST_*` style (`PROD_CLIENT_SECRET`, `TEST_GDRIVE_CLIENT_SECRET`, …). -The region is a secret rather than a literal only so that prod and test can -point at different SES accounts without editing a workflow. +So: **one new repo secret per environment**, two in total, named in the repo's +existing `PROD_*`/`TEST_*` style (`PROD_CLIENT_SECRET`, +`TEST_GDRIVE_CLIENT_SECRET`, …). This replaces the six `*_SES_*` secrets the +previous version of this section called for. They are written in the **`Create functions environment file`** step of: @@ -168,15 +164,15 @@ They are written in the **`Create functions environment file`** step of: immediately after the `TOKEN_ENCRYPTION_KEY` line. `MAIL_FROM` and `MAIL_REPLY_TO` are plain literals in the same block, like `REDIRECT_URI`. -`.github/workflows/node.js.yml` (CI) deliberately writes **none** of them — see +`.github/workflows/node.js.yml` (CI) deliberately writes **neither** — see "What happens without configuration" below. -`MAIL_FROM` must be an address on the SES-verified identity from (a), and it -must match the `ses:FromAddress` condition in (c). `MAIL_REPLY_TO` is a -forwarding alias on `jspsych.org` that reaches the operating team; it is optional (mail with -no Reply-To is deliverable, mail with a bad one is not). +`MAIL_FROM` must be an address on the Resend-verified domain from (a), and on +the domain the key is restricted to in (c). `MAIL_REPLY_TO` is a forwarding +alias on `jspsych.org` that reaches the operating team; it is optional (mail +with no Reply-To is deliverable, mail with a bad one is not). -**What happens without configuration.** Missing or blank SES config is a +**What happens without configuration.** Missing or blank config is a **terminal** delivery error with the distinct name `MailConfigMissingError`, written onto the mail document (`delivery.error.name`, and `delivery.error.message` naming the missing keys) and logged at error level @@ -184,44 +180,101 @@ naming the keys — never their values. Mail never silently vanishes, but note that terminal means terminal: **documents that failed this way are not retried once the config is fixed.** Re-drive them by deleting the `delivery` field, or accept the loss. Grep production logs for `MailConfigMissingError`; it is the -single line worth alerting on, because it means every notification the +first line worth alerting on, because it means every notification the deployment sends is being dropped. **The test site and the emulator.** -- **`datapipe-test`**: leaving the three `TEST_SES_*` secrets unset is the safe - default — the trigger records `MailConfigMissingError` and mails nobody, which - is visible rather than silent. If you do want the test site to send, the - `TEST_SES_*` credentials must belong to an SES account (or the production - one, with a separate IAM user) in which `jspsych.org` is a verified identity, - because the test `MAIL_FROM` is `datapipe-notifications@jspsych.org` too — - an address on `datapipe-test.web.app` can never be verified and was rejected - by SES. If that account is still in the SES sandbox it can only deliver - *to* verified recipient addresses as well. +- **`datapipe-test`: set `TEST_RESEND_API_KEY`.** The test site is expected to + send, and that is a deliberate reversal of the "leave it unset" default an + earlier draft of this section recommended. + + The reason is that **the test site is the only place delivery is exercised + before production.** The emulator cannot do it — see the next bullet, + `onmailcreated` returns before reading, writing or sending — and the unit + suites mock the transport at a function seam, so nothing below the test site + proves that a real message leaves the building, passes DKIM/SPF/DMARC and + lands in an inbox rather than a spam folder. Ship a mail change straight to + prod without that step and the first real send is to a researcher whose data + has stopped arriving, which is the worst possible audience for a first + attempt. + + Use a **separate sending-only key from prod, on the same Resend account.** It + has to be the same account: the test `MAIL_FROM` is + `datapipe-notifications@jspsych.org` too, and a `datapipe-test.web.app` + address can never be verified. Two keys rather than one shared key, because + test can then be revoked without touching prod, and Resend attributes sends + per key, so test traffic stays distinguishable in the dashboard. - **The emulator (and therefore CI)**: `onmailcreated` checks `FUNCTIONS_EMULATOR` and returns before doing anything at all — no read, no - write, no send. Un-delivered mail in an emulator run is expected, exactly as - it was when the extension (which also never ran against the emulator) was the - plan. This gate is also what keeps the live trigger from racing the test - suites' `mail` fixtures under `firebase emulators:exec`; see the header of + write, no send. Un-delivered mail in an emulator run is expected. This gate + matters more than it did under SES: a real key sitting in a developer's + `functions/.env` would otherwise let a local test run mail a real person. It + is also what keeps the live trigger from racing the test suites' `mail` + fixtures under `firebase emulators:exec`; see the header of `functions/src/__tests__/mail-delivery-emulator.test.js`. -### (e) Region - -Pick one region and keep the identity, the IAM policy's `Resource` ARN, and -`SES_REGION` in agreement — a verified identity exists **per region**, and an -identity verified in `us-east-1` does not exist in `us-west-2`. +**Verifying delivery before a production deploy.** Two things are worth actually +sending on `datapipe-test`, and they are not equally easy to provoke: + +- **A verification code** is trivial: change the contact email on a test + account and `send-contact-email-verification.ts` mails one. This is the one + to use for deliverability checks — DKIM/SPF alignment, inbox versus spam + folder, how the From name and Reply-To render in a real client. +- **An upload-failure notification** needs a real failure *episode*. + `upload-failure-notify.ts` opens one only on a failure DataPipe has actually + recorded, and rate-limits to one mail per experiment per 24 hours + (`RATE_LIMIT_MS`), so it cannot be provoked by repetition — you have to make + an upload genuinely fail on the test site, e.g. revoke a test experiment's + storage-provider token and then post data to it. Budget more time for this + one than for the code, and remember the 24-hour floor between attempts on the + same experiment. + +**Two cautions, both because test and prod share one sending identity.** + +- **Reputation is shared.** Both deployments send from + `datapipe-notifications@jspsych.org` on the same verified domain, so bounces + and spam complaints generated by test sends damage the reputation that + *production* notifications depend on. Send test mail only to addresses you + control, and never to an invented one — an invented address bounces, and + bounces are the expensive kind of mistake here. +- **Quota is shared.** Both keys draw on the same Resend account plan: on the + free plan that is 100/day across *both* deployments, not 100 each. The bad + case is specific — a test run consuming quota while a storage-provider outage + is firing the production burst described in (b). If the test site sends + routinely, that argues for the paid plan sooner than production volume alone + would. -`us-east-2` is this deployment's region -- the AWS project lives there, so the identity, the IAM user, and `SES_REGION` all say `us-east-2`. (Historically `us-east-1` was the SES default suggestion, -it is the region most SES documentation and tooling assumes, and it is closest -to the functions' own `us-central1`, which keeps the cross-region hop on the -send negligible. There is no data-residency argument to weigh here — the only -personal data crossing to AWS is the recipient address and the message body, -and both are transient. +### (e) Region -Whatever you choose, it must match on all three of: the verified identity, the -`arn:aws:ses::...` in the IAM policy, and `PROD_SES_REGION` / -`TEST_SES_REGION`. +A Resend domain is created in a region, and that is the whole of it — the region +lives in the dashboard, not in this repo. There is no `SES_REGION` to keep in +agreement with an identity and an IAM ARN, and no way for the three to drift +apart, which was the failure mode this section used to warn about. + +There is no data-residency argument to weigh: the only personal data reaching +Resend is the recipient address and the message body, and both are transient. + +### (f) Tearing down the SES setup + +Only relevant if the SES half was already deployed. None of it is load-bearing +any more, and all of it is credential surface: + +1. **Delete the IAM user** created for `ses:SendEmail`, which deletes its + long-lived access key with it. This is the one that matters — it is a + standing credential that nothing reads. +2. **Delete the six `*_SES_*` repo secrets** (`PROD_SES_REGION`, + `PROD_SES_ACCESS_KEY_ID`, `PROD_SES_SECRET_ACCESS_KEY`, and the `TEST_*` + three). The workflows no longer reference them. +3. **The SES verified identity and its DNS records can stay or go.** The three + `*._domainkey.jspsych.org` DKIM CNAMEs are inert once nothing sends through + SES. Leaving them costs nothing and keeps the option of going back if the + appeal ever succeeds; removing them is tidier. Do **not** remove the `_dmarc` + record — Resend wants that one too. +4. If a custom SES MAIL FROM subdomain (`mail.jspsych.org`) was set up, its MX + and SPF records are now unused. Resend uses its own `send` subdomain for the + same purpose, so these do not conflict — but leaving a stale SPF record on a + subdomain nothing sends from is a small, avoidable piece of confusion. ## 3. Backfill run procedure @@ -261,7 +314,7 @@ Run order: 3. **Target project:** the script uses the same three-way init as the other `migrations/*.cjs` scripts — `NODE_ENV=production` or `CI` for a service account from `GOOGLE_CREDENTIALS`/application-default credentials against - `FIREBASE_PROJECT_ID` (default `datapipe-prod`); `USE_LOCAL_SERVICE_ACCOUNT` + `FIREBASE_PROJECT_ID` (default `osf-relay`); `USE_LOCAL_SERVICE_ACCOUNT` for a local service-account file against `datapipe-test`; otherwise it points at the Firestore *and* Auth emulators (`localhost:8080` / `localhost:9099`) against `datapipe-test`. Double-check @@ -291,15 +344,21 @@ gcloud firestore fields ttls update 'delivery.expireAt' \ --collection-group=mail \ --enable-ttl \ --database='(default)' \ - --project=datapipe-prod + --project=osf-relay ``` (Console equivalent: Firestore → **Time-to-live (TTL)** → *Create policy* → -collection group `mail`, timestamp field `delivery.endTime`.) Repeat with +collection group `mail`, timestamp field **`delivery.expireAt`** — the same +field as the command above, NOT `delivery.endTime`. An earlier draft of this +line said `endTime`, which would have been silently wrong in the expensive +direction: native TTL deletes as soon as the timestamp passes, and `endTime` is +set the instant an outcome turns terminal, so that policy reaps every mail +document the moment it is delivered. No debugging window, and nothing anywhere +saying it happened. See the first bullet below.) Repeat with `--project=datapipe-test`. Confirm afterwards with: ``` -gcloud firestore fields ttls list --collection-group=mail --project=datapipe-prod +gcloud firestore fields ttls list --collection-group=mail --project=osf-relay ``` Three things about this policy that are worth knowing before you run it: @@ -344,6 +403,6 @@ in the main design doc, not here. Function secrets: the contact-email feature itself adds none — verification codes are SHA-256 hashed, not encrypted, and `TOKEN_ENCRYPTION_KEY` is -untouched. The three `*_SES_*` secrets in §2(d) belong to **delivery**, which +untouched. The `RESEND_API_KEY` secret in §2(d) belongs to **delivery**, which was the extension's job when that sentence was written and is now `functions/src/mail-delivery.ts`'s. diff --git a/functions/package-lock.json b/functions/package-lock.json index b79b2fa..9d54c5f 100644 --- a/functions/package-lock.json +++ b/functions/package-lock.json @@ -6,7 +6,6 @@ "": { "name": "functions", "dependencies": { - "@aws-sdk/client-sesv2": "3.1116.0", "@jspsych/metadata": "file:metadata", "archiver": "^7.0.1", "cors": "^2.8.5", @@ -36,279 +35,6 @@ "csv-parse": "^5.5.6" } }, - "node_modules/@aws-sdk/client-sesv2": { - "version": "3.1116.0", - "resolved": "https://registry.npmjs.org/@aws-sdk/client-sesv2/-/client-sesv2-3.1116.0.tgz", - "integrity": "sha512-5j2Zu5e6I0GbvrpSe86+z4I760TWGiypkBTNVGF+B46h8CkcWACG8EMwi0xVpXbjAQm1sBRvIFoinIJaEP0qcA==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/credential-provider-node": "^3.972.81", - "@aws-sdk/signature-v4-multi-region": "^3.996.46", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/fetch-http-handler": "^5.7.2", - "@smithy/node-http-handler": "^4.11.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/core": { - "version": "3.977.9", - "resolved": "https://registry.npmjs.org/@aws-sdk/core/-/core-3.977.9.tgz", - "integrity": "sha512-reqPFEQrZxDZpeGj4PFMepBeR5LGYHRqq/L0motTzgFkCRBA4rFdaVXDSLYyGHhxVz7sT2PDnPN9CluGSfgyJA==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/types": "^3.974.5", - "@aws-sdk/xml-builder": "^3.972.40", - "@aws/lambda-invoke-store": "^0.3.0", - "@smithy/core": "^3.33.3", - "@smithy/signature-v4": "^5.6.12", - "@smithy/types": "^4.17.2", - "bowser": "^2.11.0", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-env": { - "version": "3.972.70", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-env/-/credential-provider-env-3.972.70.tgz", - "integrity": "sha512-H404B7dJl2mCrBqahDEYsanB0xhdDp6tXnXcTUnXmmpy2Q3J0Ho0bUajZ2jr/RdwzCyS59Gi8xXIFwPLGBl6Uw==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-http": { - "version": "3.972.72", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-http/-/credential-provider-http-3.972.72.tgz", - "integrity": "sha512-X98zYOrVOeuosCX+6ktf29FC2N2GHPLia7qv6mzPzTc+RPAuHWCDS++Z6JK7eGYqb/v6uaW7bAXaOvDBfol+0w==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/fetch-http-handler": "^5.7.2", - "@smithy/node-http-handler": "^4.11.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-ini": { - "version": "3.973.15", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-ini/-/credential-provider-ini-3.973.15.tgz", - "integrity": "sha512-Rykg6s5ceBuynMOGWgoowO4N+27JfnqXAnVaSunZl0hOO1XodSrxGNz6sCEbnmS0lAfQZDKyb3fbr46gSuv6Sg==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/credential-provider-env": "^3.972.70", - "@aws-sdk/credential-provider-http": "^3.972.72", - "@aws-sdk/credential-provider-login": "^3.972.77", - "@aws-sdk/credential-provider-process": "^3.972.70", - "@aws-sdk/credential-provider-sso": "^3.973.14", - "@aws-sdk/credential-provider-web-identity": "^3.972.76", - "@aws-sdk/nested-clients": "^3.997.44", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/credential-provider-imds": "^4.4.16", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-login": { - "version": "3.972.77", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-login/-/credential-provider-login-3.972.77.tgz", - "integrity": "sha512-Jb59xfEISoN5mmbnA+HYqdtrSX3CgCtJoof+V5D8/TgUI56W63GEEd5Y58WijU3Ou6+WEgaLD1feVzaRXV5IDQ==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/nested-clients": "^3.997.44", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-node": { - "version": "3.972.81", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-node/-/credential-provider-node-3.972.81.tgz", - "integrity": "sha512-Rml+WitoFvXmv6JZ18U/xGdGDGGvB/mOin0ya0lTnTrdC0Z1lrVxTYh7iNklZBcvcRMrs4DoEf6xy1KWyrLQQw==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/credential-provider-env": "^3.972.70", - "@aws-sdk/credential-provider-http": "^3.972.72", - "@aws-sdk/credential-provider-ini": "^3.973.15", - "@aws-sdk/credential-provider-process": "^3.972.70", - "@aws-sdk/credential-provider-sso": "^3.973.14", - "@aws-sdk/credential-provider-web-identity": "^3.972.76", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/credential-provider-imds": "^4.4.16", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-process": { - "version": "3.972.70", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-process/-/credential-provider-process-3.972.70.tgz", - "integrity": "sha512-2ry03fGRJr4sV3jI+ocjj5JqALnFD6ymM5KiNCDZMvq8bX2GSbE0vji4aM43TVCl2nXqqLRZaUxdq/KeWRAY4Q==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-sso": { - "version": "3.973.14", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-sso/-/credential-provider-sso-3.973.14.tgz", - "integrity": "sha512-jkhg/8ocAAoc0RFyLMhCw+/zZh7gystQgd4F4hznNa8P4Cc501PQmxd+jGLiMHodPJ+7Zv/3znM62gZojyasmA==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/nested-clients": "^3.997.44", - "@aws-sdk/token-providers": "3.1116.0", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/credential-provider-web-identity": { - "version": "3.972.76", - "resolved": "https://registry.npmjs.org/@aws-sdk/credential-provider-web-identity/-/credential-provider-web-identity-3.972.76.tgz", - "integrity": "sha512-d3AGyVu759PGr35mEB2s22xxlNEA5rpdxtSPJthfPFJvoQ8dt357iVPECqWfUxXp1toJAvKmbtcIYVGigaGsCA==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/nested-clients": "^3.997.44", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/nested-clients": { - "version": "3.997.44", - "resolved": "https://registry.npmjs.org/@aws-sdk/nested-clients/-/nested-clients-3.997.44.tgz", - "integrity": "sha512-NhEgryjlBF9w38ZXqGymQV28IhkYa1mKhlbYnqIis57AYwWGVYfUPgg/qC2rLRqOUfblxx++irvju10kVTa8Vw==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/signature-v4-multi-region": "^3.996.46", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/fetch-http-handler": "^5.7.2", - "@smithy/node-http-handler": "^4.11.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/signature-v4-multi-region": { - "version": "3.996.46", - "resolved": "https://registry.npmjs.org/@aws-sdk/signature-v4-multi-region/-/signature-v4-multi-region-3.996.46.tgz", - "integrity": "sha512-L+2xZTye/2T96f3lwCws0Zw6GG2JHZW9e8FpVgGBeeExSKyeoZ6CWRpBml/7DNiK/O26jrgPM9F+Ay8VkgzUWQ==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/types": "^3.974.5", - "@smithy/signature-v4": "^5.6.12", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/token-providers": { - "version": "3.1116.0", - "resolved": "https://registry.npmjs.org/@aws-sdk/token-providers/-/token-providers-3.1116.0.tgz", - "integrity": "sha512-ygIivKqh8aHzNkucOCXHyIBgBpLPfrSI0mCqXF+vLBsPTUKqj0VSqAY0GFPe7lQl4HntjOcQ+KSyS7oUV2C54Q==", - "license": "Apache-2.0", - "dependencies": { - "@aws-sdk/core": "^3.977.9", - "@aws-sdk/nested-clients": "^3.997.44", - "@aws-sdk/types": "^3.974.5", - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/types": { - "version": "3.974.5", - "resolved": "https://registry.npmjs.org/@aws-sdk/types/-/types-3.974.5.tgz", - "integrity": "sha512-LkwLL2BLbC6wNNm4JaH9mbEqBMdOZCct6VAYqhdN4U1xrWM+fUJQEfbHwQgDypapOWTRtlk25akb5afM0P8CIQ==", - "license": "Apache-2.0", - "dependencies": { - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws-sdk/xml-builder": { - "version": "3.972.40", - "resolved": "https://registry.npmjs.org/@aws-sdk/xml-builder/-/xml-builder-3.972.40.tgz", - "integrity": "sha512-wlFmCIGUlwF4zx/kncw+bmxTQh1HeSJq4mYV/V5cZUSJadDP3kXvGW8Rn21cimj/7y9ju+47oYWXi97vF7czaA==", - "license": "Apache-2.0", - "dependencies": { - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=20.0.0" - } - }, - "node_modules/@aws/lambda-invoke-store": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/@aws/lambda-invoke-store/-/lambda-invoke-store-0.3.0.tgz", - "integrity": "sha512-sl4Bm6yiMNYrZKkqqDFWN0UfnWhlS8ivKxrYl+6t0gCLrqr8y3B2IqZZbFRkfaVVp7C/baApyh71P+LeE1A2sQ==", - "license": "Apache-2.0", - "engines": { - "node": ">=18.0.0" - } - }, "node_modules/@fastify/busboy": { "version": "3.2.0", "license": "MIT" @@ -706,87 +432,6 @@ "version": "2.0.0", "license": "BSD-3-Clause" }, - "node_modules/@smithy/core": { - "version": "3.33.3", - "resolved": "https://registry.npmjs.org/@smithy/core/-/core-3.33.3.tgz", - "integrity": "sha512-CsOeKq/9kA3y6VJHt+/+VTCtBaxJ4OTFpgrjIUhPpDIKxBci1k2bJaQASF2h/ELWrulGp+t97DZ0mevfAD8idg==", - "license": "Apache-2.0", - "dependencies": { - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=18.0.0" - } - }, - "node_modules/@smithy/credential-provider-imds": { - "version": "4.5.2", - "resolved": "https://registry.npmjs.org/@smithy/credential-provider-imds/-/credential-provider-imds-4.5.2.tgz", - "integrity": "sha512-A9uSdn72ozbRUSit0eib0TW7nXuNPlaeM0zcGkJ+nE6tFcSDbnmtwoxbTCFBukVQcszDAyvsd7+rTduPTXpygg==", - "license": "Apache-2.0", - "dependencies": { - "@smithy/core": "^3.33.2", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=18.0.0" - } - }, - "node_modules/@smithy/fetch-http-handler": { - "version": "5.7.2", - "resolved": "https://registry.npmjs.org/@smithy/fetch-http-handler/-/fetch-http-handler-5.7.2.tgz", - "integrity": "sha512-nZyWTmSpJEXl6VtWVMBJve/7x12DZu6sIX1z1a+ZMaHlQQRs9Zpu6NbTe/gmxYXVRpkjxyDYpZ5gx2IM6f/Wkw==", - "license": "Apache-2.0", - "dependencies": { - "@smithy/core": "^3.33.2", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=18.0.0" - } - }, - "node_modules/@smithy/node-http-handler": { - "version": "4.11.3", - "resolved": "https://registry.npmjs.org/@smithy/node-http-handler/-/node-http-handler-4.11.3.tgz", - "integrity": "sha512-2jY1tSpERfPfWqyBV2pH+iGFaghVsIJszJNsT7hxtQYhVJpWDyc0LqOWI+nXOxOAHaEfZ4PXXtp1wW1TGpHhkA==", - "license": "Apache-2.0", - "dependencies": { - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=18.0.0" - } - }, - "node_modules/@smithy/signature-v4": { - "version": "5.7.3", - "resolved": "https://registry.npmjs.org/@smithy/signature-v4/-/signature-v4-5.7.3.tgz", - "integrity": "sha512-7ImGm+FkHRLcBaRttIAMZ6bzJZWb2cJGoYjq46F2UjycujWzrL9GEN9h4w7eQyXJYnltrUhxbbieBAIRrdqpow==", - "license": "Apache-2.0", - "dependencies": { - "@smithy/core": "^3.33.3", - "@smithy/types": "^4.17.2", - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=18.0.0" - } - }, - "node_modules/@smithy/types": { - "version": "4.17.2", - "resolved": "https://registry.npmjs.org/@smithy/types/-/types-4.17.2.tgz", - "integrity": "sha512-FOKpVZob9MPTn2znRzGrnsMHv7BOsKVw3XiP/cOyYLDVZ9qKp4nifIiSCuUU/fIj5Vu0UOAxCFr+qRAtG0NUkA==", - "license": "Apache-2.0", - "dependencies": { - "tslib": "^2.6.2" - }, - "engines": { - "node": ">=18.0.0" - } - }, "node_modules/@tootallnate/once": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/@tootallnate/once/-/once-2.0.1.tgz", @@ -1321,12 +966,6 @@ "npm": "1.2.8000 || >= 1.4.16" } }, - "node_modules/bowser": { - "version": "2.14.1", - "resolved": "https://registry.npmjs.org/bowser/-/bowser-2.14.1.tgz", - "integrity": "sha512-tzPjzCxygAKWFOJP011oxFHs57HzIhOEracIgAePE4pqB3LikALKnSzUyU4MGs9/iCEUuHlAJTjTc5M+u7YEGg==", - "license": "MIT" - }, "node_modules/brace-expansion": { "version": "2.1.4", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", diff --git a/functions/package.json b/functions/package.json index 3c09a64..6b10510 100644 --- a/functions/package.json +++ b/functions/package.json @@ -18,7 +18,6 @@ "main": "lib/index.js", "type": "module", "dependencies": { - "@aws-sdk/client-sesv2": "3.1116.0", "@jspsych/metadata": "file:metadata", "archiver": "^7.0.1", "cors": "^2.8.5", diff --git a/functions/src/__tests__/mail-delivery-emulator.test.js b/functions/src/__tests__/mail-delivery-emulator.test.js index e09a17d..fe85a85 100644 --- a/functions/src/__tests__/mail-delivery-emulator.test.js +++ b/functions/src/__tests__/mail-delivery-emulator.test.js @@ -2,7 +2,7 @@ * @jest-environment node */ -// SES mail delivery against the Firestore emulator. +// Resend mail delivery against the Firestore emulator. // // Harness conventions are the established ones (upload-failure-notify- // emulator.test.js:1-108, upload-queue.test.js:1-90): emulator env vars set at @@ -17,14 +17,12 @@ // in-process -- the same seam handleQueueWrite(before, after, docId) and // retryPendingUploads(ownerScope) exist for. // -// THE TRANSPORT IS MOCKED AT A FUNCTION SEAM, NOT AT THE SDK. mail-delivery.ts -// exposes _setSesClientForTests(sender), where `sender` is a plain -// (input, {timeoutMs}) => Promise<{MessageId}> function. That shape is -// deliberate and it is why this file needs no jest.mock and no ESM wrestling: -// the real transport lazily `await import`s @aws-sdk/client-sesv2 inside that -// same seam, so with a sender injected the AWS package is never loaded at all. -// (The dynamic import also keeps the SDK off apidata's cold-start path -- see -// getSender in mail-delivery.ts.) +// THE TRANSPORT IS MOCKED AT A FUNCTION SEAM, NOT AT fetch. mail-delivery.ts +// exposes _setMailSenderForTests(sender), where `sender` is a plain +// (input, {timeoutMs, idempotencyKey}) => Promise<{id}> function. That shape is +// deliberate and it is why this file needs no jest.mock, no ESM wrestling and +// no HTTP interception: with a sender injected, getSender never builds the real +// one, so no test in this repo can reach api.resend.com even by accident. // // --------------------------------------------------------------------------- // CI RUNS THIS SUITE WITH THE REAL TRIGGER LIVE. READ BEFORE CHANGING. @@ -36,7 +34,7 @@ // every mail document this file creates, concurrently with the direct calls // below. That is the same hazard upload-failure-notify-emulator.test.js // documents at length, and here it would be considerably worse: the emulator's -// functions process has no SES credentials and no AWS anything, so a live +// functions process has no RESEND_API_KEY, so a live // instance that won the race would stamp a TERMINAL MailConfigMissingError on // a fixture whose test is about to assert SUCCESS. Every happy-path assertion // in this file would become CI-only flaky, in a way that never reproduces @@ -77,15 +75,13 @@ process.env.FIREBASE_CONFIG = JSON.stringify({ storageBucket: "datapipe-test.appspot.com", }); -// SES configuration. Read LAZILY, per invocation, by mail-delivery.ts (the +// Resend configuration. Read LAZILY, per invocation, by mail-delivery.ts (the // crypto-utils.ts convention), so setting it here reaches the compiled module -// even though it is imported later. These are dummies: with a sender injected -// nothing signs a request, and no value below is ever sent anywhere. +// even though it is imported later. This is a dummy: with a sender injected no +// request is ever built, and the value below is never sent anywhere. const FROM = "DataPipe "; const REPLY_TO = "contact@jspsych.org"; -process.env.SES_REGION = "us-east-1"; -process.env.SES_ACCESS_KEY_ID = "AKIATESTTESTTESTTEST"; -process.env.SES_SECRET_ACCESS_KEY = "test-secret-not-a-real-key"; +process.env.RESEND_API_KEY = "re_test_not_a_real_key"; process.env.MAIL_FROM = FROM; process.env.MAIL_REPLY_TO = REPLY_TO; @@ -93,7 +89,7 @@ jest.setTimeout(30000); let db; let deliverMailDocument; -let _setSesClientForTests; +let _setMailSenderForTests; // Read from the production module rather than restated here. let LEASE_MS; let MAX_ATTEMPTS; @@ -110,7 +106,7 @@ beforeAll(async () => { ({ deliverMailDocument, - _setSesClientForTests, + _setMailSenderForTests, LEASE_MS, MAX_ATTEMPTS, CONFIG_MISSING_ERROR, @@ -126,7 +122,7 @@ const RECIPIENT = "researcher@example.edu"; // Exactly what mail.ts's mailDocument() writes. Built through the same shape // rather than a minimal stub, because half of what this suite proves is that -// the real document maps onto a real SES request. +// the real document maps onto a real Resend request. async function seedMail(overrides = {}) { const owner = `md-user-${randomUUID()}`; const ref = db.collection("mail").doc(); @@ -154,11 +150,11 @@ async function deliveryOf(ref) { return snap.exists ? snap.data().delivery : undefined; } -// A sender that resolves with a MessageId, or one that rejects with a named -// SES-shaped error. jest.fn so call COUNT is assertable -- "zero additional -// SES calls" is the single most important assertion in this file. -function sendingOk(messageId = "ses-message-id-1") { - return jest.fn().mockResolvedValue({ MessageId: messageId }); +// A sender that resolves with a Resend message id, or one that rejects with a +// named Resend-shaped error. jest.fn so call COUNT is assertable -- "zero +// additional sends" is the single most important assertion in this file. +function sendingOk(messageId = "resend-message-id-1") { + return jest.fn().mockResolvedValue({ id: messageId }); } function sendingError(name, extra = {}) { @@ -189,7 +185,7 @@ function loggedText() { } afterEach(async () => { - _setSesClientForTests(null); + _setMailSenderForTests(null); jest.restoreAllMocks(); const batch = db.batch(); while (created.length) batch.delete(created.pop()); @@ -202,28 +198,32 @@ afterEach(async () => { describe("delivery", () => { test("sends the document and records SUCCESS, a messageId and an endTime", async () => { - const send = sendingOk("ses-0102030405"); - _setSesClientForTests(send); + const send = sendingOk("resend-0102030405"); + _setMailSenderForTests(send); const { ref, id } = await seedMail(); expect(await deliverMailDocument(id)).toBe("sent"); expect(send).toHaveBeenCalledTimes(1); const [input, options] = send.mock.calls[0]; - expect(input.FromEmailAddress).toBe(FROM); - expect(input.ReplyToAddresses).toEqual([REPLY_TO]); - expect(input.Destination.ToAddresses).toEqual([RECIPIENT]); - expect(input.Content.Simple.Subject.Data).toContain("Working Memory Span"); - expect(input.Content.Simple.Body.Text.Data).toBe("The file is not lost."); - expect(input.Content.Simple.Body.Html.Data).toContain("not lost"); + expect(input.from).toBe(FROM); + expect(input.reply_to).toEqual([REPLY_TO]); + expect(input.to).toEqual([RECIPIENT]); + expect(input.subject).toContain("Working Memory Span"); + expect(input.text).toBe("The file is not lost."); + expect(input.html).toContain("not lost"); // A send with no ceiling is a claim held until the function itself is - // killed -- see SES_TIMEOUT_MS / LEASE_MS. + // killed -- see SEND_TIMEOUT_MS / LEASE_MS. expect(options.timeoutMs).toBeGreaterThan(0); + // The document id, not a fresh uuid: the key has to be STABLE across + // attempts on one document, which is what makes a retry after an ambiguous + // timeout a no-op at Resend rather than a second copy. + expect(options.idempotencyKey).toBe(id); const delivery = await deliveryOf(ref); expect(delivery.state).toBe("SUCCESS"); expect(delivery.attempts).toBe(1); - expect(delivery.info.messageId).toBe("ses-0102030405"); + expect(delivery.info.messageId).toBe("resend-0102030405"); expect(delivery.error).toBeNull(); // endTime is what the Firestore TTL policy keys on (deploy runbook §4). // Unset here would mean this document never self-deletes. @@ -240,27 +240,26 @@ describe("delivery", () => { // (send-contact-email-verification.ts, upload-failure-notify.ts), and a // failure path is exactly where an address is most likely to be helpfully // appended to an error string. - _setSesClientForTests(sendingError("MessageRejected")); + _setMailSenderForTests(sendingError("validation_error")); const { id } = await seedMail(); await deliverMailDocument(id); const text = loggedText(); expect(text).toContain(id); - expect(text).toContain("MessageRejected"); + expect(text).toContain("validation_error"); expect(text).not.toContain(RECIPIENT); - expect(text).not.toContain(process.env.SES_SECRET_ACCESS_KEY); - expect(text).not.toContain(process.env.SES_ACCESS_KEY_ID); + expect(text).not.toContain(process.env.RESEND_API_KEY); }); test("mail with no html part is still sent, without an empty Html body", async () => { const send = sendingOk(); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { id } = await seedMail({ message: { subject: "Your DataPipe verification code", text: "123456" }, }); expect(await deliverMailDocument(id)).toBe("sent"); - expect(send.mock.calls[0][0].Content.Simple.Body).not.toHaveProperty("Html"); + expect(send.mock.calls[0][0]).not.toHaveProperty("html"); }); }); @@ -269,13 +268,13 @@ describe("delivery", () => { // --------------------------------------------------------------------------- describe("duplicate invocation", () => { - test("a second delivery of an already-sent document makes ZERO SES calls", async () => { + test("a second delivery of an already-sent document makes ZERO sends", async () => { // Firestore triggers are at-least-once, so this is the ordinary case, not // an exotic one -- and a duplicate "your uploads are failing" email is the // precise annoyance upload-failure-notify.ts spends its whole design // avoiding. Nothing downstream of this assertion is allowed to regress. const send = sendingOk("ses-once"); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail(); expect(await deliverMailDocument(id)).toBe("sent"); @@ -293,7 +292,7 @@ describe("duplicate invocation", () => { // The other half of at-least-once: a redelivery arriving WHILE the first // invocation is still inside send(). Taking it over would double-send. const send = sendingOk(); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail({ delivery: { state: "PROCESSING", @@ -315,7 +314,7 @@ describe("duplicate invocation", () => { // The claim is a transaction, so this is the same serializability property // upload-failure-notify.ts's twenty-way race test relies on, one hop later. const send = sendingOk(); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail(); const outcomes = await Promise.all( @@ -338,7 +337,7 @@ describe("duplicate invocation", () => { // The lease's only job. It is several times the function timeout, so by // the time it expires the original owner is provably dead -- see LEASE_MS. const send = sendingOk("ses-after-crash"); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail({ delivery: { state: "PROCESSING", @@ -367,7 +366,7 @@ describe("duplicate invocation", () => { describe("transient failure", () => { test("leaves a retryable ERROR with no endTime, and a rerun then succeeds", async () => { - _setSesClientForTests(sendingError("ThrottlingException")); + _setMailSenderForTests(sendingError("rate_limit_exceeded")); const { ref, id } = await seedMail(); expect(await deliverMailDocument(id)).toBe("retryable-error"); @@ -376,8 +375,8 @@ describe("transient failure", () => { expect(failed.state).toBe("ERROR"); expect(failed.retryable).toBe(true); expect(failed.attempts).toBe(1); - expect(failed.error.name).toBe("ThrottlingException"); - expect(failed.error.message).toContain("ThrottlingException"); + expect(failed.error.name).toBe("rate_limit_exceeded"); + expect(failed.error.message).toContain("rate_limit_exceeded"); // Structured, and never a stack. expect(Object.keys(failed.error).sort()).toEqual(["message", "name"]); // NOT terminal, so the TTL policy must not become eligible to reap a @@ -386,22 +385,22 @@ describe("transient failure", () => { // Lease released, so the retry does not have to wait five minutes. expect(failed.leaseExpiresAt).toBeNull(); - const send = sendingOk("ses-second-try"); - _setSesClientForTests(send); + const send = sendingOk("resend-second-try"); + _setMailSenderForTests(send); expect(await deliverMailDocument(id)).toBe("sent"); const delivered = await deliveryOf(ref); expect(delivered.state).toBe("SUCCESS"); expect(delivered.attempts).toBe(2); - expect(delivered.info.messageId).toBe("ses-second-try"); + expect(delivered.info.messageId).toBe("resend-second-try"); expect(delivered.endTime.toMillis()).toBeGreaterThan(0); expect(send).toHaveBeenCalledTimes(1); }); - test("a 5xx from SES is transient even under an unfamiliar name", async () => { - _setSesClientForTests( - sendingError("SomeFutureOutage", { $metadata: { httpStatusCode: 503 } }) - ); + test("a 5xx is transient even under an unfamiliar name", async () => { + // Resend adds error codes; an unrecognised one still has to be judged, and + // status is the fallback. + _setMailSenderForTests(sendingError("some_future_outage", { status: 503 })); const { ref, id } = await seedMail(); expect(await deliverMailDocument(id)).toBe("retryable-error"); @@ -411,10 +410,11 @@ describe("transient failure", () => { describe("permanent failure", () => { test("a rejected address is terminal immediately and is never retried", async () => { - const send = sendingError("MessageRejected", { - message: "Email address is not verified.", + const send = sendingError("validation_error", { + message: "Invalid `to` field.", + status: 422, }); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail(); expect(await deliverMailDocument(id)).toBe("terminal-error"); @@ -423,7 +423,7 @@ describe("permanent failure", () => { expect(delivery.state).toBe("ERROR"); expect(delivery.retryable).toBe(false); expect(delivery.attempts).toBe(1); - expect(delivery.error.name).toBe("MessageRejected"); + expect(delivery.error.name).toBe("validation_error"); // Terminal, so the TTL policy can eventually reap the address in `to`. expect(delivery.endTime.toMillis()).toBeGreaterThan(0); @@ -434,7 +434,7 @@ describe("permanent failure", () => { test("a document that cannot be turned into a request is terminal, not a crash", async () => { const send = sendingOk(); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail({ to: [] }); expect(await deliverMailDocument(id)).toBe("terminal-error"); @@ -446,33 +446,50 @@ describe("permanent failure", () => { expect(delivery.error.name).toBe("MailDocumentInvalidError"); }); - test("an ambiguous send is terminal -- a maybe-delivered mail is not retried", async () => { - // The request went out and no answer came back, and SESv2 SendEmail has no - // idempotency token. Retrying is a coin flip on a SECOND copy of a - // notification whose entire value is arriving once. See AMBIGUOUS_ERRORS. - _setSesClientForTests(sendingError("TimeoutError")); + test("an ambiguous send is RETRYABLE, and the retry reuses the same idempotency key", async () => { + // Inverted from the SES version of this suite, deliberately. SESv2 had no + // idempotency token, so a request that went out and never answered could + // not be retried without risking a SECOND copy of a notification whose + // entire value is arriving once -- and the code chose to lose the mail. + // + // Resend takes an Idempotency-Key. mail-delivery.ts sends the document id + // as that key on EVERY attempt, so the retry below is not a second send: + // Resend recognises the key and returns the original result. The assertion + // that both attempts carried the same key is what makes that true, and is + // the reason this test can assert `retryable` at all. + _setMailSenderForTests(sendingError("TimeoutError")); const { ref, id } = await seedMail(); - expect(await deliverMailDocument(id)).toBe("terminal-error"); - const delivery = await deliveryOf(ref); - expect(delivery.retryable).toBe(false); - // Loudly, though -- it is visible on the document and in the log, so it is - // re-drivable by hand. Never silent. - expect(delivery.error.name).toBe("TimeoutError"); + expect(await deliverMailDocument(id)).toBe("retryable-error"); + const failed = await deliveryOf(ref); + expect(failed.retryable).toBe(true); + // Still loud: visible on the document and in the log, never silent. + expect(failed.error.name).toBe("TimeoutError"); expect(loggedText()).toContain("TimeoutError"); + // Still deliverable, so the TTL must not be able to reap it. + expect(failed.endTime).toBeNull(); + + const send = sendingOk("resend-after-timeout"); + _setMailSenderForTests(send); + expect(await deliverMailDocument(id)).toBe("sent"); + expect(send.mock.calls[0][1].idempotencyKey).toBe(id); + + const delivered = await deliveryOf(ref); + expect(delivered.state).toBe("SUCCESS"); + expect(delivered.attempts).toBe(2); }); }); describe("attempts cap", () => { test("a document already at the cap is not claimed at all", async () => { const send = sendingOk(); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail({ delivery: { state: "ERROR", retryable: true, attempts: MAX_ATTEMPTS, - error: { name: "ThrottlingException", message: "throttled" }, + error: { name: "rate_limit_exceeded", message: "throttled" }, leaseExpiresAt: null, endTime: null, }, @@ -487,13 +504,13 @@ describe("attempts cap", () => { test("the attempt that reaches the cap turns a transient error terminal", async () => { // "Retryable in principle" and "still willing to retry" are two different // facts; the cap is what closes the second one. - _setSesClientForTests(sendingError("ThrottlingException")); + _setMailSenderForTests(sendingError("rate_limit_exceeded")); const { ref, id } = await seedMail({ delivery: { state: "ERROR", retryable: true, attempts: MAX_ATTEMPTS - 1, - error: { name: "ThrottlingException", message: "throttled" }, + error: { name: "rate_limit_exceeded", message: "throttled" }, leaseExpiresAt: null, endTime: null, startTime: Timestamp.now(), @@ -505,9 +522,9 @@ describe("attempts cap", () => { const delivery = await deliveryOf(ref); expect(delivery.attempts).toBe(MAX_ATTEMPTS); expect(delivery.retryable).toBe(false); - // The SES error name survives -- what changed is our willingness, not the + // The Resend error name survives -- what changed is our willingness, not the // diagnosis. - expect(delivery.error.name).toBe("ThrottlingException"); + expect(delivery.error.name).toBe("rate_limit_exceeded"); expect(delivery.endTime.toMillis()).toBeGreaterThan(0); expect(await deliverMailDocument(id)).toBe("skipped-terminal"); }); @@ -517,19 +534,19 @@ describe("attempts cap", () => { // 4. Missing configuration // --------------------------------------------------------------------------- -describe("missing SES configuration", () => { +describe("missing Resend configuration", () => { test("is terminal, distinctly named, and loudly logged -- mail never vanishes", async () => { const send = sendingOk(); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail(); - const saved = process.env.SES_SECRET_ACCESS_KEY; - delete process.env.SES_SECRET_ACCESS_KEY; + const saved = process.env.RESEND_API_KEY; + delete process.env.RESEND_API_KEY; let outcome; try { outcome = await deliverMailDocument(id); } finally { - process.env.SES_SECRET_ACCESS_KEY = saved; + process.env.RESEND_API_KEY = saved; } expect(outcome).toBe("terminal-error"); @@ -541,7 +558,7 @@ describe("missing SES configuration", () => { expect(delivery.error.name).toBe(CONFIG_MISSING_ERROR); // The document says WHICH key, by name. That is the whole difference // between a five-minute fix and an afternoon. - expect(delivery.error.message).toContain("SES_SECRET_ACCESS_KEY"); + expect(delivery.error.message).toContain("RESEND_API_KEY"); expect(delivery.endTime.toMillis()).toBeGreaterThan(0); // Loud: this line means every notification the deployment sends is being @@ -549,7 +566,7 @@ describe("missing SES configuration", () => { expect(errorSpy).toHaveBeenCalled(); const text = loggedText(); expect(text).toContain(CONFIG_MISSING_ERROR); - expect(text).toContain("SES_SECRET_ACCESS_KEY"); + expect(text).toContain("RESEND_API_KEY"); // The NAME of the missing key, never a value of any key. expect(text).not.toContain(saved); }); @@ -564,7 +581,7 @@ describe("races with account deletion", () => { // purge-user-data.ts deletes a researcher's queued mail on account // deletion. Racing that is expected, not a fault. const send = sendingOk(); - _setSesClientForTests(send); + _setMailSenderForTests(send); const { ref, id } = await seedMail(); await ref.delete(); @@ -574,10 +591,10 @@ describe("races with account deletion", () => { test("a document deleted mid-send still resolves, and says so", async () => { const { ref, id } = await seedMail(); - _setSesClientForTests( + _setMailSenderForTests( jest.fn(async () => { await ref.delete(); - return { MessageId: "ses-sent-then-purged" }; + return { id: "resend-sent-then-purged" }; }) ); @@ -592,14 +609,14 @@ describe("races with account deletion", () => { // while this invocation is inside send() -- which is what a stale-lease // takeover looks like from here. const { ref, id } = await seedMail(); - _setSesClientForTests( + _setMailSenderForTests( jest.fn(async () => { await ref.update({ "delivery.leaseOwner": "a-newer-invocation", "delivery.state": "SUCCESS", "delivery.info": { messageId: "written-by-the-newer-one" }, }); - return { MessageId: "written-by-the-loser" }; + return { id: "written-by-the-loser" }; }) ); @@ -622,7 +639,7 @@ describe("purge-user-data compatibility", () => { // (PurgeCounts.mailDocuments, asserted exactly in // purge-user-data-emulator.test.js). A delivery that touched the top level // of the document, or `datapipe`, would change that count or that query. - _setSesClientForTests(sendingOk()); + _setMailSenderForTests(sendingOk()); const { ref, id, owner } = await seedMail(); const before = (await ref.get()).data(); diff --git a/functions/src/__tests__/mail-delivery.test.js b/functions/src/__tests__/mail-delivery.test.js index e171f7c..369fa90 100644 --- a/functions/src/__tests__/mail-delivery.test.js +++ b/functions/src/__tests__/mail-delivery.test.js @@ -5,10 +5,9 @@ // Pure coverage for mail-delivery.ts's three decisions. // // Everything asserted here is a function of its arguments: the mail -// document -> SendEmailCommand mapping, the error taxonomy, and the claim -// state machine. No emulator, no network, and -- deliberately -- no AWS -// package: mail-delivery.ts imports @aws-sdk/client-sesv2 dynamically, inside -// the transport seam, so nothing in this file loads it. The end-to-end +// document -> Resend request-body mapping, the error taxonomy, and the claim +// state machine. No emulator and no network: the transport is one `fetch` +// behind the sender seam, and nothing in this file reaches it. The end-to-end // behaviour (claiming, retrying, the outcome fields) lives in // mail-delivery-emulator.test.js. // @@ -31,7 +30,7 @@ process.env.FIREBASE_CONFIG = JSON.stringify({ }); let buildSendEmailInput; -let classifySesError; +let classifyMailError; let claimDecision; let missingConfigKeys; let readMailConfig; @@ -46,7 +45,7 @@ let INVALID_DOCUMENT_ERROR; beforeAll(async () => { ({ buildSendEmailInput, - classifySesError, + classifyMailError, claimDecision, missingConfigKeys, readMailConfig, @@ -59,9 +58,7 @@ beforeAll(async () => { }); const CONFIG = { - region: "us-east-1", - accessKeyId: "AKIAEXAMPLE", - secretAccessKey: "secret", + apiKey: "re_testtesttest", from: "DataPipe ", replyTo: "contact@jspsych.org", }; @@ -81,62 +78,71 @@ function mailDoc(overrides = {}) { } // --------------------------------------------------------------------------- -// 1. Document -> SES request +// 1. Document -> Resend request body // --------------------------------------------------------------------------- describe("buildSendEmailInput", () => { - test("maps every part of the document onto Simple content", () => { + test("maps every part of the document onto the request body", () => { const input = buildSendEmailInput(mailDoc(), CONFIG); - expect(input.FromEmailAddress).toBe( - "DataPipe " - ); - expect(input.Destination.ToAddresses).toEqual(["researcher@example.edu"]); - expect(input.ReplyToAddresses).toEqual(["contact@jspsych.org"]); - expect(input.Content.Simple.Subject.Data).toBe( + expect(input.from).toBe("DataPipe "); + expect(input.to).toEqual(["researcher@example.edu"]); + expect(input.subject).toBe( "DataPipe couldn't upload data for Working Memory Span" ); - expect(input.Content.Simple.Body.Text.Data).toBe("The file is not lost."); - expect(input.Content.Simple.Body.Html.Data).toBe( - "

The file is not lost.

" - ); - // Non-ASCII shows up in experiment titles routinely; a missing charset - // renders them as mojibake in the one mail a researcher was going to read. - expect(input.Content.Simple.Subject.Charset).toBe("UTF-8"); - expect(input.Content.Simple.Body.Text.Charset).toBe("UTF-8"); - expect(input.Content.Simple.Body.Html.Charset).toBe("UTF-8"); + expect(input.text).toBe("The file is not lost."); + expect(input.html).toBe("

The file is not lost.

"); + }); + + test("spells Reply-To the way the REST API does, not the way the SDK does", () => { + // The single highest-value assertion in this file. Resend's Node SDK takes + // `replyTo`; the raw API this code speaks takes `reply_to` and IGNORES + // unknown keys silently. Get this wrong and there is no error, no bounce + // and no log line -- just every notification going out with no Reply-To, + // for however long it takes someone to notice. + const input = buildSendEmailInput(mailDoc(), CONFIG); + expect(input.reply_to).toEqual(["contact@jspsych.org"]); + expect(input).not.toHaveProperty("replyTo"); }); - test("omits the Html part entirely when the document has none", () => { - // mail.ts writes `message` WITHOUT an html key for text-only mail. An - // empty Html part is not the same thing as no Html part to a mail client. + test("sends no key at all for a part the document does not have", () => { + // mail.ts writes `message` WITHOUT an html key for text-only mail, and an + // empty html string is not the same thing as no html to a mail client. const doc = mailDoc({ message: { subject: "s", text: "t" } }); const input = buildSendEmailInput(doc, CONFIG); - expect(input.Content.Simple.Body.Text.Data).toBe("t"); - expect(input.Content.Simple.Body).not.toHaveProperty("Html"); + expect(input.text).toBe("t"); + expect(input).not.toHaveProperty("html"); }); test("omits Reply-To when none is configured", () => { const { replyTo, ...noReplyTo } = CONFIG; const input = buildSendEmailInput(mailDoc(), noReplyTo); - expect(input).not.toHaveProperty("ReplyToAddresses"); + expect(input).not.toHaveProperty("reply_to"); }); test("accepts a bare string recipient as well as an array", () => { // mail.ts always writes an array, but the extension contract this replaces // also allowed a string, and a hand-written document may well be one. const input = buildSendEmailInput(mailDoc({ to: "one@example.edu" }), CONFIG); - expect(input.Destination.ToAddresses).toEqual(["one@example.edu"]); + expect(input.to).toEqual(["one@example.edu"]); }); - test("drops junk entries rather than handing SES a null recipient", () => { + test("drops junk entries rather than handing Resend a null recipient", () => { const input = buildSendEmailInput( mailDoc({ to: [null, " keep@example.edu ", "", 42] }), CONFIG ); - expect(input.Destination.ToAddresses).toEqual(["keep@example.edu"]); + expect(input.to).toEqual(["keep@example.edu"]); + }); + + test("the body is JSON-serialisable, which is the only form it is ever used in", () => { + // buildSendEmailInput's output goes straight into JSON.stringify. A value + // that survives an assertion but not serialisation (undefined, a Date, a + // Timestamp leaked out of the document) would vanish silently on the wire. + const input = buildSendEmailInput(mailDoc(), CONFIG); + expect(JSON.parse(JSON.stringify(input))).toEqual(input); }); test("refuses a document with no usable recipient", () => { @@ -170,7 +176,7 @@ describe("buildSendEmailInput", () => { thrown = error; } expect(thrown.name).toBe(INVALID_DOCUMENT_ERROR); - expect(classifySesError(thrown).retryable).toBe(false); + expect(classifyMailError(thrown).retryable).toBe(false); }); }); @@ -178,98 +184,135 @@ describe("buildSendEmailInput", () => { // 2. Error taxonomy // --------------------------------------------------------------------------- -describe("classifySesError", () => { - function sesError(name, extra = {}) { +describe("classifyMailError", () => { + // Resend answers a refusal with {name, message, statusCode}; the transport + // turns that into a MailTransportError whose `name` is Resend's code and + // whose `status` is the HTTP status. This builds the same shape. + function apiError(name, status) { const error = new Error(`${name} happened`); error.name = name; - return Object.assign(error, extra); + if (status !== undefined) error.status = status; + return error; + } + + // A node/undici transport failure, which is a different animal: undici + // reports EVERY one as `TypeError: fetch failed` with the real diagnosis on + // .cause. Tested in that wrapped form on purpose -- the unwrapped form is + // not what this code ever receives. + function transportError(code) { + const error = new TypeError("fetch failed"); + error.cause = Object.assign(new Error(code), { code }); + return error; } const table = [ - // [error, retryable, why] - ["MessageRejected", false, "bad address / rejected content"], - ["MailFromDomainNotVerifiedException", false, "domain not verified"], - ["AccountSuspendedException", false, "account problem"], - ["SendingPausedException", false, "account problem"], - ["AccessDeniedException", false, "IAM policy problem"], - ["UnrecognizedClientException", false, "bad credentials"], - ["SignatureDoesNotMatch", false, "bad credentials"], - ["ValidationException", false, "malformed request"], - ["TooManyRequestsException", true, "throttled"], - ["ThrottlingException", true, "throttled"], - ["LimitExceededException", true, "throttled"], - ["ServiceUnavailableException", true, "SES 5xx"], - ["InternalServiceErrorException", true, "SES 5xx"], - ["ENOTFOUND", true, "never connected, so nothing was sent"], - ["ECONNREFUSED", true, "never connected, so nothing was sent"], - ["TimeoutError", false, "AMBIGUOUS: may have been accepted"], - ["AbortError", false, "AMBIGUOUS: may have been accepted"], - ["ECONNRESET", false, "AMBIGUOUS: may have been accepted"], + // [Resend error code, status, retryable, why] + ["validation_error", 403, false, "sending domain not verified"], + ["validation_error", 422, false, "malformed request"], + ["missing_required_field", 422, false, "malformed request"], + ["missing_api_key", 401, false, "not configured"], + ["restricted_api_key", 401, false, "key scoped wrong or inactive"], + ["suspended_api_key", 403, false, "account problem"], + ["invalid_permission", 403, false, "key lacks the send scope"], + ["not_found", 404, false, "wrong endpoint -- a deploy fixes it"], + ["method_not_allowed", 405, false, "wrong endpoint -- a deploy fixes it"], + ["invalid_idempotent_request", 409, false, "same key, different body"], + ["rate_limit_exceeded", 429, true, "per-second limit"], + ["daily_quota_exceeded", 429, true, "plan cap -- 100/day on the free plan"], + ["monthly_quota_exceeded", 429, true, "plan cap"], + ["concurrent_idempotent_requests", 409, true, "our own earlier attempt"], + ["application_error", 500, true, "Resend 5xx"], + ["service_unavailable", 503, true, "Resend 5xx"], ]; - test.each(table)("%s -> retryable=%s (%s)", (name, retryable) => { - const classified = classifySesError(sesError(name)); + test.each(table)("%s (%d) -> retryable=%s (%s)", (name, status, retryable) => { + const classified = classifyMailError(apiError(name, status)); expect(classified.name).toBe(name); expect(classified.retryable).toBe(retryable); }); - test("the ambiguous set is terminal on purpose, not by omission", () => { - // SESv2 SendEmail has no idempotency token, so a request that went out and - // never answered cannot be retried safely. Retrying risks a SECOND copy of - // a notification whose entire value is arriving once; not retrying loses - // at most one, and loses it loudly (the name survives on the document). - // If this assertion is ever flipped, read the AMBIGUOUS_ERRORS comment in - // mail-delivery.ts first -- it is a deliberate trade, not a default. - expect(classifySesError(sesError("TimeoutError")).retryable).toBe(false); - expect(classifySesError(sesError("ECONNRESET")).retryable).toBe(false); - // ...and the distinction from "never connected" is the whole reason the - // two lists are separate. - expect(classifySesError(sesError("ECONNREFUSED")).retryable).toBe(true); + test("unwraps undici's `TypeError: fetch failed` to the real cause", () => { + // Load-bearing. Undici hides every network error behind that TypeError. + // Without the unwrap each one classifies as an unrecognised name with no + // status -- which is TERMINAL -- so a momentary DNS blip would silently + // become a permanently lost notification. + expect(classifyMailError(transportError("ECONNREFUSED")).name).toBe( + "ECONNREFUSED" + ); + expect(classifyMailError(transportError("ECONNRESET")).name).toBe( + "ECONNRESET" + ); + expect(classifyMailError(new TypeError("fetch failed")).name).toBe( + "UnknownError" + ); + }); + + test("never-connected errors are retryable: nothing can have been sent", () => { + for (const code of ["ENOTFOUND", "ECONNREFUSED", "EAI_AGAIN"]) { + expect(classifyMailError(transportError(code)).retryable).toBe(true); + } + }); + + test("AMBIGUOUS errors are retryable BECAUSE of the idempotency key", () => { + // This assertion is inverted from the SES version of this file, and the + // inversion is the point. Under SESv2 there was no idempotency token, so a + // request that went out and never answered could not be retried safely: + // retrying risked a SECOND copy of a notification whose entire value is + // arriving once, and the code chose to lose the mail instead. + // + // Resend takes an Idempotency-Key, and mail-delivery.ts sends the mail + // document's id as that key on every attempt, so a retry after a timeout + // is not a second send -- Resend returns the original result. We get the + // retry AND exactly-once, instead of choosing. + // + // IF THIS IS EVER FLIPPED BACK, the reason will be that something can now + // retry more than 24 hours later, which is when Resend expires the key. + // Read AMBIGUOUS_ERRORS in mail-delivery.ts before touching it. + expect(classifyMailError(apiError("TimeoutError")).retryable).toBe(true); + expect(classifyMailError(apiError("AbortError")).retryable).toBe(true); + expect(classifyMailError(transportError("ECONNRESET")).retryable).toBe(true); }); test("falls back to the HTTP status when the name is unfamiliar", () => { - const throttled = classifySesError( - sesError("SomeNewThrottle", { $metadata: { httpStatusCode: 429 } }) + // Resend adds error codes; an unknown one must still be judged. + expect(classifyMailError(apiError("some_new_throttle", 429)).retryable).toBe( + true ); - expect(throttled.retryable).toBe(true); - - const serverSide = classifySesError( - sesError("SomeNewOutage", { $metadata: { httpStatusCode: 503 } }) + expect(classifyMailError(apiError("some_new_outage", 503)).retryable).toBe( + true ); - expect(serverSide.retryable).toBe(true); - - const clientSide = classifySesError( - sesError("SomeNewRefusal", { $metadata: { httpStatusCode: 400 } }) + expect(classifyMailError(apiError("some_new_refusal", 400)).retryable).toBe( + false ); - expect(clientSide.retryable).toBe(false); }); - test("honors the SDK's own $retryable hint when nothing else matched", () => { - const hinted = classifySesError( - sesError("SomethingNobodyListed", { $retryable: { throttling: true } }) - ); - expect(hinted.retryable).toBe(true); + test("a non-JSON error body still classifies, on status alone", () => { + // An edge or proxy failure answers HTML, so the transport synthesises the + // name from the status. 502 from a CDN must not be treated as terminal. + expect(classifyMailError(apiError("HttpError502", 502)).retryable).toBe(true); + expect(classifyMailError(apiError("HttpError400", 400)).retryable).toBe(false); }); test("an unknown, unnamed, statusless error does not consume the retry budget", () => { - // More likely a defect on our side than a blip on SES's. - expect(classifySesError(new Error("boom")).retryable).toBe(false); - expect(classifySesError(new Error("boom")).name).toBe("UnknownError"); - expect(classifySesError(undefined).name).toBe("UnknownError"); - expect(classifySesError("just a string").message).toBe("Unknown error"); + // More likely a defect on our side than a blip at Resend. + expect(classifyMailError(new Error("boom")).retryable).toBe(false); + expect(classifyMailError(new Error("boom")).name).toBe("UnknownError"); + expect(classifyMailError(undefined).name).toBe("UnknownError"); + expect(classifyMailError("just a string").message).toBe("Unknown error"); }); test("reads a node-style `code` when there is no useful name", () => { const error = new Error("socket hang up"); error.code = "ECONNRESET"; - expect(classifySesError(error).name).toBe("ECONNRESET"); + expect(classifyMailError(error).name).toBe("ECONNRESET"); }); test("never carries a stack, and truncates the message", () => { - const error = sesError("MessageRejected", { message: "x".repeat(5000) }); - const classified = classifySesError(error); + const error = apiError("validation_error", 422); + error.message = "x".repeat(5000); + const classified = classifyMailError(error); expect(classified).toEqual({ - name: "MessageRejected", + name: "validation_error", message: "x".repeat(500), retryable: false, }); @@ -388,9 +431,7 @@ describe("claimDecision", () => { describe("configuration", () => { const FULL = { - SES_REGION: "us-east-1", - SES_ACCESS_KEY_ID: "AKIAEXAMPLE", - SES_SECRET_ACCESS_KEY: "secret", + RESEND_API_KEY: "re_testtesttest", MAIL_FROM: "DataPipe ", MAIL_REPLY_TO: "contact@jspsych.org", }; @@ -406,24 +447,21 @@ describe("configuration", () => { }); test("names every missing key, and blank counts as missing", () => { - expect(missingConfigKeys({ ...FULL, SES_SECRET_ACCESS_KEY: " " })).toEqual([ - "SES_SECRET_ACCESS_KEY", - ]); - expect(missingConfigKeys({})).toEqual([ - "SES_REGION", - "SES_ACCESS_KEY_ID", - "SES_SECRET_ACCESS_KEY", - "MAIL_FROM", + expect(missingConfigKeys({ ...FULL, RESEND_API_KEY: " " })).toEqual([ + "RESEND_API_KEY", ]); + expect(missingConfigKeys({})).toEqual(["RESEND_API_KEY", "MAIL_FROM"]); }); - test("readMailConfig trims, so a stray newline from the .env heredoc cannot break signing", () => { + test("readMailConfig trims, so a stray newline from the .env heredoc cannot 401", () => { + // A trailing newline inside a bearer token is not a visible problem: it is + // a 401 that looks exactly like a wrong key. const config = readMailConfig({ ...FULL, - SES_ACCESS_KEY_ID: " AKIAEXAMPLE\n", + RESEND_API_KEY: " re_testtesttest\n", MAIL_REPLY_TO: " contact@jspsych.org ", }); - expect(config.accessKeyId).toBe("AKIAEXAMPLE"); + expect(config.apiKey).toBe("re_testtesttest"); expect(config.replyTo).toBe("contact@jspsych.org"); }); @@ -431,8 +469,8 @@ describe("configuration", () => { // It is the alert handle: it means every notification this deployment // sends is being dropped on the floor. expect(CONFIG_MISSING_ERROR).toBe("MailConfigMissingError"); - const error = new Error("Missing configuration: SES_REGION"); + const error = new Error("Missing configuration: RESEND_API_KEY"); error.name = CONFIG_MISSING_ERROR; - expect(classifySesError(error).retryable).toBe(false); + expect(classifyMailError(error).retryable).toBe(false); }); }); diff --git a/functions/src/index.ts b/functions/src/index.ts index 98f494e..416eeea 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -14,8 +14,9 @@ import { onExperimentGrew, onUploadQueueChanged } from "./compaction-triggers.js // onUploadQueueChanged above -- see the header of upload-failure-notify.ts. import { onUploadFailure } from "./upload-failure-notify.js"; // Delivery for the `mail` collection. Replaces the deprecated Firebase -// "Trigger Email" extension with a direct Amazon SES send; mail.ts's document -// contract is unchanged, so nothing on the write side moved. +// "Trigger Email" extension with a direct Resend send (Amazon SES until AWS +// 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"; // 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 diff --git a/functions/src/mail-delivery.ts b/functions/src/mail-delivery.ts index e76d6b8..4edca2e 100644 --- a/functions/src/mail-delivery.ts +++ b/functions/src/mail-delivery.ts @@ -12,10 +12,14 @@ // // The plan was the Firebase "Trigger Email" extension // (firebase/firestore-send-email). That platform is deprecated, so delivery -// moves in-repo, on Amazon SES via the AWS API directly -- no SMTP, no -// nodemailer. mail.ts's header still describes the extension; its DOCUMENT -// contract is unchanged and still authoritative, which is the whole reason -// the swap costs nothing on the write side. +// moved in-repo. The transport was then Amazon SES, until AWS denied the +// production-access request that would have let it send to anyone but a +// verified address; it is now Resend, over its plain JSON HTTP API -- no SMTP, +// no nodemailer, and no SDK at all (one `fetch` to one endpoint). +// +// mail.ts's header still describes the extension; its DOCUMENT contract is +// unchanged and still authoritative, which is the whole reason both swaps cost +// nothing on the write side. // // The outcome fields written below are deliberately the extension's: // `delivery.state`, `delivery.attempts`, `delivery.startTime`, @@ -75,10 +79,10 @@ import { MAIL_COLLECTION } from "./mail.js"; // Timings. The relationship between these three numbers is load-bearing. // --------------------------------------------------------------------------- -// Hard ceiling on one SES call. SES answers in well under a second in the -// normal case; ten seconds is "the network eating the request", not "SES is -// thinking". -export const SES_TIMEOUT_MS = 10 * 1000; +// Hard ceiling on one Resend API call. Resend answers in well under a second +// in the normal case; ten seconds is "the network eating the request", not +// "Resend is thinking". +export const SEND_TIMEOUT_MS = 10 * 1000; // The function's own timeout. Nothing here holds data -- one document read, // one HTTPS call, one document write -- so this is generous by an order of @@ -94,14 +98,17 @@ export const FUNCTION_TIMEOUT_SECONDS = 60; // value trades two failures against each other: // // too SHORT -> a second invocation takes over a claim whose original owner -// is still alive and still inside client.send(). Both send. -// That is the double-send this whole file exists to prevent. +// is still alive and still inside the send. Both send. That is +// the double-send this whole file exists to prevent. (Resend's +// Idempotency-Key makes that collision survivable now -- see +// the transport seam -- but "survivable" is not "fine", and +// the lease is still the thing that prevents it.) // too LONG -> a genuinely crashed claim sits undelivered for that long. // // The floor is therefore the longest an invocation can still be running after // it claimed, and that is bounded by the FUNCTION timeout (60s), not by the -// SES timeout (10s): past 60 seconds the platform has killed the invocation, -// so it is provably not in client.send() any more. 5 minutes is 5x that +// send timeout (10s): past 60 seconds the platform has killed the invocation, +// so it is provably not in the send any more. 5 minutes is 5x that // bound, which absorbs clock skew between instances and any future increase // of the function timeout short of five minutes. The cost of the choice is // that a crashed claim waits up to five minutes -- irrelevant for mail whose @@ -136,25 +143,27 @@ const TRANSACTION_ATTEMPTS = 10; // convention crypto-utils.ts / payload-crypto.ts use for TOKEN_ENCRYPTION_KEY, // and for the same reason: tests set these after module load. // -// The three secret values are written into functions/.env by the deploy -// workflows from repo secrets, exactly as TOKEN_ENCRYPTION_KEY already is +// RESEND_API_KEY is written into functions/.env by the deploy workflows from a +// repo secret, exactly as TOKEN_ENCRYPTION_KEY already is // (.github/workflows/firebase-deploy.yml, firebase-deploy-test.yml). MAIL_FROM // and MAIL_REPLY_TO are not secret and are written as literals in the same // block. See docs/deploy-contact-email.md §2. +// +// ONE secret, not the three SES needed: Resend authenticates with a bearer +// token, so there is no region to keep in sync with a verified identity and no +// request signing to be broken by a stray newline. The trim() below survives +// from the SES version anyway -- a trailing newline in a bearer token is a 401, +// which is just as fatal and rather harder to see. // --------------------------------------------------------------------------- -export const REQUIRED_CONFIG_KEYS = [ - "SES_REGION", - "SES_ACCESS_KEY_ID", - "SES_SECRET_ACCESS_KEY", - "MAIL_FROM", -] as const; +export const REQUIRED_CONFIG_KEYS = ["RESEND_API_KEY", "MAIL_FROM"] as const; export interface MailConfig { - region: string; - accessKeyId: string; - secretAccessKey: string; + // Resend API key, "re_..." -- sent as `Authorization: Bearer `. + apiKey: string; // "DataPipe " -- the extension's DEFAULT_FROM. + // Must be on a domain verified in the Resend account, or every send is a 403 + // validation_error. from: string; // The extension's DEFAULT_REPLY_TO. Optional: mail with no Reply-To is // deliverable, mail with a bad one is not. @@ -179,40 +188,32 @@ export function readMailConfig( ): MailConfig { const replyTo = env.MAIL_REPLY_TO?.trim(); return { - region: (env.SES_REGION ?? "").trim(), - accessKeyId: (env.SES_ACCESS_KEY_ID ?? "").trim(), - secretAccessKey: (env.SES_SECRET_ACCESS_KEY ?? "").trim(), + apiKey: (env.RESEND_API_KEY ?? "").trim(), from: (env.MAIL_FROM ?? "").trim(), ...(replyTo ? { replyTo } : {}), }; } // --------------------------------------------------------------------------- -// The SES request, built from the mail document. +// The Resend request, built from the mail document. // --------------------------------------------------------------------------- -interface SesContentPart { - Data: string; - Charset: string; -} - -// SESv2 SendEmailCommand input, "Simple" content. Declared structurally rather -// than imported from the SDK so this shape is assertable in a pure test with -// no AWS package loaded at all. +// The JSON body of POST https://api.resend.com/emails. +// +// SNAKE_CASE IS NOT A TYPO. Resend's Node SDK takes `replyTo`; the raw REST +// API this file speaks takes `reply_to`, and silently ignores keys it does not +// recognise -- so a camelCase `replyTo` here would not error, it would just +// send every notification with no Reply-To header and nothing would say so. +// That is exactly the class of bug buildSendEmailInput is exported to catch. export interface SendEmailInput { - FromEmailAddress: string; - Destination: { ToAddresses: string[] }; - ReplyToAddresses?: string[]; - Content: { - Simple: { - Subject: SesContentPart; - Body: { Text: SesContentPart; Html?: SesContentPart }; - }; - }; + from: string; + to: string[]; + subject: string; + text: string; + html?: string; + reply_to?: string[]; } -const CHARSET = "UTF-8"; - // A mail document that cannot be turned into a request. Permanent by // definition: no amount of retrying fixes a missing recipient. export const INVALID_DOCUMENT_ERROR = "MailDocumentInvalidError"; @@ -228,7 +229,7 @@ export class MailInputError extends Error { // mail.ts always writes `to` as an array; the extension contract also allowed // a bare string, and a hand-written document may well be one. Accept both, and -// drop anything that is not a non-empty string rather than handing SES a +// drop anything that is not a non-empty string rather than handing Resend a // `null` recipient. function recipients(to: unknown): string[] { const list: unknown[] = Array.isArray(to) ? (to as unknown[]) : [to]; @@ -239,11 +240,11 @@ function recipients(to: unknown): string[] { } /** - * mail document -> SESv2 SendEmailCommand input. + * mail document -> Resend request body. * - * Pure, and exported for it: this mapping is where a silent wrong-From or a - * dropped html part would hide, and it needs no emulator and no AWS package to - * assert. + * Pure, and exported for it: this mapping is where a silent wrong-From, a + * dropped html part, or a camelCased `reply_to` would hide, and it needs no + * emulator and no network to assert. */ export function buildSendEmailInput( data: FirebaseFirestore.DocumentData, @@ -266,23 +267,17 @@ export function buildSendEmailInput( } const input: SendEmailInput = { - FromEmailAddress: config.from, - Destination: { ToAddresses: to }, - Content: { - Simple: { - Subject: { Data: subject, Charset: CHARSET }, - // Text always, Html only when the document carries one -- mail.ts - // omits `html` entirely for text-only mail, and an empty Html part is - // not the same thing as no Html part to a mail client. - Body: { - Text: { Data: text, Charset: CHARSET }, - ...(html ? { Html: { Data: html, Charset: CHARSET } } : {}), - }, - }, - }, + from: config.from, + to, + subject, + // Text always, html only when the document carries one -- mail.ts omits + // `html` entirely for text-only mail, and an empty html part is not the + // same thing as no html part to a mail client. + text, + ...(html ? { html } : {}), }; if (config.replyTo) { - input.ReplyToAddresses = [config.replyTo]; + input.reply_to = [config.replyTo]; } return input; } @@ -297,94 +292,159 @@ export interface ClassifiedError { retryable: boolean; } +// Resend names its refusals in the response body (`name`), and those names are +// the precise signal -- more precise than the status, which reuses 403 for +// "unverified domain", "suspended key" and "over quota" alike. Status is the +// fallback for anything not listed here (Resend adds codes); see +// https://resend.com/docs/api-reference/errors. + // Permanent. Retrying changes nothing; a human has to change something. -// MessageRejected is the bad-address case, the rest are configuration and -// account problems. +// Everything here is a configuration, credential, or bad-document problem. const PERMANENT_ERRORS = new Set([ - "MessageRejected", - "MailFromDomainNotVerifiedException", - "AccountSuspendedException", - "SendingPausedException", - "NotFoundException", - "BadRequestException", - "ValidationException", - "InvalidParameterValue", - "InvalidParameterValueException", - "AccessDeniedException", - "AccessDenied", - "UnrecognizedClientException", - "InvalidClientTokenId", - "SignatureDoesNotMatch", - "InvalidSignatureException", - "ExpiredTokenException", - "CredentialsProviderError", - "IncompleteSignature", - "OptInRequired", + // 400/403/422 -- bad field, unverified sending domain, or a free-account + // restriction to the account owner's own address. The last one is the + // Resend equivalent of the SES sandbox that started this migration, and it + // fails exactly as loudly: terminal, named, on the document. + "validation_error", + "missing_required_field", + "missing_required_parameter", + "invalid_parameter", + "invalid_attachment", + // 401/403 -- the key is absent, wrong, scoped wrong, or switched off. + "missing_api_key", + "restricted_api_key", + "suspended_api_key", + "invalid_permission", + // 403 -- the account cannot send this at all. + "email_above_quota", + // 404/405 -- this code is calling the wrong endpoint. A deploy fixes it, a + // retry does not. + "not_found", + "method_not_allowed", + // 400/409 -- our Idempotency-Key is malformed, or was reused with a + // different body. Both are defects here, not conditions at Resend. + "invalid_idempotency_key", + "invalid_idempotent_request", INVALID_DOCUMENT_ERROR, CONFIG_MISSING_ERROR, ]); -// Transient. SES answered, and its answer was "not now". +// Transient. Resend answered, and its answer was "not now". const TRANSIENT_ERRORS = new Set([ - "TooManyRequestsException", - "ThrottlingException", - "Throttling", - "RequestThrottled", - "RequestThrottledException", - "SlowDown", - "LimitExceededException", - "ServiceUnavailable", - "ServiceUnavailableException", - "InternalServiceErrorException", - "InternalFailure", - "InternalServerError", + // 429 -- the per-second limit. Genuinely momentary. + "rate_limit_exceeded", + // 429 -- THE PLAN CAP, and worth understanding before it happens. Nothing + // retries a retryable error on a timer (see the header): marking these + // retryable makes the document say "still deliverable" and makes an + // operator re-drive work, but the mail does not resend itself tomorrow. + // On the free plan the cap is 100/day, and the burst case that reaches it + // is a storage-provider outage putting many experiments into failure + // episodes at once -- i.e. exactly when these notifications matter most. + // `daily_quota_exceeded` in the logs is the signal to move to a paid plan; + // it is the second line worth alerting on after MailConfigMissingError. + "daily_quota_exceeded", + "monthly_quota_exceeded", + // 409 -- a previous attempt on the same Idempotency-Key is still in flight + // at Resend. It will finish; ours should stand down and let a later + // delivery read the result. + "concurrent_idempotent_requests", + "resource_locked", + // 500/503. + "application_error", + "service_unavailable", + "internal_server_error", ]); -// AMBIGUOUS: the request went out and no answer came back. SES may or may not -// have accepted the message, and SESv2 SendEmail has no idempotency token that -// would let a retry be safe. +// Never connected at all, so nothing can have been sent. +const NEVER_CONNECTED_ERRORS = new Set([ + "ENOTFOUND", + "ECONNREFUSED", + "EAI_AGAIN", + "EHOSTUNREACH", + "ENETUNREACH", + "UND_ERR_CONNECT_TIMEOUT", +]); + +// AMBIGUOUS: the request went out and no answer came back. Resend may or may +// not have accepted the message. +// +// THIS SET IS RETRYABLE NOW, AND IT WAS NOT UNDER SES. That is the one +// behavioural change the transport swap carries, and it is an improvement +// rather than a slip. The old comment here read: "SESv2 SendEmail has no +// idempotency token that would let a retry be safe", and so it chose to lose +// an ambiguous mail rather than risk delivering a second copy of a +// notification whose whole value is arriving once. +// +// Resend takes an Idempotency-Key header, and the transport below sends the +// mail document's own id as that key on every attempt. So a retry after a +// timeout is not a second send: Resend recognises the key and returns the +// original result. The trade the old comment was making no longer exists -- +// we can now retry ambiguity AND keep the exactly-once guarantee, instead of +// choosing between them. // -// These are treated as TERMINAL, not retryable, and that is a deliberate -// asymmetry rather than an oversight. Retrying an ambiguous send is a coin -// flip on delivering a second copy of a notification whose whole value is that -// it arrives once; not retrying loses at most one mail, LOUDLY -- the error -// name is preserved on the document and logged at error level, so it is -// visible and re-drivable by hand, never silent. Given the choice the feature -// itself already made (upload-failure-notify.ts: one mail per episode, a -// 24-hour floor, and an explicit preference for saying nothing over saying it -// twice), losing the coin flip in the quiet direction is the consistent call. +// THE ONE THING THAT WOULD BREAK THIS: Resend expires an idempotency key after +// 24 hours. Every retry path here is minutes wide (at-least-once redelivery of +// the create event, or a lease takeover bounded by LEASE_MS), so nothing comes +// close. If a retry mechanism is ever added that can fire a day later, this set +// goes back to terminal, or the key stops being sufficient. const AMBIGUOUS_ERRORS = new Set([ "TimeoutError", - "RequestTimeout", - "RequestTimeoutException", "AbortError", + "RequestTimeout", "ECONNRESET", "EPIPE", "ECONNABORTED", - "ERR_SOCKET_CONNECTION_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", + "UND_ERR_BODY_TIMEOUT", + "UND_ERR_SOCKET", ]); -// Never connected at all, so nothing can have been sent. Safe to retry, -// unlike the ambiguous set above -- the distinction is the whole reason these -// are not one list. -const NEVER_CONNECTED_ERRORS = new Set([ - "ENOTFOUND", - "ECONNREFUSED", - "EAI_AGAIN", - "EHOSTUNREACH", - "ENETUNREACH", -]); +/** + * 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. + */ +export class MailTransportError extends Error { + status?: number; + constructor(name: string, message: string, status?: number) { + super(message); + this.name = name; + this.status = status; + } +} function errorName(error: unknown): string { if (error === null || typeof error !== "object") return "UnknownError"; - const e = error as { name?: unknown; code?: unknown }; + const e = error as { name?: unknown; code?: unknown; cause?: unknown }; + // `!== "Error"` because a bare `new Error(...)` carries no diagnosis at all; // its node-style `code` (ECONNRESET and friends) is the useful signal. - if (typeof e.name === "string" && e.name.length > 0 && e.name !== "Error") { + // + // `!== "TypeError"` for the same reason, and it is load-bearing: undici -- + // the fetch implementation in Node 22 -- wraps EVERY transport failure as + // `TypeError: fetch failed` and hangs the real diagnosis off `.cause`. Take + // the name at face value here and every network error in this file becomes + // an unrecognised "TypeError", which classifies terminal, which silently + // turns every transient blip into a permanently lost notification. + if ( + typeof e.name === "string" && + e.name.length > 0 && + e.name !== "Error" && + e.name !== "TypeError" + ) { return e.name; } - if (typeof e.code === "string" && e.code.length > 0) { - return e.code; + if (typeof e.code === "string" && e.code.length > 0) return e.code; + + const cause = e.cause as { name?: unknown; code?: unknown } | undefined; + if (cause && typeof cause === "object") { + if (typeof cause.code === "string" && cause.code.length > 0) return cause.code; + if ( + typeof cause.name === "string" && + cause.name.length > 0 && + cause.name !== "Error" + ) { + return cause.name; + } } return "UnknownError"; } @@ -392,39 +452,34 @@ function errorName(error: unknown): string { /** * Classify a failed send. * - * Never returns a stack. `message` is the SES/SDK message text, which - * describes the refusal ("Email address is not verified"), not the payload -- - * a stack here would end up in a Firestore document and in a log line, and - * neither is a place for one. + * Never returns a stack. `message` is Resend's own message text, which + * describes the refusal ("The gmail.com domain is not verified"), not the + * payload -- a stack here would end up in a Firestore document and in a log + * line, and neither is a place for one. */ -export function classifySesError(error: unknown): ClassifiedError { +export function classifyMailError(error: unknown): ClassifiedError { const name = errorName(error); const raw = (error as { message?: unknown })?.message; const message = typeof raw === "string" && raw.length > 0 ? raw.slice(0, 500) : "Unknown error"; - // Explicit names win over status codes: SES returns some permanent refusals - // with unhelpful status codes, and the name is the precise signal. + // Explicit names win over status codes: Resend reuses 403 for problems that + // are not alike, and the name is the precise signal. if (PERMANENT_ERRORS.has(name)) return { name, message, retryable: false }; - if (AMBIGUOUS_ERRORS.has(name)) return { name, message, retryable: false }; if (NEVER_CONNECTED_ERRORS.has(name)) return { name, message, retryable: true }; + if (AMBIGUOUS_ERRORS.has(name)) return { name, message, retryable: true }; if (TRANSIENT_ERRORS.has(name)) return { name, message, retryable: true }; - const status = (error as { $metadata?: { httpStatusCode?: number } })?.$metadata - ?.httpStatusCode; + const status = (error as { status?: unknown })?.status; if (typeof status === "number") { - // 429 and 5xx: SES answered and declined to do the work now. + // 429 and 5xx: Resend answered and declined to do the work now. if (status === 429 || status >= 500) return { name, message, retryable: true }; // Any other 4xx is a refusal of THIS request, and it will refuse it again. if (status >= 400) return { name, message, retryable: false }; } - // The SDK's own judgement, last, because it is coarser than the lists above. - const retryableHint = (error as { $retryable?: unknown })?.$retryable; - if (retryableHint) return { name, message, retryable: true }; - // Unknown, unnamed, no status. More likely a defect on our side than a blip - // on SES's, so it does not get to consume the retry budget. + // at Resend, so it does not get to consume the retry budget. return { name, message, retryable: false }; } @@ -482,7 +537,7 @@ export function claimDecision( if (state === "SUCCESS") return "skip-delivered"; if (state === "PROCESSING") { - // Someone holds the claim and may be inside client.send() right now. + // Someone holds the claim and may be inside the send right now. if (millisOrZero(delivery?.leaseExpiresAt) > nowMs) return "skip-in-flight"; // Lease expired: the claimant is provably dead (see LEASE_MS). Recoverable. } else if (state === "ERROR") { @@ -505,82 +560,108 @@ export function claimDecision( // --------------------------------------------------------------------------- export interface SendResult { - MessageId?: string; + // Resend's `id` for the accepted message. Recorded as + // delivery.info.messageId, which is the extension's field name. + id?: string; +} + +export interface SendOptions { + timeoutMs: number; + // Sent as the Idempotency-Key header. The mail document's id: stable across + // every attempt on one document, unique across documents. See + // AMBIGUOUS_ERRORS for what this buys. + idempotencyKey: string; } export type MailSender = ( input: SendEmailInput, - options: { timeoutMs: number } + options: SendOptions ) => Promise; -// The AWS SDK, described by what this file actually uses. Declared structurally -// so nothing here depends on the SDK's exact exported types, and so the module -// under test never has to load the package. -interface SesSdk { - SESv2Client: new (config: Record) => { - send(command: unknown, options?: unknown): Promise; - }; - SendEmailCommand: new (input: SendEmailInput) => unknown; -} +const RESEND_ENDPOINT = "https://api.resend.com/emails"; let injectedSender: MailSender | null = null; -let cachedSender: MailSender | null = null; -let cachedSenderKey = ""; /** - * Test seam. Replaces the SES transport with a plain function. + * Test seam. Replaces the Resend transport with a plain function. * * A function rather than a client object on purpose: it means no test in this - * repo -- pure or emulator-backed -- ever loads @aws-sdk/client-sesv2, and it - * means the mocked surface is exactly the one line of behaviour that matters - * (an input goes in, a MessageId or a throw comes out). Pass null to restore. + * repo -- pure or emulator-backed -- ever makes a network call, and it means + * the mocked surface is exactly the one line of behaviour that matters (an + * input goes in, an id or a throw comes out). Pass null to restore. */ -export function _setSesClientForTests(sender: MailSender | null): void { +export function _setMailSenderForTests(sender: MailSender | null): void { injectedSender = sender; - cachedSender = null; - cachedSenderKey = ""; } -async function getSender(config: MailConfig): Promise { - if (injectedSender) return injectedSender; - - // Keyed so a credential rotation between invocations on a warm instance - // builds a new client instead of reusing one signed with the old key. - const key = `${config.region}:${config.accessKeyId}`; - if (cachedSender && cachedSenderKey === key) return cachedSender; - - // Dynamic, not top-level: index.ts imports every module in this codebase, so - // a top-level AWS import would be paid on the cold start of apidata -- - // DataPipe's hot path -- to send no mail at all. - const sdk = (await import("@aws-sdk/client-sesv2")) as unknown as SesSdk; - const client = new sdk.SESv2Client({ - region: config.region, - credentials: { - accessKeyId: config.accessKeyId, - secretAccessKey: config.secretAccessKey, - }, - // The SDK's own retries are turned OFF. Attempts are counted on the - // document and capped there; an invisible second attempt inside send() - // would be a second send this file cannot see or bound. - maxAttempts: 1, - }); +/** + * The real transport: one POST, no SDK. + * + * There is nothing to cache and nothing to construct between calls -- which is + * why the SES version's client cache and its credential-rotation cache key are + * both gone. `fetch` is global in Node 22, so this also drops the dynamic + * import that existed to keep @aws-sdk/client-sesv2 off the cold-start path of + * apidata (index.ts imports every module in this codebase). There is now no + * mail dependency to keep off it. + */ +function resendSender(config: MailConfig): MailSender { + return async (input, { timeoutMs, idempotencyKey }) => { + const response = await fetch(RESEND_ENDPOINT, { + method: "POST", + headers: { + Authorization: `Bearer ${config.apiKey}`, + "Content-Type": "application/json", + "Idempotency-Key": idempotencyKey, + }, + body: JSON.stringify(input), + // No retries of its own, deliberately -- the same reason the SES client + // was built with maxAttempts: 1. Attempts are counted on the document + // and capped there; an invisible second attempt inside the transport + // would be a send this file cannot see or bound. + signal: AbortSignal.timeout(timeoutMs), + }); - cachedSender = async (input, { timeoutMs }) => { - const controller = new AbortController(); - const timer: ReturnType = setTimeout( - () => controller.abort(), - timeoutMs - ); + if (!response.ok) { + // Resend answers errors as {name, message, statusCode}, but that shape + // is not in its published contract and an edge/proxy failure is HTML. + // So: try for the body, fall back to the status, never throw from here + // -- a parse failure must not be reported as the reason the mail failed. + let name = `HttpError${response.status}`; + let message = response.statusText || `HTTP ${response.status}`; + try { + const body = (await response.json()) as { + name?: unknown; + message?: unknown; + }; + if (body && typeof body === "object") { + if (typeof body.name === "string" && body.name.length > 0) { + name = body.name; + } + if (typeof body.message === "string" && body.message.length > 0) { + message = body.message; + } + } + } catch { + // Not JSON. The status-derived name and message above stand, and + // classifyMailError falls back to `status` for the verdict. + } + throw new MailTransportError(name, message, response.status); + } + + // 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 { - return await client.send(new sdk.SendEmailCommand(input), { - abortSignal: controller.signal, - }); - } finally { - clearTimeout(timer); + const body = (await response.json()) as { id?: unknown }; + return { id: typeof body?.id === "string" ? body.id : undefined }; + } catch { + return {}; } }; - cachedSenderKey = key; - return cachedSender; +} + +function getSender(config: MailConfig): MailSender { + return injectedSender ?? resendSender(config); } // --------------------------------------------------------------------------- @@ -714,7 +795,7 @@ export async function deliverMailDocument(docId: string): Promise= MAX_ATTEMPTS; @@ -776,7 +863,7 @@ export async function deliverMailDocument(docId: string): Promise