fix: Track and report partial send failures in sendDigest (apn-obg9x)

patchnotes-email/package.json

@@ -9,7 +9,7 @@
     "clean": "rimraf dist",
     "prestart": "pnpm clean && pnpm build",
     "start": "func start",
-    "test": "echo \"No tests yet...\"",
+    "test": "vitest run",
     "db:pull": "prisma db pull",
     "db:generate": "prisma generate",
     "db:smoke-test": "pnpm tsx scripts/prisma-smoke-test.ts"
@@ -27,6 +27,7 @@
     "dotenv": "^17.3.1",
     "prisma": "^7.4.0",
     "rimraf": "^6.1.2",
-    "typescript": "^5.0.0"
+    "typescript": "^5.0.0",
+    "vitest": "^4.0.18"
   }
 }

patchnotes-email/src/functions/sendDigest.test.ts

@@ -0,0 +1,189 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+const { mockSend, mockFindMany } = vi.hoisted(() => ({
+    mockSend: vi.fn(),
+    mockFindMany: vi.fn(),
+}));
+
+vi.mock("../lib/resend", () => ({
+    resend: { emails: { send: mockSend } },
+    FROM_ADDRESS: "PatchNotes <notifications@patchnotes.dev>",
+    escapeHtml: (s: string) => s.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;").replace(/"/g, "&quot;"),
+    emailFooter: () => "<footer/>",
+    sanitizeSubject: (s: string) => s.replace(/[\r\n]+/g, " ").trim(),
+    isValidEmail: (email: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email),
+}));
+
+vi.mock("../lib/prisma", () => ({
+    getPrismaClient: () => ({ users: { findMany: mockFindMany } }),
+}));
+
+import { sendDigest } from "./sendDigest";
+
+function makeContext() {
+    return {
+        log: vi.fn(),
+        warn: vi.fn(),
+        error: vi.fn(),
+    } as any;
+}
+
+function makeTimer(): any {
+    return { isPastDue: false };
+}
+
+function makeUser(email: string, name: string, releases: Array<{ tag: string; major: number }>) {
+    return {
+        Email: email,
+        Name: name,
+        Watchlists: [
+            {
+                Packages: {
+                    Name: "test-package",
+                    Releases: releases.map((r) => ({
+                        Tag: r.tag,
+                        MajorVersion: r.major,
+                        IsPrerelease: false,
+                    })),
+                    ReleaseSummaries: releases.map((r) => ({
+                        Summary: `Summary for ${r.tag}`,
+                        MajorVersion: r.major,
+                        IsPrerelease: false,
+                    })),
+                },
+            },
+        ],
+    };
+}
+
+describe("sendDigest", () => {
+    beforeEach(() => {
+        vi.clearAllMocks();
+    });
+
+    it("returns early when no users have pending items", async () => {
+        mockFindMany.mockResolvedValue([]);
+        const context = makeContext();
+
+        await sendDigest(makeTimer(), context);
+
+        expect(mockSend).not.toHaveBeenCalled();
+        expect(context.log).toHaveBeenCalledWith("No users with pending digest items");
+    });
+
+    it("sends digests to all users and logs summary on full success", async () => {
+        mockFindMany.mockResolvedValue([
+            makeUser("a@test.com", "Alice", [{ tag: "v1.0.0", major: 1 }]),
+            makeUser("b@test.com", "Bob", [{ tag: "v2.0.0", major: 2 }]),
+        ]);
+        mockSend.mockResolvedValue({ error: null });
+        const context = makeContext();
+
+        await sendDigest(makeTimer(), context);
+
+        expect(mockSend).toHaveBeenCalledTimes(2);
+        expect(context.log).toHaveBeenCalledWith(
+            "Digest summary: 2 sent, 0 failed, 0 skipped out of 2 users"
+        );
+    });
+
+    it("throws when some sends fail with API error", async () => {
+        mockFindMany.mockResolvedValue([
+            makeUser("a@test.com", "Alice", [{ tag: "v1.0.0", major: 1 }]),
+            makeUser("b@test.com", "Bob", [{ tag: "v2.0.0", major: 2 }]),
+            makeUser("c@test.com", "Carol", [{ tag: "v3.0.0", major: 3 }]),
+        ]);
+        mockSend
+            .mockResolvedValueOnce({ error: null })
+            .mockResolvedValueOnce({ error: { message: "rate limited" } })
+            .mockResolvedValueOnce({ error: null });
+        const context = makeContext();
+
+        await expect(sendDigest(makeTimer(), context)).rejects.toThrow(
+            "Digest send partially failed: 1/3 sends failed. Failed: b@test.com"
+        );
+        expect(context.log).toHaveBeenCalledWith(
+            "Digest summary: 2 sent, 1 failed, 0 skipped out of 3 users"
+        );
+    });
+
+    it("throws when sends throw exceptions", async () => {
+        mockFindMany.mockResolvedValue([
+            makeUser("a@test.com", "Alice", [{ tag: "v1.0.0", major: 1 }]),
+            makeUser("b@test.com", "Bob", [{ tag: "v2.0.0", major: 2 }]),
+        ]);
+        mockSend
+            .mockResolvedValueOnce({ error: null })
+            .mockRejectedValueOnce(new Error("network error"));
+        const context = makeContext();
+
+        await expect(sendDigest(makeTimer(), context)).rejects.toThrow(
+            "Digest send partially failed: 1/2 sends failed. Failed: b@test.com"
+        );
+    });
+
+    it("throws when all sends fail", async () => {
+        mockFindMany.mockResolvedValue([
+            makeUser("a@test.com", "Alice", [{ tag: "v1.0.0", major: 1 }]),
+            makeUser("b@test.com", "Bob", [{ tag: "v2.0.0", major: 2 }]),
+        ]);
+        mockSend.mockResolvedValue({ error: { message: "service down" } });
+        const context = makeContext();
+
+        await expect(sendDigest(makeTimer(), context)).rejects.toThrow(
+            "Digest send partially failed: 2/2 sends failed. Failed: a@test.com, b@test.com"
+        );
+    });
+
+    it("skips users with invalid emails and counts them", async () => {
+        mockFindMany.mockResolvedValue([
+            makeUser("valid@test.com", "Valid", [{ tag: "v1.0.0", major: 1 }]),
+            makeUser("not-an-email", "Invalid", [{ tag: "v2.0.0", major: 2 }]),
+        ]);
+        mockSend.mockResolvedValue({ error: null });
+        const context = makeContext();
+
+        await sendDigest(makeTimer(), context);
+
+        expect(mockSend).toHaveBeenCalledTimes(1);
+        expect(context.warn).toHaveBeenCalledWith(
+            "Skipping digest for user with invalid email: not-an-email"
+        );
+        expect(context.log).toHaveBeenCalledWith(
+            "Digest summary: 1 sent, 0 failed, 1 skipped out of 2 users"
+        );
+    });
+
+    it("skips users with no releases and counts them", async () => {
+        mockFindMany.mockResolvedValue([
+            makeUser("a@test.com", "Alice", [{ tag: "v1.0.0", major: 1 }]),
+            makeUser("b@test.com", "Bob", []),  // no releases
+        ]);
+        mockSend.mockResolvedValue({ error: null });
+        const context = makeContext();
+
+        await sendDigest(makeTimer(), context);
+
+        expect(mockSend).toHaveBeenCalledTimes(1);
+        expect(context.log).toHaveBeenCalledWith(
+            "Digest summary: 1 sent, 0 failed, 1 skipped out of 2 users"
+        );
+    });
+
+    it("continues sending to remaining users after a failure", async () => {
+        mockFindMany.mockResolvedValue([
+            makeUser("a@test.com", "Alice", [{ tag: "v1.0.0", major: 1 }]),
+            makeUser("b@test.com", "Bob", [{ tag: "v2.0.0", major: 2 }]),
+            makeUser("c@test.com", "Carol", [{ tag: "v3.0.0", major: 3 }]),
+        ]);
+        mockSend
+            .mockRejectedValueOnce(new Error("fail"))
+            .mockResolvedValueOnce({ error: null })
+            .mockResolvedValueOnce({ error: null });
+        const context = makeContext();
+
+        await expect(sendDigest(makeTimer(), context)).rejects.toThrow();
+        // All 3 users should have been attempted (not short-circuited)
+        expect(mockSend).toHaveBeenCalledTimes(3);
+    });
+});

patchnotes-email/src/functions/sendDigest.ts

@@ -66,6 +66,10 @@ export async function sendDigest(
 
     context.log(`Sending digests to ${users.length} users`);
 
+    const failures: Array<{ email: string; error: unknown }> = [];
+    let sentCount = 0;
+    let skippedCount = 0;
+
     for (const user of users) {
         const releases: Array<{ packageName: string; version: string; summary: string }> = [];
 
@@ -82,10 +86,14 @@ export async function sendDigest(
             }
         }
 
-        if (releases.length === 0) continue;
+        if (releases.length === 0) {
+            skippedCount++;
+            continue;
+        }
 
         if (!user.Email || !isValidEmail(user.Email)) {
             context.warn(`Skipping digest for user with invalid email: ${user.Email}`);
+            skippedCount++;
             continue;
         }
 
@@ -112,13 +120,27 @@ export async function sendDigest(
 
             if (error) {
                 context.error(`Failed to send digest to ${user.Email}:`, error);
+                failures.push({ email: user.Email!, error });
             } else {
+                sentCount++;
                 context.log(`Digest sent to ${user.Email}`);
             }
         } catch (err) {
             context.error(`Error sending to ${user.Email}:`, err);
+            failures.push({ email: user.Email!, error: err });
         }
     }
+
+    context.log(
+        `Digest summary: ${sentCount} sent, ${failures.length} failed, ${skippedCount} skipped out of ${users.length} users`
+    );
+
+    if (failures.length > 0) {
+        const failedEmails = failures.map((f) => f.email).join(", ");
+        throw new Error(
+            `Digest send partially failed: ${failures.length}/${sentCount + failures.length} sends failed. Failed: ${failedEmails}`
+        );
+    }
 }
 
 // Runs every Monday at 9:00 AM UTC

patchnotes-email/tsconfig.json

@@ -10,6 +10,7 @@
   },
   "exclude": [
     "scripts",
-    "prisma.config.ts"
+    "prisma.config.ts",
+    "**/*.test.ts"
   ]
 }