From 726080994ad0e7d1761f7f57d01b283ea8375492 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 30 Jun 2026 15:29:10 -0500 Subject: [PATCH 1/5] feat(emails): log every POST /api/emails attempt to email_send_log (7-day observability) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an empty/footer-only email went out we could not recover what the API received: safeParseJson discards malformed bodies, the route logged nothing, and the sandbox that built the request is ephemeral. This records every attempt — sent, send_failed, and rejected (empty/malformed) — so a send is debuggable days later. - lib/supabase/email_send_log/insertEmailSendLog.ts: typed insert. - lib/emails/logEmailAttempt.ts: builds the row (computes body_parsed, truncates raw_body to 10k); best-effort — never throws, so logging can't break a send. - sendEmailHandler: capture the raw body via request.clone() and log on every outcome (rejected attempts keep the raw body, so a malformed send is visible). - types/database.types.ts: email_send_log added so the typed insert compiles. The migration lives in the recoupable/database repo (mono/database) — applied via its CI — not here. Tests: logEmailAttempt unit (parsed/malformed/truncation/swallow-errors) + handler asserts the sent/rejected/send_failed log calls. Full lib/emails: 150. Ref: recoupable/chat#1829 Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/emails/__tests__/logEmailAttempt.test.ts | 56 ++++++++++++++++++ lib/emails/__tests__/sendEmailHandler.test.ts | 14 +++++ lib/emails/logEmailAttempt.ts | 59 +++++++++++++++++++ lib/emails/sendEmailHandler.ts | 40 +++++++++++++ .../email_send_log/insertEmailSendLog.ts | 16 +++++ types/database.types.ts | 48 +++++++++++++++ 6 files changed, 233 insertions(+) create mode 100644 lib/emails/__tests__/logEmailAttempt.test.ts create mode 100644 lib/emails/logEmailAttempt.ts create mode 100644 lib/supabase/email_send_log/insertEmailSendLog.ts diff --git a/lib/emails/__tests__/logEmailAttempt.test.ts b/lib/emails/__tests__/logEmailAttempt.test.ts new file mode 100644 index 00000000..a352a9e3 --- /dev/null +++ b/lib/emails/__tests__/logEmailAttempt.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { logEmailAttempt } from "../logEmailAttempt"; + +const mockInsert = vi.fn(); +vi.mock("@/lib/supabase/email_send_log/insertEmailSendLog", () => ({ + insertEmailSendLog: (...args: unknown[]) => mockInsert(...args), +})); + +describe("logEmailAttempt", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockInsert.mockResolvedValue({ error: null }); + }); + + it("records a sent attempt with parsed body, lengths, and resend id", async () => { + await logEmailAttempt({ + rawBody: JSON.stringify({ to: ["a@b.com"], html: "

hi

" }), + status: "sent", + accountId: "acc-1", + to: ["a@b.com"], + subject: "S", + html: "

hi

", + chatId: "chat-1", + resendId: "re_123", + }); + expect(mockInsert).toHaveBeenCalledTimes(1); + const row = mockInsert.mock.calls[0][0]; + expect(row.status).toBe("sent"); + expect(row.body_parsed).toBe(true); + expect(row.account_id).toBe("acc-1"); + expect(row.to_count).toBe(1); + expect(row.html_length).toBe("

hi

".length); + expect(row.resend_id).toBe("re_123"); + expect(row.chat_id).toBe("chat-1"); + }); + + it("marks body_parsed false for a malformed body and still logs", async () => { + await logEmailAttempt({ rawBody: '{"to":[', status: "rejected" }); + const row = mockInsert.mock.calls[0][0]; + expect(row.body_parsed).toBe(false); + expect(row.status).toBe("rejected"); + expect(row.account_id).toBeNull(); + }); + + it("truncates a large raw body to 10000 chars", async () => { + await logEmailAttempt({ rawBody: "x".repeat(20000), status: "rejected" }); + const row = mockInsert.mock.calls[0][0]; + expect(row.raw_body.length).toBe(10000); + }); + + it("never throws when the insert fails", async () => { + mockInsert.mockRejectedValue(new Error("db down")); + await expect(logEmailAttempt({ rawBody: "{}", status: "rejected" })).resolves.toBeUndefined(); + }); +}); diff --git a/lib/emails/__tests__/sendEmailHandler.test.ts b/lib/emails/__tests__/sendEmailHandler.test.ts index 3b5fa6e8..03610bb1 100644 --- a/lib/emails/__tests__/sendEmailHandler.test.ts +++ b/lib/emails/__tests__/sendEmailHandler.test.ts @@ -4,6 +4,7 @@ import { sendEmailHandler } from "../sendEmailHandler"; const mockValidateSendEmailBody = vi.fn(); const mockProcessAndSendEmail = vi.fn(); +const mockLogEmailAttempt = vi.fn(); vi.mock("@/lib/emails/validateSendEmailBody", () => ({ validateSendEmailBody: (...args: unknown[]) => mockValidateSendEmailBody(...args), @@ -13,6 +14,10 @@ vi.mock("@/lib/emails/processAndSendEmail", () => ({ processAndSendEmail: (...args: unknown[]) => mockProcessAndSendEmail(...args), })); +vi.mock("@/lib/emails/logEmailAttempt", () => ({ + logEmailAttempt: (...args: unknown[]) => mockLogEmailAttempt(...args), +})); + vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); @@ -60,6 +65,9 @@ describe("sendEmailHandler", () => { room_id: "chat-1", }), ); + expect(mockLogEmailAttempt).toHaveBeenCalledWith( + expect.objectContaining({ status: "sent", resendId: "resend-id-1" }), + ); }); it("propagates the NextResponse from validateSendEmailBody (auth/validation/recipient errors)", async () => { @@ -69,6 +77,9 @@ describe("sendEmailHandler", () => { const response = await sendEmailHandler(createRequest()); expect(response.status).toBe(403); expect(mockProcessAndSendEmail).not.toHaveBeenCalled(); + expect(mockLogEmailAttempt).toHaveBeenCalledWith( + expect.objectContaining({ status: "rejected" }), + ); }); it("returns 502 when Resend delivery fails", async () => { @@ -77,5 +88,8 @@ describe("sendEmailHandler", () => { expect(response.status).toBe(502); const json = await response.json(); expect(json.error).toBe("resend boom"); + expect(mockLogEmailAttempt).toHaveBeenCalledWith( + expect.objectContaining({ status: "send_failed", error: "resend boom" }), + ); }); }); diff --git a/lib/emails/logEmailAttempt.ts b/lib/emails/logEmailAttempt.ts new file mode 100644 index 00000000..969d2ea0 --- /dev/null +++ b/lib/emails/logEmailAttempt.ts @@ -0,0 +1,59 @@ +import { insertEmailSendLog } from "@/lib/supabase/email_send_log/insertEmailSendLog"; + +/** Cap the stored raw body so a 12KB+ HTML report doesn't bloat the log row. */ +const MAX_RAW_BODY = 10000; + +export type EmailAttemptStatus = "sent" | "send_failed" | "rejected"; + +export type EmailAttemptLog = { + /** The raw request body, as received (used to record body_parsed + a truncated copy). */ + rawBody: string; + status: EmailAttemptStatus; + accountId?: string | null; + to?: string[]; + subject?: string; + html?: string; + text?: string; + chatId?: string; + resendId?: string; + error?: string; +}; + +/** + * Records one POST /api/emails attempt in `email_send_log` so a send can be + * debugged days later without the (ephemeral) sandbox — capturing whether the + * body parsed, a truncated copy of it, the resolved fields, and the outcome. + * + * Best-effort: this never throws. A logging failure must not affect the send. + * + * @param attempt - The attempt to record. + */ +export async function logEmailAttempt(attempt: EmailAttemptLog): Promise { + let bodyParsed = false; + try { + if (attempt.rawBody && attempt.rawBody.trim()) { + JSON.parse(attempt.rawBody); + bodyParsed = true; + } + } catch { + bodyParsed = false; + } + + try { + await insertEmailSendLog({ + account_id: attempt.accountId ?? null, + chat_id: attempt.chatId ?? null, + status: attempt.status, + body_parsed: bodyParsed, + raw_body: attempt.rawBody ? attempt.rawBody.slice(0, MAX_RAW_BODY) : null, + to_count: attempt.to?.length ?? null, + subject: attempt.subject ?? null, + html_length: attempt.html?.length ?? null, + text_length: attempt.text?.length ?? null, + resend_id: attempt.resendId ?? null, + error: attempt.error ?? null, + }); + } catch { + // Best-effort logging — swallow so the email send result is unaffected. + } +} diff --git a/lib/emails/sendEmailHandler.ts b/lib/emails/sendEmailHandler.ts index 8eac17e5..6a884db9 100644 --- a/lib/emails/sendEmailHandler.ts +++ b/lib/emails/sendEmailHandler.ts @@ -2,6 +2,16 @@ import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateSendEmailBody } from "@/lib/emails/validateSendEmailBody"; import { processAndSendEmail } from "@/lib/emails/processAndSendEmail"; +import { logEmailAttempt } from "@/lib/emails/logEmailAttempt"; + +/** Read the raw request body without consuming it (validation reads the original). */ +async function readRawBody(request: NextRequest): Promise { + try { + return await request.clone().text(); + } catch { + return ""; + } +} /** * Handler for POST /api/emails. @@ -13,12 +23,19 @@ import { processAndSendEmail } from "@/lib/emails/processAndSendEmail"; * Body validation, auth, and the recipient restriction all live in * `validateSendEmailBody`. * + * Every attempt — sent, send-failed, and rejected (e.g. an empty/malformed + * body) — is recorded in `email_send_log` via `logEmailAttempt`, so a send can + * be debugged days later without the ephemeral sandbox that built the request. + * * @param request - The request object. * @returns A NextResponse with the send result. */ export async function sendEmailHandler(request: NextRequest): Promise { + const rawBody = await readRawBody(request); + const validated = await validateSendEmailBody(request); if (validated instanceof NextResponse) { + await logEmailAttempt({ rawBody, status: "rejected" }); return validated; } @@ -35,12 +52,35 @@ export async function sendEmailHandler(request: NextRequest): Promise { + const { error } = await supabase.from("email_send_log").insert(row); + return { error }; +} diff --git a/types/database.types.ts b/types/database.types.ts index 872750b7..3e15dda8 100644 --- a/types/database.types.ts +++ b/types/database.types.ts @@ -8,6 +8,54 @@ export type Database = { }; public: { Tables: { + email_send_log: { + Row: { + account_id: string | null; + body_parsed: boolean; + chat_id: string | null; + created_at: string; + error: string | null; + html_length: number | null; + id: string; + raw_body: string | null; + resend_id: string | null; + status: string; + subject: string | null; + text_length: number | null; + to_count: number | null; + }; + Insert: { + account_id?: string | null; + body_parsed?: boolean; + chat_id?: string | null; + created_at?: string; + error?: string | null; + html_length?: number | null; + id?: string; + raw_body?: string | null; + resend_id?: string | null; + status: string; + subject?: string | null; + text_length?: number | null; + to_count?: number | null; + }; + Update: { + account_id?: string | null; + body_parsed?: boolean; + chat_id?: string | null; + created_at?: string; + error?: string | null; + html_length?: number | null; + id?: string; + raw_body?: string | null; + resend_id?: string | null; + status?: string; + subject?: string | null; + text_length?: number | null; + to_count?: number | null; + }; + Relationships: []; + }; account_api_keys: { Row: { account: string | null; From 0151c418155ff5baf3b0b368d9ea16be68af1ced Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 30 Jun 2026 17:56:51 -0500 Subject: [PATCH 2/5] refactor(emails): reconcile email_send_log writer to the final 7-column schema database#37 + #38 shipped email_send_log as 7 columns (id, created_at, account_id, chat_id, status, resend_id, raw_body) with RLS. Regenerate types/database.types.ts from the live schema (replaces the earlier hand-edit; also picks up unrelated drift) and trim the writer to match: - logEmailAttempt: insert only account_id/chat_id/status/resend_id/raw_body; drop body_parsed, error, subject, to_count, html_length, text_length; store the FULL raw_body (no truncation). - sendEmailHandler: drop the removed fields from the send_failed/sent log calls. - tests updated accordingly. Full lib/emails suite: 149 passing; tsc + lint clean. Ref: recoupable/chat#1829 Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/emails/__tests__/logEmailAttempt.test.ts | 32 ++--- lib/emails/__tests__/sendEmailHandler.test.ts | 2 +- lib/emails/logEmailAttempt.ts | 34 +----- lib/emails/sendEmailHandler.ts | 9 -- types/database.types.ts | 110 +++++------------- 5 files changed, 47 insertions(+), 140 deletions(-) diff --git a/lib/emails/__tests__/logEmailAttempt.test.ts b/lib/emails/__tests__/logEmailAttempt.test.ts index a352a9e3..5570a42d 100644 --- a/lib/emails/__tests__/logEmailAttempt.test.ts +++ b/lib/emails/__tests__/logEmailAttempt.test.ts @@ -13,44 +13,34 @@ describe("logEmailAttempt", () => { mockInsert.mockResolvedValue({ error: null }); }); - it("records a sent attempt with parsed body, lengths, and resend id", async () => { + it("records account, chat, status, resend id, and the raw body", async () => { + const raw = JSON.stringify({ html: "

hi

" }); await logEmailAttempt({ - rawBody: JSON.stringify({ to: ["a@b.com"], html: "

hi

" }), + rawBody: raw, status: "sent", accountId: "acc-1", - to: ["a@b.com"], - subject: "S", - html: "

hi

", chatId: "chat-1", resendId: "re_123", }); expect(mockInsert).toHaveBeenCalledTimes(1); const row = mockInsert.mock.calls[0][0]; expect(row.status).toBe("sent"); - expect(row.body_parsed).toBe(true); expect(row.account_id).toBe("acc-1"); - expect(row.to_count).toBe(1); - expect(row.html_length).toBe("

hi

".length); - expect(row.resend_id).toBe("re_123"); expect(row.chat_id).toBe("chat-1"); + expect(row.resend_id).toBe("re_123"); + expect(row.raw_body).toBe(raw); }); - it("marks body_parsed false for a malformed body and still logs", async () => { - await logEmailAttempt({ rawBody: '{"to":[', status: "rejected" }); - const row = mockInsert.mock.calls[0][0]; - expect(row.body_parsed).toBe(false); - expect(row.status).toBe("rejected"); - expect(row.account_id).toBeNull(); - }); - - it("truncates a large raw body to 10000 chars", async () => { + it("stores the full raw body (no truncation)", async () => { await logEmailAttempt({ rawBody: "x".repeat(20000), status: "rejected" }); - const row = mockInsert.mock.calls[0][0]; - expect(row.raw_body.length).toBe(10000); + expect(mockInsert.mock.calls[0][0].raw_body.length).toBe(20000); }); - it("never throws when the insert fails", async () => { + it("nulls optional ids and never throws when the insert fails", async () => { mockInsert.mockRejectedValue(new Error("db down")); await expect(logEmailAttempt({ rawBody: "{}", status: "rejected" })).resolves.toBeUndefined(); + const row = mockInsert.mock.calls[0][0]; + expect(row.account_id).toBeNull(); + expect(row.chat_id).toBeNull(); }); }); diff --git a/lib/emails/__tests__/sendEmailHandler.test.ts b/lib/emails/__tests__/sendEmailHandler.test.ts index 03610bb1..3c732737 100644 --- a/lib/emails/__tests__/sendEmailHandler.test.ts +++ b/lib/emails/__tests__/sendEmailHandler.test.ts @@ -89,7 +89,7 @@ describe("sendEmailHandler", () => { const json = await response.json(); expect(json.error).toBe("resend boom"); expect(mockLogEmailAttempt).toHaveBeenCalledWith( - expect.objectContaining({ status: "send_failed", error: "resend boom" }), + expect.objectContaining({ status: "send_failed" }), ); }); }); diff --git a/lib/emails/logEmailAttempt.ts b/lib/emails/logEmailAttempt.ts index 969d2ea0..a87f424c 100644 --- a/lib/emails/logEmailAttempt.ts +++ b/lib/emails/logEmailAttempt.ts @@ -1,57 +1,33 @@ import { insertEmailSendLog } from "@/lib/supabase/email_send_log/insertEmailSendLog"; -/** Cap the stored raw body so a 12KB+ HTML report doesn't bloat the log row. */ -const MAX_RAW_BODY = 10000; - export type EmailAttemptStatus = "sent" | "send_failed" | "rejected"; export type EmailAttemptLog = { - /** The raw request body, as received (used to record body_parsed + a truncated copy). */ + /** The raw request body, as received (stored in full). */ rawBody: string; status: EmailAttemptStatus; accountId?: string | null; - to?: string[]; - subject?: string; - html?: string; - text?: string; - chatId?: string; + chatId?: string | null; resendId?: string; - error?: string; }; /** * Records one POST /api/emails attempt in `email_send_log` so a send can be - * debugged days later without the (ephemeral) sandbox — capturing whether the - * body parsed, a truncated copy of it, the resolved fields, and the outcome. + * debugged days later without the (ephemeral) sandbox — the request body as + * received, the outcome, and pointers to the account/chat and the Resend id. * * Best-effort: this never throws. A logging failure must not affect the send. * * @param attempt - The attempt to record. */ export async function logEmailAttempt(attempt: EmailAttemptLog): Promise { - let bodyParsed = false; - try { - if (attempt.rawBody && attempt.rawBody.trim()) { - JSON.parse(attempt.rawBody); - bodyParsed = true; - } - } catch { - bodyParsed = false; - } - try { await insertEmailSendLog({ account_id: attempt.accountId ?? null, chat_id: attempt.chatId ?? null, status: attempt.status, - body_parsed: bodyParsed, - raw_body: attempt.rawBody ? attempt.rawBody.slice(0, MAX_RAW_BODY) : null, - to_count: attempt.to?.length ?? null, - subject: attempt.subject ?? null, - html_length: attempt.html?.length ?? null, - text_length: attempt.text?.length ?? null, resend_id: attempt.resendId ?? null, - error: attempt.error ?? null, + raw_body: attempt.rawBody || null, }); } catch { // Best-effort logging — swallow so the email send result is unaffected. diff --git a/lib/emails/sendEmailHandler.ts b/lib/emails/sendEmailHandler.ts index 6a884db9..5b4d63bf 100644 --- a/lib/emails/sendEmailHandler.ts +++ b/lib/emails/sendEmailHandler.ts @@ -56,12 +56,7 @@ export async function sendEmailHandler(request: NextRequest): Promise Date: Tue, 30 Jun 2026 18:22:55 -0500 Subject: [PATCH 3/5] refactor(emails): own raw-body read in validate (SRP), log once (DRY) Address review on #731: - SRP: validateSendEmailBody now reads the request body once (readRawBody) and returns rawBody on both the rejected and accepted paths, so the handler no longer reads the request itself. - DRY: sendEmailHandler computes a single outcome and calls logEmailAttempt exactly once for every path (sent / send_failed / rejected). - Surface a returned insert error to server logs (observability) and store the raw body verbatim (preserve empty string). - Type insertEmailSendLog's error as PostgrestError | null. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/emails/__tests__/logEmailAttempt.test.ts | 11 ++ lib/emails/__tests__/sendEmailHandler.test.ts | 32 +++-- .../__tests__/validateSendEmailBody.test.ts | 78 ++++++------ lib/emails/logEmailAttempt.ts | 19 +-- lib/emails/sendEmailHandler.ts | 103 +++++++--------- lib/emails/validateSendEmailBody.ts | 114 ++++++++++-------- .../email_send_log/insertEmailSendLog.ts | 7 +- 7 files changed, 191 insertions(+), 173 deletions(-) diff --git a/lib/emails/__tests__/logEmailAttempt.test.ts b/lib/emails/__tests__/logEmailAttempt.test.ts index 5570a42d..4026da1e 100644 --- a/lib/emails/__tests__/logEmailAttempt.test.ts +++ b/lib/emails/__tests__/logEmailAttempt.test.ts @@ -8,9 +8,11 @@ vi.mock("@/lib/supabase/email_send_log/insertEmailSendLog", () => ({ })); describe("logEmailAttempt", () => { + let errorSpy: ReturnType; beforeEach(() => { vi.clearAllMocks(); mockInsert.mockResolvedValue({ error: null }); + errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); }); it("records account, chat, status, resend id, and the raw body", async () => { @@ -42,5 +44,14 @@ describe("logEmailAttempt", () => { const row = mockInsert.mock.calls[0][0]; expect(row.account_id).toBeNull(); expect(row.chat_id).toBeNull(); + expect(errorSpy).toHaveBeenCalled(); + }); + + it("surfaces a returned insert error to the server logs", async () => { + mockInsert.mockResolvedValue({ error: { message: "rls denied" } }); + await logEmailAttempt({ rawBody: "{}", status: "sent" }); + expect(errorSpy).toHaveBeenCalledWith("email_send_log insert failed:", { + message: "rls denied", + }); }); }); diff --git a/lib/emails/__tests__/sendEmailHandler.test.ts b/lib/emails/__tests__/sendEmailHandler.test.ts index 3c732737..8bc132c1 100644 --- a/lib/emails/__tests__/sendEmailHandler.test.ts +++ b/lib/emails/__tests__/sendEmailHandler.test.ts @@ -34,12 +34,15 @@ describe("sendEmailHandler", () => { beforeEach(() => { vi.clearAllMocks(); mockValidateSendEmailBody.mockResolvedValue({ - to: ["dest@example.com"], - cc: ["cc@example.com"], - subject: "Weekly report", - text: "body", - chat_id: "chat-1", - accountId: "account-123", + rawBody: '{"subject":"Weekly report"}', + data: { + to: ["dest@example.com"], + cc: ["cc@example.com"], + subject: "Weekly report", + text: "body", + chat_id: "chat-1", + accountId: "account-123", + }, }); mockProcessAndSendEmail.mockResolvedValue({ success: true, @@ -65,15 +68,24 @@ describe("sendEmailHandler", () => { room_id: "chat-1", }), ); + // Single call, on every path (DRY); rawBody comes from validateSendEmailBody (SRP). + expect(mockLogEmailAttempt).toHaveBeenCalledTimes(1); expect(mockLogEmailAttempt).toHaveBeenCalledWith( - expect.objectContaining({ status: "sent", resendId: "resend-id-1" }), + expect.objectContaining({ + status: "sent", + resendId: "resend-id-1", + rawBody: '{"subject":"Weekly report"}', + accountId: "account-123", + chatId: "chat-1", + }), ); }); it("propagates the NextResponse from validateSendEmailBody (auth/validation/recipient errors)", async () => { - mockValidateSendEmailBody.mockResolvedValue( - NextResponse.json({ status: "error", error: "Forbidden" }, { status: 403 }), - ); + mockValidateSendEmailBody.mockResolvedValue({ + rawBody: "{}", + error: NextResponse.json({ status: "error", error: "Forbidden" }, { status: 403 }), + }); const response = await sendEmailHandler(createRequest()); expect(response.status).toBe(403); expect(mockProcessAndSendEmail).not.toHaveBeenCalled(); diff --git a/lib/emails/__tests__/validateSendEmailBody.test.ts b/lib/emails/__tests__/validateSendEmailBody.test.ts index a53a2c56..9fdb86ee 100644 --- a/lib/emails/__tests__/validateSendEmailBody.test.ts +++ b/lib/emails/__tests__/validateSendEmailBody.test.ts @@ -22,10 +22,6 @@ vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); -vi.mock("@/lib/networking/safeParseJson", () => ({ - safeParseJson: vi.fn(async (req: Request) => req.json()), -})); - function createRequest(body: unknown, headers: Record = {}): NextRequest { return new NextRequest("http://localhost/api/emails", { method: "POST", @@ -58,10 +54,10 @@ describe("validateSendEmailBody", () => { ); const result = await validateSendEmailBody(request); - expect(result).toBeInstanceOf(NextResponse); - if (result instanceof NextResponse) { - expect(result.status).toBe(403); - const json = await result.json(); + expect("error" in result).toBe(true); + if ("error" in result) { + expect(result.error.status).toBe(403); + const json = await result.error.json(); expect(json.disallowed_recipients).toEqual(["stranger@example.com"]); } }); @@ -87,18 +83,18 @@ describe("validateSendEmailBody", () => { { "x-api-key": "k" }, ); const result = await validateSendEmailBody(request); - expect(result).not.toBeInstanceOf(NextResponse); - if (!(result instanceof NextResponse)) { - expect(result.subject).toBe("Pulse Report"); + expect("data" in result).toBe(true); + if ("data" in result) { + expect(result.data.subject).toBe("Pulse Report"); } }); it("defaults to `Message from Recoup` when subject and body are empty", async () => { const request = createRequest({ to: ["d@example.com"] }, { "x-api-key": "k" }); const result = await validateSendEmailBody(request); - expect(result).not.toBeInstanceOf(NextResponse); - if (!(result instanceof NextResponse)) { - expect(result.subject).toBe("Message from Recoup"); + expect("data" in result).toBe(true); + if ("data" in result) { + expect(result.data.subject).toBe("Message from Recoup"); } }); }); @@ -111,12 +107,12 @@ describe("validateSendEmailBody", () => { ); const result = await validateSendEmailBody(request); - expect(result).not.toBeInstanceOf(NextResponse); - if (!(result instanceof NextResponse)) { - expect(result.to).toEqual(["dest@example.com"]); - expect(result.subject).toBe("Hi"); - expect(result.text).toBe("Hello world"); - expect(result.accountId).toBe("account-123"); + expect("data" in result).toBe(true); + if ("data" in result) { + expect(result.data.to).toEqual(["dest@example.com"]); + expect(result.data.subject).toBe("Hi"); + expect(result.data.text).toBe("Hello world"); + expect(result.data.accountId).toBe("account-123"); } }); @@ -135,13 +131,13 @@ describe("validateSendEmailBody", () => { ); const result = await validateSendEmailBody(request); - expect(result).not.toBeInstanceOf(NextResponse); - if (!(result instanceof NextResponse)) { - expect(result.to).toEqual(["a@example.com", "b@example.com"]); - expect(result.cc).toEqual(["cc@example.com"]); - expect(result.html).toBe("

h

"); - expect(result.headers).toEqual({ "X-Custom": "v" }); - expect(result.chat_id).toBe("chat-1"); + expect("data" in result).toBe(true); + if ("data" in result) { + expect(result.data.to).toEqual(["a@example.com", "b@example.com"]); + expect(result.data.cc).toEqual(["cc@example.com"]); + expect(result.data.html).toBe("

h

"); + expect(result.data.headers).toEqual({ "X-Custom": "v" }); + expect(result.data.chat_id).toBe("chat-1"); } }); @@ -163,9 +159,9 @@ describe("validateSendEmailBody", () => { const request = createRequest({ subject: "s", text: "hi" }, { "x-api-key": "k" }); const result = await validateSendEmailBody(request); - expect(result).not.toBeInstanceOf(NextResponse); - if (!(result instanceof NextResponse)) { - expect(result.to).toEqual(["owner@example.com"]); + expect("data" in result).toBe(true); + if ("data" in result) { + expect(result.data.to).toEqual(["owner@example.com"]); } expect(mockSelectAccountEmails).toHaveBeenCalledWith({ accountIds: "account-123" }); }); @@ -178,9 +174,9 @@ describe("validateSendEmailBody", () => { const request = createRequest({ subject: "s" }, { "x-api-key": "k" }); const result = await validateSendEmailBody(request); - expect(result).not.toBeInstanceOf(NextResponse); - if (!(result instanceof NextResponse)) { - expect(result.to).toEqual(["owner@example.com", "owner.alt@example.com"]); + expect("data" in result).toBe(true); + if ("data" in result) { + expect(result.data.to).toEqual(["owner@example.com", "owner.alt@example.com"]); } }); @@ -188,8 +184,8 @@ describe("validateSendEmailBody", () => { mockSelectAccountEmails.mockResolvedValue([]); const request = createRequest({ subject: "s" }, { "x-api-key": "k" }); const result = await validateSendEmailBody(request); - expect(result).toBeInstanceOf(NextResponse); - if (result instanceof NextResponse) expect(result.status).toBe(400); + expect("error" in result).toBe(true); + if ("error" in result) expect(result.error.status).toBe(400); }); it("does not resolve account emails when 'to' is provided", async () => { @@ -206,15 +202,15 @@ describe("validateSendEmailBody", () => { it("rejects an empty 'to' array", async () => { const request = createRequest({ to: [], subject: "s" }, { "x-api-key": "k" }); const result = await validateSendEmailBody(request); - expect(result).toBeInstanceOf(NextResponse); - if (result instanceof NextResponse) expect(result.status).toBe(400); + expect("error" in result).toBe(true); + if ("error" in result) expect(result.error.status).toBe(400); }); it("rejects a non-email recipient", async () => { const request = createRequest({ to: ["not-an-email"], subject: "s" }, { "x-api-key": "k" }); const result = await validateSendEmailBody(request); - expect(result).toBeInstanceOf(NextResponse); - if (result instanceof NextResponse) expect(result.status).toBe(400); + expect("error" in result).toBe(true); + if ("error" in result) expect(result.error.status).toBe(400); }); // subject is now optional (defaults from the body) — covered by "subject defaulting" above. @@ -227,8 +223,8 @@ describe("validateSendEmailBody", () => { ); const request = createRequest({ to: ["d@example.com"], subject: "s" }); const result = await validateSendEmailBody(request); - expect(result).toBeInstanceOf(NextResponse); - if (result instanceof NextResponse) expect(result.status).toBe(401); + expect("error" in result).toBe(true); + if ("error" in result) expect(result.error.status).toBe(401); }); }); }); diff --git a/lib/emails/logEmailAttempt.ts b/lib/emails/logEmailAttempt.ts index a87f424c..14c13f0c 100644 --- a/lib/emails/logEmailAttempt.ts +++ b/lib/emails/logEmailAttempt.ts @@ -3,7 +3,7 @@ import { insertEmailSendLog } from "@/lib/supabase/email_send_log/insertEmailSen export type EmailAttemptStatus = "sent" | "send_failed" | "rejected"; export type EmailAttemptLog = { - /** The raw request body, as received (stored in full). */ + /** The request body as received (stored in full; "" when empty/unreadable). */ rawBody: string; status: EmailAttemptStatus; accountId?: string | null; @@ -13,23 +13,26 @@ export type EmailAttemptLog = { /** * Records one POST /api/emails attempt in `email_send_log` so a send can be - * debugged days later without the (ephemeral) sandbox — the request body as - * received, the outcome, and pointers to the account/chat and the Resend id. + * debugged days later without the (ephemeral) sandbox. * - * Best-effort: this never throws. A logging failure must not affect the send. + * Best-effort: never throws (a logging failure must not affect the send), but a + * failed write IS surfaced to server logs so observability gaps are visible. * * @param attempt - The attempt to record. */ export async function logEmailAttempt(attempt: EmailAttemptLog): Promise { try { - await insertEmailSendLog({ + const { error } = await insertEmailSendLog({ account_id: attempt.accountId ?? null, chat_id: attempt.chatId ?? null, status: attempt.status, resend_id: attempt.resendId ?? null, - raw_body: attempt.rawBody || null, + raw_body: attempt.rawBody, }); - } catch { - // Best-effort logging — swallow so the email send result is unaffected. + if (error) { + console.error("email_send_log insert failed:", error); + } + } catch (err) { + console.error("logEmailAttempt threw (swallowed):", err); } } diff --git a/lib/emails/sendEmailHandler.ts b/lib/emails/sendEmailHandler.ts index 5b4d63bf..7993fd01 100644 --- a/lib/emails/sendEmailHandler.ts +++ b/lib/emails/sendEmailHandler.ts @@ -1,45 +1,17 @@ import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateSendEmailBody } from "@/lib/emails/validateSendEmailBody"; +import { + validateSendEmailBody, + type ValidatedSendEmailRequest, +} from "@/lib/emails/validateSendEmailBody"; import { processAndSendEmail } from "@/lib/emails/processAndSendEmail"; -import { logEmailAttempt } from "@/lib/emails/logEmailAttempt"; +import { logEmailAttempt, type EmailAttemptLog } from "@/lib/emails/logEmailAttempt"; -/** Read the raw request body without consuming it (validation reads the original). */ -async function readRawBody(request: NextRequest): Promise { - try { - return await request.clone().text(); - } catch { - return ""; - } -} - -/** - * Handler for POST /api/emails. - * - * Sends an email to the explicit recipients in the request body via Resend - * (from `Agent by Recoup `), reusing the same - * `processAndSendEmail` domain function as the `send_email` MCP tool. - * Account-scoped: requires authentication via x-api-key or Authorization Bearer. - * Body validation, auth, and the recipient restriction all live in - * `validateSendEmailBody`. - * - * Every attempt — sent, send-failed, and rejected (e.g. an empty/malformed - * body) — is recorded in `email_send_log` via `logEmailAttempt`, so a send can - * be debugged days later without the ephemeral sandbox that built the request. - * - * @param request - The request object. - * @returns A NextResponse with the send result. - */ -export async function sendEmailHandler(request: NextRequest): Promise { - const rawBody = await readRawBody(request); - - const validated = await validateSendEmailBody(request); - if (validated instanceof NextResponse) { - await logEmailAttempt({ rawBody, status: "rejected" }); - return validated; - } - - const { to, cc = [], subject, text, html = "", headers = {}, chat_id } = validated; +/** Sends a validated email; returns the HTTP response plus the attempt to log. */ +async function deliver( + data: ValidatedSendEmailRequest, +): Promise<{ response: NextResponse; attempt: Omit }> { + const { to, cc = [], subject, text, html = "", headers = {}, chat_id, accountId } = data; const result = await processAndSendEmail({ to, @@ -52,28 +24,41 @@ export async function sendEmailHandler(request: NextRequest): Promise { + const validated = await validateSendEmailBody(request); + + const { response, attempt } = + "data" in validated + ? await deliver(validated.data) + : { response: validated.error, attempt: { status: "rejected" as const } }; - return NextResponse.json( - { success: true, message: result.message, id: result.id }, - { status: 200, headers: getCorsHeaders() }, - ); + await logEmailAttempt({ rawBody: validated.rawBody, ...attempt }); + return response; } diff --git a/lib/emails/validateSendEmailBody.ts b/lib/emails/validateSendEmailBody.ts index 041437da..7a6d1eb2 100644 --- a/lib/emails/validateSendEmailBody.ts +++ b/lib/emails/validateSendEmailBody.ts @@ -3,7 +3,6 @@ import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { assertRecipientsAllowed } from "@/lib/emails/assertRecipientsAllowed"; import { resolveEmailSubject } from "@/lib/emails/resolveEmailSubject"; -import { safeParseJson } from "@/lib/networking/safeParseJson"; import selectAccountEmails from "@/lib/supabase/account_emails/selectAccountEmails"; import { z } from "zod"; @@ -30,47 +29,60 @@ export type ValidatedSendEmailRequest = Omit & }; /** - * Validates POST /api/emails: parses the body, authenticates via - * validateAuthContext (x-api-key or Bearer), resolves the recipients, then - * enforces the recipient restriction (without a payment method on file, - * `to`/`cc` are limited to the account's own email). - * - * `to` is optional: when omitted, the email defaults to the authenticated - * account's own email address(es) (via `account_emails`), so a caller can - * "email me this" without restating their address. `subject` is optional too: - * when omitted it defaults to the body's first heading/line, else - * `Message from Recoup`. Returns the resolved `to` + `subject`. + * Validation outcome. Always carries `rawBody` — the request body as received — + * so the handler can record it in `email_send_log` on both the rejected and the + * accepted paths without re-reading the request. + */ +export type ValidateSendEmailResult = + | { rawBody: string; error: NextResponse } + | { rawBody: string; data: ValidatedSendEmailRequest }; + +/** Read the request body once, as text (the source for both parsing and logging). */ +async function readRawBody(request: NextRequest): Promise { + try { + return await request.text(); + } catch { + return ""; + } +} + +/** + * Validates POST /api/emails: reads the raw body, parses + schema-checks it, + * authenticates (x-api-key or Bearer), resolves the recipients, and enforces the + * recipient restriction. `to` defaults to the account's own email when omitted; + * `subject` is derived from the body when omitted. Always returns `rawBody`. * * @param request - The NextRequest object - * @returns A NextResponse with an error if validation/auth/recipients fail, or the validated request data. + * @returns `{ rawBody, error }` if validation/auth/recipients fail, else `{ rawBody, data }`. */ export async function validateSendEmailBody( request: NextRequest, -): Promise { - const body = await safeParseJson(request); - const result = sendEmailBodySchema.safeParse(body); +): Promise { + const rawBody = await readRawBody(request); + let parsed: unknown = {}; + if (rawBody) { + try { + parsed = JSON.parse(rawBody); + } catch { + parsed = {}; + } + } + const result = sendEmailBodySchema.safeParse(parsed); if (!result.success) { const firstError = result.error.issues[0]; - return NextResponse.json( - { - status: "error", - missing_fields: firstError.path, - error: firstError.message, - }, - { - status: 400, - headers: getCorsHeaders(), - }, - ); + return { + rawBody, + error: NextResponse.json( + { status: "error", missing_fields: firstError.path, error: firstError.message }, + { status: 400, headers: getCorsHeaders() }, + ), + }; } - const authContext = await validateAuthContext(request, { - accountId: result.data.account_id, - }); - + const authContext = await validateAuthContext(request, { accountId: result.data.account_id }); if (authContext instanceof NextResponse) { - return authContext; + return { rawBody, error: authContext }; } let to = result.data.to ?? []; @@ -78,13 +90,13 @@ export async function validateSendEmailBody( const ownRows = await selectAccountEmails({ accountIds: authContext.accountId }); to = ownRows.map(row => row.email); if (to.length === 0) { - return NextResponse.json( - { - status: "error", - error: "No email address found for the authenticated account.", - }, - { status: 400, headers: getCorsHeaders() }, - ); + return { + rawBody, + error: NextResponse.json( + { status: "error", error: "No email address found for the authenticated account." }, + { status: 400, headers: getCorsHeaders() }, + ), + }; } } @@ -93,14 +105,17 @@ export async function validateSendEmailBody( recipients: [...to, ...(result.data.cc ?? [])], }); if (recipientCheck.allowed === false) { - return NextResponse.json( - { - status: "error", - error: `Without a payment method on file, emails can only be sent to the account's own address. Disallowed recipients: ${recipientCheck.disallowed.join(", ")}. Add a payment method to send to any recipient.`, - disallowed_recipients: recipientCheck.disallowed, - }, - { status: 403, headers: getCorsHeaders() }, - ); + return { + rawBody, + error: NextResponse.json( + { + status: "error", + error: `Without a payment method on file, emails can only be sent to the account's own address. Disallowed recipients: ${recipientCheck.disallowed.join(", ")}. Add a payment method to send to any recipient.`, + disallowed_recipients: recipientCheck.disallowed, + }, + { status: 403, headers: getCorsHeaders() }, + ), + }; } const subject = resolveEmailSubject({ @@ -109,10 +124,5 @@ export async function validateSendEmailBody( html: result.data.html, }); - return { - ...result.data, - to, - subject, - accountId: authContext.accountId, - }; + return { rawBody, data: { ...result.data, to, subject, accountId: authContext.accountId } }; } diff --git a/lib/supabase/email_send_log/insertEmailSendLog.ts b/lib/supabase/email_send_log/insertEmailSendLog.ts index cb16241a..07cf540d 100644 --- a/lib/supabase/email_send_log/insertEmailSendLog.ts +++ b/lib/supabase/email_send_log/insertEmailSendLog.ts @@ -1,16 +1,17 @@ import supabase from "../serverClient"; import type { Database } from "@/types/database.types"; +import type { PostgrestError } from "@supabase/supabase-js"; /** - * Inserts one row into `email_send_log`. Best-effort: callers should not let a - * logging failure affect the email send, so errors are returned, never thrown. + * Inserts one row into `email_send_log`. Best-effort: returns the Supabase error + * rather than throwing, so a logging failure never affects the email send. * * @param row - The email_send_log insert payload. * @returns `{ error }` — null on success. */ export async function insertEmailSendLog( row: Database["public"]["Tables"]["email_send_log"]["Insert"], -): Promise<{ error: unknown }> { +): Promise<{ error: PostgrestError | null }> { const { error } = await supabase.from("email_send_log").insert(row); return { error }; } From 70c155e8de297861cece046eea5b0bd2a6082f09 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 30 Jun 2026 18:32:19 -0500 Subject: [PATCH 4/5] refactor(emails): extract deliverEmail + readRawBody to own files (SRP) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the one-exported-function-per-file convention (review on #731): - lib/networking/readRawBody.ts — body-as-text read (sibling of safeParseJson); validateSendEmailBody imports it. - lib/emails/deliverEmail.ts — Resend send + response/attempt shaping; sendEmailHandler delegates to it. Handler is now pure orchestration. Both extractions get dedicated unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/emails/__tests__/deliverEmail.test.ts | 50 ++++++++++++++++++ lib/emails/deliverEmail.ts | 52 +++++++++++++++++++ lib/emails/sendEmailHandler.ts | 53 +++----------------- lib/emails/validateSendEmailBody.ts | 10 +--- lib/networking/__tests__/readRawBody.test.ts | 23 +++++++++ lib/networking/readRawBody.ts | 17 +++++++ 6 files changed, 150 insertions(+), 55 deletions(-) create mode 100644 lib/emails/__tests__/deliverEmail.test.ts create mode 100644 lib/emails/deliverEmail.ts create mode 100644 lib/networking/__tests__/readRawBody.test.ts create mode 100644 lib/networking/readRawBody.ts diff --git a/lib/emails/__tests__/deliverEmail.test.ts b/lib/emails/__tests__/deliverEmail.test.ts new file mode 100644 index 00000000..2f21a5c8 --- /dev/null +++ b/lib/emails/__tests__/deliverEmail.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { deliverEmail } from "../deliverEmail"; +import type { ValidatedSendEmailRequest } from "../validateSendEmailBody"; + +const mockProcessAndSendEmail = vi.fn(); +vi.mock("@/lib/emails/processAndSendEmail", () => ({ + processAndSendEmail: (...args: unknown[]) => mockProcessAndSendEmail(...args), +})); +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +const data: ValidatedSendEmailRequest = { + to: ["dest@example.com"], + cc: ["cc@example.com"], + subject: "Weekly report", + text: "body", + chat_id: "chat-1", + accountId: "account-123", +}; + +describe("deliverEmail", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns a 200 + sent attempt and maps chat_id to room_id on success", async () => { + mockProcessAndSendEmail.mockResolvedValue({ success: true, message: "ok", id: "re_1" }); + const { response, attempt } = await deliverEmail(data); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ success: true, message: "ok", id: "re_1" }); + expect(mockProcessAndSendEmail).toHaveBeenCalledWith( + expect.objectContaining({ to: ["dest@example.com"], room_id: "chat-1" }), + ); + expect(attempt).toEqual({ + status: "sent", + accountId: "account-123", + chatId: "chat-1", + resendId: "re_1", + }); + }); + + it("returns a 502 + send_failed attempt (no resendId) on delivery failure", async () => { + mockProcessAndSendEmail.mockResolvedValue({ success: false, error: "resend boom" }); + const { response, attempt } = await deliverEmail(data); + + expect(response.status).toBe(502); + expect((await response.json()).error).toBe("resend boom"); + expect(attempt).toEqual({ status: "send_failed", accountId: "account-123", chatId: "chat-1" }); + }); +}); diff --git a/lib/emails/deliverEmail.ts b/lib/emails/deliverEmail.ts new file mode 100644 index 00000000..2de16069 --- /dev/null +++ b/lib/emails/deliverEmail.ts @@ -0,0 +1,52 @@ +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { processAndSendEmail } from "@/lib/emails/processAndSendEmail"; +import type { ValidatedSendEmailRequest } from "@/lib/emails/validateSendEmailBody"; +import type { EmailAttemptLog } from "@/lib/emails/logEmailAttempt"; + +export type DeliverEmailResult = { + /** The HTTP response to return to the caller. */ + response: NextResponse; + /** The attempt to record (status + ids); the caller supplies `rawBody`. */ + attempt: Omit; +}; + +/** + * Sends a validated email via Resend (`processAndSendEmail`) and shapes both the + * HTTP response and the attempt to log, so the handler records every outcome in + * one place. `chat_id` maps to the internal `room_id` footer arg. + * + * @param data - The validated send-email request. + * @returns The HTTP response plus the attempt to log (`sent` or `send_failed`). + */ +export async function deliverEmail(data: ValidatedSendEmailRequest): Promise { + const { to, cc = [], subject, text, html = "", headers = {}, chat_id, accountId } = data; + + const result = await processAndSendEmail({ + to, + cc, + subject, + text, + html, + headers, + room_id: chat_id, + }); + + if (result.success === false) { + return { + response: NextResponse.json( + { status: "error", error: result.error }, + { status: 502, headers: getCorsHeaders() }, + ), + attempt: { status: "send_failed", accountId, chatId: chat_id }, + }; + } + + return { + response: NextResponse.json( + { success: true, message: result.message, id: result.id }, + { status: 200, headers: getCorsHeaders() }, + ), + attempt: { status: "sent", accountId, chatId: chat_id, resendId: result.id }, + }; +} diff --git a/lib/emails/sendEmailHandler.ts b/lib/emails/sendEmailHandler.ts index 7993fd01..ef7e9fe0 100644 --- a/lib/emails/sendEmailHandler.ts +++ b/lib/emails/sendEmailHandler.ts @@ -1,51 +1,12 @@ import { NextRequest, NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { - validateSendEmailBody, - type ValidatedSendEmailRequest, -} from "@/lib/emails/validateSendEmailBody"; -import { processAndSendEmail } from "@/lib/emails/processAndSendEmail"; -import { logEmailAttempt, type EmailAttemptLog } from "@/lib/emails/logEmailAttempt"; - -/** Sends a validated email; returns the HTTP response plus the attempt to log. */ -async function deliver( - data: ValidatedSendEmailRequest, -): Promise<{ response: NextResponse; attempt: Omit }> { - const { to, cc = [], subject, text, html = "", headers = {}, chat_id, accountId } = data; - - const result = await processAndSendEmail({ - to, - cc, - subject, - text, - html, - headers, - room_id: chat_id, - }); - - if (result.success === false) { - return { - response: NextResponse.json( - { status: "error", error: result.error }, - { status: 502, headers: getCorsHeaders() }, - ), - attempt: { status: "send_failed", accountId, chatId: chat_id }, - }; - } - - return { - response: NextResponse.json( - { success: true, message: result.message, id: result.id }, - { status: 200, headers: getCorsHeaders() }, - ), - attempt: { status: "sent", accountId, chatId: chat_id, resendId: result.id }, - }; -} +import { validateSendEmailBody } from "@/lib/emails/validateSendEmailBody"; +import { deliverEmail } from "@/lib/emails/deliverEmail"; +import { logEmailAttempt } from "@/lib/emails/logEmailAttempt"; /** - * Handler for POST /api/emails. Sends to the explicit recipients via Resend, - * reusing `processAndSendEmail`. Auth + body validation + the recipient - * restriction live in `validateSendEmailBody`, which also returns the raw body. + * Handler for POST /api/emails. Auth + body validation + the recipient + * restriction live in `validateSendEmailBody` (which also returns the raw body); + * the actual Resend send + response shaping live in `deliverEmail`. * * Every attempt — sent, send_failed, rejected — is recorded in `email_send_log` * with a single `logEmailAttempt` call, so a send is debuggable days later @@ -56,7 +17,7 @@ export async function sendEmailHandler(request: NextRequest): Promise { - try { - return await request.text(); - } catch { - return ""; - } -} - /** * Validates POST /api/emails: reads the raw body, parses + schema-checks it, * authenticates (x-api-key or Bearer), resolves the recipients, and enforces the diff --git a/lib/networking/__tests__/readRawBody.test.ts b/lib/networking/__tests__/readRawBody.test.ts new file mode 100644 index 00000000..b3b11a6f --- /dev/null +++ b/lib/networking/__tests__/readRawBody.test.ts @@ -0,0 +1,23 @@ +import { describe, it, expect } from "vitest"; +import { NextRequest } from "next/server"; +import { readRawBody } from "../readRawBody"; + +describe("readRawBody", () => { + it("returns the body verbatim", async () => { + const raw = '{"subject":"Hi","html":"

x

"}'; + const request = new NextRequest("http://localhost/api/emails", { method: "POST", body: raw }); + expect(await readRawBody(request)).toBe(raw); + }); + + it("returns an empty string for an empty body", async () => { + const request = new NextRequest("http://localhost/api/emails", { method: "POST" }); + expect(await readRawBody(request)).toBe(""); + }); + + it("returns an empty string when reading the body throws", async () => { + const broken = { + text: () => Promise.reject(new Error("stream consumed")), + } as unknown as NextRequest; + expect(await readRawBody(broken)).toBe(""); + }); +}); diff --git a/lib/networking/readRawBody.ts b/lib/networking/readRawBody.ts new file mode 100644 index 00000000..27b23980 --- /dev/null +++ b/lib/networking/readRawBody.ts @@ -0,0 +1,17 @@ +import { NextRequest } from "next/server"; + +/** + * Reads a request body once, as text — the single source for both JSON parsing + * and raw-body logging. Returns an empty string if the body can't be read, so + * callers never throw on a missing/unreadable body. + * + * @param request - The NextRequest object + * @returns The body as a string, or "" when it can't be read + */ +export async function readRawBody(request: NextRequest): Promise { + try { + return await request.text(); + } catch { + return ""; + } +} From 780fe79e360217d028759daad333b5a54f199d54 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 30 Jun 2026 18:40:21 -0500 Subject: [PATCH 5/5] refactor(emails): inline deliver into the handler (drop deliverEmail.ts) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review discussion on #731: deliverEmail's {response, attempt} return was handler-internal glue with a single caller, not a reusable unit. Inline it back into sendEmailHandler — still one logEmailAttempt call (DRY), still ~24 lines (under the cap). Keep readRawBody as its own file (cohesive, reusable). Mapping coverage stays in sendEmailHandler.test.ts via the processAndSendEmail mock. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/emails/__tests__/deliverEmail.test.ts | 50 ------------------- lib/emails/deliverEmail.ts | 52 ------------------- lib/emails/sendEmailHandler.ts | 61 +++++++++++++++++++---- 3 files changed, 50 insertions(+), 113 deletions(-) delete mode 100644 lib/emails/__tests__/deliverEmail.test.ts delete mode 100644 lib/emails/deliverEmail.ts diff --git a/lib/emails/__tests__/deliverEmail.test.ts b/lib/emails/__tests__/deliverEmail.test.ts deleted file mode 100644 index 2f21a5c8..00000000 --- a/lib/emails/__tests__/deliverEmail.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { deliverEmail } from "../deliverEmail"; -import type { ValidatedSendEmailRequest } from "../validateSendEmailBody"; - -const mockProcessAndSendEmail = vi.fn(); -vi.mock("@/lib/emails/processAndSendEmail", () => ({ - processAndSendEmail: (...args: unknown[]) => mockProcessAndSendEmail(...args), -})); -vi.mock("@/lib/networking/getCorsHeaders", () => ({ - getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), -})); - -const data: ValidatedSendEmailRequest = { - to: ["dest@example.com"], - cc: ["cc@example.com"], - subject: "Weekly report", - text: "body", - chat_id: "chat-1", - accountId: "account-123", -}; - -describe("deliverEmail", () => { - beforeEach(() => vi.clearAllMocks()); - - it("returns a 200 + sent attempt and maps chat_id to room_id on success", async () => { - mockProcessAndSendEmail.mockResolvedValue({ success: true, message: "ok", id: "re_1" }); - const { response, attempt } = await deliverEmail(data); - - expect(response.status).toBe(200); - expect(await response.json()).toEqual({ success: true, message: "ok", id: "re_1" }); - expect(mockProcessAndSendEmail).toHaveBeenCalledWith( - expect.objectContaining({ to: ["dest@example.com"], room_id: "chat-1" }), - ); - expect(attempt).toEqual({ - status: "sent", - accountId: "account-123", - chatId: "chat-1", - resendId: "re_1", - }); - }); - - it("returns a 502 + send_failed attempt (no resendId) on delivery failure", async () => { - mockProcessAndSendEmail.mockResolvedValue({ success: false, error: "resend boom" }); - const { response, attempt } = await deliverEmail(data); - - expect(response.status).toBe(502); - expect((await response.json()).error).toBe("resend boom"); - expect(attempt).toEqual({ status: "send_failed", accountId: "account-123", chatId: "chat-1" }); - }); -}); diff --git a/lib/emails/deliverEmail.ts b/lib/emails/deliverEmail.ts deleted file mode 100644 index 2de16069..00000000 --- a/lib/emails/deliverEmail.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { processAndSendEmail } from "@/lib/emails/processAndSendEmail"; -import type { ValidatedSendEmailRequest } from "@/lib/emails/validateSendEmailBody"; -import type { EmailAttemptLog } from "@/lib/emails/logEmailAttempt"; - -export type DeliverEmailResult = { - /** The HTTP response to return to the caller. */ - response: NextResponse; - /** The attempt to record (status + ids); the caller supplies `rawBody`. */ - attempt: Omit; -}; - -/** - * Sends a validated email via Resend (`processAndSendEmail`) and shapes both the - * HTTP response and the attempt to log, so the handler records every outcome in - * one place. `chat_id` maps to the internal `room_id` footer arg. - * - * @param data - The validated send-email request. - * @returns The HTTP response plus the attempt to log (`sent` or `send_failed`). - */ -export async function deliverEmail(data: ValidatedSendEmailRequest): Promise { - const { to, cc = [], subject, text, html = "", headers = {}, chat_id, accountId } = data; - - const result = await processAndSendEmail({ - to, - cc, - subject, - text, - html, - headers, - room_id: chat_id, - }); - - if (result.success === false) { - return { - response: NextResponse.json( - { status: "error", error: result.error }, - { status: 502, headers: getCorsHeaders() }, - ), - attempt: { status: "send_failed", accountId, chatId: chat_id }, - }; - } - - return { - response: NextResponse.json( - { success: true, message: result.message, id: result.id }, - { status: 200, headers: getCorsHeaders() }, - ), - attempt: { status: "sent", accountId, chatId: chat_id, resendId: result.id }, - }; -} diff --git a/lib/emails/sendEmailHandler.ts b/lib/emails/sendEmailHandler.ts index ef7e9fe0..6fcc3ab5 100644 --- a/lib/emails/sendEmailHandler.ts +++ b/lib/emails/sendEmailHandler.ts @@ -1,24 +1,63 @@ import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateSendEmailBody } from "@/lib/emails/validateSendEmailBody"; -import { deliverEmail } from "@/lib/emails/deliverEmail"; -import { logEmailAttempt } from "@/lib/emails/logEmailAttempt"; +import { processAndSendEmail } from "@/lib/emails/processAndSendEmail"; +import { logEmailAttempt, type EmailAttemptLog } from "@/lib/emails/logEmailAttempt"; /** * Handler for POST /api/emails. Auth + body validation + the recipient - * restriction live in `validateSendEmailBody` (which also returns the raw body); - * the actual Resend send + response shaping live in `deliverEmail`. + * restriction live in `validateSendEmailBody` (which also returns the raw body). + * Here we send the validated email via Resend, shape the response, and record + * the attempt. * - * Every attempt — sent, send_failed, rejected — is recorded in `email_send_log` - * with a single `logEmailAttempt` call, so a send is debuggable days later - * without the ephemeral sandbox. + * Every outcome — sent, send_failed, rejected — is resolved into a single + * `{ response, attempt }` and logged with ONE `logEmailAttempt` call, so a send + * is debuggable days later without the ephemeral sandbox. */ export async function sendEmailHandler(request: NextRequest): Promise { const validated = await validateSendEmailBody(request); - const { response, attempt } = - "data" in validated - ? await deliverEmail(validated.data) - : { response: validated.error, attempt: { status: "rejected" as const } }; + let response: NextResponse; + let attempt: Omit; + + if ("data" in validated) { + const { + to, + cc = [], + subject, + text, + html = "", + headers = {}, + chat_id, + accountId, + } = validated.data; + const result = await processAndSendEmail({ + to, + cc, + subject, + text, + html, + headers, + room_id: chat_id, + }); + + if (result.success === false) { + response = NextResponse.json( + { status: "error", error: result.error }, + { status: 502, headers: getCorsHeaders() }, + ); + attempt = { status: "send_failed", accountId, chatId: chat_id }; + } else { + response = NextResponse.json( + { success: true, message: result.message, id: result.id }, + { status: 200, headers: getCorsHeaders() }, + ); + attempt = { status: "sent", accountId, chatId: chat_id, resendId: result.id }; + } + } else { + response = validated.error; + attempt = { status: "rejected" }; + } await logEmailAttempt({ rawBody: validated.rawBody, ...attempt }); return response;