Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions lib/emails/__tests__/logEmailAttempt.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
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", () => {
let errorSpy: ReturnType<typeof vi.spyOn>;
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 () => {
const raw = JSON.stringify({ html: "<p>hi</p>" });
await logEmailAttempt({
rawBody: raw,
status: "sent",
accountId: "acc-1",
chatId: "chat-1",
resendId: "re_123",
});
expect(mockInsert).toHaveBeenCalledTimes(1);
const row = mockInsert.mock.calls[0][0];
expect(row.status).toBe("sent");
expect(row.account_id).toBe("acc-1");
expect(row.chat_id).toBe("chat-1");
expect(row.resend_id).toBe("re_123");
expect(row.raw_body).toBe(raw);
});

it("stores the full raw body (no truncation)", async () => {
await logEmailAttempt({ rawBody: "x".repeat(20000), status: "rejected" });
expect(mockInsert.mock.calls[0][0].raw_body.length).toBe(20000);
});

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();
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",
});
});
});
44 changes: 35 additions & 9 deletions lib/emails/__tests__/sendEmailHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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": "*" })),
}));
Expand All @@ -29,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,
Expand All @@ -60,15 +68,30 @@ describe("sendEmailHandler", () => {
room_id: "chat-1",
}),
);
// Single call, on every path (DRY); rawBody comes from validateSendEmailBody (SRP).
expect(mockLogEmailAttempt).toHaveBeenCalledTimes(1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Rejected and send_failed tests don't assert toHaveBeenCalledTimes(1), so a double-log regression on error paths wouldn't be caught. Add the count assertion for consistency.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/__tests__/sendEmailHandler.test.ts, line 72:

<comment>Rejected and send_failed tests don't assert `toHaveBeenCalledTimes(1)`, so a double-log regression on error paths wouldn't be caught. Add the count assertion for consistency.</comment>

<file context>
@@ -65,15 +68,24 @@ describe("sendEmailHandler", () => {
       }),
     );
+    // 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" }),
</file context>

expect(mockLogEmailAttempt).toHaveBeenCalledWith(
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: "{}",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Rejected and send_failed test assertions omit rawBody, so a regression where rawBody is silently dropped on error paths would not be caught. Add rawBody to both assertions.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/__tests__/sendEmailHandler.test.ts, line 86:

<comment>Rejected and send_failed test assertions omit `rawBody`, so a regression where `rawBody` is silently dropped on error paths would not be caught. Add `rawBody` to both assertions.</comment>

<file context>
@@ -65,15 +68,24 @@ describe("sendEmailHandler", () => {
-      NextResponse.json({ status: "error", error: "Forbidden" }, { status: 403 }),
-    );
+    mockValidateSendEmailBody.mockResolvedValue({
+      rawBody: "{}",
+      error: NextResponse.json({ status: "error", error: "Forbidden" }, { status: 403 }),
+    });
</file context>

error: NextResponse.json({ status: "error", error: "Forbidden" }, { status: 403 }),
});
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 () => {
Expand All @@ -77,5 +100,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" }),
);
});
});
78 changes: 37 additions & 41 deletions lib/emails/__tests__/validateSendEmailBody.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {}): NextRequest {
return new NextRequest("http://localhost/api/emails", {
method: "POST",
Expand Down Expand Up @@ -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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Same: the validation-error test (empty body → 400) asserts shape with "error" in result but never checks rawBody contents.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/__tests__/validateSendEmailBody.test.ts, line 57:

<comment>Same: the validation-error test (empty body → 400) asserts shape with `"error" in result` but never checks `rawBody` contents.</comment>

<file context>
@@ -58,10 +54,10 @@ describe("validateSendEmailBody", () => {
-      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);
</file context>

if ("error" in result) {
expect(result.error.status).toBe(403);
const json = await result.error.json();
expect(json.disallowed_recipients).toEqual(["stranger@example.com"]);
}
});
Expand All @@ -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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Tests assert "data" in result / "error" in result to check the discriminated-union shape, but never verify the rawBody field — the central contract of the return type.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/__tests__/validateSendEmailBody.test.ts, line 86:

<comment>Tests assert `"data" in result` / `"error" in result` to check the discriminated-union shape, but never verify the `rawBody` field — the central contract of the return type.</comment>

<file context>
@@ -87,18 +83,18 @@ describe("validateSendEmailBody", () => {
-      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");
</file context>

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");
}
});
});
Expand All @@ -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");
}
});

Expand All @@ -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("<p>h</p>");
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("<p>h</p>");
expect(result.data.headers).toEqual({ "X-Custom": "v" });
expect(result.data.chat_id).toBe("chat-1");
}
});

Expand All @@ -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" });
});
Expand All @@ -178,18 +174,18 @@ 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"]);
}
});

it("returns 400 when 'to' is omitted and the account has no email on file", async () => {
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 () => {
Expand All @@ -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.
Expand All @@ -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);
});
});
});
38 changes: 38 additions & 0 deletions lib/emails/logEmailAttempt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { insertEmailSendLog } from "@/lib/supabase/email_send_log/insertEmailSendLog";

export type EmailAttemptStatus = "sent" | "send_failed" | "rejected";

export type EmailAttemptLog = {
/** The request body as received (stored in full; "" when empty/unreadable). */
rawBody: string;
status: EmailAttemptStatus;
accountId?: string | null;
chatId?: string | null;
resendId?: string;
};

/**
* Records one POST /api/emails attempt in `email_send_log` so a send can be
* debugged days later without the (ephemeral) sandbox.
*
* 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<void> {
try {
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,
});
if (error) {
console.error("email_send_log insert failed:", error);
}
} catch (err) {
console.error("logEmailAttempt threw (swallowed):", err);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading
Loading