SLICE-4: harden mailer failure path (send-before-commit + 502 delivery_failed) #7

Closed
opened 2026-06-10 17:53:06 +00:00 by ben.stull · 0 comments
Owner

Harden the mailer failure path when SmtpMailer lands (SLICE-4)

Surfaced by the SLICE-2 final pre-merge review (PR #6). Harmless in SLICE-2 because LogMailer never fails, but SmtpMailer (SLICE-4) will raise on a refused/unreachable relay, so SLICE-4 must close two gaps. SD-0001 scopes 502 delivery_failed to SLICE-4, so this is the natural home.

I-1 — request_code commits the code before dispatching it

backend/app/domains/accounts/service.pyrequest_code does conn.commit() (code row durable) and then mailer.send(...). If send() raises, the code is committed with consumed_at IS NULL, so the 60s resend cooldown fires on the user's next attempt — they're honestly told the code didn't go out (INV-9) but cannot immediately request a fresh one, violating §6.9 ("codes are re-requestable").

Fix (SLICE-4): send-before-commit (and roll back on failure), or delete the just-inserted row on send() failure before re-raising. Mind holding the txn across the SMTP call vs. cleanup-on-failure — pick the simpler correct shape.

I-2 — BFF has no 502 delivery_failed handler

backend/app/main.py — the request-code handler has no except for a mailer delivery error, so a raised send() would yield a 500, not the 502 delivery_failed envelope §6.4 specifies (INV-9). The Mailer port docstring already promises "Raises on delivery failure (INV-9 → §6.4 502)".

Fix (SLICE-4): define a delivery-failure exception the port raises, catch it in request_code, return _error(502, "delivery_failed", ...). Add a test (a stub mailer that raises) — the §6.6 PPE rehearsal also exercises real delivery.

Also worth a look in SLICE-4 (minor, MVP-accepted today)

  • request_code cooldown check isn't serialized — two concurrent requests for one email can both issue a code (low probability; not a security issue, allows more codes not fewer).
  • 429 responses carry retry_after_s in the JSON body (per §6.4) but no Retry-After HTTP header (RFC 6585) — add the header for API clients.

Context

  • Branch merged: slice-2-identitymain @ 1b40bde (PR #6).
  • Spec: wiggleverse-ecomm-content/specs/SD-0001-mvp-sign-up-and-single-storefront.md §6.4, §6.9, §7.2 SLICE-4.
  • Reviewer write-up is in the session 0022 transcript's Deferred decisions.
## Harden the mailer failure path when `SmtpMailer` lands (SLICE-4) Surfaced by the SLICE-2 final pre-merge review (PR #6). Harmless in SLICE-2 because `LogMailer` never fails, but `SmtpMailer` (SLICE-4) **will** raise on a refused/unreachable relay, so SLICE-4 must close two gaps. SD-0001 scopes `502 delivery_failed` to SLICE-4, so this is the natural home. ### I-1 — `request_code` commits the code *before* dispatching it `backend/app/domains/accounts/service.py` — `request_code` does `conn.commit()` (code row durable) and *then* `mailer.send(...)`. If `send()` raises, the code is committed with `consumed_at IS NULL`, so the 60s resend cooldown fires on the user's next attempt — they're honestly told the code didn't go out (INV-9) but **cannot immediately request a fresh one**, violating §6.9 ("codes are re-requestable"). **Fix (SLICE-4):** send-before-commit (and roll back on failure), or delete the just-inserted row on `send()` failure before re-raising. Mind holding the txn across the SMTP call vs. cleanup-on-failure — pick the simpler correct shape. ### I-2 — BFF has no `502 delivery_failed` handler `backend/app/main.py` — the `request-code` handler has no `except` for a mailer delivery error, so a raised `send()` would yield a 500, not the `502 delivery_failed` envelope §6.4 specifies (INV-9). The `Mailer` port docstring already promises "Raises on delivery failure (INV-9 → §6.4 502)". **Fix (SLICE-4):** define a delivery-failure exception the port raises, catch it in `request_code`, return `_error(502, "delivery_failed", ...)`. Add a test (a stub mailer that raises) — the §6.6 PPE rehearsal also exercises real delivery. ### Also worth a look in SLICE-4 (minor, MVP-accepted today) - `request_code` cooldown check isn't serialized — two concurrent requests for one email can both issue a code (low probability; not a security issue, allows *more* codes not fewer). - 429 responses carry `retry_after_s` in the JSON body (per §6.4) but no `Retry-After` HTTP header (RFC 6585) — add the header for API clients. ### Context - Branch merged: `slice-2-identity` → `main` @ 1b40bde (PR #6). - Spec: `wiggleverse-ecomm-content/specs/SD-0001-mvp-sign-up-and-single-storefront.md` §6.4, §6.9, §7.2 SLICE-4. - Reviewer write-up is in the session 0022 transcript's Deferred decisions.
ben.stull added the type/taskpriority/P2 labels 2026-06-10 17:53:06 +00:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: wiggleverse/wiggleverse-ecomm#7